All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] git-p4: Rename and copy detection improvements.
@ 2010-11-19  1:38 Vitor Antunes
  2010-11-19  1:38 ` [PATCH 1/2] git-p4: Don't edit renamed files if not necessary Vitor Antunes
  2010-11-19  1:38 ` [PATCH 2/2] git-p4: Added copy detection support Vitor Antunes
  0 siblings, 2 replies; 15+ messages in thread
From: Vitor Antunes @ 2010-11-19  1:38 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes

Two patches to improve the rename and copy detection implementation in git-p4.
Also added some configuration options for ease of use.

Vitor Antunes (2):
  git-p4: Don't edit renamed files if not necessary.
  git-p4: Added copy detection support

 contrib/fast-import/git-p4 |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)

-- 
1.7.2.3

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] git-p4: Don't edit renamed files if not necessary.
  2010-11-19  1:38 [PATCH 0/2] git-p4: Rename and copy detection improvements Vitor Antunes
@ 2010-11-19  1:38 ` Vitor Antunes
  2010-11-19 12:14   ` Vitor Antunes
  2010-11-25  1:26   ` [PATCH] git-p4: Corrected typo Vitor Antunes
  2010-11-19  1:38 ` [PATCH 2/2] git-p4: Added copy detection support Vitor Antunes
  1 sibling, 2 replies; 15+ messages in thread
From: Vitor Antunes @ 2010-11-19  1:38 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes

Only open files for edit after integrating if the SHA1 of source and
destination differ from each other.
Also added git config option to allow permanent rename detection.
The detectRenames option should be set to the desired threshold value.
Rename detection can still be enabled through -M option.
---
 contrib/fast-import/git-p4 |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 04ce7e3..ba18512 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -613,7 +613,13 @@ class P4Submit(Command):
 
     def applyCommit(self, id):
         print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
-        diffOpts = ("", "-M")[self.detectRename]
+
+        detectRenames = gitConfig("git-p4.detectRenames")
+        if len(detectRenames) > 0:
+            diffOpts = "-M%s" % detectRenames
+        else:
+            diffOpts = ("", "-M")[self.detectRenames]
+
         diff = read_pipe_lines("git diff-tree -r %s \"%s^\" \"%s\"" % (diffOpts, id, id))
         filesToAdd = set()
         filesToDelete = set()
@@ -640,7 +646,8 @@ class P4Submit(Command):
             elif modifier == "R":
                 src, dest = diff['src'], diff['dst']
                 p4_system("integrate -Dt \"%s\" \"%s\"" % (src, dest))
-                p4_system("edit \"%s\"" % (dest))
+                if diff['src_sha1'] != diff['dst_sha1']:
+                    p4_system("edit \"%s\"" % (dest))
                 if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
                     filesToChangeExecBit[dest] = diff['dst_mode']
                 os.unlink(dest)
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] git-p4: Added copy detection support
  2010-11-19  1:38 [PATCH 0/2] git-p4: Rename and copy detection improvements Vitor Antunes
  2010-11-19  1:38 ` [PATCH 1/2] git-p4: Don't edit renamed files if not necessary Vitor Antunes
@ 2010-11-19  1:38 ` Vitor Antunes
  1 sibling, 0 replies; 15+ messages in thread
From: Vitor Antunes @ 2010-11-19  1:38 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes

Added new config options:
    git-p4.detectCopies         - Enable copy detection.
    git-p4.detectCopiesHarder   - Find copies harder.
The detectCopies option should be set to the desired threshold value.
The detectCopiesHarder option receives a simple true/false value.
P4Submit is now able to process diff-tree C status.
---
 contrib/fast-import/git-p4 |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index ba18512..0ea3a44 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -620,6 +620,14 @@ class P4Submit(Command):
         else:
             diffOpts = ("", "-M")[self.detectRenames]
 
