larsxschnei...@gmail.com writes:
> From: Lars Schneider <larsxschnei...@gmail.com>
>
> PROBLEM:
> We run P4 servers on Linux and P4 clients on Windows. For an unknown
> reason the file path for a number of files in P4 does not match the
> directory path with respect to case sensitivity.
>
> E.g. `p4 files` might return
> //depot/path/to/file1
> //depot/PATH/to/file2
>
> If you use P4/P4V then these files end up in the same directory, e.g.
> //depot/path/to/file1
> //depot/path/to/file2
>
> If you use git-p4 then all files not matching the correct file path
> (e.g. `file2`) will be ignored.
>
> SOLUTION:
> Identify path names that are different with respect to case sensitivity.
> If there are any then run `p4 dirs` to build up a dictionary
> containing the "correct" cases for each path. It looks like P4
> interprets "correct" here as the existing path of the first file in a
> directory. The path dictionary is used later on to fix all paths.
>
> This is only applied if the parameter "--ignore-path-case" is passed to
> the git-p4 clone command.
>
>
> Signed-off-by: Lars Schneider <larsxschnei...@gmail.com>
> ---
Thanks. A few comments.
- Have you checked "git log" on our history and notice how nobody
says "PROBLEM:" and "SOLUTION:" in capital letters? Don't try to
be original in the form; your contributions are already original
and valuable in the substance ;-)
- I think I saw v3 yesterday. It would be nice to see a brief
description of what has been updated in this version.
I do not do Perforce and am not familiar enough with this code to
judge myself. Will wait for area experts (you have them CC'ed, which
is good) to comment.
> diff --git a/git-p4.py b/git-p4.py
> index 073f87b..61a587b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1931,7 +1931,7 @@ class View(object):
> (self.client_prefix, clientFile))
> return clientFile[len(self.client_prefix):]
>
> - def update_client_spec_path_cache(self, files):
> + def update_client_spec_path_cache(self, files, fixPathCase = None):
I didn't check the remainder of the file, but I thought it is
customery *not* to have spaces around '=' when the parameter is
written with its default value in Python?
> diff --git a/t/t9821-git-p4-path-variations.sh
> b/t/t9821-git-p4-path-variations.sh
> ...
> +test_expect_success 'Clone the repo and WITHOUT path fixing' '
> + client_view "//depot/One/... //client/..." &&
> + git p4 clone --use-client-spec --destination="$git" //depot &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + # This method is used instead of "test -f" to ensure the case is
> + # checked even if the test is executed on case-insensitive file
> systems.
> + cat >expect <<-\EOF &&
> + two/File2.txt
> + EOF
I think we usually write the body of the indented here text
(i.e. "<<-") indented to the same level as the command and
delimiter, i.e.
cat >expect <<-\EOF &&
body of the here document line 1
body of the here document line 2
...
EOF
> + git ls-files >actual &&
> + test_cmp expect actual
> + )
> +'
I think you used to check the result with "find .", i.e. what the
filesystem actually recorded. "ls-files" gives what the index
records (i.e. to be committed if you were to make a new commit from
that index). They can be different in case on case-insensitive
filesystems, which I think are the ones that are most problematic
with the issue your patch is trying to address.
You are verifying what the set of "canonical" paths should be by
checking ls-files output. I think that is what you intended to do
(i.e. I am saying "I think the code is more correct than the earlier
round that used find"), but I just am double checking.
> +test_expect_success 'Add a new file and clone the repo WITH path fixing' '
> + client_view "//depot/... //client/..." &&
> + (
> + cd "$cli" &&
> +
> + mkdir -p One/two &&
A blank after 'cd' only in this one but not others. A blank line is
a good vehicle to convey that blocks of text above and below it are
logically separate, but use it consistently. I _think_ this blank
line can go.
> + >One/two/File0.txt &&
> + p4 add One/two/File0.txt &&
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
