Re: [PATCH] git-p4: add failing tests for case-folding p4d

[Luke Diamand <luke@diamand.org>]

(Adding Pete, Vitor, and Fusion in case they have any thoughts on 
working with P4 servers that do case-folding, or at least failing 
gracefully).

On 29/04/15 00:01, Lex Spoon wrote:
> The last comment in the test took me a minute to decipher. I would
> suggest "no repo path called LC" instead of "no repo called LC". Also,
> it would have helped me to either have a little comment on the "UC"
> version of the test, or to make the previous comment a little more
> neutral so that it will apply to both test cases.

OK, thanks!

>
> Otherwise, while I am not a regular maintainer of this code, the patch
> does LGTM. Certainly it's good to have more test coverage.
>
> For the underlying problem, I haven't thought about it very much, but
> it looks like a plausible first step might be to simply probe the
> given file name and see if it comes back the same way. If it comes
> back differently, then maybe the command should abort?

I think the problem may be a bit trickier than that.

I think what's happening when cloning is that when files come back from 
the server, git-p4 checks that they are contained within the directory 
it is cloning. This happens in p4StartsWith(), (called from 
extractFilesFromCommit()) which already tries to fix this problem by 
checking 'core.ignorecase'. However, that won't work if the local 
machine is case sensitive but the server isn't (e.g. Linux client, 
Windows server).

git-p4 does this because it's fetching *commits* from Perforce, and a 
commit might have files that are outside the directory being cloned.

I tried teaching p4StartsWith() to ask the server if it is case-folding 
('p4 info') and that then means that the git-p4 clone actually succeeds. 
However, git-p4 submit then fails because it gets terribly confused 
about pathnames - it probably needs to do some lowercasing somewhere. So 
that might be worth pursuing.

Open to other suggestions though!

>
>
> What a tough problem all around...

Indeed!

Luke