{"thread":{"id":"49770","subject":"[PATCH 0/1] mingw: handle absolute paths in expand_user_path()","startedAt":"2018-11-06T14:53:19Z","lastAt":"2021-07-28T17:08:48Z","messageCount":53,"participants":["Johannes Schindelin via GitGitGadget","Ramsay Jones","Duy Nguyen","Johannes Sixt","Junio C Hamano","Johannes Schindelin","Jeff King","Eric Sunshine","Joseph Moisan","Fabian Stelzer"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"362575","messageId":"pull.66.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":null,"subject":"[PATCH 0/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-06T14:53:16Z","receivedAt":"2018-11-06T14:53:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"While this patch has been \"in production\" in Git for Windows for a good\nwhile, this patch series is half meant as a request for comments.\n\nThe reason is this: something like this (make paths specified e.g. via \nhttp.sslCAInfo relative to the runtime prefix) is potentially useful also in\nthe non-Windows context, as long as Git was built with the runtime prefix\nfeature.\n\nThe main problem with non-Windows platforms is that paths starting with a\nsingle slash are unambiguously absolute, whereas we can say with certainty\nthat they are not absolute Windows paths.\n\nSo maybe someone on this list has a clever idea how we could specify paths\n(unambiguously, even on non-Windows platforms) that Git should interpret as\nrelative to the runtime prefix?\n\nJohannes Schindelin (1):\n mingw: handle absolute paths in expand_user_path()\n\n path.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\n\nbase-commit: cd69ec8cde54af1817630331fc441f493866f0d4\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-66%2Fdscho%2Fmingw-expand-absolute-user-path-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-66/dscho/mingw-expand-absolute-user-path-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/66\n-- \ngitgitgadget\n"},{"id":"362576","messageId":"2287dd96cf0b9e9e250fdf92a32dcf666510e67d.1541515994.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-11-06T14:53:17Z","receivedAt":"2018-11-06T14:53:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOn Windows, an absolute POSIX path needs to be turned into a Windows\none.\n\nSigned-off-by: Johannes Schindelin \n---\n path.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/path.c b/path.c\nindex 34f0f98349..a72abf0e1f 100644\n--- a/path.c\n+++ b/path.c\n@@ -11,6 +11,7 @@\n #include \"path.h\"\n #include \"packfile.h\"\n #include \"object-store.h\"\n+#include \"exec-cmd.h\"\n \n static int get_st_mode_bits(const char *path, int *mode)\n {\n@@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n \n \tif (path == NULL)\n \t\tgoto return_null;\n+#ifdef __MINGW32__\n+\tif (path[0] == '/')\n+\t\treturn system_path(path + 1);\n+#endif\n \tif (path[0] == '~') {\n \t\tconst char *first_slash = strchrnul(path, '/');\n \t\tconst char *username = path + 1;\n-- \ngitgitgadget\n"},{"id":"362580","messageId":"9174a750-3498-c2fc-d7fa-29c1926c95fc@ramsayjones.plus.com","threadId":"49770","inReplyTo":"2287dd96cf0b9e9e250fdf92a32dcf666510e67d.1541515994.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-11-06T15:54:47Z","receivedAt":"2018-11-06T15:54:51Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 06/11/2018 14:53, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> On Windows, an absolute POSIX path needs to be turned into a Windows\n> one.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> path.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/path.c b/path.c\n> index 34f0f98349..a72abf0e1f 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -11,6 +11,7 @@\n> #include \"path.h\"\n> #include \"packfile.h\"\n> #include \"object-store.h\"\n> +#include \"exec-cmd.h\"\n> \n> static int get_st_mode_bits(const char *path, int *mode)\n> {\n> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n> \n> \tif (path == NULL)\n> \t\tgoto return_null;\n> +#ifdef __MINGW32__\n> +\tif (path[0] == '/')\n> +\t\treturn system_path(path + 1);\n> +#endif\n\nHmm, this doesn't quite fit with the intended use of this\nfunction! ;-) (even on windows!)\n\nI haven't looked very deeply, but doesn't this affect all\nabsolute paths in the config read by git_config_pathname(),\nalong with all 'included config' files?\n\nI am pretty sure that I would not want the absolute paths\nin my config file(s) magically 'moved' depending on whether\ngit has been compiled with 'runtime prefix' support or not!\n\nATB,\nRamsay Jones\n\n> \tif (path[0] == '~') {\n> \t\tconst char *first_slash = strchrnul(path, '/');\n> \t\tconst char *username = path + 1;\n> \n"},{"id":"362584","messageId":"48e90389-399e-a727-a1f3-965bf0f32f18@ramsayjones.plus.com","threadId":"49770","inReplyTo":"9174a750-3498-c2fc-d7fa-29c1926c95fc@ramsayjones.plus.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-11-06T16:10:38Z","receivedAt":"2018-11-06T16:10:43Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 06/11/2018 15:54, Ramsay Jones wrote:\n> \n> \n> On 06/11/2018 14:53, Johannes Schindelin via GitGitGadget wrote:\n>> From: Johannes Schindelin \n>>\n>> On Windows, an absolute POSIX path needs to be turned into a Windows\n>> one.\n>>\n>> Signed-off-by: Johannes Schindelin \n>> ---\n>> path.c | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/path.c b/path.c\n>> index 34f0f98349..a72abf0e1f 100644\n>> --- a/path.c\n>> +++ b/path.c\n>> @@ -11,6 +11,7 @@\n>> #include \"path.h\"\n>> #include \"packfile.h\"\n>> #include \"object-store.h\"\n>> +#include \"exec-cmd.h\"\n>> \n>> static int get_st_mode_bits(const char *path, int *mode)\n>> {\n>> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n>> \n>> \tif (path == NULL)\n>> \t\tgoto return_null;\n>> +#ifdef __MINGW32__\n>> +\tif (path[0] == '/')\n>> +\t\treturn system_path(path + 1);\n>> +#endif\n> \n> Hmm, this doesn't quite fit with the intended use of this\n> function! ;-) (even on windows!)\n> \n> I haven't looked very deeply, but doesn't this affect all\n> absolute paths in the config read by git_config_pathname(),\n> along with all 'included config' files?\n> \n> I am pretty sure that I would not want the absolute paths\n> in my config file(s) magically 'moved' depending on whether\n> git has been compiled with 'runtime prefix' support or not!\n\nSo, I hit 'send' before finishing my thought ...\n\nI can't think of a non-backwards compatible way of doing\nwhat you want. If backward compatibility wasn't an issue,\nthen we could (maybe) have used some kind of pathname prefix\nlike 'system:/path/relative/to/git/executable', or somesuch.\n\nATB,\nRamsay Jones\n\n"},{"id":"362597","messageId":"CACsJy8D47sDgdpmPSqVbBu7Omc1=4yAn895-PE91-PLs9+83bw@mail.gmail.com","threadId":"49770","inReplyTo":"2287dd96cf0b9e9e250fdf92a32dcf666510e67d.1541515994.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-06T18:24:59Z","receivedAt":"2018-11-06T18:25:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Nov 6, 2018 at 3:55 PM Johannes Schindelin via GitGitGadget\n wrote:\n>\n> From: Johannes Schindelin \n>\n> On Windows, an absolute POSIX path needs to be turned into a Windows\n> one.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> path.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/path.c b/path.c\n> index 34f0f98349..a72abf0e1f 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -11,6 +11,7 @@\n> #include \"path.h\"\n> #include \"packfile.h\"\n> #include \"object-store.h\"\n> +#include \"exec-cmd.h\"\n>\n> static int get_st_mode_bits(const char *path, int *mode)\n> {\n> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n>\n> if (path == NULL)\n> goto return_null;\n> +#ifdef __MINGW32__\n> + if (path[0] == '/')\n> + return system_path(path + 1);\n> +#endif\n\nShould this behavior be documented somewhere, maybe in config.txt?\n\n> if (path[0] == '~') {\n> const char *first_slash = strchrnul(path, '/');\n> const char *username = path + 1;\n> --\n> gitgitgadget\n-- \nDuy\n"},{"id":"362598","messageId":"CACsJy8DmyU_Kn4yytu_PQdpppXO8wLcuuzQ-fjwnyjA0ntB2Dw@mail.gmail.com","threadId":"49770","inReplyTo":"48e90389-399e-a727-a1f3-965bf0f32f18@ramsayjones.plus.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-06T18:27:06Z","receivedAt":"2018-11-06T18:27:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Nov 6, 2018 at 7:15 PM Ramsay Jones wrote:\n> >> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n> >>\n> >> if (path == NULL)\n> >> goto return_null;\n> >> +#ifdef __MINGW32__\n> >> + if (path[0] == '/')\n> >> + return system_path(path + 1);\n> >> +#endif\n> >\n> > Hmm, this doesn't quite fit with the intended use of this\n> > function! ;-) (even on windows!)\n> >\n> > I haven't looked very deeply, but doesn't this affect all\n> > absolute paths in the config read by git_config_pathname(),\n> > along with all 'included config' files?\n> >\n> > I am pretty sure that I would not want the absolute paths\n> > in my config file(s) magically 'moved' depending on whether\n> > git has been compiled with 'runtime prefix' support or not!\n>\n> So, I hit 'send' before finishing my thought ...\n>\n> I can't think of a non-backwards compatible way of doing\n> what you want. If backward compatibility wasn't an issue,\n> then we could (maybe) have used some kind of pathname prefix\n> like 'system:/path/relative/to/git/executable', or somesuch.\n\nA pseudo variable might work, like $ROOT/path/relative/to/somewhere\n-- \nDuy\n"},{"id":"362614","messageId":"a374e4bb-1970-9ec7-fe94-a273f1206d6b@kdbg.org","threadId":"49770","inReplyTo":"2287dd96cf0b9e9e250fdf92a32dcf666510e67d.1541515994.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-06T21:32:38Z","receivedAt":"2018-11-06T21:32:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 06.11.18 um 15:53 schrieb Johannes Schindelin via GitGitGadget:\n> From: Johannes Schindelin \n> \n> On Windows, an absolute POSIX path needs to be turned into a Windows\n> one.\n\nIf I were picky, I would say that in a pure Windows application there \ncannot be POSIX paths to begin with.\n\nEven if a path looks like a POSIX paths, i.e. it starts with a directory \nseparator, but not with drive-letter-colon, it still has a particular \nmeaning, namely (as far as I know) that the path is anchored at the root \nof the drive of the current working directory.\n\nIf a user specifies such a path on Windows, and it must undergo \nexpand_user_path(), then that is a user error, or the user knows what \nthey are doing.\n\nI think it is wrong to interpret such a path as relative to some random \nother directory, like this patch seems to do.\n\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> path.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/path.c b/path.c\n> index 34f0f98349..a72abf0e1f 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -11,6 +11,7 @@\n> #include \"path.h\"\n> #include \"packfile.h\"\n> #include \"object-store.h\"\n> +#include \"exec-cmd.h\"\n> \n> static int get_st_mode_bits(const char *path, int *mode)\n> {\n> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n> \n> \tif (path == NULL)\n> \t\tgoto return_null;\n> +#ifdef __MINGW32__\n> +\tif (path[0] == '/')\n> +\t\treturn system_path(path + 1);\n> +#endif\n> \tif (path[0] == '~') {\n> \t\tconst char *first_slash = strchrnul(path, '/');\n> \t\tconst char *username = path + 1;\n> \n\n"},{"id":"362625","messageId":"xmqqo9b1d6na.fsf@gitster-ct.c.googlers.com","threadId":"49770","inReplyTo":"9174a750-3498-c2fc-d7fa-29c1926c95fc@ramsayjones.plus.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-07T01:21:29Z","receivedAt":"2018-11-07T01:21:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> On 06/11/2018 14:53, Johannes Schindelin via GitGitGadget wrote:\n>> From: Johannes Schindelin \n>> \n>> On Windows, an absolute POSIX path needs to be turned into a Windows\n>> one.\n>> \n>> Signed-off-by: Johannes Schindelin \n>> ---\n>> path.c | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>> \n>> diff --git a/path.c b/path.c\n>> index 34f0f98349..a72abf0e1f 100644\n>> --- a/path.c\n>> +++ b/path.c\n>> @@ -11,6 +11,7 @@\n>> #include \"path.h\"\n>> #include \"packfile.h\"\n>> #include \"object-store.h\"\n>> +#include \"exec-cmd.h\"\n>> \n>> static int get_st_mode_bits(const char *path, int *mode)\n>> {\n>> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n>> \n>> \tif (path == NULL)\n>> \t\tgoto return_null;\n>> +#ifdef __MINGW32__\n>> +\tif (path[0] == '/')\n>> +\t\treturn system_path(path + 1);\n>> +#endif\n>\n> Hmm, this doesn't quite fit with the intended use of this\n> function! ;-) (even on windows!)\n>\n> I haven't looked very deeply, but doesn't this affect all\n> absolute paths in the config read by git_config_pathname(),\n> along with all 'included config' files?\n\nI think so. I have not thought things through to say if replacing a\n\"full path in the current drive\" with system_path() is a sensible\nthing to do in the first place, but I am getting the impression from\nreview comments that it probably is not.\n\n> I am pretty sure that I would not want the absolute paths\n> in my config file(s) magically 'moved' depending on whether\n> git has been compiled with 'runtime prefix' support or not!\n\nIn any case, the helper is about expanding ~/foo and ~who/foo to\nabsolute paths, without touching other paths, so it is a wrong\nfunction to implement it in, even if the motivation were sensible.\n"},{"id":"362643","messageId":"nycvar.QRO.7.76.6.1811071217280.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"xmqqo9b1d6na.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-07T11:19:22Z","receivedAt":"2018-11-07T11:19:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Nov 2018, Junio C Hamano wrote:\n\n> Ramsay Jones writes:\n> \n> > On 06/11/2018 14:53, Johannes Schindelin via GitGitGadget wrote:\n> >> From: Johannes Schindelin \n> >> \n> >> On Windows, an absolute POSIX path needs to be turned into a Windows\n> >> one.\n> >> \n> >> Signed-off-by: Johannes Schindelin \n> >> ---\n> >> path.c | 5 +++++\n> >> 1 file changed, 5 insertions(+)\n> >> \n> >> diff --git a/path.c b/path.c\n> >> index 34f0f98349..a72abf0e1f 100644\n> >> --- a/path.c\n> >> +++ b/path.c\n> >> @@ -11,6 +11,7 @@\n> >> #include \"path.h\"\n> >> #include \"packfile.h\"\n> >> #include \"object-store.h\"\n> >> +#include \"exec-cmd.h\"\n> >> \n> >> static int get_st_mode_bits(const char *path, int *mode)\n> >> {\n> >> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n> >> \n> >> \tif (path == NULL)\n> >> \t\tgoto return_null;\n> >> +#ifdef __MINGW32__\n> >> +\tif (path[0] == '/')\n> >> +\t\treturn system_path(path + 1);\n> >> +#endif\n> >\n> > Hmm, this doesn't quite fit with the intended use of this\n> > function! ;-) (even on windows!)\n> >\n> > I haven't looked very deeply, but doesn't this affect all\n> > absolute paths in the config read by git_config_pathname(),\n> > along with all 'included config' files?\n> \n> I think so. I have not thought things through to say if replacing a\n> \"full path in the current drive\" with system_path() is a sensible\n> thing to do in the first place, but I am getting the impression from\n> review comments that it probably is not.\n> \n> > I am pretty sure that I would not want the absolute paths\n> > in my config file(s) magically 'moved' depending on whether\n> > git has been compiled with 'runtime prefix' support or not!\n\nThe cute thing is: your absolute paths would not be moved because we are\ntalking about Windows. Therefore your absolute paths would not start with\na forward slash.\n\n> In any case, the helper is about expanding ~/foo and ~who/foo to\n> absolute paths, without touching other paths, so it is a wrong\n> function to implement it in, even if the motivation were sensible.\n\nIt could be renamed. In any case, for this feature we would need to expand\na path that is not the final path, and this here location is the most\nlogical place to do so.\n\nCiao,\nDscho\n"},{"id":"362644","messageId":"nycvar.QRO.7.76.6.1811071219350.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"CACsJy8D47sDgdpmPSqVbBu7Omc1=4yAn895-PE91-PLs9+83bw@mail.gmail.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-07T11:19:57Z","receivedAt":"2018-11-07T11:20:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Nov 2018, Duy Nguyen wrote:\n\n> On Tue, Nov 6, 2018 at 3:55 PM Johannes Schindelin via GitGitGadget\n> wrote:\n> >\n> > From: Johannes Schindelin \n> >\n> > On Windows, an absolute POSIX path needs to be turned into a Windows\n> > one.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > path.c | 5 +++++\n> > 1 file changed, 5 insertions(+)\n> >\n> > diff --git a/path.c b/path.c\n> > index 34f0f98349..a72abf0e1f 100644\n> > --- a/path.c\n> > +++ b/path.c\n> > @@ -11,6 +11,7 @@\n> > #include \"path.h\"\n> > #include \"packfile.h\"\n> > #include \"object-store.h\"\n> > +#include \"exec-cmd.h\"\n> >\n> > static int get_st_mode_bits(const char *path, int *mode)\n> > {\n> > @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n> >\n> > if (path == NULL)\n> > goto return_null;\n> > +#ifdef __MINGW32__\n> > + if (path[0] == '/')\n> > + return system_path(path + 1);\n> > +#endif\n> \n> Should this behavior be documented somewhere, maybe in config.txt?\n\nFirst we need to find a consensus how this should work.\n\nCiao,\nDscho\n\n> \n> > if (path[0] == '~') {\n> > const char *first_slash = strchrnul(path, '/');\n> > const char *username = path + 1;\n> > --\n> > gitgitgadget\n> -- \n> Duy\n> \n"},{"id":"362645","messageId":"nycvar.QRO.7.76.6.1811071222200.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"a374e4bb-1970-9ec7-fe94-a273f1206d6b@kdbg.org","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-07T11:23:16Z","receivedAt":"2018-11-07T11:23:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Hannes,\n\nOn Tue, 6 Nov 2018, Johannes Sixt wrote:\n\n> Am 06.11.18 um 15:53 schrieb Johannes Schindelin via GitGitGadget:\n> > From: Johannes Schindelin \n> > \n> > On Windows, an absolute POSIX path needs to be turned into a Windows\n> > one.\n> \n> If I were picky, I would say that in a pure Windows application there cannot\n> be POSIX paths to begin with.\n> \n> Even if a path looks like a POSIX paths, i.e. it starts with a directory\n> separator, but not with drive-letter-colon, it still has a particular meaning,\n> namely (as far as I know) that the path is anchored at the root of the drive\n> of the current working directory.\n> \n> If a user specifies such a path on Windows, and it must undergo\n> expand_user_path(), then that is a user error, or the user knows what they are\n> doing.\n> \n> I think it is wrong to interpret such a path as relative to some random other\n> directory, like this patch seems to do.\n\nOkay, now we know everything you find wrong with the current patch. Do you\nhave any suggestion how to make it right? I.e. what would you suggest as a\nway to specify in a gitconfig in a portable Git where the certificate\nbundle is?\n\nThanks,\nDscho\n\n> \n> > \n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > path.c | 5 +++++\n> > 1 file changed, 5 insertions(+)\n> > \n> > diff --git a/path.c b/path.c\n> > index 34f0f98349..a72abf0e1f 100644\n> > --- a/path.c\n> > +++ b/path.c\n> > @@ -11,6 +11,7 @@\n> > #include \"path.h\"\n> > #include \"packfile.h\"\n> > #include \"object-store.h\"\n> > +#include \"exec-cmd.h\"\n> > \n> > static int get_st_mode_bits(const char *path, int *mode)\n> > {\n> > @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)\n> > \n> > if (path == NULL)\n> > \t\tgoto return_null;\n> > +#ifdef __MINGW32__\n> > +\tif (path[0] == '/')\n> > +\t\treturn system_path(path + 1);\n> > +#endif\n> > if (path[0] == '~') {\n> > const char *first_slash = strchrnul(path, '/');\n> > const char *username = path + 1;\n> > \n> \n> \n> \n"},{"id":"362667","messageId":"d7a70226-3441-76c4-df6a-e8fb32249f27@ramsayjones.plus.com","threadId":"49770","inReplyTo":"nycvar.QRO.7.76.6.1811071217280.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-11-07T17:42:00Z","receivedAt":"2018-11-07T17:42:05Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 07/11/2018 11:19, Johannes Schindelin wrote:\n[snip]\n>>> Hmm, this doesn't quite fit with the intended use of this\n>>> function! ;-) (even on windows!)\n>>>\n>>> I haven't looked very deeply, but doesn't this affect all\n>>> absolute paths in the config read by git_config_pathname(),\n>>> along with all 'included config' files?\n>>\n>> I think so. I have not thought things through to say if replacing a\n>> \"full path in the current drive\" with system_path() is a sensible\n>> thing to do in the first place, but I am getting the impression from\n>> review comments that it probably is not.\n>>\n>>> I am pretty sure that I would not want the absolute paths\n>>> in my config file(s) magically 'moved' depending on whether\n>>> git has been compiled with 'runtime prefix' support or not!\n> \n> The cute thing is: your absolute paths would not be moved because we are\n> talking about Windows. Therefore your absolute paths would not start with\n> a forward slash.\n\nAh, sorry, I must have misunderstood a comment in your cover letter:\n\n The reason is this: something like this (make paths specified e.g. via \n http.sslCAInfo relative to the runtime prefix) is potentially useful\n also in the non-Windows context, as long as Git was built with the\n runtime prefix feature.\n\n... so I thought you meant to add this code for POSIX systems as well.\n\nMy mistake. :(\n\nATB,\nRamsay Jones\n\n\n"},{"id":"362668","messageId":"efd57458-07f6-2813-483b-dc7fba785dc0@kdbg.org","threadId":"49770","inReplyTo":"nycvar.QRO.7.76.6.1811071222200.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-07T18:52:28Z","receivedAt":"2018-11-07T18:52:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 07.11.18 um 12:23 schrieb Johannes Schindelin:\n> On Tue, 6 Nov 2018, Johannes Sixt wrote:\n> \n>> Am 06.11.18 um 15:53 schrieb Johannes Schindelin via GitGitGadget:\n>> Even if a path looks like a POSIX paths, i.e. it starts with a directory\n>> separator, but not with drive-letter-colon, it still has a particular meaning,\n>> namely (as far as I know) that the path is anchored at the root of the drive\n>> of the current working directory.\n>>\n>> If a user specifies such a path on Windows, and it must undergo\n>> expand_user_path(), then that is a user error, or the user knows what they are\n>> doing.\n>>\n>> I think it is wrong to interpret such a path as relative to some random other\n>> directory, like this patch seems to do.\n> \n> Okay, now we know everything you find wrong with the current patch. Do you\n> have any suggestion how to make it right? I.e. what would you suggest as a\n> way to specify in a gitconfig in a portable Git where the certificate\n> bundle is?\n\nAh, so your actual problem is quite a different one!\n\nDo I understand correctly, that you use a leading slash as an indicator \nto construct a path relative to system_path(). How about a \"reserved\" \nuser name? For example,\n\n [http] sslcert = ~system_path/what/ever\n\nalthough a more unique name, perhaps with some punctuation, may be \ndesirable.\n\n-- Hannes\n"},{"id":"362670","messageId":"20181107204142.GA30078@sigill.intra.peff.net","threadId":"49770","inReplyTo":"efd57458-07f6-2813-483b-dc7fba785dc0@kdbg.org","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-07T20:41:42Z","receivedAt":"2018-11-07T20:41:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 07, 2018 at 07:52:28PM +0100, Johannes Sixt wrote:\n\n> > Okay, now we know everything you find wrong with the current patch. Do you\n> > have any suggestion how to make it right? I.e. what would you suggest as a\n> > way to specify in a gitconfig in a portable Git where the certificate\n> > bundle is?\n> \n> Ah, so your actual problem is quite a different one!\n> \n> Do I understand correctly, that you use a leading slash as an indicator to\n> construct a path relative to system_path(). How about a \"reserved\" user\n> name? For example,\n> \n> [http] sslcert = ~system_path/what/ever\n> \n> although a more unique name, perhaps with some punctuation, may be\n> desirable.\n\nIt's syntactically a bit further afield, but something like:\n\n [http]\n sslcert = $RUNTIME_PREFIX/what/ever\n\nwould make sense to me, and is a bit less subtle than the fake user. I\ndon't know if that would confuse people into thinking that we\ninterpolate arbitrary environment variables, though.\n\n-Peff\n"},{"id":"362673","messageId":"e7ff6f22-fe5a-3cca-9305-2c8a6fb55d45@kdbg.org","threadId":"49770","inReplyTo":"20181107204142.GA30078@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-11-07T21:36:52Z","receivedAt":"2018-11-07T21:36:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 07.11.18 um 21:41 schrieb Jeff King:\n> On Wed, Nov 07, 2018 at 07:52:28PM +0100, Johannes Sixt wrote:\n>> Do I understand correctly, that you use a leading slash as an indicator to\n>> construct a path relative to system_path(). How about a \"reserved\" user\n>> name? For example,\n>>\n>> [http] sslcert = ~system_path/what/ever\n>>\n>> although a more unique name, perhaps with some punctuation, may be\n>> desirable.\n> \n> It's syntactically a bit further afield, but something like:\n> \n> [http]\n> sslcert = $RUNTIME_PREFIX/what/ever\n> \n> would make sense to me, and is a bit less subtle than the fake user. I\n> don't know if that would confuse people into thinking that we\n> interpolate arbitrary environment variables, though.\n\nThe expansion of a fake user name would have to go in \nexpand_user_path(), a fake variable name would have to be expanded \nelsewhere. Now, Dscho mentions in the cover letter that his patch has \nalready seen some field testing (\"has been 'in production' for a good \nwhile\"). So, we have gained some confidence that the point where the \nsubstitution happens, in expand_user_path(), is suitable. Therefore, I \nhave slight preference for a fake user.\n\n-- Hannes\n"},{"id":"362674","messageId":"20181107220320.GA8970@sigill.intra.peff.net","threadId":"49770","inReplyTo":"e7ff6f22-fe5a-3cca-9305-2c8a6fb55d45@kdbg.org","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-07T22:03:20Z","receivedAt":"2018-11-07T22:03:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 07, 2018 at 10:36:52PM +0100, Johannes Sixt wrote:\n\n> Am 07.11.18 um 21:41 schrieb Jeff King:\n> > On Wed, Nov 07, 2018 at 07:52:28PM +0100, Johannes Sixt wrote:\n> > > Do I understand correctly, that you use a leading slash as an indicator to\n> > > construct a path relative to system_path(). How about a \"reserved\" user\n> > > name? For example,\n> > > \n> > > [http] sslcert = ~system_path/what/ever\n> > > \n> > > although a more unique name, perhaps with some punctuation, may be\n> > > desirable.\n> > \n> > It's syntactically a bit further afield, but something like:\n> > \n> > [http]\n> > sslcert = $RUNTIME_PREFIX/what/ever\n> > \n> > would make sense to me, and is a bit less subtle than the fake user. I\n> > don't know if that would confuse people into thinking that we\n> > interpolate arbitrary environment variables, though.\n> \n> The expansion of a fake user name would have to go in expand_user_path(), a\n> fake variable name would have to be expanded elsewhere. Now, Dscho mentions\n> in the cover letter that his patch has already seen some field testing (\"has\n> been 'in production' for a good while\"). So, we have gained some confidence\n> that the point where the substitution happens, in expand_user_path(), is\n> suitable. Therefore, I have slight preference for a fake user.\n\nI don't think that necessarily needs to limit us. expand_user_path() is\nnamed that because right now it just expands \"~\". But there's no reason\nit couldn't be used for general expansion. The problem in the original\npatch is that it expands _even when the user has not asked us to do it_.\nLooking at the callers, most of them would be fine with the new\nexpansion (the only exception is the one in daemon.c, though it manually\nchecks for \"~\" already).\n\nNow I agree that the new function would probably need a better name, at\nwhich point you could easily have a function expand_path() which just\nwraps expand_user_path().\n\nAll that said, if we're just interested in allowing this for config,\nthen we already have such a wrapper function: git_config_pathname().\n\nSo I don't think it's a big deal to implement it in any of these ways.\nIt's much more important to get the syntax right, because that's\nuser-facing and will be with us forever.\n\n-Peff\n"},{"id":"362681","messageId":"xmqqr2fwa0ew.fsf@gitster-ct.c.googlers.com","threadId":"49770","inReplyTo":"d7a70226-3441-76c4-df6a-e8fb32249f27@ramsayjones.plus.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-08T00:16:39Z","receivedAt":"2018-11-08T00:16:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n>> The cute thing is: your absolute paths would not be moved because we are\n>> talking about Windows. Therefore your absolute paths would not start with\n>> a forward slash.\n>\n> Ah, sorry, I must have misunderstood a comment in your cover letter:\n>\n> The reason is this: something like this (make paths specified e.g. via \n> http.sslCAInfo relative to the runtime prefix) is potentially useful\n> also in the non-Windows context, as long as Git was built with the\n> runtime prefix feature.\n>\n> ... so I thought you meant to add this code for POSIX systems as well.\n>\n> My mistake. :(\n\nWell, I do not think you should feel so bad about it, as you are not\nalone.\n\nIt wasn't clear to me either that it was to introduce a new syntax\nthat users would not have otherwise used before (i.e. avoids\nnegatively affecting existing settings---because the users would\nhave used a path that begins with a backslash if they meant \"full\npath down from the top of the current drive\") to mean a new thing\n(i.e. \"relative to the runtime prefix\") that the patch wanted to do.\n\nIf that is the motivation, then it does make sense to extend this\nfunction, and possibly rename it, like this patch does, as we would\nwant this new syntax available in anywhere we take a pathname to an\nuntracked thing. And for such a pathname, we should be consistently\nusing expand_user_path() *anyway* even without this new \"now we know\nhow to spell 'relative to the runtime prefix'\" feature.\n\nSo I agree with the patch that the issue it wants to address is\nworth addressing, and the function to enhance is this one.\n\nI am still unsure about the solution, though.\n\nI suspect that any build that uses runtime prefix would want access\nto this feature, regardless of the platform. The new syntax that is\n\"a pathname that begins with a slash is not an absolute path but is\nrelative to the runtime prefix\" cannot avoid ambiguity with users'\nexisting settings on platforms other than Windows, which means this\nfeature cannot be made platform neutral in its posted form. The\ndocumentation must say \"On windows, the way to spell runtime prefix\nrelative paths is this; on macs, you must spell it differently like\nthis.\" etc. Which is rather unfortunate.\n\nPerhaps we need to make an effort to find a syntax that is\nuniversally unambiguous and use it consistently across platforms?\n\nI am tempted to say \"///\" might also be such a\nway, even in the POSIX world, but am not brave enough to do so, as I\nsuspect that may have a fallout in the Windows world X-<.\n\nAn earlier suggestion by Duy in [*1*] to pretend as if we take\n$VARIABLE and define special variables might be a better direction.\n\nAre there security implications if we started allowing references to\nenvironment varibables in strings we pass expand_user_path()? If we\ncannot convince ourselves that it is safe to allow access to any and\nall environment variables, then we probably can start by specific\n\"pseudo variables\" under our control, like GIT_EXEC_PATH and\nGIT_INSTALL_ROOT, that do not even have to be tied to environment\nvariables, perhaps with further restriction to allow it only at the\nbeginning of the string, or something like that, if necessary.\n\n[References]\n\n*1* \n"},{"id":"362682","messageId":"xmqqlg649zs8.fsf@gitster-ct.c.googlers.com","threadId":"49770","inReplyTo":"20181107220320.GA8970@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-08T00:30:15Z","receivedAt":"2018-11-08T00:30:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Nov 07, 2018 at 10:36:52PM +0100, Johannes Sixt wrote:\n>\n> All that said, if we're just interested in allowing this for config,\n> then we already have such a wrapper function: git_config_pathname().\n>\n> So I don't think it's a big deal to implement it in any of these ways.\n> It's much more important to get the syntax right, because that's\n> user-facing and will be with us forever.\n\nAll of us are on the same page after seeing the clarification by\nDscho, it seems. I came to pretty much the same conclusion this\nmorning before reading this subthread. Outside config values, the\ncallers of expand_user_path() only feed \"~/.git$constant\", and they\nare all about \"personal customization\" that do not want to be shared\nwith other users of the same installation, so \"relative to runtime\nprefix\" feature would not be wanted. But we do not know about new\ncaller's needs. For now I am OK to have it in expand_user_path(),\npossibly renaming the function if people feel it is needed (I don't).\n\nBetween ~ and $VARIABLE_LOOKING_THINGS, I do not have\na strong preference either way, but I am getting an impression that\nthe latter is more generally favoured in the discussion?\n"},{"id":"362686","messageId":"20181108011813.GA10148@sigill.intra.peff.net","threadId":"49770","inReplyTo":"xmqqlg649zs8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-08T01:18:14Z","receivedAt":"2018-11-08T01:18:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 08, 2018 at 09:30:15AM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Wed, Nov 07, 2018 at 10:36:52PM +0100, Johannes Sixt wrote:\n> >\n> > All that said, if we're just interested in allowing this for config,\n> > then we already have such a wrapper function: git_config_pathname().\n> >\n> > So I don't think it's a big deal to implement it in any of these ways.\n> > It's much more important to get the syntax right, because that's\n> > user-facing and will be with us forever.\n> \n> All of us are on the same page after seeing the clarification by\n> Dscho, it seems. I came to pretty much the same conclusion this\n> morning before reading this subthread. Outside config values, the\n> callers of expand_user_path() only feed \"~/.git$constant\", and they\n> are all about \"personal customization\" that do not want to be shared\n> with other users of the same installation, so \"relative to runtime\n> prefix\" feature would not be wanted. But we do not know about new\n> caller's needs. For now I am OK to have it in expand_user_path(),\n> possibly renaming the function if people feel it is needed (I don't).\n\nI think we would want to carefully think about the call in enter_repo().\nWe do not want git-daemon to accidentally expose repositories in\n$RUNTIME_PREFIX.\n\nLooking over the code, I think this is OK. The expansion happens in\nenter_repo(), and then we take the path that was found and do our\nok_paths checks on it (which makes sense -- even now you'd ask to export\n\"/home/\" and it would need to look at \"~peff/repo.git\" and expand that\nto \"/home/peff/repo.git\" before doing a simple string check.\n\n> Between ~ and $VARIABLE_LOOKING_THINGS, I do not have\n> a strong preference either way, but I am getting an impression that\n> the latter is more generally favoured in the discussion?\n\nI certainly prefer the latter, but I thought I was the only one to have\nexpressed support so far. ;)\n\n-Peff\n"},{"id":"362695","messageId":"xmqqlg648d1u.fsf@gitster-ct.c.googlers.com","threadId":"49770","inReplyTo":"20181108011813.GA10148@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-08T03:26:37Z","receivedAt":"2018-11-08T03:26:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think we would want to carefully think about the call in enter_repo().\n> We do not want git-daemon to accidentally expose repositories in\n> $RUNTIME_PREFIX.\n>\n> Looking over the code, I think this is OK. The expansion happens in\n> enter_repo(), and then we take the path that was found and do our\n> ok_paths checks on it (which makes sense -- even now you'd ask to export\n> \"/home/\" and it would need to look at \"~peff/repo.git\" and expand that\n> to \"/home/peff/repo.git\" before doing a simple string check.\n\nYup, that is another reason why I think this new \"expansion feature\"\nbelongs to the function, not to a wrapper that is aware of this new\nfeature in addition to ~tilde expansion.\n\n>> Between ~ and $VARIABLE_LOOKING_THINGS, I do not have\n>> a strong preference either way, but I am getting an impression that\n>> the latter is more generally favoured in the discussion?\n>\n> I certainly prefer the latter, but I thought I was the only one to have\n> expressed support so far. ;)\n\nThe first mention of pseudo-variable I saw was in Duy's message,\nwondering if $ROOT is more appropriate than \"/\", and I counted it as\nsupporting the $VARIABLE syntax.\n"},{"id":"362733","messageId":"nycvar.QRO.7.76.6.1811081356350.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"xmqqr2fwa0ew.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-08T13:04:17Z","receivedAt":"2018-11-08T13:04:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 8 Nov 2018, Junio C Hamano wrote:\n\n> I am tempted to say \"///\" might also be such a\n> way, even in the POSIX world, but am not brave enough to do so, as I\n> suspect that may have a fallout in the Windows world X-<.\n\nIt does. //server/share is the way we refer to UNC paths (AKA network\ndrives).\n\n> An earlier suggestion by Duy in [*1*] to pretend as if we take\n> $VARIABLE and define special variables might be a better direction.\n\nMy only qualm with this is that `$VARIABLE` is a perfectly allowed part of\na path. That is, you *could* create a directory called `$VARIABLE` and\nreference that, and then the expand_user_path() function (or whatever name\nwe will give it) would always expand this, with no way to switch it off.\n\nGranted, this is a highly unlikely scenario, but I would feel a bit more\ncomfortable with something like\n\n\t/ssl/certs/ca-bundle.crt\n\nOf course, `` is *also* a perfectly valid directory name,\nbut I would argue that it is even less likely to exist than\n`$RUNTIME_PREFIX` because the user would have to escape *two* characters\nrather than one.\n\n> Are there security implications if we started allowing references to\n> environment varibables in strings we pass expand_user_path()?\n\nProbably. But then, the runtime prefix is not even available as\nenvironment variable...\n\nCiao,\nDscho\n\n> If we cannot convince ourselves that it is safe to allow access to any\n> and all environment variables, then we probably can start by specific\n> \"pseudo variables\" under our control, like GIT_EXEC_PATH and\n> GIT_INSTALL_ROOT, that do not even have to be tied to environment\n> variables, perhaps with further restriction to allow it only at the\n> beginning of the string, or something like that, if necessary.\n> \n> [References]\n> \n> *1* \n> \n"},{"id":"362734","messageId":"nycvar.QRO.7.76.6.1811081408310.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"20181107220320.GA8970@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-08T13:11:53Z","receivedAt":"2018-11-08T13:12:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 7 Nov 2018, Jeff King wrote:\n\n> All that said, if we're just interested in allowing this for config,\n> then we already have such a wrapper function: git_config_pathname().\n\nGood point. I agree that `git_config_pathname()` is a better home for this\nfeature than `expand_user_path()`.\n\nBut now I have a really crazy idea: how about ~~/ssl/certs/ca-bundle.crt?\nThe `~` prefix is *already* a reserved character, and while it would\nprobably not be super intuitive to have `~~` be expanded to the runtime\nprefix, at least it would be less ambiguous than `$RUNTIME_PREFIX` (which\n*is* a valid directory name).\n\nOf course, `~~` is also a valid directory name, but then, so is `~` and we\ndo not have any way to specify *that* because `expand_user_path()` will\nalways expand it to the home directory. Or am I wrong? Do we have a way to\ndisable the expansion?\n\nCiao,\nDscho\n"},{"id":"362735","messageId":"CACsJy8AdRqKKFFO80p8jdMGaH6+Pj833nUEd7Xe6SWLQB=80UQ@mail.gmail.com","threadId":"49770","inReplyTo":"nycvar.QRO.7.76.6.1811081408310.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-08T14:25:30Z","receivedAt":"2018-11-08T14:25:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Nov 8, 2018 at 2:14 PM Johannes Schindelin\n wrote:\n>\n> Hi Peff,\n>\n> On Wed, 7 Nov 2018, Jeff King wrote:\n>\n> > All that said, if we're just interested in allowing this for config,\n> > then we already have such a wrapper function: git_config_pathname().\n>\n> Good point. I agree that `git_config_pathname()` is a better home for this\n> feature than `expand_user_path()`.\n>\n> But now I have a really crazy idea: how about ~~/ssl/certs/ca-bundle.crt?\n> The `~` prefix is *already* a reserved character, and while it would\n> probably not be super intuitive to have `~~` be expanded to the runtime\n> prefix, at least it would be less ambiguous than `$RUNTIME_PREFIX` (which\n> *is* a valid directory name).\n\nOne thing I had in mind when proposing $VARIABLE is that it opens up a\nnamespace for us to expand more things (*) for example $GIT_DIR (from\n~/.gitconfig).\n\n(*) but in a controlled way, it may look like an environment variable,\nbut we only accept a selected few. And it's only expanded at the\nbeginning of a path.\n\n> Of course, `~~` is also a valid directory name, but then, so is `~` and we\n> do not have any way to specify *that* because `expand_user_path()` will\n> always expand it to the home directory. Or am I wrong? Do we have a way to\n> disable the expansion?\n>\n> Ciao,\n> Dscho\n\n\n\n-- \nDuy\n"},{"id":"362736","messageId":"xmqqr2fv7hp8.fsf@gitster-ct.c.googlers.com","threadId":"49770","inReplyTo":"nycvar.QRO.7.76.6.1811081356350.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-08T14:43:47Z","receivedAt":"2018-11-08T14:43:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Thu, 8 Nov 2018, Junio C Hamano wrote:\n>\n>> I am tempted to say \"///\" might also be such a\n>> way, even in the POSIX world, but am not brave enough to do so, as I\n>> suspect that may have a fallout in the Windows world X-<.\n>\n> It does. //server/share is the way we refer to UNC paths (AKA network\n> drives).\n\nShucks. That would mean the patch that started this thread would\nnot be a good idea, as an end-user could already be writing\n\"//server/share/some/path\" and the code with the patch would see '/'\nthat begins it, and start treating it differently than the code\nbefore the patch X-<.\n\n> Granted, this is a highly unlikely scenario, but I would feel a bit more\n> comfortable with something like\n>\n> \t/ssl/certs/ca-bundle.crt\n>\n> Of course, `` is *also* a perfectly valid directory name,\n> but I would argue that it is even less likely to exist than\n> `$RUNTIME_PREFIX` because the user would have to escape *two* characters\n> rather than one.\n\nYes, and it is naturally extensible by allowing \ninside the special bra-ket pair (just like $OTHER_THINGS can be a\nway to extend the system if we used a special variable syntax).\n\n>> Are there security implications if we started allowing references to\n>> environment varibables in strings we pass expand_user_path()?\n>\n> Probably. But then, the runtime prefix is not even available as\n> environment variable...\n\nAh, sorry. I thought it was clear that I would next be suggesting to\nadd an environmet variable for it, _if_ the approach to allow env\nreferences turns out to be viable.\n"},{"id":"362737","messageId":"xmqqmuqj7hiv.fsf@gitster-ct.c.googlers.com","threadId":"49770","inReplyTo":"nycvar.QRO.7.76.6.1811081408310.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-08T14:47:36Z","receivedAt":"2018-11-08T14:47:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> But now I have a really crazy idea: how about ~~/ssl/certs/ca-bundle.crt?\n> The `~` prefix is *already* a reserved character,...\n\nWe would need to prepare for a future where we need yet another\nspecial thing to be expanded, and it will quickly become cryptic if\nyou said \"~/ is HOME, ~USER/ is USER's HOME, ~~/ is runtime prefix,\nand ~~~/ is this new thing...\". ~runtime_prefix~/ (i.e. carve out\nthe namespace for USERNAME by reserving any names that ends with a\ntilde) may be a viable way to do this, though. It is just as good\nas your other idea, , in an earlier message.\n\n"},{"id":"362738","messageId":"nycvar.QRO.7.76.6.1811081637240.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"xmqqr2fv7hp8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-08T15:39:15Z","receivedAt":"2018-11-08T15:39:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 8 Nov 2018, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Thu, 8 Nov 2018, Junio C Hamano wrote:\n> >\n> >> I am tempted to say \"///\" might also be such a\n> >> way, even in the POSIX world, but am not brave enough to do so, as I\n> >> suspect that may have a fallout in the Windows world X-<.\n> >\n> > It does. //server/share is the way we refer to UNC paths (AKA network\n> > drives).\n> \n> Shucks. That would mean the patch that started this thread would\n> not be a good idea, as an end-user could already be writing\n> \"//server/share/some/path\" and the code with the patch would see '/'\n> that begins it, and start treating it differently than the code\n> before the patch X-<.\n\nOuch. You're right!\n\n> > Granted, this is a highly unlikely scenario, but I would feel a bit more\n> > comfortable with something like\n> >\n> > \t/ssl/certs/ca-bundle.crt\n> >\n> > Of course, `` is *also* a perfectly valid directory name,\n> > but I would argue that it is even less likely to exist than\n> > `$RUNTIME_PREFIX` because the user would have to escape *two* characters\n> > rather than one.\n> \n> Yes, and it is naturally extensible by allowing \n> inside the special bra-ket pair (just like $OTHER_THINGS can be a\n> way to extend the system if we used a special variable syntax).\n\nTrue.\n\n> >> Are there security implications if we started allowing references to\n> >> environment varibables in strings we pass expand_user_path()?\n> >\n> > Probably. But then, the runtime prefix is not even available as\n> > environment variable...\n> \n> Ah, sorry. I thought it was clear that I would next be suggesting to\n> add an environmet variable for it, _if_ the approach to allow env\n> references turns out to be viable.\n\nOf course, I should have assumed that. Sorry!\n\nCiao,\nDscho\n"},{"id":"362739","messageId":"nycvar.QRO.7.76.6.1811081639210.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"CACsJy8AdRqKKFFO80p8jdMGaH6+Pj833nUEd7Xe6SWLQB=80UQ@mail.gmail.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-08T15:45:16Z","receivedAt":"2018-11-08T15:45:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 8 Nov 2018, Duy Nguyen wrote:\n\n> On Thu, Nov 8, 2018 at 2:14 PM Johannes Schindelin\n> wrote:\n> >\n> > On Wed, 7 Nov 2018, Jeff King wrote:\n> >\n> > > All that said, if we're just interested in allowing this for config,\n> > > then we already have such a wrapper function: git_config_pathname().\n> >\n> > Good point. I agree that `git_config_pathname()` is a better home for this\n> > feature than `expand_user_path()`.\n> >\n> > But now I have a really crazy idea: how about ~~/ssl/certs/ca-bundle.crt?\n> > The `~` prefix is *already* a reserved character, and while it would\n> > probably not be super intuitive to have `~~` be expanded to the runtime\n> > prefix, at least it would be less ambiguous than `$RUNTIME_PREFIX` (which\n> > *is* a valid directory name).\n> \n> One thing I had in mind when proposing $VARIABLE is that it opens up a\n> namespace for us to expand more things (*) for example $GIT_DIR (from\n> ~/.gitconfig).\n> \n> (*) but in a controlled way, it may look like an environment variable,\n> but we only accept a selected few. And it's only expanded at the\n> beginning of a path.\n\nI understand that desire, and I even agree. But the fact that it looks\nlike an environment variable, but is not, is actually a point in favor of\n*not* doing this. It would violate the principle of least astonishment.\n\nThe form `/abc/def` would not be confused with anything\nthat it is not, I would think. The only thing against this form (at least\nthat I can think of) is that some people use this way to talk about paths\nthat vary between different setups, with the implicit assumption that the\nreader will \"interpolate\" this *before* applying. So for example, when I\ntell a user to please edit their /config, I implicitly assume\nthat they know to not type out, literally, but instead fill in\nthe correct path.\n\nCiao,\nDscho\n\n> > Of course, `~~` is also a valid directory name, but then, so is `~` and we\n> > do not have any way to specify *that* because `expand_user_path()` will\n> > always expand it to the home directory. Or am I wrong? Do we have a way to\n> > disable the expansion?\n> >\n> > Ciao,\n> > Dscho\n> \n> \n> \n> -- \n> Duy\n> \n"},{"id":"362740","messageId":"nycvar.QRO.7.76.6.1811081645280.39@tvgsbejvaqbjf.bet","threadId":"49770","inReplyTo":"xmqqmuqj7hiv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-08T15:46:51Z","receivedAt":"2018-11-08T15:47:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 8 Nov 2018, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > But now I have a really crazy idea: how about ~~/ssl/certs/ca-bundle.crt?\n> > The `~` prefix is *already* a reserved character,...\n> \n> We would need to prepare for a future where we need yet another\n> special thing to be expanded, and it will quickly become cryptic if\n> you said \"~/ is HOME, ~USER/ is USER's HOME, ~~/ is runtime prefix,\n> and ~~~/ is this new thing...\". ~runtime_prefix~/ (i.e. carve out\n> the namespace for USERNAME by reserving any names that ends with a\n> tilde) may be a viable way to do this, though. It is just as good\n> as your other idea, , in an earlier message.\n\nIndeed. Your \"cryptic\" matches my \"unintuitive\".\n\nCiao,\nDscho\n"},{"id":"362746","messageId":"CAPig+cQMR8PGhtQUTtdshNy80Ej-L2vT8_3yHXm=jCM58_fAjQ@mail.gmail.com","threadId":"49770","inReplyTo":"nycvar.QRO.7.76.6.1811081639210.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-11-08T17:40:17Z","receivedAt":"2018-11-08T17:40:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 8, 2018 at 10:45 AM Johannes Schindelin\n wrote:\n> On Thu, 8 Nov 2018, Duy Nguyen wrote:\n> > One thing I had in mind when proposing $VARIABLE is that it opens up a\n> > namespace for us to expand more things (*) for example $GIT_DIR (from\n> > ~/.gitconfig).\n> >\n> > (*) but in a controlled way, it may look like an environment variable,\n> > but we only accept a selected few. And it's only expanded at the\n> > beginning of a path.\n>\n> I understand that desire, and I even agree. But the fact that it looks\n> like an environment variable, but is not, is actually a point in favor of\n> *not* doing this. It would violate the principle of least astonishment.\n\nPerhaps something like $:VARIABLE, which gives the convenience of\n$VARIABLE, but looks sufficiently different that it shouldn't trip up\nreaders into thinking it is one. (Well-written code checking for a\nDOS/Windows drive letter at the beginning of a path shouldn't be\nconfused by it.)\n"},{"id":"362765","messageId":"DM5P132MB0143B9D5FAF40A3999131517E7C60@DM5P132MB0143.NAMP132.PROD.OUTLOOK.COM","threadId":"49770","inReplyTo":"xmqqr2fwa0ew.fsf@gitster-ct.c.googlers.com","subject":"RE: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Joseph Moisan","fromEmail":"joseph.moisan@jci.com","sentAt":"2018-11-09T02:05:48Z","receivedAt":"2018-11-09T02:05:56Z","isPatch":true,"sender":{"key":"joseph.moisan@jci.com","avatar":null},"body":"Can someone please tell me how to unsubscribe from this email. I am no longer interested in receiving these emails, and cannot find how to unsubscribe.\nThank you in advance.\n\nJoseph Moisan | Software Engineer\nBuilding Technologies and Solutions\nTel: +1-978-731-8950\nJoseph.Moisan@JCI.com\n\n-----Original Message-----\nFrom: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On Behalf Of Junio C Hamano\nSent: Wednesday, November 7, 2018 7:17 PM\nTo: Ramsay Jones \nCc: Johannes Schindelin ; Johannes Schindelin via GitGitGadget ; git@vger.kernel.org\nSubject: Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()\n\nRamsay Jones writes:\n\n>> The cute thing is: your absolute paths would not be moved because we \n>> are talking about Windows. Therefore your absolute paths would not \n>> start with a forward slash.\n>\n> Ah, sorry, I must have misunderstood a comment in your cover letter:\n>\n> The reason is this: something like this (make paths specified e.g. via \n> http.sslCAInfo relative to the runtime prefix) is potentially useful\n> also in the non-Windows context, as long as Git was built with the\n> runtime prefix feature.\n>\n> ... so I thought you meant to add this code for POSIX systems as well.\n>\n> My mistake. :(\n\nWell, I do not think you should feel so bad about it, as you are not alone.\n\nIt wasn't clear to me either that it was to introduce a new syntax that users would not have otherwise used before (i.e. avoids negatively affecting existing settings---because the users would have used a path that begins with a backslash if they meant \"full path down from the top of the current drive\") to mean a new thing (i.e. \"relative to the runtime prefix\") that the patch wanted to do.\n\nIf that is the motivation, then it does make sense to extend this function, and possibly rename it, like this patch does, as we would want this new syntax available in anywhere we take a pathname to an untracked thing. And for such a pathname, we should be consistently using expand_user_path() *anyway* even without this new \"now we know how to spell 'relative to the runtime prefix'\" feature.\n\nSo I agree with the patch that the issue it wants to address is worth addressing, and the function to enhance is this one.\n\nI am still unsure about the solution, though.\n\nI suspect that any build that uses runtime prefix would want access to this feature, regardless of the platform. The new syntax that is \"a pathname that begins with a slash is not an absolute path but is relative to the runtime prefix\" cannot avoid ambiguity with users'\nexisting settings on platforms other than Windows, which means this feature cannot be made platform neutral in its posted form. The documentation must say \"On windows, the way to spell runtime prefix relative paths is this; on macs, you must spell it differently like this.\" etc. Which is rather unfortunate.\n\nPerhaps we need to make an effort to find a syntax that is universally unambiguous and use it consistently across platforms?\n\nI am tempted to say \"///\" might also be such a way, even in the POSIX world, but am not brave enough to do so, as I suspect that may have a fallout in the Windows world X-<.\n\nAn earlier suggestion by Duy in [*1*] to pretend as if we take $VARIABLE and define special variables might be a better direction.\n\nAre there security implications if we started allowing references to environment varibables in strings we pass expand_user_path()? If we cannot convince ourselves that it is safe to allow access to any and all environment variables, then we probably can start by specific \"pseudo variables\" under our control, like GIT_EXEC_PATH and GIT_INSTALL_ROOT, that do not even have to be tied to environment variables, perhaps with further restriction to allow it only at the beginning of the string, or something like that, if necessary.\n\n[References]\n\n*1* \n"},{"id":"362813","messageId":"20181109101918.GC7410@sigill.intra.peff.net","threadId":"49770","inReplyTo":"nycvar.QRO.7.76.6.1811081639210.39@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-09T10:19:18Z","receivedAt":"2018-11-09T10:19:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 08, 2018 at 04:45:16PM +0100, Johannes Schindelin wrote:\n\n> > One thing I had in mind when proposing $VARIABLE is that it opens up a\n> > namespace for us to expand more things (*) for example $GIT_DIR (from\n> > ~/.gitconfig).\n> > \n> > (*) but in a controlled way, it may look like an environment variable,\n> > but we only accept a selected few. And it's only expanded at the\n> > beginning of a path.\n> \n> I understand that desire, and I even agree. But the fact that it looks\n> like an environment variable, but is not, is actually a point in favor of\n> *not* doing this. It would violate the principle of least astonishment.\n\nI agree that it is potentially surprising. OTOH, it is at least pretty\nobvious when you see in the wild something like:\n\n [core]\n foo = $RUNTIME_PREFIX/bar\n\nwhat it is _trying_ to do. My big concern with \"~\"-based things is that\nthey're kind of subtle. If I saw:\n\n [core]\n foo = ~~/bar\n\nI'm not sure what I would think it does. ;)\n\n> The form `/abc/def` would not be confused with anything\n> that it is not, I would think. The only thing against this form (at least\n> that I can think of) is that some people use this way to talk about paths\n> that vary between different setups, with the implicit assumption that the\n> reader will \"interpolate\" this *before* applying. So for example, when I\n> tell a user to please edit their /config, I implicitly assume\n> that they know to not type out, literally, but instead fill in\n> the correct path.\n\nSo yeah, some alternative syntax that is verbose but distinct makes\nsense to me. We use %-substitution elsewhere. Could we do something with\nthat? \"%RUNTIME_PREFIX%\" gives me too many flashbacks, but something\nlike \"%(RUNTIME_PREFIX)\" matches our formatting language.\n\nI dunno. I actually do not think \"$FOO\" is so bad, as long as we clearly\ndocument that:\n\n - we do not allow arbitrary $FOO from the environment (though I am\n actually not all that opposed to doing so)\n\n - we add some special $FOOs that are not actually environment\n variables\n\n-Peff\n"},{"id":"362814","messageId":"20181109102117.GD7410@sigill.intra.peff.net","threadId":"49770","inReplyTo":"DM5P132MB0143B9D5FAF40A3999131517E7C60@DM5P132MB0143.NAMP132.PROD.OUTLOOK.COM","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-09T10:21:17Z","receivedAt":"2018-11-09T10:21:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 09, 2018 at 02:05:48AM +0000, Joseph Moisan wrote:\n\n> Can someone please tell me how to unsubscribe from this email. I am\n> no longer interested in receiving these emails, and cannot find how to\n> unsubscribe.\n\nDetails are at http://vger.kernel.org/vger-lists.html#git.\n\n-Peff\n"},{"id":"362831","messageId":"CACsJy8BzbEnXMRw6kB1rg+WMfu2wwJZWkAtVate98CZRSY_dLA@mail.gmail.com","threadId":"49770","inReplyTo":"20181109101918.GC7410@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-11-09T16:16:41Z","receivedAt":"2018-11-09T16:17:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Nov 9, 2018 at 11:19 AM Jeff King wrote:\n> > The form `/abc/def` would not be confused with anything\n> > that it is not, I would think. The only thing against this form (at least\n> > that I can think of) is that some people use this way to talk about paths\n> > that vary between different setups, with the implicit assumption that the\n> > reader will \"interpolate\" this *before* applying. So for example, when I\n> > tell a user to please edit their /config, I implicitly assume\n> > that they know to not type out, literally, but instead fill in\n> > the correct path.\n>\n> So yeah, some alternative syntax that is verbose but distinct makes\n> sense to me. We use %-substitution elsewhere. Could we do something with\n> that? \"%RUNTIME_PREFIX%\" gives me too many flashbacks, but something\n> like \"%(RUNTIME_PREFIX)\" matches our formatting language.\n\nFWIW I don't have any preference, as long as the variable can still\nhave a name (that is not a symbol).\n\nA side question regardless of syntax. What do we do with\n%(unrecognized name)/foo? I see three options\n\n - expand to empty, so \"/foo\"\n - keep it and try the literal path \"%(unrecognized name)/foo\"\n - somehow tell the caller that the path is invalid and treat it like\nnon-existing path, even if there is some real thing at \"%(unrecognized\nname)/foo\"\n\nThe last option is more predictable. But we need to be more careful\nabout the syntax because if \"%(some path like this)\" actually exists\nsomewhere, then it will be broken. And I think it's also more work.\n-- \nDuy\n"},{"id":"362955","messageId":"xmqqr2fr3skw.fsf@gitster-ct.c.googlers.com","threadId":"49770","inReplyTo":"CACsJy8BzbEnXMRw6kB1rg+WMfu2wwJZWkAtVate98CZRSY_dLA@mail.gmail.com","subject":"Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-12T03:03:43Z","receivedAt":"2018-11-12T03:03:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> FWIW I don't have any preference, as long as the variable can still\n> have a name (that is not a symbol).\n\nSame here.\n\n> A side question regardless of syntax. What do we do with\n> %(unrecognized name)/foo? I see three options\n>\n> - expand to empty, so \"/foo\"\n> - keep it and try the literal path \"%(unrecognized name)/foo\"\n\nNeither of these is good for future proofing purposes.\n\n> - somehow tell the caller that the path is invalid and treat it like\n> non-existing path, even if there is some real thing at \"%(unrecognized\n> name)/foo\"\n\nAnother thing to worry about is how to spell the real thing that has\nsuch a funny name, perhaps by escaping like \"%%(unrecognized\nname)/foo\".\n\nAnd from that point of view, \"~runtime-prefix~/foo\" (i.e. what J6t\nstarted) may make the most sense, as I do not think we need to\nsupport a username that ends with a tilde by introducing a quoting\nconvention.\n"},{"id":"428947","messageId":"pull.66.v2.git.1625155388.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-01T16:03:05Z","receivedAt":"2021-07-01T16:03:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In Git for Windows, we ran with a patch \"in production\" for quite a while\nwhere paths starting with a slash (i.e. looking like Unix paths, not like\nWindows paths) were interpreted as being relative to the runtime prefix,\nwhen expanded via expand_user_path().\n\nThis was sent to the Git mailing list as a discussion starter, and it was\npointed out that this is neither portable nor unambiguous.\n\nAfter the dust settled, I thought about the presented ideas for a while\n(quite a while...), and ended up with the following: any path starting with\n/ is expanded. This is ambiguous because it could be a valid\npath. But then, it is unlikely, and if someone really wants to specify such\na path, it is easy to slap a ./ in front and they're done.\n\nChanges since v1:\n\n * Included a test for the RUNTIME_PREFIX that I had meant to send for ages\n already, and based on which...\n * A test case was added to verify that this actually works as intended\n * It is no longer Windows-specific\n * I added some documentation\n\nJohannes Schindelin (2):\n tests: exercise the RUNTIME_PREFIX feature\n expand_user_path(): support specifying paths relative to the runtime\n prefix\n\n Documentation/config.txt | 10 ++++++++++\n Makefile | 5 +++++\n path.c | 5 +++++\n t/t0060-path-utils.sh | 26 ++++++++++++++++++++++++++\n 4 files changed, 46 insertions(+)\n\n\nbase-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-66%2Fdscho%2Fmingw-expand-absolute-user-path-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-66/dscho/mingw-expand-absolute-user-path-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/66\n\nRange-diff vs v1:\n\n -: ----------- > 1: cc8f09baba9 tests: exercise the RUNTIME_PREFIX feature\n 1: 2287dd96cf0 ! 2: 66df56f5db0 mingw: handle absolute paths in expand_user_path()\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - mingw: handle absolute paths in expand_user_path()\n + expand_user_path(): support specifying paths relative to the runtime prefix\n \n - On Windows, an absolute POSIX path needs to be turned into a Windows\n - one.\n + Ever since Git learned to detect its install location at runtime, there\n + was the slightly awkward problem that it was impossible to specify paths\n + relative to said location.\n +\n + For example, if a version of Git was shipped with custom SSL\n + certificates to use, there was no portable way to specify\n + `http.sslCAInfo`.\n +\n + In Git for Windows, the problem was \"solved\" for years by interpreting\n + paths starting with a slash as relative to the runtime prefix.\n +\n + However, this is not correct: such paths _are_ legal on Windows, and\n + they are interpreted as absolute paths in the same drive as the current\n + directory.\n +\n + After a lengthy discussion, and a way lengthier time to mull over the\n + problem and its best solution, we decided to introduce support for the\n + magic sequence `/`. If a path starts with this, the\n + remainder is interpreted as relative to the detected runtime prefix.\n +\n + This solves the problem, but what new problems does it stir up? Here are\n + the two most obvious ones:\n +\n + - What if Git was not compiled with support for a runtime prefix?\n +\n + In that case, we will simply use the compiled-in hard-coded prefix.\n +\n + - What if a user _wants_ to specify a path starting with the magic\n + sequence?\n +\n + In that case, the user will simply need to prefix the magic sequence\n + with `./` and voilà, the path won't be expanded.\n \n Signed-off-by: Johannes Schindelin \n \n + ## Documentation/config.txt ##\n +@@ Documentation/config.txt: pathname::\n + \ttilde expansion happens to such a string: `~/`\n + \tis expanded to the value of `$HOME`, and `~user/` to the\n + \tspecified user's home directory.\n +++\n ++If a path starts with `/`, the remainder is\n ++interpreted as a path relative to Git's \"runtime prefix\", i.e. relative\n ++to the location where Git itself was installed. For example,\n ++`/bin/` refers to the directory in which the Git\n ++executable itself lives. If Git was compiled without runtime prefix\n ++support, the compiled-in prefix will be subsituted instead. In the\n ++unlikely event that a literal path needs to be specified that should\n ++_not_ be expanded, it needs to be prefixed by `./`, like so:\n ++`.//bin`.\n + \n + \n + Variables\n +\n ## path.c ##\n @@\n - #include \"path.h\"\n #include \"packfile.h\"\n #include \"object-store.h\"\n + #include \"lockfile.h\"\n +#include \"exec-cmd.h\"\n \n static int get_st_mode_bits(const char *path, int *mode)\n @@ path.c: char *expand_user_path(const char *path, int real_home)\n \n \tif (path == NULL)\n \t\tgoto return_null;\n -+#ifdef __MINGW32__\n -+\tif (path[0] == '/')\n -+\t\treturn system_path(path + 1);\n -+#endif\n ++\n ++\tif (skip_prefix(path, \"/\", &path))\n ++\t\treturn system_path(path);\n ++\n \tif (path[0] == '~') {\n \t\tconst char *first_slash = strchrnul(path, '/');\n \t\tconst char *username = path + 1;\n +\n + ## t/t0060-path-utils.sh ##\n +@@ t/t0060-path-utils.sh: test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' '\n + \tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n + \tGIT_EXEC_PATH= ./pretend/bin/git here >actual &&\n + \techo HERE >expect &&\n ++\ttest_cmp expect actual'\n ++\n ++test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '/ works' '\n ++\tmkdir -p pretend/bin &&\n ++\tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n ++\tgit config yes.path \"/yes\" &&\n ++\tGIT_EXEC_PATH= ./pretend/bin/git config --path yes.path >actual &&\n ++\techo \"$(pwd)/pretend/yes\" >expect &&\n + \ttest_cmp expect actual\n + '\n + \n\n-- \ngitgitgadget\n"},{"id":"428948","messageId":"cc8f09baba94546a13087e923e9c2374284c8136.1625155388.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v2.git.1625155388.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] tests: exercise the RUNTIME_PREFIX feature","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-01T16:03:06Z","receivedAt":"2021-07-01T16:03:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOriginally, we refrained from adding a regression test in 7b6c6496374\n(system_path(): Add prefix computation at runtime if RUNTIME_PREFIX set,\n2008-08-10), and in 226c0ddd0d6 (exec_cmd: RUNTIME_PREFIX on some POSIX\nsystems, 2018-04-10).\n\nThe reason was that it was deemed too tricky to test.\n\nTurns out that it is not tricky to test at all: we simply create a\npseudo-root, copy the `git` executable into the `git/` subdirectory of\nthat pseudo-root, then copy a script into the `libexec/git-core/`\ndirectory and expect that to be picked up.\n\nAs long as the trash directory is in a location where binaries can be\nexecuted, this works.\n\nSigned-off-by: Johannes Schindelin \n---\n Makefile | 5 +++++\n t/t0060-path-utils.sh | 18 ++++++++++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex c3565fc0f8f..4c3e4ccabcd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2826,6 +2826,11 @@ ifdef GIT_TEST_INDEX_VERSION\n endif\n ifdef GIT_TEST_PERL_FATAL_WARNINGS\n \t@echo GIT_TEST_PERL_FATAL_WARNINGS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_PERL_FATAL_WARNINGS)))'\\' >>$@+\n+endif\n+ifdef RUNTIME_PREFIX\n+\t@echo RUNTIME_PREFIX=\\'true\\' >>$@+\n+else\n+\t@echo RUNTIME_PREFIX=\\'false\\' >>$@+\n endif\n \t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n \ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex de4960783f0..a76728c27bf 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -525,4 +525,22 @@ test_expect_success MINGW 'is_valid_path() on Windows' '\n \t\t\"PRN./abc\"\n '\n \n+test_lazy_prereq RUNTIME_PREFIX '\n+\ttest true = \"$RUNTIME_PREFIX\"\n+'\n+\n+test_lazy_prereq CAN_EXEC_IN_PWD '\n+\tcp \"$GIT_EXEC_PATH\"/git$X ./ &&\n+\t./git rev-parse\n+'\n+\n+test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' '\n+\tmkdir -p pretend/bin pretend/libexec/git-core &&\n+\techo \"echo HERE\" | write_script pretend/libexec/git-core/git-here &&\n+\tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n+\tGIT_EXEC_PATH= ./pretend/bin/git here >actual &&\n+\techo HERE >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"428949","messageId":"66df56f5db02fa43b28111bb9fcc9b06f8542154.1625155388.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v2.git.1625155388.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] expand_user_path(): support specifying paths relative to the runtime prefix","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-01T16:03:07Z","receivedAt":"2021-07-01T16:03:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEver since Git learned to detect its install location at runtime, there\nwas the slightly awkward problem that it was impossible to specify paths\nrelative to said location.\n\nFor example, if a version of Git was shipped with custom SSL\ncertificates to use, there was no portable way to specify\n`http.sslCAInfo`.\n\nIn Git for Windows, the problem was \"solved\" for years by interpreting\npaths starting with a slash as relative to the runtime prefix.\n\nHowever, this is not correct: such paths _are_ legal on Windows, and\nthey are interpreted as absolute paths in the same drive as the current\ndirectory.\n\nAfter a lengthy discussion, and a way lengthier time to mull over the\nproblem and its best solution, we decided to introduce support for the\nmagic sequence `/`. If a path starts with this, the\nremainder is interpreted as relative to the detected runtime prefix.\n\nThis solves the problem, but what new problems does it stir up? Here are\nthe two most obvious ones:\n\n- What if Git was not compiled with support for a runtime prefix?\n\n In that case, we will simply use the compiled-in hard-coded prefix.\n\n- What if a user _wants_ to specify a path starting with the magic\n sequence?\n\n In that case, the user will simply need to prefix the magic sequence\n with `./` and voilà, the path won't be expanded.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 10 ++++++++++\n path.c | 5 +++++\n t/t0060-path-utils.sh | 8 ++++++++\n 3 files changed, 23 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bf82766a6a2..fd56e2c1220 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -298,6 +298,16 @@ pathname::\n \ttilde expansion happens to such a string: `~/`\n \tis expanded to the value of `$HOME`, and `~user/` to the\n \tspecified user's home directory.\n++\n+If a path starts with `/`, the remainder is\n+interpreted as a path relative to Git's \"runtime prefix\", i.e. relative\n+to the location where Git itself was installed. For example,\n+`/bin/` refers to the directory in which the Git\n+executable itself lives. If Git was compiled without runtime prefix\n+support, the compiled-in prefix will be subsituted instead. In the\n+unlikely event that a literal path needs to be specified that should\n+_not_ be expanded, it needs to be prefixed by `./`, like so:\n+`.//bin`.\n \n \n Variables\ndiff --git a/path.c b/path.c\nindex 7bccd830e95..d8542a7b27b 100644\n--- a/path.c\n+++ b/path.c\n@@ -12,6 +12,7 @@\n #include \"packfile.h\"\n #include \"object-store.h\"\n #include \"lockfile.h\"\n+#include \"exec-cmd.h\"\n \n static int get_st_mode_bits(const char *path, int *mode)\n {\n@@ -732,6 +733,10 @@ char *expand_user_path(const char *path, int real_home)\n \n \tif (path == NULL)\n \t\tgoto return_null;\n+\n+\tif (skip_prefix(path, \"/\", &path))\n+\t\treturn system_path(path);\n+\n \tif (path[0] == '~') {\n \t\tconst char *first_slash = strchrnul(path, '/');\n \t\tconst char *username = path + 1;\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex a76728c27bf..cb7fbfb9af2 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -540,6 +540,14 @@ test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' '\n \tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n \tGIT_EXEC_PATH= ./pretend/bin/git here >actual &&\n \techo HERE >expect &&\n+\ttest_cmp expect actual'\n+\n+test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '/ works' '\n+\tmkdir -p pretend/bin &&\n+\tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n+\tgit config yes.path \"/yes\" &&\n+\tGIT_EXEC_PATH= ./pretend/bin/git config --path yes.path >actual &&\n+\techo \"$(pwd)/pretend/yes\" >expect &&\n \ttest_cmp expect actual\n '\n \n-- \ngitgitgadget\n"},{"id":"428975","messageId":"xmqq1r8irkvk.fsf@gitster.g","threadId":"49770","inReplyTo":"pull.66.v2.git.1625155388.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-01T17:42:39Z","receivedAt":"2021-07-01T17:42:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> In Git for Windows, we ran with a patch \"in production\" for quite a while\n> where paths starting with a slash (i.e. looking like Unix paths, not like\n> Windows paths) were interpreted as being relative to the runtime prefix,\n> when expanded via expand_user_path().\n>\n> This was sent to the Git mailing list as a discussion starter, and it was\n> pointed out that this is neither portable nor unambiguous.\n>\n> After the dust settled, I thought about the presented ideas for a while\n> (quite a while...), and ended up with the following: any path starting with\n> / is expanded. This is ambiguous because it could be a valid\n> path. But then, it is unlikely, and if someone really wants to specify such\n> a path, it is easy to slap a ./ in front and they're done.\n\nI just went back to briefly scan the discussion in late 2018.\n\nI think the rough consensus back then was that \n\n * It indeed is a problem that there is no syntax for users of\n \"relocatable Git\" to use to point at things that come as part of\n the \"relocatable Git\" package.\n\n * A change to expand_user_path() would be too broad, it makes sense\n for this feature to be in git_config_pathname();\n\n * We need to get the syntax right.\n\nAs to the last item, there were a handful of choices raised:\n\n - Use \"~~\"--the downside is that this is not extensible. Use\n \"~runtime-prefix/\" would be a better choice (the likelyhood of\n , $RUNTIME_PREFIX, and any other random choice\n happens to be used as a valid directory name is just as slim as\n the likelyhood of \"runtime-prefix\" used as a valid username).\n\n - \"$RUNTIME_PREFIX\" to make it read like a variable---the downside\n was that it looked TOO MUCH like a variable and risked user\n confusion (e.g. it may be upsetting that \"$HOME/.gitconfig\" is\n not expanded like \"~/.gitconfig\" is).\n\n - %(RUNTIME-PREFIX) to make it similar to how Git replaces various\n tokens that are understood only in the context of Git.\n\n - ---the downside is that this is an unnecessary\n new syntax we do not have to introduce. If is\n unlikely to be used as a valid directory name, %(RUNTIME-PREFIX)\n is just as unlikely.\n\nThere might have been other ideas floated back then. I have to say\nthat the one you chose in this round is the least favourite of mine,\nand if you consulted me privately before redoing this round, I would\nprobably have said %(RUNTIME_PREFIX) would make the most sense among\nthe candidates.\n\nThanks.\n"},{"id":"431122","messageId":"pull.66.v3.git.1627164413.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v2.git.1625155388.gitgitgadget@gmail.com","subject":"[PATCH v3 0/5] mingw: handle absolute paths in expand_user_path()","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-24T22:06:48Z","receivedAt":"2021-07-24T22:06:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"In Git for Windows, we ran with a patch \"in production\" for quite a while\nwhere paths starting with a slash (i.e. looking like Unix paths, not like\nWindows paths) were interpreted as being relative to the runtime prefix,\nwhen expanded via expand_user_path().\n\nThis was sent to the Git mailing list as a discussion starter, and it was\npointed out that this is neither portable nor unambiguous.\n\nAfter the dust settled, I thought about the presented ideas for a while\n(quite a while...), ended up with a solution, then adapted it to Junio's\npreference: any path starting with %(prefix)/ is expanded. This is ambiguous\nbecause it could be a valid path. But then, it is unlikely, and if someone\nreally wants to specify such a path, it is easy to slap a ./ in front and\nthey're done.\n\nChanges since v2:\n\n * Adjusted to Junio's preference %(...) over <...>. Of course, the\n preferred preference has the disadvantage of actually being allowed in\n Win32 filenames, but then, the workaround is as easy as on non-Windows\n platforms.\n * Since our convention of %(...) interpolation does not involve uppercase\n keywords, I now use a lowercase one.\n * Since this keyword is interpolated to the compiled-in prefix if built\n without runtime prefix support, I dropped the runtime part of the\n keyword.\n * Renamed the expand_user_path() to interpolate_path(), to remove the\n distraction as to the implementation detail which things get to be\n interpolated (because we extend it to interpolate more than just a home\n directory, which might well be unclear from the former name, anyway).\n * Adjusted the code comment above the interpolate_path() to remove a stale\n part, clarify another part, and to extend it to talk about the prefix\n expansion, too.\n\nChanges since v1:\n\n * Included a test for the RUNTIME_PREFIX that I had meant to send for ages\n already, and based on which...\n * A test case was added to verify that this actually works as intended\n * It is no longer Windows-specific\n * I added some documentation\n\nJohannes Schindelin (5):\n tests: exercise the RUNTIME_PREFIX feature\n expand_user_path(): remove stale part of the comment\n expand_user_path(): clarify the role of the `real_home` parameter\n Use a better name for the function interpolating paths\n interpolate_path(): allow specifying paths relative to the runtime\n prefix\n\n Documentation/config.txt | 9 +++++++++\n Makefile | 5 +++++\n builtin/credential-cache.c | 2 +-\n builtin/credential-store.c | 2 +-\n builtin/gc.c | 2 +-\n cache.h | 2 +-\n config.c | 8 ++++----\n path.c | 19 +++++++++++++------\n sequencer.c | 2 +-\n t/t0060-path-utils.sh | 26 ++++++++++++++++++++++++++\n 10 files changed, 62 insertions(+), 15 deletions(-)\n\n\nbase-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-66%2Fdscho%2Fmingw-expand-absolute-user-path-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-66/dscho/mingw-expand-absolute-user-path-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/66\n\nRange-diff vs v2:\n\n 1: cc8f09baba9 = 1: cc8f09baba9 tests: exercise the RUNTIME_PREFIX feature\n -: ----------- > 2: a4ff3a461bc expand_user_path(): remove stale part of the comment\n -: ----------- > 3: 7b79ba66dd0 expand_user_path(): clarify the role of the `real_home` parameter\n -: ----------- > 4: 19fd9c3c803 Use a better name for the function interpolating paths\n 2: 66df56f5db0 ! 5: d286583082e expand_user_path(): support specifying paths relative to the runtime prefix\n @@ Metadata\n Author: Johannes Schindelin \n \n ## Commit message ##\n - expand_user_path(): support specifying paths relative to the runtime prefix\n + interpolate_path(): allow specifying paths relative to the runtime prefix\n \n Ever since Git learned to detect its install location at runtime, there\n was the slightly awkward problem that it was impossible to specify paths\n @@ Commit message\n they are interpreted as absolute paths in the same drive as the current\n directory.\n \n - After a lengthy discussion, and a way lengthier time to mull over the\n - problem and its best solution, we decided to introduce support for the\n - magic sequence `/`. If a path starts with this, the\n - remainder is interpreted as relative to the detected runtime prefix.\n + After a lengthy discussion, and an even lengthier time to mull over the\n + problem and its best solution, and then more discussions, we eventually\n + decided to introduce support for the magic sequence `%(prefix)/`. If a\n + path starts with this, the remainder is interpreted as relative to the\n + detected (runtime) prefix. If built without runtime prefix support, Git\n + will simply interpolate the compiled-in prefix.\n \n - This solves the problem, but what new problems does it stir up? Here are\n - the two most obvious ones:\n -\n - - What if Git was not compiled with support for a runtime prefix?\n -\n - In that case, we will simply use the compiled-in hard-coded prefix.\n -\n - - What if a user _wants_ to specify a path starting with the magic\n - sequence?\n -\n - In that case, the user will simply need to prefix the magic sequence\n - with `./` and voilà, the path won't be expanded.\n + If a user _wants_ to specify a path starting with the magic sequence,\n + they can prefix the magic sequence with `./` and voilà, the path won't\n + be expanded.\n \n Signed-off-by: Johannes Schindelin \n \n @@ Documentation/config.txt: pathname::\n \tis expanded to the value of `$HOME`, and `~user/` to the\n \tspecified user's home directory.\n ++\n -+If a path starts with `/`, the remainder is\n -+interpreted as a path relative to Git's \"runtime prefix\", i.e. relative\n -+to the location where Git itself was installed. For example,\n -+`/bin/` refers to the directory in which the Git\n -+executable itself lives. If Git was compiled without runtime prefix\n -+support, the compiled-in prefix will be subsituted instead. In the\n -+unlikely event that a literal path needs to be specified that should\n -+_not_ be expanded, it needs to be prefixed by `./`, like so:\n -+`.//bin`.\n ++If a path starts with `%(prefix)/`, the remainder is interpreted as a\n ++path relative to Git's \"runtime prefix\", i.e. relative to the location\n ++where Git itself was installed. For example, `%(prefix)/bin/` refers to\n ++the directory in which the Git executable itself lives. If Git was\n ++compiled without runtime prefix support, the compiled-in prefix will be\n ++subsituted instead. In the unlikely event that a literal path needs to\n ++be specified that should _not_ be expanded, it needs to be prefixed by\n ++`./`, like so: `./%(prefix)/bin`.\n \n \n Variables\n @@ path.c\n \n static int get_st_mode_bits(const char *path, int *mode)\n {\n -@@ path.c: char *expand_user_path(const char *path, int real_home)\n +@@ path.c: static struct passwd *getpw_str(const char *username, size_t len)\n + * failure or if path is NULL.\n + *\n + * If real_home is true, strbuf_realpath($HOME) is used in the `~/` expansion.\n ++ *\n ++ * If the path starts with `%(prefix)/`, the remainder is interpreted as\n ++ * relative to where Git is installed, and expanded to the absolute path.\n + */\n + char *interpolate_path(const char *path, int real_home)\n + {\n +@@ path.c: char *interpolate_path(const char *path, int real_home)\n \n \tif (path == NULL)\n \t\tgoto return_null;\n +\n -+\tif (skip_prefix(path, \"/\", &path))\n ++\tif (skip_prefix(path, \"%(prefix)/\", &path))\n +\t\treturn system_path(path);\n +\n \tif (path[0] == '~') {\n @@ t/t0060-path-utils.sh: test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTI\n \techo HERE >expect &&\n +\ttest_cmp expect actual'\n +\n -+test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '/ works' '\n ++test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' '\n +\tmkdir -p pretend/bin &&\n +\tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n -+\tgit config yes.path \"/yes\" &&\n ++\tgit config yes.path \"%(prefix)/yes\" &&\n +\tGIT_EXEC_PATH= ./pretend/bin/git config --path yes.path >actual &&\n +\techo \"$(pwd)/pretend/yes\" >expect &&\n \ttest_cmp expect actual\n\n-- \ngitgitgadget\n"},{"id":"431123","messageId":"cc8f09baba94546a13087e923e9c2374284c8136.1627164413.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v3.git.1627164413.gitgitgadget@gmail.com","subject":"[PATCH v3 1/5] tests: exercise the RUNTIME_PREFIX feature","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-24T22:06:49Z","receivedAt":"2021-07-24T22:07:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nOriginally, we refrained from adding a regression test in 7b6c6496374\n(system_path(): Add prefix computation at runtime if RUNTIME_PREFIX set,\n2008-08-10), and in 226c0ddd0d6 (exec_cmd: RUNTIME_PREFIX on some POSIX\nsystems, 2018-04-10).\n\nThe reason was that it was deemed too tricky to test.\n\nTurns out that it is not tricky to test at all: we simply create a\npseudo-root, copy the `git` executable into the `git/` subdirectory of\nthat pseudo-root, then copy a script into the `libexec/git-core/`\ndirectory and expect that to be picked up.\n\nAs long as the trash directory is in a location where binaries can be\nexecuted, this works.\n\nSigned-off-by: Johannes Schindelin \n---\n Makefile | 5 +++++\n t/t0060-path-utils.sh | 18 ++++++++++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex c3565fc0f8f..4c3e4ccabcd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2826,6 +2826,11 @@ ifdef GIT_TEST_INDEX_VERSION\n endif\n ifdef GIT_TEST_PERL_FATAL_WARNINGS\n \t@echo GIT_TEST_PERL_FATAL_WARNINGS=\\''$(subst ','\\'',$(subst ','\\'',$(GIT_TEST_PERL_FATAL_WARNINGS)))'\\' >>$@+\n+endif\n+ifdef RUNTIME_PREFIX\n+\t@echo RUNTIME_PREFIX=\\'true\\' >>$@+\n+else\n+\t@echo RUNTIME_PREFIX=\\'false\\' >>$@+\n endif\n \t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n \ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex de4960783f0..a76728c27bf 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -525,4 +525,22 @@ test_expect_success MINGW 'is_valid_path() on Windows' '\n \t\t\"PRN./abc\"\n '\n \n+test_lazy_prereq RUNTIME_PREFIX '\n+\ttest true = \"$RUNTIME_PREFIX\"\n+'\n+\n+test_lazy_prereq CAN_EXEC_IN_PWD '\n+\tcp \"$GIT_EXEC_PATH\"/git$X ./ &&\n+\t./git rev-parse\n+'\n+\n+test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' '\n+\tmkdir -p pretend/bin pretend/libexec/git-core &&\n+\techo \"echo HERE\" | write_script pretend/libexec/git-core/git-here &&\n+\tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n+\tGIT_EXEC_PATH= ./pretend/bin/git here >actual &&\n+\techo HERE >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"431124","messageId":"7b79ba66dd0d43f2565d2cbba268f01d3d36d052.1627164413.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v3.git.1627164413.gitgitgadget@gmail.com","subject":"[PATCH v3 3/5] expand_user_path(): clarify the role of the `real_home` parameter","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-24T22:06:51Z","receivedAt":"2021-07-24T22:07:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe `real_home` parameter only has an effect when expanding paths\nstarting with `~/`, not when expanding paths starting with `~/`.\nLet's make that clear.\n\nSigned-off-by: Johannes Schindelin \n---\n path.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/path.c b/path.c\nindex 3318ad24336..bf329e535cf 100644\n--- a/path.c\n+++ b/path.c\n@@ -722,7 +722,7 @@ static struct passwd *getpw_str(const char *username, size_t len)\n * Return a string with ~ and ~user expanded via getpw*. Returns NULL on getpw\n * failure or if path is NULL.\n *\n- * If real_home is true, strbuf_realpath($HOME) is used in the expansion.\n+ * If real_home is true, strbuf_realpath($HOME) is used in the `~/` expansion.\n */\n char *expand_user_path(const char *path, int real_home)\n {\n-- \ngitgitgadget\n\n"},{"id":"431125","messageId":"19fd9c3c803a300b586c76736301a7379c4c6226.1627164413.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v3.git.1627164413.gitgitgadget@gmail.com","subject":"[PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-24T22:06:52Z","receivedAt":"2021-07-24T22:07:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIt is not immediately clear what `expand_user_path()` means, so let's\nrename it to `interpolate_path()`. This also opens the path for\ninterpolating more than just a home directory.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/credential-cache.c | 2 +-\n builtin/credential-store.c | 2 +-\n builtin/gc.c | 2 +-\n cache.h | 2 +-\n config.c | 8 ++++----\n path.c | 4 ++--\n sequencer.c | 2 +-\n 7 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/credential-cache.c b/builtin/credential-cache.c\nindex 76a6ba37223..e8a74157471 100644\n--- a/builtin/credential-cache.c\n+++ b/builtin/credential-cache.c\n@@ -90,7 +90,7 @@ static char *get_socket_path(void)\n {\n \tstruct stat sb;\n \tchar *old_dir, *socket;\n-\told_dir = expand_user_path(\"~/.git-credential-cache\", 0);\n+\told_dir = interpolate_path(\"~/.git-credential-cache\", 0);\n \tif (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode))\n \t\tsocket = xstrfmt(\"%s/socket\", old_dir);\n \telse\ndiff --git a/builtin/credential-store.c b/builtin/credential-store.c\nindex ae3c1ba75fe..62a4f3c2653 100644\n--- a/builtin/credential-store.c\n+++ b/builtin/credential-store.c\n@@ -173,7 +173,7 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)\n \tif (file) {\n \t\tstring_list_append(&fns, file);\n \t} else {\n-\t\tif ((file = expand_user_path(\"~/.git-credentials\", 0)))\n+\t\tif ((file = interpolate_path(\"~/.git-credentials\", 0)))\n \t\t\tstring_list_append_nodup(&fns, file);\n \t\tfile = xdg_config_home(\"credentials\");\n \t\tif (file)\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex f05d2f0a1ac..6ce5ca45126 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1542,7 +1542,7 @@ static char *launchctl_service_filename(const char *name)\n \tstruct strbuf filename = STRBUF_INIT;\n \tstrbuf_addf(&filename, \"~/Library/LaunchAgents/%s.plist\", name);\n \n-\texpanded = expand_user_path(filename.buf, 1);\n+\texpanded = interpolate_path(filename.buf, 1);\n \tif (!expanded)\n \t\tdie(_(\"failed to expand path '%s'\"), filename.buf);\n \ndiff --git a/cache.h b/cache.h\nindex ba04ff8bd36..87e4cbe9c5f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1246,7 +1246,7 @@ typedef int create_file_fn(const char *path, void *cb);\n int raceproof_create_file(const char *path, create_file_fn fn, void *cb);\n \n int mkdir_in_gitdir(const char *path);\n-char *expand_user_path(const char *path, int real_home);\n+char *interpolate_path(const char *path, int real_home);\n const char *enter_repo(const char *path, int strict);\n static inline int is_absolute_path(const char *path)\n {\ndiff --git a/config.c b/config.c\nindex f9c400ad306..c17b9797292 100644\n--- a/config.c\n+++ b/config.c\n@@ -137,7 +137,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \tif (!path)\n \t\treturn config_error_nonbool(\"include.path\");\n \n-\texpanded = expand_user_path(path, 0);\n+\texpanded = interpolate_path(path, 0);\n \tif (!expanded)\n \t\treturn error(_(\"could not expand include path '%s'\"), path);\n \tpath = expanded;\n@@ -185,7 +185,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)\n \tchar *expanded;\n \tint prefix = 0;\n \n-\texpanded = expand_user_path(pat->buf, 1);\n+\texpanded = interpolate_path(pat->buf, 1);\n \tif (expanded) {\n \t\tstrbuf_reset(pat);\n \t\tstrbuf_addstr(pat, expanded);\n@@ -1270,7 +1270,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value)\n {\n \tif (!value)\n \t\treturn config_error_nonbool(var);\n-\t*dest = expand_user_path(value, 0);\n+\t*dest = interpolate_path(value, 0);\n \tif (!*dest)\n \t\tdie(_(\"failed to expand user dir in: '%s'\"), value);\n \treturn 0;\n@@ -1844,7 +1844,7 @@ void git_global_config(char **user_out, char **xdg_out)\n \tchar *xdg_config = NULL;\n \n \tif (!user_config) {\n-\t\tuser_config = expand_user_path(\"~/.gitconfig\", 0);\n+\t\tuser_config = interpolate_path(\"~/.gitconfig\", 0);\n \t\txdg_config = xdg_config_home(\"config\");\n \t}\n \ndiff --git a/path.c b/path.c\nindex bf329e535cf..8dc5ad2cb55 100644\n--- a/path.c\n+++ b/path.c\n@@ -724,7 +724,7 @@ static struct passwd *getpw_str(const char *username, size_t len)\n *\n * If real_home is true, strbuf_realpath($HOME) is used in the `~/` expansion.\n */\n-char *expand_user_path(const char *path, int real_home)\n+char *interpolate_path(const char *path, int real_home)\n {\n \tstruct strbuf user_path = STRBUF_INIT;\n \tconst char *to_copy = path;\n@@ -811,7 +811,7 @@ const char *enter_repo(const char *path, int strict)\n \t\tstrbuf_add(&validated_path, path, len);\n \n \t\tif (used_path.buf[0] == '~') {\n-\t\t\tchar *newpath = expand_user_path(used_path.buf, 0);\n+\t\t\tchar *newpath = interpolate_path(used_path.buf, 0);\n \t\t\tif (!newpath)\n \t\t\t\treturn NULL;\n \t\t\tstrbuf_attach(&used_path, newpath, strlen(newpath),\ndiff --git a/sequencer.c b/sequencer.c\nindex 0bec01cf38e..007a851804d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1241,7 +1241,7 @@ N_(\"Your name and email address were configured automatically based\\n\"\n \n static const char *implicit_ident_advice(void)\n {\n-\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n+\tchar *user_config = interpolate_path(\"~/.gitconfig\", 0);\n \tchar *xdg_config = xdg_config_home(\"config\");\n \tint config_exists = file_exists(user_config) || file_exists(xdg_config);\n \n-- \ngitgitgadget\n\n"},{"id":"431126","messageId":"a4ff3a461bc42d11e5ef9e40deeb90d17919943d.1627164413.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v3.git.1627164413.gitgitgadget@gmail.com","subject":"[PATCH v3 2/5] expand_user_path(): remove stale part of the comment","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-24T22:06:50Z","receivedAt":"2021-07-24T22:07:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 395de250d9d (Expand ~ and ~user in core.excludesfile,\ncommit.template, 2009-11-17), the `user_path()` function was refactored\ninto the `expand_user_path()`. During that refactoring, the `buf`\nparameter was lost, but the code comment above said function still talks\nabout it. Let's remove that stale part of the comment.\n\nSigned-off-by: Johannes Schindelin \n---\n path.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex 7bccd830e95..3318ad24336 100644\n--- a/path.c\n+++ b/path.c\n@@ -719,9 +719,8 @@ static struct passwd *getpw_str(const char *username, size_t len)\n }\n \n /*\n- * Return a string with ~ and ~user expanded via getpw*. If buf != NULL,\n- * then it is a newly allocated string. Returns NULL on getpw failure or\n- * if path is NULL.\n+ * Return a string with ~ and ~user expanded via getpw*. Returns NULL on getpw\n+ * failure or if path is NULL.\n *\n * If real_home is true, strbuf_realpath($HOME) is used in the expansion.\n */\n-- \ngitgitgadget\n\n"},{"id":"431127","messageId":"d286583082e13be0ed4e1d4d923b6bdcf0c234a6.1627164414.git.gitgitgadget@gmail.com","threadId":"49770","inReplyTo":"pull.66.v3.git.1627164413.gitgitgadget@gmail.com","subject":"[PATCH v3 5/5] interpolate_path(): allow specifying paths relative to the runtime prefix","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-24T22:06:53Z","receivedAt":"2021-07-24T22:07:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEver since Git learned to detect its install location at runtime, there\nwas the slightly awkward problem that it was impossible to specify paths\nrelative to said location.\n\nFor example, if a version of Git was shipped with custom SSL\ncertificates to use, there was no portable way to specify\n`http.sslCAInfo`.\n\nIn Git for Windows, the problem was \"solved\" for years by interpreting\npaths starting with a slash as relative to the runtime prefix.\n\nHowever, this is not correct: such paths _are_ legal on Windows, and\nthey are interpreted as absolute paths in the same drive as the current\ndirectory.\n\nAfter a lengthy discussion, and an even lengthier time to mull over the\nproblem and its best solution, and then more discussions, we eventually\ndecided to introduce support for the magic sequence `%(prefix)/`. If a\npath starts with this, the remainder is interpreted as relative to the\ndetected (runtime) prefix. If built without runtime prefix support, Git\nwill simply interpolate the compiled-in prefix.\n\nIf a user _wants_ to specify a path starting with the magic sequence,\nthey can prefix the magic sequence with `./` and voilà, the path won't\nbe expanded.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/config.txt | 9 +++++++++\n path.c | 8 ++++++++\n t/t0060-path-utils.sh | 8 ++++++++\n 3 files changed, 25 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bf82766a6a2..0c0e6b859f1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -298,6 +298,15 @@ pathname::\n \ttilde expansion happens to such a string: `~/`\n \tis expanded to the value of `$HOME`, and `~user/` to the\n \tspecified user's home directory.\n++\n+If a path starts with `%(prefix)/`, the remainder is interpreted as a\n+path relative to Git's \"runtime prefix\", i.e. relative to the location\n+where Git itself was installed. For example, `%(prefix)/bin/` refers to\n+the directory in which the Git executable itself lives. If Git was\n+compiled without runtime prefix support, the compiled-in prefix will be\n+subsituted instead. In the unlikely event that a literal path needs to\n+be specified that should _not_ be expanded, it needs to be prefixed by\n+`./`, like so: `./%(prefix)/bin`.\n \n \n Variables\ndiff --git a/path.c b/path.c\nindex 8dc5ad2cb55..0bc788ea40f 100644\n--- a/path.c\n+++ b/path.c\n@@ -12,6 +12,7 @@\n #include \"packfile.h\"\n #include \"object-store.h\"\n #include \"lockfile.h\"\n+#include \"exec-cmd.h\"\n \n static int get_st_mode_bits(const char *path, int *mode)\n {\n@@ -723,6 +724,9 @@ static struct passwd *getpw_str(const char *username, size_t len)\n * failure or if path is NULL.\n *\n * If real_home is true, strbuf_realpath($HOME) is used in the `~/` expansion.\n+ *\n+ * If the path starts with `%(prefix)/`, the remainder is interpreted as\n+ * relative to where Git is installed, and expanded to the absolute path.\n */\n char *interpolate_path(const char *path, int real_home)\n {\n@@ -731,6 +735,10 @@ char *interpolate_path(const char *path, int real_home)\n \n \tif (path == NULL)\n \t\tgoto return_null;\n+\n+\tif (skip_prefix(path, \"%(prefix)/\", &path))\n+\t\treturn system_path(path);\n+\n \tif (path[0] == '~') {\n \t\tconst char *first_slash = strchrnul(path, '/');\n \t\tconst char *username = path + 1;\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex a76728c27bf..34d1061f321 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -540,6 +540,14 @@ test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' '\n \tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n \tGIT_EXEC_PATH= ./pretend/bin/git here >actual &&\n \techo HERE >expect &&\n+\ttest_cmp expect actual'\n+\n+test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' '\n+\tmkdir -p pretend/bin &&\n+\tcp \"$GIT_EXEC_PATH\"/git$X pretend/bin/ &&\n+\tgit config yes.path \"%(prefix)/yes\" &&\n+\tGIT_EXEC_PATH= ./pretend/bin/git config --path yes.path >actual &&\n+\techo \"$(pwd)/pretend/yes\" >expect &&\n \ttest_cmp expect actual\n '\n \n-- \ngitgitgadget\n"},{"id":"431206","messageId":"xmqqk0ldj6w3.fsf@gitster.g","threadId":"49770","inReplyTo":"pull.66.v3.git.1627164413.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/5] mingw: handle absolute paths in expand_user_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-26T17:56:44Z","receivedAt":"2021-07-26T17:56:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> * Since our convention of %(...) interpolation does not involve uppercase\n> keywords, I now use a lowercase one.\n\nMakes sense.\n\n> * Since this keyword is interpolated to the compiled-in prefix if built\n> without runtime prefix support, I dropped the runtime part of the\n> keyword.\n\nI have this nagging feeling that %(prefix) may be (mistakenly)\nexpected to interporate to $(git rev-parse --show-prefix). Of\ncourse, nobody would expect that in paths in the configuration\nfiles, but because we are borrowing %(token) convention from\nelsewhere, it is not outragous to imagine that either \"for-each-ref\"\nfamily or \"log\" family of placeholders may want to use %(prefix)\"\nfor such purpose (for that matter, they may also be helped to have\nthe runtime-prefix information available).\n\nPerhaps %(installPrefix) or something may have less chance of making\nus regret later. I am just raising this as a possible problem; I\npersonally would not be confused if we settled on the %(prefix).\n\n> * Renamed the expand_user_path() to interpolate_path(), to remove the\n> distraction as to the implementation detail which things get to be\n> interpolated (because we extend it to interpolate more than just a home\n> directory, which might well be unclear from the former name, anyway).\n\nGreat.\n\n> * Adjusted the code comment above the interpolate_path() to remove a stale\n> part, clarify another part, and to extend it to talk about the prefix\n> expansion, too.\n\nThese looked good, too.\n\nThanks.\n"},{"id":"431239","messageId":"xmqqh7gghgtd.fsf@gitster.g","threadId":"49770","inReplyTo":"19fd9c3c803a300b586c76736301a7379c4c6226.1627164413.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-26T22:05:18Z","receivedAt":"2021-07-26T22:05:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> It is not immediately clear what `expand_user_path()` means, so let's\n> rename it to `interpolate_path()`. This also opens the path for\n> interpolating more than just a home directory.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> ...\n> diff --git a/cache.h b/cache.h\n> index ba04ff8bd36..87e4cbe9c5f 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1246,7 +1246,7 @@ typedef int create_file_fn(const char *path, void *cb);\n> int raceproof_create_file(const char *path, create_file_fn fn, void *cb);\n> \n> int mkdir_in_gitdir(const char *path);\n> -char *expand_user_path(const char *path, int real_home);\n> +char *interpolate_path(const char *path, int real_home);\n\nThis of course breaks any topic in flight that adds more places to\nuse expand_user_path().\n\nI think Fabian's \"ssh signing\" is not as ready as this topic, and it\ncan afford to wait by rebasing on top of this topic. By the time\n\"ssh signing\" gets into testable shape (right now, it does not pass\ntests when merged to 'seen'), hopefully the \"expand install-prefix\"\ntopic may already be in 'next' if not in 'master'.\n\nIn the meantime, I am adding this band-aid at the tip of this topic\nto help these two topics play together better.\n\nThanks.\n\n\ndiff --git a/cache.h b/cache.h\nindex 87e4cbe9c5..679a27e17c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1247,6 +1247,8 @@ int raceproof_create_file(const char *path, create_file_fn fn, void *cb);\n \n int mkdir_in_gitdir(const char *path);\n char *interpolate_path(const char *path, int real_home);\n+/* NEEDSWORK: remove this synonym once in-flight topics have migrated */\n+#define expand_user_path interpolate_path\n const char *enter_repo(const char *path, int strict);\n static inline int is_absolute_path(const char *path)\n {\n"},{"id":"431290","messageId":"99332fdf-24af-8938-2f55-b6e25ca12aad@gigacodes.de","threadId":"49770","inReplyTo":"xmqqh7gghgtd.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-07-27T07:57:48Z","receivedAt":"2021-07-27T07:57:57Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 27.07.21 00:05, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n>> From: Johannes Schindelin \n>>\n>> It is not immediately clear what `expand_user_path()` means, so let's\n>> rename it to `interpolate_path()`. This also opens the path for\n>> interpolating more than just a home directory.\n>>\n>> Signed-off-by: Johannes Schindelin \n>> ---\n>> ...\n>> diff --git a/cache.h b/cache.h\n>> index ba04ff8bd36..87e4cbe9c5f 100644\n>> --- a/cache.h\n>> +++ b/cache.h\n>> @@ -1246,7 +1246,7 @@ typedef int create_file_fn(const char *path, void *cb);\n>> int raceproof_create_file(const char *path, create_file_fn fn, void *cb);\n>> \n>> int mkdir_in_gitdir(const char *path);\n>> -char *expand_user_path(const char *path, int real_home);\n>> +char *interpolate_path(const char *path, int real_home);\n> This of course breaks any topic in flight that adds more places to\n> use expand_user_path().\n>\n> I think Fabian's \"ssh signing\" is not as ready as this topic, and it\n> can afford to wait by rebasing on top of this topic. By the time\n> \"ssh signing\" gets into testable shape (right now, it does not pass\n> tests when merged to 'seen'), hopefully the \"expand install-prefix\"\n> topic may already be in 'next' if not in 'master'.\nI think the test problem is not due to my patch.\nLike Ævar wrote it's \"hn/reftable probably interacting with my \nab/refs-files-cleanup\" [1]\nThe failed tests i can see in [2] are either t0031-reftable.sh or a \ncompile failure referencing reftable as well.\nIf i merge the ssh code into the current seen branch everything works \nfine for me. Let me know if you have any other results / CI runs that \nmight help.\n\n[1] https://lore.kernel.org/git/87a6mevkrx.fsf@evledraar.gmail.com/\n[2] https://github.com/git/git/actions/runs/1053603028\n>\n> In the meantime, I am adding this band-aid at the tip of this topic\n> to help these two topics play together better.\n>\n> Thanks.\n>\n>\n> diff --git a/cache.h b/cache.h\n> index 87e4cbe9c5..679a27e17c 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1247,6 +1247,8 @@ int raceproof_create_file(const char *path, create_file_fn fn, void *cb);\n> \n> int mkdir_in_gitdir(const char *path);\n> char *interpolate_path(const char *path, int real_home);\n> +/* NEEDSWORK: remove this synonym once in-flight topics have migrated */\n> +#define expand_user_path interpolate_path\n> const char *enter_repo(const char *path, int strict);\n> static inline int is_absolute_path(const char *path)\n> {\n"},{"id":"431370","messageId":"xmqqfsvzcqmo.fsf@gitster.g","threadId":"49770","inReplyTo":"99332fdf-24af-8938-2f55-b6e25ca12aad@gigacodes.de","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-27T22:56:47Z","receivedAt":"2021-07-27T22:56:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Stelzer writes:\n\n>> I think Fabian's \"ssh signing\" is not as ready as this topic, and it\n>> can afford to wait by rebasing on top of this topic. By the time\n>> \"ssh signing\" gets into testable shape (right now, it does not pass\n>> tests when merged to 'seen'), hopefully the \"expand install-prefix\"\n>> topic may already be in 'next' if not in 'master'.\n> I think the test problem is not due to my patch.\n\nI've been seeing these test failers locally, every time\nfs/ssh-signing topic is merged to 'seen' (without the reftable\nthing).\n\nTest Summary Report\n-------------------\nt5534-push-signed.sh (Wstat: 256 Tests: 13 Failed: 2)\n Failed tests: 8, 12\n Non-zero exit status: 1\nt7528-signed-commit-ssh.sh (Wstat: 256 Tests: 23 Failed: 2)\n Failed tests: 13, 17\n Non-zero exit status: 1\n\nWhen reftable thing is merged, either compilation fails or t0031\nfails, and I suspect that these are not due to the ssh signing\ntopic.\n\n"},{"id":"431371","messageId":"xmqq8s1rcn0t.fsf@gitster.g","threadId":"49770","inReplyTo":"xmqqfsvzcqmo.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-28T00:14:42Z","receivedAt":"2021-07-28T00:14:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Fabian Stelzer writes:\n>\n>>> I think Fabian's \"ssh signing\" is not as ready as this topic, and it\n>>> can afford to wait by rebasing on top of this topic. By the time\n>>> \"ssh signing\" gets into testable shape (right now, it does not pass\n>>> tests when merged to 'seen'), hopefully the \"expand install-prefix\"\n>>> topic may already be in 'next' if not in 'master'.\n>> I think the test problem is not due to my patch.\n>\n> I've been seeing these test failers locally, every time\n> fs/ssh-signing topic is merged to 'seen' (without the reftable\n> thing).\n>\n> Test Summary Report\n> -------------------\n> t5534-push-signed.sh (Wstat: 256 Tests: 13 Failed: 2)\n> Failed tests: 8, 12\n> Non-zero exit status: 1\n> t7528-signed-commit-ssh.sh (Wstat: 256 Tests: 23 Failed: 2)\n> Failed tests: 13, 17\n> Non-zero exit status: 1\n>\n> When reftable thing is merged, either compilation fails or t0031\n> fails, and I suspect that these are not due to the ssh signing\n> topic.\n\nInteresting. It seems that the failure has some correlation with\nthe use of --root= option.\n\n $ sh t5534-push-signed.sh -i\n ok 1 - setup\n ok 2 - unsigned push does not send push certificate\n ok 3 - talking with a receiver without push certificate support\n ok 4 - push --signed fails with a receiver without push certificate\n support\n ok 5 - push --signed=1 is accepted\n ok 6 - no certificate for a signed push with no update\n ok 7 - signed push sends push certificate\n ok 8 - ssh signed push sends push certificate\n ok 9 - inconsistent push options in signed push not allowed\n ok 10 - fail without key and heed user.signingkey\n ok 11 - fail without key and heed user.signingkey x509\n ok 12 - fail without key and heed user.signingkey ssh\n ok 13 - failed atomic push does not execute GPG\n # passed all 13 test(s)\n 1..13\n\npasses just fine, but\n\n $ TESTPEN=/dev/shm/testpen.$$\n $ rm -fr \"$TESTPEN\" && mkdir \"$TESTPEN\"\n $ sh t5534-push-signed.sh --root=$TESTPEN -i -v\n\ndies like this:\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/.git/\nexpecting success of 5534.1 'setup': \n\t# main, ff and noff branches pointing at the same commit\n\ttest_tick &&\n\tgit commit --allow-empty -m initial &&\n\n\tgit checkout -b noop &&\n\tgit checkout -b ff &&\n\tgit checkout -b noff &&\n\n\t# noop stays the same, ff advances, noff rewrites\n\ttest_tick &&\n\tgit commit --allow-empty --amend -m rewritten &&\n\tgit checkout ff &&\n\n\ttest_tick &&\n\tgit commit --allow-empty -m second\n\n[main (root-commit) 66fe8b3] initial\n Author: A U Thor \nSwitched to a new branch 'noop'\nSwitched to a new branch 'ff'\nSwitched to a new branch 'noff'\n[noff 6391b7f] rewritten\n Author: A U Thor \n Date: Thu Apr 7 15:13:13 2005 -0700\nSwitched to branch 'ff'\n[ff 566fbd3] second\n Author: A U Thor \nok 1 - setup\n\nexpecting success of 5534.2 'unsigned push does not send push certificate': \n\tprepare_dst &&\n\tmkdir -p dst/.git/hooks &&\n\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n\t# discard the update list\n\tcat >/dev/null\n\t# record the push certificate\n\tif test -n \"${GIT_PUSH_CERT-}\"\n\tthen\n\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n\tfi\n\tEOF\n\n\tgit push dst noop ff +noff &&\n\t! test -f dst/push-cert\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/\nTo dst\n * [new branch] main -> noop\n * [new branch] main -> ff\n * [new branch] main -> noff\nTo dst\n 66fe8b3..566fbd3 ff -> ff\n + 66fe8b3...6391b7f noff -> noff (forced update)\nok 2 - unsigned push does not send push certificate\n\nexpecting success of 5534.3 'talking with a receiver without push certificate support': \n\tprepare_dst &&\n\tmkdir -p dst/.git/hooks &&\n\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n\t# discard the update list\n\tcat >/dev/null\n\t# record the push certificate\n\tif test -n \"${GIT_PUSH_CERT-}\"\n\tthen\n\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n\tfi\n\tEOF\n\n\tgit push dst noop ff +noff &&\n\t! test -f dst/push-cert\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/\nTo dst\n * [new branch] main -> noop\n * [new branch] main -> ff\n * [new branch] main -> noff\nTo dst\n 66fe8b3..566fbd3 ff -> ff\n + 66fe8b3...6391b7f noff -> noff (forced update)\nok 3 - talking with a receiver without push certificate support\n\nexpecting success of 5534.4 'push --signed fails with a receiver without push certificate support': \n\tprepare_dst &&\n\tmkdir -p dst/.git/hooks &&\n\ttest_must_fail git push --signed dst noop ff +noff 2>err &&\n\ttest_i18ngrep \"the receiving end does not support\" err\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/\nTo dst\n * [new branch] main -> noop\n * [new branch] main -> ff\n * [new branch] main -> noff\nfatal: the receiving end does not support --signed push\nok 4 - push --signed fails with a receiver without push certificate support\n\nexpecting success of 5534.5 'push --signed=1 is accepted': \n\tprepare_dst &&\n\tmkdir -p dst/.git/hooks &&\n\ttest_must_fail git push --signed=1 dst noop ff +noff 2>err &&\n\ttest_i18ngrep \"the receiving end does not support\" err\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/\nTo dst\n * [new branch] main -> noop\n * [new branch] main -> ff\n * [new branch] main -> noff\nfatal: the receiving end does not support --signed push\nok 5 - push --signed=1 is accepted\n\nchecking prerequisite: GPG\n\nmkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-GPG\" &&\n(\n\tcd \"$TRASH_DIRECTORY/prereq-test-dir-GPG\" &&\n\tgpg_version=$(gpg --version 2>&1)\n\ttest $? != 127 || exit 1\n\n\t# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19\n\t# the gpg version 1.0.6 did not parse trust packets correctly, so for\n\t# that version, creation of signed tags using the generated key fails.\n\tcase \"$gpg_version\" in\n\t\"gpg (GnuPG) 1.0.6\"*)\n\t\tsay \"Your version of gpg (1.0.6) is too buggy for testing\"\n\t\texit 1\n\t\t;;\n\t*)\n\t\t# Available key info:\n\t\t# * Type DSA and Elgamal, size 2048 bits, no expiration date,\n\t\t# name and email: C O Mitter \n\t\t# * Type RSA, size 2048 bits, no expiration date,\n\t\t# name and email: Eris Discordia \n\t\t# No password given, to enable non-interactive operation.\n\t\t# To generate new key:\n\t\t#\tgpg --homedir /tmp/gpghome --gen-key\n\t\t# To write armored exported key to keyring:\n\t\t#\tgpg --homedir /tmp/gpghome --export-secret-keys \\\n\t\t#\t\t--armor 0xDEADBEEF >> lib-gpg/keyring.gpg\n\t\t#\tgpg --homedir /tmp/gpghome --export \\\n\t\t#\t\t--armor 0xDEADBEEF >> lib-gpg/keyring.gpg\n\t\t# To export ownertrust:\n\t\t#\tgpg --homedir /tmp/gpghome --export-ownertrust \\\n\t\t#\t\t> lib-gpg/ownertrust\n\t\tmkdir \"$GNUPGHOME\" &&\n\t\tchmod 0700 \"$GNUPGHOME\" &&\n\t\t(gpgconf --kill gpg-agent || : ) &&\n\t\tgpg --homedir \"${GNUPGHOME}\" --import \\\n\t\t\t\"$TEST_DIRECTORY\"/lib-gpg/keyring.gpg &&\n\t\tgpg --homedir \"${GNUPGHOME}\" --import-ownertrust \\\n\t\t\t\"$TEST_DIRECTORY\"/lib-gpg/ownertrust &&\n\t\tgpg --homedir \"${GNUPGHOME}\" /dev/null \\\n\t\t\t--sign -u committer@example.com\n\t\t;;\n\tesac\n\n)\ngpg: keybox '/dev/shm/testpen.9441/trash directory.t5534-push-signed/gpghome/pubring.kbx' created\ngpg: /dev/shm/testpen.9441/trash directory.t5534-push-signed/gpghome/trustdb.gpg: trustdb created\ngpg: key 13B6F51ECDDE430D: public key \"C O Mitter \" imported\ngpg: key 13B6F51ECDDE430D: secret key imported\ngpg: key 61092E85B7227189: public key \"Eris Discordia \" imported\ngpg: key 61092E85B7227189: secret key imported\ngpg: key 13B6F51ECDDE430D: \"C O Mitter \" not changed\ngpg: key 61092E85B7227189: \"Eris Discordia \" not changed\ngpg: Total number processed: 4\ngpg: imported: 2\ngpg: unchanged: 2\ngpg: secret keys read: 2\ngpg: secret keys imported: 2\ngpg: inserting ownertrust of 6\ngpg: inserting ownertrust of 3\nprerequisite GPG ok\nexpecting success of 5534.6 'no certificate for a signed push with no update': \n\tprepare_dst &&\n\tmkdir -p dst/.git/hooks &&\n\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n\tif test -n \"${GIT_PUSH_CERT-}\"\n\tthen\n\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n\tfi\n\tEOF\n\tgit push dst noop &&\n\t! test -f dst/push-cert\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/\nTo dst\n * [new branch] main -> noop\n * [new branch] main -> ff\n * [new branch] main -> noff\nEverything up-to-date\nok 6 - no certificate for a signed push with no update\n\nexpecting success of 5534.7 'signed push sends push certificate': \n\tprepare_dst &&\n\tmkdir -p dst/.git/hooks &&\n\tgit -C dst config receive.certnonceseed sekrit &&\n\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n\t# discard the update list\n\tcat >/dev/null\n\t# record the push certificate\n\tif test -n \"${GIT_PUSH_CERT-}\"\n\tthen\n\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n\tfi &&\n\n\tcat >../push-cert-status <\n\t\tKEY=13B6F51ECDDE430D\n\t\tSTATUS=G\n\t\tNONCE_STATUS=OK\n\t\tEOF\n\t\tsed -n -e \"s/^nonce /NONCE=/p\" -e \"/^$/q\" dst/push-cert\n\t) >expect &&\n\n\tnoop=$(git rev-parse noop) &&\n\tff=$(git rev-parse ff) &&\n\tnoff=$(git rev-parse noff) &&\n\tgrep \"$noop $ff refs/heads/ff\" dst/push-cert &&\n\tgrep \"$noop $noff refs/heads/noff\" dst/push-cert &&\n\ttest_cmp expect dst/push-cert-status\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/\nTo dst\n * [new branch] main -> noop\n * [new branch] main -> ff\n * [new branch] main -> noff\nTo dst\n 66fe8b3..566fbd3 ff -> ff\n + 66fe8b3...6391b7f noff -> noff (forced update)\n66fe8b3f2df5c2a6e67944af865f3a0893093d69 566fbd34a75c18947f0bcd052512caf55e7144ba refs/heads/ff\n66fe8b3f2df5c2a6e67944af865f3a0893093d69 6391b7f36bc1393eab3cad0aaf8c08cdacbe78fa refs/heads/noff\nok 7 - signed push sends push certificate\n\nchecking prerequisite: GPGSSH\n\nmkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-GPGSSH\" &&\n(\n\tcd \"$TRASH_DIRECTORY/prereq-test-dir-GPGSSH\" &&\n\tssh_version=$(ssh-keygen -Y find-principals -n \"git\" 2>&1)\n\ttest $? != 127 || exit 1\n\techo $ssh_version | grep -q \"find-principals:missing signature file\"\n\ttest $? = 0 || exit 1;\n\tmkdir -p \"${GNUPGHOME}\" &&\n\tchmod 0700 \"${GNUPGHOME}\" &&\n\tssh-keygen -t ed25519 -N \"\" -f \"${GNUPGHOME}/ed25519_ssh_signing_key\" >/dev/null &&\n\tssh-keygen -t rsa -b 2048 -N \"\" -f \"${GNUPGHOME}/rsa_2048_ssh_signing_key\" >/dev/null &&\n\tssh-keygen -t ed25519 -N \"super_secret\" -f \"${GNUPGHOME}/protected_ssh_signing_key\" >/dev/null &&\n\tfind \"${GNUPGHOME}\" -name *ssh_signing_key.pub -exec cat {} \\; | awk \"{print \\\"\\\\\\\"principal with number \\\" NR \\\"\\\\\\\" \\\" \\$0}\" > \"${GNUPGHOME}/ssh.all_valid.allowedSignersFile\" &&\n\tcat \"${GNUPGHOME}/ssh.all_valid.allowedSignersFile\" &&\n\tssh-keygen -t ed25519 -N \"\" -f \"${GNUPGHOME}/untrusted_ssh_signing_key\" >/dev/null\n\n)\n\"principal with number 1\" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIK+hDFZT7sYN3M+1ciMGLTM6pu/xmJrNDTK/vN+QIVnq jch@gitster.c.googlers.com\n\"principal with number 2\" ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDWLnqVpDYNstR7jCPKr1FaWzxt7NNw/kEV61GgKThW9S54/p/0WN+SCtUI0j7dCv/NkNhy87WNhohC9rCvZowPf/HRflHZ28vVk5d0DERCmlLBHnTq65Buyzj2R5xMtyVOBBMd+Io6tXk6GfJ6Dvq9l1wIJLv3OodOLBym3idV+8C+GOw9teTOlQWwkZD2hAt0n1Dy5WtaeGm3O2aUwRyg7KuxxmsgadBJ13a9thMYpwkS1McnwB+7oS81VXeeF0WcKAa2tGvxlg8NdBZuvvLJkl5vcISnpjt0NIuhqHzMlBIprpKujkZ5ze18YHXA52w6Y+9hG40n819EKjQfBVtD jch@gitster.c.googlers.com\n\"principal with number 3\" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAgtFx51cu1d0gzZOjIdw4M9oBYgV+tX6Bsm2L+riv/Z jch@gitster.c.googlers.com\nprerequisite GPGSSH ok\nexpecting success of 5534.8 'ssh signed push sends push certificate': \n\tprepare_dst &&\n\tmkdir -p dst/.git/hooks &&\n\tgit -C dst config gpg.ssh.allowedSignersFile \"${SIGNING_ALLOWED_SIGNERS}\" &&\n\tgit -C dst config receive.certnonceseed sekrit &&\n\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n\t# discard the update list\n\tcat >/dev/null\n\t# record the push certificate\n\tif test -n \"${GIT_PUSH_CERT-}\"\n\tthen\n\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n\tfi &&\n\n\tcat >../push-cert-status <expect &&\n\n\tnoop=$(git rev-parse noop) &&\n\tff=$(git rev-parse ff) &&\n\tnoff=$(git rev-parse noff) &&\n\tgrep \"$noop $ff refs/heads/ff\" dst/push-cert &&\n\tgrep \"$noop $noff refs/heads/noff\" dst/push-cert &&\n\ttest_cmp expect dst/push-cert-status\n\nInitialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/\nTo dst\n * [new branch] main -> noop\n * [new branch] main -> ff\n * [new branch] main -> noff\nTo dst\n 66fe8b3..566fbd3 ff -> ff\n + 66fe8b3...6391b7f noff -> noff (forced update)\n66fe8b3f2df5c2a6e67944af865f3a0893093d69 566fbd34a75c18947f0bcd052512caf55e7144ba refs/heads/ff\n66fe8b3f2df5c2a6e67944af865f3a0893093d69 6391b7f36bc1393eab3cad0aaf8c08cdacbe78fa refs/heads/noff\n--- expect\t2021-07-28 00:11:20.863019887 +0000\n+++ dst/push-cert-status\t2021-07-28 00:11:20.855019156 +0000\n@@ -1,4 +1,4 @@\n-SIGNER=principal with number 1\n+SIGNER=principal with number 3\n KEY=SHA256:Szd5rzYOrMBJFTR+gnRUu60YRVqg98UvpcSvmAm89rE\n STATUS=G\n NONCE_STATUS=OK\nnot ok 8 - ssh signed push sends push certificate\n#\t\n#\t\tprepare_dst &&\n#\t\tmkdir -p dst/.git/hooks &&\n#\t\tgit -C dst config gpg.ssh.allowedSignersFile \"${SIGNING_ALLOWED_SIGNERS}\" &&\n#\t\tgit -C dst config receive.certnonceseed sekrit &&\n#\t\twrite_script dst/.git/hooks/post-receive <<-\\EOF &&\n#\t\t# discard the update list\n#\t\tcat >/dev/null\n#\t\t# record the push certificate\n#\t\tif test -n \"${GIT_PUSH_CERT-}\"\n#\t\tthen\n#\t\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n#\t\tfi &&\n#\t\n#\t\tcat >../push-cert-status <expect &&\n#\t\n#\t\tnoop=$(git rev-parse noop) &&\n#\t\tff=$(git rev-parse ff) &&\n#\t\tnoff=$(git rev-parse noff) &&\n#\t\tgrep \"$noop $ff refs/heads/ff\" dst/push-cert &&\n#\t\tgrep \"$noop $noff refs/heads/noff\" dst/push-cert &&\n#\t\ttest_cmp expect dst/push-cert-status\n#\t\n"},{"id":"431372","messageId":"xmqq4kcfclw8.fsf@gitster.g","threadId":"49770","inReplyTo":"xmqq8s1rcn0t.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-28T00:39:03Z","receivedAt":"2021-07-28T00:39:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> Fabian Stelzer writes:\n>>\n>>>> I think Fabian's \"ssh signing\" is not as ready as this topic, and it\n>>>> can afford to wait by rebasing on top of this topic. By the time\n>>>> \"ssh signing\" gets into testable shape (right now, it does not pass\n>>>> tests when merged to 'seen'), hopefully the \"expand install-prefix\"\n>>>> topic may already be in 'next' if not in 'master'.\n>>> I think the test problem is not due to my patch.\n>>\n>> I've been seeing these test failers locally, every time\n>> fs/ssh-signing topic is merged to 'seen' (without the reftable\n>> thing).\n>>\n>> Test Summary Report\n>> -------------------\n>> t5534-push-signed.sh (Wstat: 256 Tests: 13 Failed: 2)\n>> Failed tests: 8, 12\n>> Non-zero exit status: 1\n>> t7528-signed-commit-ssh.sh (Wstat: 256 Tests: 23 Failed: 2)\n>> Failed tests: 13, 17\n>> Non-zero exit status: 1\n>>\n>> When reftable thing is merged, either compilation fails or t0031\n>> fails, and I suspect that these are not due to the ssh signing\n>> topic.\n>\n> Interesting. It seems that the failure has some correlation with\n> the use of --root= option.\n>\n> $ sh t5534-push-signed.sh -i\n\nAnd 7528 fails with --root set to a /dev/shm/ trash directory.\n\nSo,... this \"principal with number #\" differences come from the\ndifferences in location of the trash directory? Why?\n\n> --- expect\t2021-07-28 00:11:20.863019887 +0000\n> +++ dst/push-cert-status\t2021-07-28 00:11:20.855019156 +0000\n> @@ -1,4 +1,4 @@\n> -SIGNER=principal with number 1\n> +SIGNER=principal with number 3\n> KEY=SHA256:Szd5rzYOrMBJFTR+gnRUu60YRVqg98UvpcSvmAm89rE\n> STATUS=G\n> NONCE_STATUS=OK\n"},{"id":"431384","messageId":"xmqqczr3at7n.fsf@gitster.g","threadId":"49770","inReplyTo":"xmqq4kcfclw8.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-28T05:43:56Z","receivedAt":"2021-07-28T05:44:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> Fabian Stelzer writes:\n>>>\n>>>>> I think Fabian's \"ssh signing\" is not as ready as this topic, and it\n>>>>> can afford to wait by rebasing on top of this topic. By the time\n>>>>> \"ssh signing\" gets into testable shape (right now, it does not pass\n>>>>> tests when merged to 'seen'), hopefully the \"expand install-prefix\"\n>>>>> topic may already be in 'next' if not in 'master'.\n>>>> I think the test problem is not due to my patch.\n>>>\n>>> I've been seeing these test failers locally, every time\n>>> fs/ssh-signing topic is merged to 'seen' (without the reftable\n>>> thing).\n>>> ...\n>> Interesting. It seems that the failure has some correlation with\n>> the use of --root= option.\n>>\n>> $ sh t5534-push-signed.sh -i\n>\n> And 7528 fails with --root set to a /dev/shm/ trash directory.\n\nAn update. The same failure can be seen _without_ merging the topic\nto 'seen'. The topic by itself will fail t5534 and t7528 when run\nwith --root= set to somewhere:\n\n $ make\n $ testpen=/dev/shm/testpen.$$\n $ rm -fr \"$testpen\" && mkdir \"$testpen\"\n $ cd t\n $ sh t5534-*.sh --root=$testpen -i\n $ sh t7528-*.sh --root=$testpen -i\n\non the branch itself, without getting interference by any other\ntopic, should hopefully be an easy enough way to reproduce the\nproblem.\n\nThanks.\n"},{"id":"431389","messageId":"6fdc3b2f-6135-d540-1868-080a423af0e8@gigacodes.de","threadId":"49770","inReplyTo":"xmqqczr3at7n.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-07-28T08:18:59Z","receivedAt":"2021-07-28T08:19:27Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"On 28.07.21 07:43, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> Junio C Hamano writes:\n>>>\n>>>> Fabian Stelzer writes:\n>>>>\n>>>>>> I think Fabian's \"ssh signing\" is not as ready as this topic, and it\n>>>>>> can afford to wait by rebasing on top of this topic. By the time\n>>>>>> \"ssh signing\" gets into testable shape (right now, it does not pass\n>>>>>> tests when merged to 'seen'), hopefully the \"expand install-prefix\"\n>>>>>> topic may already be in 'next' if not in 'master'.\n>>>>> I think the test problem is not due to my patch.\n>>>> I've been seeing these test failers locally, every time\n>>>> fs/ssh-signing topic is merged to 'seen' (without the reftable\n>>>> thing).\n>>>> ...\n>>> Interesting. It seems that the failure has some correlation with\n>>> the use of --root= option.\n>>>\n>>> $ sh t5534-push-signed.sh -i\n>> And 7528 fails with --root set to a /dev/shm/ trash directory.\n> An update. The same failure can be seen _without_ merging the topic\n> to 'seen'. The topic by itself will fail t5534 and t7528 when run\n> with --root= set to somewhere:\n>\n> $ make\n> $ testpen=/dev/shm/testpen.$$\n> $ rm -fr \"$testpen\" && mkdir \"$testpen\"\n> $ cd t\n> $ sh t5534-*.sh --root=$testpen -i\n> $ sh t7528-*.sh --root=$testpen -i\n>\n> on the branch itself, without getting interference by any other\n> topic, should hopefully be an easy enough way to reproduce the\n> problem.\n>\n> Thanks.\n\n\nok, funny issue. in the ssh test setup i generated a few ssh keys for \ntesting and (wanting to be clever) concatenated them with a prefixed \nprincipal into an allowedSigners file using find & awk.\n\nTurns out the directory entries in /dev/shm are the other way around.\n\ntouch ./t1 ./t2 /dev/shm/t1 /dev/shm/t2\n\nfind ./ -name 't[0-9]' results in:\n./t1\n./t2\n\na find /dev/shm -name 't[0-9]' returns:\n/dev/shm/t2\n/dev/shm/t1\n\nI'll change the test setup code to do this statically for each key. Not \nsuch a good idea to rely on the file order in the dir anyway.\n\nThanks\n\n"},{"id":"431406","messageId":"xmqqzgu69xik.fsf@gitster.g","threadId":"49770","inReplyTo":"6fdc3b2f-6135-d540-1868-080a423af0e8@gigacodes.de","subject":"Re: [PATCH v3 4/5] Use a better name for the function interpolating paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-28T17:08:35Z","receivedAt":"2021-07-28T17:08:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Stelzer writes:\n\n> ok, funny issue. in the ssh test setup i generated a few ssh keys for\n> testing and (wanting to be clever) concatenated them with a prefixed \n> principal into an allowedSigners file using find & awk.\n>\n> Turns out the directory entries in /dev/shm are the other way around.\n\nGood finding. Yes, relying on the order \"find\" discovers filesystem\nentities is asking for trouble.\n\n> I'll change the test setup code to do this statically for each\n> key. Not such a good idea to rely on the file order in the dir anyway.\n\nThanks.\n"}]}