+        detectCopies = gitConfig("git-p4.detectCopies")
+        if len(detectCopies) > 0:
+            diffOpts += " -C%s" % detectCopies
+
+        detectCopiesHarder = gitConfig("git-p4.detectCopiesHarder")
+        if len(detectCopiesHarder) > 0 and detectCopiesHarder.lower() != "false":
+            diffOpts += " --find-copies-harder"
+
         diff = read_pipe_lines("git diff-tree -r %s \"%s^\" \"%s\"" % (diffOpts, id, id))
         filesToAdd = set()
         filesToDelete = set()
@@ -643,6 +651,15 @@ class P4Submit(Command):
                 filesToDelete.add(path)
                 if path in filesToAdd:
                     filesToAdd.remove(path)
+            elif modifier == "C":
+                src, dest = diff['src'], diff['dst']
+                p4_system("integrate -Dt \"%s\" \"%s\"" % (src, dest))
+                if diff['src_sha1'] != diff['dst_sha1']:
+                    p4_system("edit \"%s\"" % (dest))
+                if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
+                    filesToChangeExecBit[dest] = diff['dst_mode']
+                os.unlink(dest)
+                editedFiles.add(dest)
             elif modifier == "R":
                 src, dest = diff['src'], diff['dst']
                 p4_system("integrate -Dt \"%s\" \"%s\"" % (src, dest))
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] git-p4: Don't edit renamed files if not necessary.
  2010-11-19  1:38 ` [PATCH 1/2] git-p4: Don't edit renamed files if not necessary Vitor Antunes
@ 2010-11-19 12:14   ` Vitor Antunes
  2010-11-25  1:26   ` [PATCH] git-p4: Corrected typo Vitor Antunes
  1 sibling, 0 replies; 15+ messages in thread
From: Vitor Antunes @ 2010-11-19 12:14 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes

There is a typo in this patch. I'll send a patch this night.

Sorry.

On Fri, Nov 19, 2010 at 1:38 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> Only open files for edit after integrating if the SHA1 of source and
> destination differ from each other.
> Also added git config option to allow permanent rename detection.
> The detectRenames option should be set to the desired threshold value.
> Rename detection can still be enabled through -M option.
> ---
>  contrib/fast-import/git-p4 |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 04ce7e3..ba18512 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -613,7 +613,13 @@ class P4Submit(Command):
>
>     def applyCommit(self, id):
>         print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
> -        diffOpts = ("", "-M")[self.detectRename]
> +
> +        detectRenames = gitConfig("git-p4.detectRenames")
> +        if len(detectRenames) > 0:
> +            diffOpts = "-M%s" % detectRenames
> +        else:
> +            diffOpts = ("", "-M")[self.detectRenames]
> +
>         diff = read_pipe_lines("git diff-tree -r %s \"%s^\" \"%s\"" % (diffOpts, id, id))
>         filesToAdd = set()
>         filesToDelete = set()
> @@ -640,7 +646,8 @@ class P4Submit(Command):
>             elif modifier == "R":
>                 src, dest = diff['src'], diff['dst']
>                 p4_system("integrate -Dt \"%s\" \"%s\"" % (src, dest))
> -                p4_system("edit \"%s\"" % (dest))
> +                if diff['src_sha1'] != diff['dst_sha1']:
> +                    p4_system("edit \"%s\"" % (dest))
>                 if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
>                     filesToChangeExecBit[dest] = diff['dst_mode']
>                 os.unlink(dest)
> --
> 1.7.2.3
>
>



-- 
Vitor Antunes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] git-p4: Corrected typo.
  2010-11-19  1:38 ` [PATCH 1/2] git-p4: Don't edit renamed files if not necessary Vitor Antunes
  2010-11-19 12:14   ` Vitor Antunes
@ 2010-11-25  1:26   ` Vitor Antunes
  2011-01-27 23:35     ` Vitor Antunes
  1 sibling, 1 reply; 15+ messages in thread
From: Vitor Antunes @ 2010-11-25  1:26 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes

---
 contrib/fast-import/git-p4 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 0ea3a44..a466847 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -618,7 +618,7 @@ class P4Submit(Command):
         if len(detectRenames) > 0:
             diffOpts = "-M%s" % detectRenames
         else:
-            diffOpts = ("", "-M")[self.detectRenames]
+            diffOpts = ("", "-M")[self.detectRename]
 
         detectCopies = gitConfig("git-p4.detectCopies")
         if len(detectCopies) > 0:
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2010-11-25  1:26   ` [PATCH] git-p4: Corrected typo Vitor Antunes
@ 2011-01-27 23:35     ` Vitor Antunes
  2011-01-28 15:19       ` Thomas Berg
  0 siblings, 1 reply; 15+ messages in thread
