Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25 August 2015 at 19:24, Luke Diamand wrote: > On 25/08/15 14:14, Lars Schneider wrote: >>> >>> >>> So the choices are: >>> >>> 1. A new command-line option which would silently set core.ignorecase >>> 2. Users just have to know to set core.ignorecase manually before >>> using git-p4 (i.e. Lars' patch v5) >>> 3. Fix fast-import to take a --casefold option (but that's a much bigger >>> change) >>> >> I vote for 2 because that solves the problem consistently with the >> existing implementation for now. That means we don’t surprise git-p4 users. >> In addition I would try to fix (3), the —casefold option, in a separate >> patch. Although this (3) patch could take a bit as I have two more git-p4 >> patches in the queue that I want to propose to the mailing list first. > > > That works for me. Ack. > > Thanks! > Luke Lars - could you resubmit PATCHv5 as v6, with Acked-by from me added after the Signed-off-by: line. It then this from SubmittingPatches: > After the list reached a consensus that it is a good idea to apply the > patch, re-send it with "To:" set to the maintainer [*1*] and "cc:" the > list [*2*] for inclusion. Thanks Luke -- 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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25/08/15 14:14, Lars Schneider wrote: So the choices are: 1. A new command-line option which would silently set core.ignorecase 2. Users just have to know to set core.ignorecase manually before using git-p4 (i.e. Lars' patch v5) 3. Fix fast-import to take a --casefold option (but that's a much bigger change) I vote for 2 because that solves the problem consistently with the existing implementation for now. That means we don’t surprise git-p4 users. In addition I would try to fix (3), the —casefold option, in a separate patch. Although this (3) patch could take a bit as I have two more git-p4 patches in the queue that I want to propose to the mailing list first. That works for me. Ack. Thanks! Luke -- 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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25 Aug 2015, at 13:57, Luke Diamand wrote: > On 25/08/15 11:30, larsxschnei...@gmail.com wrote: > >> Unfortunately the command line option is not sufficient as the resulting >> paths are still messed up. I added the switch but it looks like as >> core.ignorecase does some additional magic on fast-import. You can see my >> changes here: >> https://github.com/larsxschneider/git/commit/b4399179ff542161c2c5b83c34c5b4901287ceb0 >> >> You can also run the unit tests to see the results here: >> https://github.com/larsxschneider/git/tree/lars/fix-path-v5-with-command-switch >> >> The only way I could image to fix that is to request every path from P4 as >> shown in my PATCH v4. This would be slow and the change would be rather huge. > > Yes, you're right - fast-import has special handling based on core.ignorecase. > > There was a thread a while back saying that it shouldn't do this, and > instead should have a new --casefold option, which would make more > sense, but isn't the case at present. > > http://www.spinics.net/lists/git/msg243264.html > >> I am curious: >> I run all my P4 -> git migrations on a Linux box with EXT4 and >> core.ignorecase=True. I did not realize that this might cause trouble. What >> could happen and what should I look out for? > > An obvious way it could go wrong would be if you had a a repo that > _did_ care about case (e.g. had Makefile and makefile in the same > directory) and you then tried to git-p4 clone a separate repo into a > different branch. In an ideal world, you would only use the > case-folding on the git-p4 based repo. I think while fast-import just > checks core.ignorecase, that's not possible. OK. Thanks for the explanation. > > So the choices are: > > 1. A new command-line option which would silently set core.ignorecase > 2. Users just have to know to set core.ignorecase manually before > using git-p4 (i.e. Lars' patch v5) > 3. Fix fast-import to take a --casefold option (but that's a much bigger > change) > I vote for 2 because that solves the problem consistently with the existing implementation for now. That means we don’t surprise git-p4 users. In addition I would try to fix (3), the —casefold option, in a separate patch. Although this (3) patch could take a bit as I have two more git-p4 patches in the queue that I want to propose to the mailing list first. - Lars-- 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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25/08/15 11:30, larsxschnei...@gmail.com wrote: > Unfortunately the command line option is not sufficient as the resulting > paths are still messed up. I added the switch but it looks like as > core.ignorecase does some additional magic on fast-import. You can see my > changes here: > https://github.com/larsxschneider/git/commit/b4399179ff542161c2c5b83c34c5b4901287ceb0 > > You can also run the unit tests to see the results here: > https://github.com/larsxschneider/git/tree/lars/fix-path-v5-with-command-switch > > The only way I could image to fix that is to request every path from P4 as > shown in my PATCH v4. This would be slow and the change would be rather huge. Yes, you're right - fast-import has special handling based on core.ignorecase. There was a thread a while back saying that it shouldn't do this, and instead should have a new --casefold option, which would make more sense, but isn't the case at present. http://www.spinics.net/lists/git/msg243264.html > I am curious: > I run all my P4 -> git migrations on a Linux box with EXT4 and > core.ignorecase=True. I did not realize that this might cause trouble. What > could happen and what should I look out for? An obvious way it could go wrong would be if you had a a repo that _did_ care about case (e.g. had Makefile and makefile in the same directory) and you then tried to git-p4 clone a separate repo into a different branch. In an ideal world, you would only use the case-folding on the git-p4 based repo. I think while fast-import just checks core.ignorecase, that's not possible. So the choices are: 1. A new command-line option which would silently set core.ignorecase 2. Users just have to know to set core.ignorecase manually before using git-p4 (i.e. Lars' patch v5) 3. Fix fast-import to take a --casefold option (but that's a much bigger change) Luke -- 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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25 Aug 2015, at 10:33, Torsten Bögershausen wrote: > On 08/25/2015 08:54 AM, Luke Diamand wrote: >> On 24/08/15 22:30, larsxschnei...@gmail.com wrote: >>> From: Lars Schneider >>> >>> Thanks to Luke Diamand I realized the core problem and propose here a >>> substiantially simpler fix to my PATCH v4. >>> >>> The test cases remain and prove the problem. In particular >>> "8 - Clone path (ignorecase)" and >>> "Add a new file and clone path with new file (ignorecase)" fail with the >>> current implementation on OS X and Linux. >> >> That's a lot simpler, thanks! >> >> Could we give this its own command line option and git config variable? >> >> Core.ignorecase gets set if the client is on a filing system that ignores >> case. This is slightly different - it squashes case in depot files for >> people with depots that have incorrectly jumbled-up case. >> >> Conflating the two seems like it would cause confusion at some point - for >> example, I have no idea how the rest of git behaves if core.ignorecase is >> set to True on a case-preserving file system. >> > That doesn't work as expected and is not allowed (or say strictly forbidden, > or strongly recommended not to do) > > Remembering older discussions about importers from foreign VCS: > This should work best for most people: > Look at the command line option, if no one is given, > look at core.ignorecase. > > So the command line option overrides core.ignorecase, > and the user can either run > --ignore-path-case > or > --no-ignore-path-case Unfortunately the command line option is not sufficient as the resulting paths are still messed up. I added the switch but it looks like as core.ignorecase does some additional magic on fast-import. You can see my changes here: https://github.com/larsxschneider/git/commit/b4399179ff542161c2c5b83c34c5b4901287ceb0 You can also run the unit tests to see the results here: https://github.com/larsxschneider/git/tree/lars/fix-path-v5-with-command-switch The only way I could image to fix that is to request every path from P4 as shown in my PATCH v4. This would be slow and the change would be rather huge. I am curious: I run all my P4 -> git migrations on a Linux box with EXT4 and core.ignorecase=True. I did not realize that this might cause trouble. What could happen and what should I look out for? One thing to keep in mind: This is a one time migration and we don’t develop on these Linux boxes. We usually develop on Windows and Mac. I just use the Linux boxes as migration workers as these scripts usually run a while. - Lars-- 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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 08/25/2015 08:54 AM, Luke Diamand wrote: On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular "8 - Clone path (ignorecase)" and "Add a new file and clone path with new file (ignorecase)" fail with the current implementation on OS X and Linux. That's a lot simpler, thanks! Could we give this its own command line option and git config variable? Core.ignorecase gets set if the client is on a filing system that ignores case. This is slightly different - it squashes case in depot files for people with depots that have incorrectly jumbled-up case. Conflating the two seems like it would cause confusion at some point - for example, I have no idea how the rest of git behaves if core.ignorecase is set to True on a case-preserving file system. That doesn't work as expected and is not allowed (or say strictly forbidden, or strongly recommended not to do) Remembering older discussions about importers from foreign VCS: This should work best for most people: Look at the command line option, if no one is given, look at core.ignorecase. So the command line option overrides core.ignorecase, and the user can either run --ignore-path-case or --no-ignore-path-case PS: Need to drop Eric from the list: the MX/A from from web.de problem still exists -- 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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25 Aug 2015, at 08:54, Luke Diamand wrote: > On 24/08/15 22:30, larsxschnei...@gmail.com wrote: >> From: Lars Schneider >> >> Thanks to Luke Diamand I realized the core problem and propose here a >> substiantially simpler fix to my PATCH v4. >> >> The test cases remain and prove the problem. In particular >> "8 - Clone path (ignorecase)" and >> "Add a new file and clone path with new file (ignorecase)" fail with the >> current implementation on OS X and Linux. > > That's a lot simpler, thanks! > > Could we give this its own command line option and git config variable? Can you outline the command line option you have in mind? > > Core.ignorecase gets set if the client is on a filing system that ignores > case. This is slightly different - it squashes case in depot files for people > with depots that have incorrectly jumbled-up case. Well, you can’t tell if the depots have incorrectly jumbled-up case. It could be intentional after all. Therefore I believe the “ignorecase” flag is a good fit because we explicitly state that we are not interested in case sensitivity on the client. > Conflating the two seems like it would cause confusion at some point - for > example, I have no idea how the rest of git behaves if core.ignorecase is set > to True on a case-preserving file system. > > It would probably be necessary to change p4PathStartsWith() to also check the > same flag. > > >> >> Lars Schneider (1): >> git-p4: Obey core.ignorecase when using P4 client specs. >> >> git-p4.py | 7 ++ >> t/t9821-git-p4-path-variations.sh | 200 >> ++ >> 2 files changed, 207 insertions(+) >> create mode 100755 t/t9821-git-p4-path-variations.sh >> >> -- >> 1.9.5 (Apple Git-50.3) >> > -- 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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular "8 - Clone path (ignorecase)" and "Add a new file and clone path with new file (ignorecase)" fail with the current implementation on OS X and Linux. That's a lot simpler, thanks! Could we give this its own command line option and git config variable? Core.ignorecase gets set if the client is on a filing system that ignores case. This is slightly different - it squashes case in depot files for people with depots that have incorrectly jumbled-up case. Conflating the two seems like it would cause confusion at some point - for example, I have no idea how the rest of git behaves if core.ignorecase is set to True on a case-preserving file system. It would probably be necessary to change p4PathStartsWith() to also check the same flag. Lars Schneider (1): git-p4: Obey core.ignorecase when using P4 client specs. git-p4.py | 7 ++ t/t9821-git-p4-path-variations.sh | 200 ++ 2 files changed, 207 insertions(+) create mode 100755 t/t9821-git-p4-path-variations.sh -- 1.9.5 (Apple Git-50.3) -- 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
[PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
From: Lars Schneider 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 and clone the code via client spec "//depot/path/..." then all files not matching the case in the client spec will be ignored (in the example above "file2"). This is correct if core.ignorecase=false but not otherwise. Signed-off-by: Lars Schneider --- git-p4.py | 7 ++ t/t9821-git-p4-path-variations.sh | 200 ++ 2 files changed, 207 insertions(+) create mode 100755 t/t9821-git-p4-path-variations.sh diff --git a/git-p4.py b/git-p4.py index 073f87b..0093fa3 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1950,10 +1950,14 @@ class View(object): if "unmap" in res: # it will list all of them, but only one not unmap-ped continue +if gitConfigBool("core.ignorecase"): +res['depotFile'] = res['depotFile'].lower() self.client_spec_path_cache[res['depotFile']] = self.convert_client_path(res["clientFile"]) # not found files or unmap files set to "" for depotFile in fileArgs: +if gitConfigBool("core.ignorecase"): +depotFile = depotFile.lower() if depotFile not in self.client_spec_path_cache: self.client_spec_path_cache[depotFile] = "" @@ -1962,6 +1966,9 @@ class View(object): depot file should live. Returns "" if the file should not be mapped in the client.""" +if gitConfigBool("core.ignorecase"): +depot_path = depot_path.lower() + if depot_path in self.client_spec_path_cache: return self.client_spec_path_cache[depot_path] diff --git a/t/t9821-git-p4-path-variations.sh b/t/t9821-git-p4-path-variations.sh new file mode 100755 index 000..599d16c --- /dev/null +++ b/t/t9821-git-p4-path-variations.sh @@ -0,0 +1,200 @@ +#!/bin/sh + +test_description='Clone repositories with path case variations' + +. ./lib-git-p4.sh + +test_expect_success 'start p4d with case folding enabled' ' + start_p4d -C1 +' + +test_expect_success 'Create a repo with path case variations' ' + client_view "//depot/... //client/..." && + ( + cd "$cli" && + + mkdir -p One/two && + >One/two/File2.txt && + p4 add One/two/File2.txt && + p4 submit -d "Add file2" && + rm -rf One && + + mkdir -p one/TWO && + >one/TWO/file1.txt && + p4 add one/TWO/file1.txt && + p4 submit -d "Add file1" && + rm -rf one && + + mkdir -p one/two && + >one/two/file3.txt && + p4 add one/two/file3.txt && + p4 submit -d "Add file3" && + rm -rf one && + + mkdir -p outside-spec && + >outside-spec/file4.txt && + p4 add outside-spec/file4.txt && + p4 submit -d "Add file4" && + rm -rf outside-spec + ) +' + +test_expect_success 'Clone root' ' + client_view "//depot/... //client/..." && + test_when_finished cleanup_git && + ( + cd "$git" && + git init . && + git config core.ignorecase false && + git p4 clone --use-client-spec --destination="$git" //depot && + # 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. + # All files are there as expected although the path cases look random. + cat >expect <<-\EOF && + One/two/File2.txt + one/TWO/file1.txt + one/two/file3.txt + outside-spec/file4.txt + EOF + git ls-files >actual && + test_cmp expect actual + ) +' + +test_expect_success 'Clone root (ignorecase)' ' + client_view "//depot/... //client/..." && + test_when_finished cleanup_git && + ( + cd "$git" && + git init . && + git config core.ignorecase true && + git p4 clone --use-client-spec --destination="$git" //depot && + # 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. + # All files are there as expected although the path cases look random. + cat >expect <<-\EOF && + one/TWO/File2.txt + one/TWO/file1.txt +
[PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
From: Lars Schneider Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular "8 - Clone path (ignorecase)" and "Add a new file and clone path with new file (ignorecase)" fail with the current implementation on OS X and Linux. Lars Schneider (1): git-p4: Obey core.ignorecase when using P4 client specs. git-p4.py | 7 ++ t/t9821-git-p4-path-variations.sh | 200 ++ 2 files changed, 207 insertions(+) create mode 100755 t/t9821-git-p4-path-variations.sh -- 1.9.5 (Apple Git-50.3) -- 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