Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25 August 2015 at 19:24, Luke Diamand l...@diamand.org 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 08/25/2015 08:54 AM, Luke Diamand wrote: On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com 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 l...@diamand.org wrote: On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com 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 larsxschnei...@gmail.com 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
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 10:33, Torsten Bögershausen tbo...@web.de wrote: On 08/25/2015 08:54 AM, Luke Diamand wrote: On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com 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 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 13:57, Luke Diamand l...@diamand.org 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
[PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
From: Lars Schneider larsxschnei...@gmail.com 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 larsxschnei...@gmail.com --- 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 + one/TWO/file3.txt + outside-spec/file4.txt + EOF
[PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
From: Lars Schneider larsxschnei...@gmail.com 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