From: Vitor Antunes @ 2011-01-27 23:35 UTC (permalink / raw)
  To: git; +Cc: Vitor Antunes

Hi everyone,

Could anyone comment the 3 patches I sent (being this the last one)?

Thanks in advance,
Vitor

On Thu, Nov 25, 2010 at 1:26 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> ---
>  contrib/fast-import/git-p4 |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 0ea3a44..a466847 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -618,7 +618,7 @@ class P4Submit(Command):
>         if len(detectRenames) > 0:
>             diffOpts = "-M%s" % detectRenames
>         else:
> -            diffOpts = ("", "-M")[self.detectRenames]
> +            diffOpts = ("", "-M")[self.detectRename]
>
>         detectCopies = gitConfig("git-p4.detectCopies")
>         if len(detectCopies) > 0:
> --
> 1.7.2.3
>
>



-- 
Vitor Antunes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-27 23:35     ` Vitor Antunes
@ 2011-01-28 15:19       ` Thomas Berg
  2011-01-29  2:41         ` Vitor Antunes
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Berg @ 2011-01-28 15:19 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git

Hi,

On Fri, Jan 28, 2011 at 12:35 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> Hi everyone,
>
> Could anyone comment the 3 patches I sent (being this the last one)?
>
[...]
> On Thu, Nov 25, 2010 at 1:26 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
>> ---
>>  contrib/fast-import/git-p4 |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
>> index 0ea3a44..a466847 100755
>> --- a/contrib/fast-import/git-p4
>> +++ b/contrib/fast-import/git-p4
>> @@ -618,7 +618,7 @@ class P4Submit(Command):
>>         if len(detectRenames) > 0:
>>             diffOpts = "-M%s" % detectRenames
>>         else:
>> -            diffOpts = ("", "-M")[self.detectRenames]
>> +            diffOpts = ("", "-M")[self.detectRename]
>>

This appears to me to be a bugfix for one of the other patches you
sent, is that right?

If so, maybe you could squash it with the previous patch and re-send
it all to the list?

My other comments for now are:
- you have forgotten to sign off on the patches
- commit messages are normally in imperative rather than past tense
(see Documentation/SubmittingPatches in git)

- In your first patch you wrote:
> The detectRenames option should be set to the desired threshold value.
I'm not sure what threshold value you refer to here, and what values
you can set it to. Am I missing something?
(I'm not very familiar with git rename detection options)

I'm a git-p4 user, so I can test your changes and look a bit more at
your code. Someone verifying it could help getting the patches
applied.

Thanks for improving git-p4!

Cheers,
Thomas Berg

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-28 15:19       ` Thomas Berg
@ 2011-01-29  2:41         ` Vitor Antunes
  2011-01-30 23:21           ` Vitor Antunes
  0 siblings, 1 reply; 15+ messages in thread
From: Vitor Antunes @ 2011-01-29  2:41 UTC (permalink / raw)
  To: Thomas Berg; +Cc: git

Hi Thomas,

First of all I'd like to thank you on your feedback. It's my first try
on creating submitting a patch, so having someone's guidance helps a
lot :)

I'll rebase my patches against the head of the tree and squash the fix
to avoid multiple commits. While I do that I'll also review my commit
message and sign-off the patches according to what you said. Hopefully
I will be able to do this during this weekend.

From git-diff-tree man page:

