Re: [PATCH 2/2] git-p4: Add copy detection support

From: Vitor Antunes <vitor.hda@gmail.com>
Date: Feb 7, 2011, 11:11 UTC
Message-ID: <AANLkTikO4=froHvYN-JCOMZF7zgGHaD4J=nbN1ij_RjW@mail.gmail.com>
In-Reply-To: <20110206220546.GA9024@mew.padd.com>

Hi Pete,

On Sun, Feb 6, 2011 at 10:05 PM, Pete Wyckoff <pw@padd.com> wrote:

Show 14 quoted lines

>> (Copying help text:
>>       The -t flag makes the source file's filetype propagate to the target
>>       file.  Normally, the target file retains its previous filetype.
>>       Newly branched files always use the source file's filetype.  The
>>       filetype can still be changed before 'p4 submit' with 'p4 reopen'.
>> )
>>
>> Since in git we're only considering newly branched files, I think in
>> this case "-t" will not add anything. In fact, what is being done here
>> is detecting exec bit changes from source to target files - we're not
>> trying to force P4 to use the source's exec bit. Do you agree?
>
> That sounds fine to me.  The code seemed to indicate that
> sometimes the destination file exists.

I've basically copied the code from the rename detection part and adapted it to copying. Nevertheless, even if git detects a copy to a already existing file I think that the integrate command should behave correctly. I should create a test case for this, though.

Show 14 quoted lines

>> +            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)
>
> If you're happy the dest never exists, you may be able to get rid
> of the edit step and the mode-change check entirely.  As long as
> you've tested this, you're the expert here.  The change makes
> sense overall.

I'm not that happy with this... and I'm no expert! I will really need to test this possibility. Just need to understand how I can make git detect a copy to an existing file... :)

Thanks for your feedback,

-- 
Vitor Antunes