Re: [PATCH 1/2] git-p4: Improve rename detection support.

From: Pete Wyckoff <pw@padd.com>
Date: Feb 6, 2011, 00:21 UTC
Message-ID: <20110206002113.GA31245@arf.padd.com>
In-Reply-To: <1296429563-18390-1-git-send-email-vitor.hda@gmail.com>

vitor.hda@gmail.com wrote on Sun, 30 Jan 2011 23:19 +0000:

> Only open files for edit after integrating if the SHA1 of source and destination
> differ from each other.
> Add git config option detectRenames to allow permanent rename detection. This
> options should be set to a true/false value.

I like the idea.

Show 9 quoted lines

>      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) and detectRenames.lower() != "false" > 0:
> +            diffOpts = "-M"
> +        else:
> +            diffOpts = ("", "-M")[self.detectRename]

The comparisons confuse me. detectRenames != "false" > 0 ? How about just detectRenames == "true"?

You could rename the existing self.detectRename to add an "s" so it all makes a bit more sense.

    if not self.detectRenames:
	# not explicitly set, check the config variable
	b = gitConfig("git-p4.detectRenames")
	if b == "true":
	    self.detectRenames = "-M"

    if self.detectRenames:
	diffOpts = "-M"
    else:
	diffOpts = ""

Show 13 quoted lines

>          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)

If you rename the file and also cause its perms to change (say add +x), does it still work if dest is not open?

		-- Pete