"""
-M[<n>]
    Detect renames. If n is specified, it is a is a threshold on the
similarity index (i.e. amount of addition/deletions compared to the
file’s
    size). For example, -M90% means git should consider a delete/add
pair to be a rename if more than 90% of the file hasn’t changed.
"""

But from my latest tests I think that this option is ignored in
diff-tree (I think it's only used in git log). With this in mind I'll
need to add some code to implement the check of the score value of
diff-tree output string. Again from its man page:

"""
Status letters C and R are always followed by a score (denoting the
percentage of similarity between the source and target of the move or
copy), and are the only ones to be so.
"""

Thanks,
Vitor

On Fri, Jan 28, 2011 at 3:19 PM, Thomas Berg <merlin66b@gmail.com> wrote:
> Hi,
>
> On Fri, Jan 28, 2011 at 12:35 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
>> Hi everyone,
>>
>> Could anyone comment the 3 patches I sent (being this the last one)?
>>
> [...]
>> On Thu, Nov 25, 2010 at 1:26 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
>>> ---
>>>  contrib/fast-import/git-p4 |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
>>> index 0ea3a44..a466847 100755
>>> --- a/contrib/fast-import/git-p4
>>> +++ b/contrib/fast-import/git-p4
>>> @@ -618,7 +618,7 @@ class P4Submit(Command):
>>>         if len(detectRenames) > 0:
>>>             diffOpts = "-M%s" % detectRenames
>>>         else:
>>> -            diffOpts = ("", "-M")[self.detectRenames]
>>> +            diffOpts = ("", "-M")[self.detectRename]
>>>
>
> This appears to me to be a bugfix for one of the other patches you
> sent, is that right?
>
> If so, maybe you could squash it with the previous patch and re-send
> it all to the list?
>
> My other comments for now are:
> - you have forgotten to sign off on the patches
> - commit messages are normally in imperative rather than past tense
> (see Documentation/SubmittingPatches in git)
>
> - In your first patch you wrote:
>> The detectRenames option should be set to the desired threshold value.
> I'm not sure what threshold value you refer to here, and what values
> you can set it to. Am I missing something?
> (I'm not very familiar with git rename detection options)
>
> I'm a git-p4 user, so I can test your changes and look a bit more at
> your code. Someone verifying it could help getting the patches
> applied.
>
> Thanks for improving git-p4!
>
> Cheers,
> Thomas Berg
>



-- 
Vitor Antunes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-29  2:41         ` Vitor Antunes
@ 2011-01-30 23:21           ` Vitor Antunes
  2011-01-30 23:34             ` Thomas Berg
  0 siblings, 1 reply; 15+ messages in thread
From: Vitor Antunes @ 2011-01-30 23:21 UTC (permalink / raw)
  To: Thomas Berg; +Cc: git

Hi Thomas,

I've just sent out the patches to the mailing list. I'm looking
forward to receive some feedback from you :)

I also have some branch detection related patch prepared. Do you use
this feature often?

Thanks,
Vitor

On Sat, Jan 29, 2011 at 2:41 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> Hi Thomas,
>
> First of all I'd like to thank you on your feedback. It's my first try
> on creating submitting a patch, so having someone's guidance helps a
> lot :)
>
> I'll rebase my patches against the head of the tree and squash the fix
> to avoid multiple commits. While I do that I'll also review my commit
> message and sign-off the patches according to what you said. Hopefully
> I will be able to do this during this weekend.
>
> From git-diff-tree man page:
>
> """
> -M[<n>]
>    Detect renames. If n is specified, it is a is a threshold on the
> similarity index (i.e. amount of addition/deletions compared to the
> file’s
>    size). For example, -M90% means git should consider a delete/add
> pair to be a rename if more than 90% of the file hasn’t changed.
> """
>
> But from my latest tests I think that this option is ignored in
> diff-tree (I think it's only used in git log). With this in mind I'll
> need to add some code to implement the check of the score value of
> diff-tree output string. Again from its man page:
>
> """
> Status letters C and R are always followed by a score (denoting the
> percentage of similarity between the source and target of the move or
> copy), and are the only ones to be so.
> """
>
> Thanks,
> Vitor
>
> On Fri, Jan 28, 2011 at 3:19 PM, Thomas Berg <merlin66b@gmail.com> wrote:
>> Hi,
>>
>> On Fri, Jan 28, 2011 at 12:35 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
>>> Hi everyone,
>>>
>>> Could anyone comment the 3 patches I sent (being this the last one)?
>>>
>> [...]
>>> On Thu, Nov 25, 2010 at 1:26 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
>>>> ---
>>>>  contrib/fast-import/git-p4 |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
>>>> index 0ea3a44..a466847 100755
>>>> --- a/contrib/fast-import/git-p4
>>>> +++ b/contrib/fast-import/git-p4
>>>> @@ -618,7 +618,7 @@ class P4Submit(Command):
>>>>         if len(detectRenames) > 0:
>>>>             diffOpts = "-M%s" % detectRenames
>>>>         else:
>>>> -            diffOpts = ("", "-M")[self.detectRenames]
>>>> +            diffOpts = ("", "-M")[self.detectRename]
>>>>
>>
>> This appears to me to be a bugfix for one of the other patches you
>> sent, is that right?
>>
>> If so, maybe you could squash it with the previous patch and re-send
>> it all to the list?
>>
>> My other comments for now are:
>> - you have forgotten to sign off on the patches
>> - commit messages are normally in imperative rather than past tense
>> (see Documentation/SubmittingPatches in git)
>>
>> - In your first patch you wrote:
>>> The detectRenames option should be set to the desired threshold value.
>> I'm not sure what threshold value you refer to here, and what values
>> you can set it to. Am I missing something?
>> (I'm not very familiar with git rename detection options)
>>
>> I'm a git-p4 user, so I can test your changes and look a bit more at
>> your code. Someone verifying it could help getting the patches
>> applied.
>>
>> Thanks for improving git-p4!
>>
>> Cheers,
>> Thomas Berg
>>
>
>
>
> --
> Vitor Antunes
>



-- 
Vitor Antunes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-30 23:21           ` Vitor Antunes
@ 2011-01-30 23:34             ` Thomas Berg
  2011-01-31 11:25               ` Vitor Antunes
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Berg @ 2011-01-30 23:34 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git

Hi Vitor,

On Mon, Jan 31, 2011 at 12:21 AM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> Hi Thomas,
>
> I've just sent out the patches to the mailing list. I'm looking
> forward to receive some feedback from you :)

Thanks, I will try to get it tested tomorrow.

By the way, on this mailing list please don't top post. I think the
preferred style is interleaved posting:
http://en.wikipedia.org/wiki/Posting_style#Interleaved_style

>
> I also have some branch detection related patch prepared. Do you use
> this feature often?

No, the Perforce repo I work with is so non-standard that the only
solution has been to import all the branches separately and graft the
history together. This covers all my needs at the moment.

Cheers,
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-30 23:34             ` Thomas Berg
@ 2011-01-31 11:25               ` Vitor Antunes
  2011-01-31 12:51                 ` Thomas Berg
  0 siblings, 1 reply; 15+ messages in thread
From: Vitor Antunes @ 2011-01-31 11:25 UTC (permalink / raw)
  To: Thomas Berg; +Cc: git

Hi Thomas,

On Sun, Jan 30, 2011 at 11:34 PM, Thomas Berg <merlin66b@gmail.com> wrote:
> By the way, on this mailing list please don't top post. I think the
> preferred style is interleaved posting:
> http://en.wikipedia.org/wiki/Posting_style#Interleaved_style

Thanks for introducing me to these "rules" that are being used within
the git community :)

>> I also have some branch detection related patch prepared. Do you use
>> this feature often?
>
> No, the Perforce repo I work with is so non-standard that the only
> solution has been to import all the branches separately and graft the
> history together. This covers all my needs at the moment.

Maybe I'm not seeing some obvious limitation, but I can't imagine a
branching structure that can't be imported into git. Could please you
give me an example?

Thanks,

-- 
Vitor Antunes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-31 11:25               ` Vitor Antunes
@ 2011-01-31 12:51                 ` Thomas Berg
  2011-01-31 13:39                   ` Vitor Antunes
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Berg @ 2011-01-31 12:51 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git

Hi Vitor,

On Mon, Jan 31, 2011 at 12:25 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:
>> No, the Perforce repo I work with is so non-standard that the only
>> solution has been to import all the branches separately and graft the
>> history together. This covers all my needs at the moment.
>
> Maybe I'm not seeing some obvious limitation, but I can't imagine a
> branching structure that can't be imported into git. Could please you
> give me an example?

Here I was thinking of the fact that git-p4 (last time I checked the
implementation ) uses the list of branch specs in Perforce for
figuring out the parent of a branch. Our branch specs have changed
over time (they are used for different integration purposes), so they
are no longer usable for this purpose.

I also discovered bugs in git-p4: in some cases, if the first submit
to a new branch in Perforce is not identical to the branch it derives
from, the import was not correct.

One other issue with Perforce, CVS and many other systems is that they
branch per file. Therefore Perforce can represent partial merges
between two branches, which git cannot. Because of this, translating
merges in Perforce to merges in git is not always possible or
desirable:
- if you integrate just one file from one branch to another in
Perforce, and leave the rest unmerged, you probably want to represent
it as a normal git commit (not a merge)
- if you merge almost everything, but leave out a file for some
reason, you may still want to represent it as a merge in git

The git-p4raw tool has excellent handling of merges, see details in
this file around line 4300:
https://github.com/samv/git-p4raw/blob/master/git-p4raw
It supports several algorithms for automatic merge detection, as well
as manually changing it after the import is done.

Cheers,
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-31 12:51                 ` Thomas Berg
@ 2011-01-31 13:39                   ` Vitor Antunes
  2011-01-31 15:54                     ` Vitor Antunes
  0 siblings, 1 reply; 15+ messages in thread
From: Vitor Antunes @ 2011-01-31 13:39 UTC (permalink / raw)
  To: Thomas Berg; +Cc: git

Hi Thomas,

On Mon, Jan 31, 2011 at 12:51 PM, Thomas Berg <merlin66b@gmail.com> wrote:
>> Maybe I'm not seeing some obvious limitation, but I can't imagine a
>> branching structure that can't be imported into git. Could please you
>> give me an example?
>
> Here I was thinking of the fact that git-p4 (last time I checked the
> implementation ) uses the list of branch specs in Perforce for
> figuring out the parent of a branch. Our branch specs have changed
> over time (they are used for different integration purposes), so they
> are no longer usable for this purpose.

My personal git-p4 script uses a configuration option to define the
list of branches. I also added an option to get the list of branches
filtered by user (p4 branches -u), which avoids waiting for the server
since I don't have any branches defined.

> I also discovered bugs in git-p4: in some cases, if the first submit
> to a new branch in Perforce is not identical to the branch it derives
> from, the import was not correct.

Another thing that I modified was the following lines:

1559                         for (prev, cur) in
zip(self.previousDepotPaths, depotPaths):
1560                             for i in range(0, min(len(cur), len(prev))):
1561                                 if cur[i] <> prev[i]:
1562                                     i = i - 1
1563                                     break

This tries to find the root directory of all branches, but does that
comparing char by char. So, if you have something like:

//depot/branches/branch
//depot/branches/branch_test1
//depot/branches/branch_test2

It will assume that your root branch is //depot/branches/branch, which is wrong.
I've modified this to split the string by "/" and compare those items,
making sure it will detect //depot/branches as the root directory.

> One other issue with Perforce, CVS and many other systems is that they
> branch per file. Therefore Perforce can represent partial merges
> between two branches, which git cannot. Because of this, translating
> merges in Perforce to merges in git is not always possible or
> desirable:
> - if you integrate just one file from one branch to another in
> Perforce, and leave the rest unmerged, you probably want to represent
> it as a normal git commit (not a merge)
> - if you merge almost everything, but leave out a file for some
> reason, you may still want to represent it as a merge in git

Yes, merge detection is something that is working. I never tried to
look into this because I don't find it too important for my work flow.

> The git-p4raw tool has excellent handling of merges, see details in
> this file around line 4300:
> https://github.com/samv/git-p4raw/blob/master/git-p4raw
> It supports several algorithms for automatic merge detection, as well
> as manually changing it after the import is done.

I'll have to look into this later :)

Bye,
-- 
Vitor Antunes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-31 13:39                   ` Vitor Antunes
@ 2011-01-31 15:54                     ` Vitor Antunes
  2011-02-04  8:59                       ` Thomas Berg
  0 siblings, 1 reply; 15+ messages in thread
From: Vitor Antunes @ 2011-01-31 15:54 UTC (permalink / raw)
  To: Thomas Berg; +Cc: git

Hi Thomas,

> On Mon, Jan 31, 2011 at 12:51 PM, Thomas Berg <merlin66b@gmail.com> wrote:
>> I also discovered bugs in git-p4: in some cases, if the first submit
>> to a new branch in Perforce is not identical to the branch it derives
>> from, the import was not correct.

I forgot to answer this specific topic. I also noticed this bug.
Basically, git-p4 choses the first commit from the origin branch to
start the branch from. My idea was to walk through the commit tree in
the original branch until I find a commit in which the diff is null.
Unfortunately, I don't know what is the best approach to achieve this
in git. Do you have any ideas?

Thanks,
-- 
Vitor Antunes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] git-p4: Corrected typo.
  2011-01-31 15:54                     ` Vitor Antunes
@ 2011-02-04  8:59                       ` Thomas Berg
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Berg @ 2011-02-04  8:59 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git

