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
