Re: [PATCH] Teach git-p4 to ignore case in perforce filenames if configured.

From: Pete Wyckoff <pw@padd.com>
Date: Feb 11, 2011, 12:22 UTC
Message-ID: <20110211122253.GA14662@mew.padd.com>
In-Reply-To: <1297163499-18776-1-git-send-email-torarvid@gmail.com>

torarvid@gmail.com wrote on Tue, 08 Feb 2011 12:11 +0100:

Show 13 quoted lines

> When files are added to perforce, the path to that file has whichever case
> configuration that exists on the machine of the user who added the file.
> What does that mean? It means that when Alice adds a file
> 
> //depot/DirA/FileA.txt
> 
> ... and Bob adds:
> 
> //depot/dirA/FileB.txt
> 
> ... we may or may not get a problem. If a user sets the config variable
> git-p4.ignorecase to "true", we will consider //depot/DirA and //depot/dirA
> to be the same directory.

That's horrid. Seriously? A and B can both generate mixed-case paths, but with a different mix?

If it's all just cast to lower, does it make sense to use core.ignorecase for this?

Show 18 quoted lines

> ---
>  contrib/fast-import/git-p4     |   22 ++++++++++++++--------
>  contrib/fast-import/git-p4.txt |   12 ++++++++++++
>  2 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 04ce7e3..ca3cea0 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -452,6 +452,12 @@ def p4ChangesForPaths(depotPaths, changeRange):
>      changelist.sort()
>      return changelist
>  
> +def p4PathStartsWith(path, prefix):
> +    ignorecase = gitConfig("git-p4.ignorecase").lower()
> +    if ignorecase in ["true", "yes", "1"]:
> +        return path.lower().startswith(prefix.lower())
> +    return path.startswith(prefix)

git config --bool will always return "true" or "false" (or ""). I think we should start looking for "true" that rather than checking for all four possible versions of true (+ "on").

Can you put a comment in this function explaining the mixed-case problem? When reading the code, it's easier than searching through the doc to learn about it.

Rest of patch looks fine.

		-- Pete