On Mon, Jan 31, 2011 at 4:54 PM, Vitor Antunes <vitor.hda@gmail.com> wrote:
> Hi Thomas,
>
>> On Mon, Jan 31, 2011 at 12:51 PM, Thomas Berg <merlin66b@gmail.com> wrote:
>>> I also discovered bugs in git-p4: in some cases, if the first submit
>>> to a new branch in Perforce is not identical to the branch it derives
>>> from, the import was not correct.
>
> I forgot to answer this specific topic. I also noticed this bug.
> Basically, git-p4 choses the first commit from the origin branch to
> start the branch from. My idea was to walk through the commit tree in
> the original branch until I find a commit in which the diff is null.
> Unfortunately, I don't know what is the best approach to achieve this
> in git. Do you have any ideas?

The thing is that the diff doesn't have to be null either, if an
"evil" branching has happened  - the submit that creates the new
branch contains a different tree than the source branch. It would
probably work well in most cases though, you could just fall back to
the current behaviour if no zero-diff parent is found. I'm sure it's
easy with some git plumbing commands, but I'm of little help there.

I haven't had time to test your patches yet, hope to get some time soon...

Cheers,
Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2011-02-04  8:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-19  1:38 [PATCH 0/2] git-p4: Rename and copy detection improvements Vitor Antunes
2010-11-19  1:38 ` [PATCH 1/2] git-p4: Don't edit renamed files if not necessary Vitor Antunes
2010-11-19 12:14   ` Vitor Antunes
2010-11-25  1:26   ` [PATCH] git-p4: Corrected typo Vitor Antunes
2011-01-27 23:35     ` Vitor Antunes
2011-01-28 15:19       ` Thomas Berg
2011-01-29  2:41         ` Vitor Antunes
2011-01-30 23:21           ` Vitor Antunes
2011-01-30 23:34             ` Thomas Berg
2011-01-31 11:25               ` Vitor Antunes
2011-01-31 12:51                 ` Thomas Berg
2011-01-31 13:39                   ` Vitor Antunes
2011-01-31 15:54                     ` Vitor Antunes
2011-02-04  8:59                       ` Thomas Berg
2010-11-19  1:38 ` [PATCH 2/2] git-p4: Added copy detection support Vitor Antunes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.