{"thread":{"id":"17485","subject":"026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","startedAt":"2009-02-01T18:24:33Z","lastAt":"2009-02-05T22:16:13Z","messageCount":13,"participants":["A Large Angry SCM","Junio C Hamano","Steffen Prohaska","Pascal Obry","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"102759","messageId":"4985E8E1.90303@gmail.com","threadId":"17485","inReplyTo":null,"subject":"026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2009-02-01T18:24:33Z","receivedAt":"2009-02-01T18:24:33Z","isPatch":false,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"In 026fa0d5ad9538ca76838070861531c037d7b9ba, \"Move computation of \nabsolute paths from Makefile to runtime (in preparation for \nRUNTIME_PREFIX)\", the following change was made to the Makefile. The \nproblem is that $(abspath names...) is a relatively recent addition to \nGNU make and when used in an older GNU make, the test always fails \nresulting incorrect installation dirs for the templates and commands.\n\nThe new test is also wrong; (for *nix systems) in that it really wants \nto test if the first character is a '/' but GNU make doesn't have a way \nto do that directly. Instead, it tests if the first character is a / \n_AND_ the path string does not include . or .. components,\n\nThe older test has problems itself but at least it allowed you to \nspecify absolute paths.\n\n@@ -1407,17 +1417,17 @@ remove-dashes:\n\n ### Installation rules\n\n-ifeq ($(firstword $(subst /, ,$(template_dir))),..)\n-template_instdir = $(bindir)/$(template_dir)\n-else\n+ifeq ($(abspath $(template_dir)),$(template_dir))\n template_instdir = $(template_dir)\n+else\n+template_instdir = $(prefix)/$(template_dir)\n endif\n export template_instdir\n\n-ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)\n-gitexec_instdir = $(bindir)/$(gitexecdir)\n-else\n+ifeq ($(abspath $(gitexecdir)),$(gitexecdir))\n gitexec_instdir = $(gitexecdir)\n+else\n+gitexec_instdir = $(prefix)/$(gitexecdir)\n endif\n gitexec_instdir_SQ = $(subst ','\\'',$(gitexec_instdir))\n export gitexec_instdir\n\n\nI see 3 ways to fix the problem:\n\t1) go back to using the old test.\n\t2) keep the new test but add a test that will break the build if\n\t\t$(abspath names...) does not work.\n\t3) something like the following (untested)\n\n\n ### Installation rules\n\n\n temp = $(subst \" \",x,$(template_dir))\n temp = $(subst //,/,$(temp))\n temp = $(addsuffix x,$(temp))\n temp = $(subst /, ,$(temp))\n ifeq ($(strip $(temp)),$(temp))\n template_instdir = $(template_dir)\n else\n template_instdir = $(prefix)/$(template_dir)\n endif\n export template_instdir\n\n temp = $(subst \" \",x,$(gitexecdir))\n temp = $(subst //,/,$(temp))\n temp = $(addsuffix x,$(temp))\n temp = $(subst /, ,$(temp))\n ifeq ($(strip $(temp)),$(temp))\n gitexec_instdir = $(gitexecdir)\n else\n gitexec_instdir = $(prefix)/$(gitexecdir)\n endif\n gitexec_instdir_SQ = $(subst ','\\'',$(gitexec_instdir))\n export gitexec_instdir\n"},{"id":"103279","messageId":"7vab91icu0.fsf@gitster.siamese.dyndns.org","threadId":"17485","inReplyTo":"4985E8E1.90303@gmail.com","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T07:04:39Z","receivedAt":"2009-02-05T07:04:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A Large Angry SCM writes:\n\n> In 026fa0d5ad9538ca76838070861531c037d7b9ba, \"Move computation of\n> absolute paths from Makefile to runtime (in preparation for\n> RUNTIME_PREFIX)\", the following change was made to the Makefile. The\n> problem is that $(abspath names...) is a relatively recent addition to\n> GNU make and when used in an older GNU make, the test always fails\n> resulting incorrect installation dirs for the templates and commands.\n\nIs there anything being done on this issue?\n\nI could revert ed096c4 (Merge branch 'sp/runtime-prefix', 2009-01-31), but\nI'd rather not if we can avoid it.\n"},{"id":"103280","messageId":"54B6E81F-FCC5-453D-9F38-9F29C41F5613@zib.de","threadId":"17485","inReplyTo":"7vab91icu0.fsf@gitster.siamese.dyndns.org","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2009-02-05T07:13:30Z","receivedAt":"2009-02-05T07:13:30Z","isPatch":false,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Feb 5, 2009, at 8:04 AM, Junio C Hamano wrote:\n\n> A Large Angry SCM writes:\n>\n>> In 026fa0d5ad9538ca76838070861531c037d7b9ba, \"Move computation of\n>> absolute paths from Makefile to runtime (in preparation for\n>> RUNTIME_PREFIX)\", the following change was made to the Makefile. The\n>> problem is that $(abspath names...) is a relatively recent addition \n>> to\n>> GNU make and when used in an older GNU make, the test always fails\n>> resulting incorrect installation dirs for the templates and commands.\n>\n> Is there anything being done on this issue?\n\nNot yet. I haven't had time.\n\n\n> I could revert ed096c4 (Merge branch 'sp/runtime-prefix', \n> 2009-01-31), but\n> I'd rather not if we can avoid it.\n\nI'll propose a fix on Saturday.\n\n\tSteffen\n"},{"id":"103281","messageId":"7v63jpibe8.fsf@gitster.siamese.dyndns.org","threadId":"17485","inReplyTo":"4985E8E1.90303@gmail.com","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T07:35:43Z","receivedAt":"2009-02-05T07:35:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A Large Angry SCM writes:\n\n> The new test is also wrong; (for *nix systems) in that it really wants\n> to test if the first character is a '/' but GNU make doesn't have a\n> way to do that directly.\n\nHmph. Isn't it just the matter of doing something silly like this?\n\nifeq ($(filter /%,$(template_dir)),)\n\t... it does not start with a slash\nelse\n\t... it does\nendif\n"},{"id":"103283","messageId":"a2633edd0902042338n25dc7be4vbb202d011eb4ad78@mail.gmail.com","threadId":"17485","inReplyTo":"7v63jpibe8.fsf@gitster.siamese.dyndns.org","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2009-02-05T07:38:36Z","receivedAt":"2009-02-05T07:38:36Z","isPatch":false,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"I have just proposed a patch to fix this in another thread.\n\nPascal.\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"103286","messageId":"7vzlh1gwf4.fsf@gitster.siamese.dyndns.org","threadId":"17485","inReplyTo":"7v63jpibe8.fsf@gitster.siamese.dyndns.org","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T07:44:31Z","receivedAt":"2009-02-05T07:44:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> A Large Angry SCM writes:\n>\n>> The new test is also wrong; (for *nix systems) in that it really wants\n>> to test if the first character is a '/' but GNU make doesn't have a\n>> way to do that directly.\n>\n> Hmph. Isn't it just the matter of doing something silly like this?\n>\n> ifeq ($(filter /%,$(template_dir)),)\n> \t... it does not start with a slash\n> else\n> \t... it does\n> endif\n\nThat is, something like this.\n\nThe patch also protects the $(filter) check from sick users that use\n\n\ttemplate_dir = My Files /the-leading-dir-ends-with-a-space\n\nby using $(firstword)\n\nNo, I didn't test it.\n\n Makefile | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git c/Makefile w/Makefile\nindex a82f173..605b147 100644\n--- c/Makefile\n+++ w/Makefile\n@@ -1434,14 +1434,14 @@ remove-dashes:\n \n ### Installation rules\n \n-ifeq ($(abspath $(template_dir)),$(template_dir))\n+ifneq ($(filter /%,$(firstword $(template_dir))),)\n template_instdir = $(template_dir)\n else\n template_instdir = $(prefix)/$(template_dir)\n endif\n export template_instdir\n \n-ifeq ($(abspath $(gitexecdir)),$(gitexecdir))\n+ifneq ($(filter /%,$(firstword $(gitexecdir))),)\n gitexec_instdir = $(gitexecdir)\n else\n gitexec_instdir = $(prefix)/$(gitexecdir)\n"},{"id":"103288","messageId":"7vvdrpgvzl.fsf@gitster.siamese.dyndns.org","threadId":"17485","inReplyTo":"a2633edd0902042338n25dc7be4vbb202d011eb4ad78@mail.gmail.com","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T07:53:50Z","receivedAt":"2009-02-05T07:53:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pascal Obry writes:\n\n> I have just proposed a patch to fix this in another thread.\n\nDoes your patch address the lack of $(abspath) in older GNU make?\n\nI do not know if mine that used firstword did for that matter, either,\nthough.\n\nRegardless, I think your patch to config.mak is independently good,\nbecause the way Makefile defines these directories is without the trailing\nslash, and it would be better to be consistent.\n"},{"id":"103289","messageId":"a2633edd0902042357s35ab8fe7g59cb3ef109e9b0bb@mail.gmail.com","threadId":"17485","inReplyTo":"7vvdrpgvzl.fsf@gitster.siamese.dyndns.org","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2009-02-05T07:57:22Z","receivedAt":"2009-02-05T07:57:22Z","isPatch":false,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"Junio,\n\n> Does your patch address the lack of $(abspath) in older GNU make?\n\nDon't think so.\n\n> Regardless, I think your patch to config.mak is independently good,\n> because the way Makefile defines these directories is without the trailing\n> slash, and it would be better to be consistent.\n\nRight, better to have no slash ending directories to avoid double\nslashes in $(dir)/file.c.\n\nPascal.\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"103291","messageId":"7vprhxgvnc.fsf@gitster.siamese.dyndns.org","threadId":"17485","inReplyTo":"7vvdrpgvzl.fsf@gitster.siamese.dyndns.org","subject":"Re: 026fa0d5ad Breaks installs with absolue $(gitexecdir) and $(template_dir) variables using older GNU makes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T08:01:11Z","receivedAt":"2009-02-05T08:01:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Regardless, I think your patch to config.mak is independently good,\n> because the way Makefile defines these directories is without the trailing\n> slash, and it would be better to be consistent.\n\nIn other words, I am thinking about applying your patch after amending the \nlog message like this.\n\n [PATCH] config.mak.in: define paths without trailing slash\n\n The main Makefile defines gitexecdir and template_dir without trailing\n slash. config.mak.in should do the same to be consistent.\n\n Signed-off-by: Pascal Obry \n\nI think the misuse of abspath is a separate issue.\n\nThe main Makefile should be made robust not to misdetect the relative vs\nabsolute, especially because not everybody uses config.mak.autogen, and\nbecause people should be able to override these variables from the command\nline of make invocation and the name of the directory with or without\ntrailing slash ought to mean the same thing.\n"},{"id":"103303","messageId":"7vhc39guv2.fsf@gitster.siamese.dyndns.org","threadId":"17485","inReplyTo":"4985E8E1.90303@gmail.com","subject":"[PATCH] Makefile: fix misdetection of relative pathnames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T08:18:09Z","receivedAt":"2009-02-05T08:18:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\nA Large Angry SCM writes:\n\n> In 026fa0d5ad9538ca76838070861531c037d7b9ba, \"Move computation of\n> absolute paths from Makefile to runtime (in preparation for\n> RUNTIME_PREFIX)\", the following change was made to the Makefile. The\n> problem is that $(abspath names...) is a relatively recent addition to\n> GNU make and when used in an older GNU make, the test always fails\n> resulting incorrect installation dirs for the templates and commands.\n>\n> The new test is also wrong; (for *nix systems) in that it really wants\n> to test if the first character is a '/'\n\nOk, here is the final one from me that I am considering to commit, but it\nwould be nice to hear success/failure feedback from people who had trouble\nwith Steffen's changes. I'd also like to hear from people who have been\nhappy with Steffen's changes that this does not break things for them.\n\n-- >8 --\n\nThe installation rules wanted to differentiate between a template_dir that\nis given as an absolute path (e.g. /usr/share/git-core/templates) and a\nrelative one (e.g. share/git-core/templates) but it was done by checking\nif $(abspath $(template_dir)) and $(template_dir) yield the same string.\n\nThis was wrong in number of ways.\n\n * The user can give template_dir with a trailing slash from the command\n line to invoke make, or in the included config.mak. A directory path\n ought to mean the same thing with or without such a trailing slash but\n use of $(abspath) means with an absolute path with a trailing slash\n fails the test.\n\n * The compilation could be done inside /usr directory with a relative\n pathname share/git-core/templates and it will be mistaken as an\n absolute path.\n\n * Versions of GNU make older than 3.81 do not have $(abspath) to begin\n with.\n\nThis changes the detection logic to see if the given path begins with a\nslash.\n\nSigned-off-by: Junio C Hamano \n---\n Makefile | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex a82f173..605b147 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1434,14 +1434,14 @@ remove-dashes:\n \n ### Installation rules\n \n-ifeq ($(abspath $(template_dir)),$(template_dir))\n+ifneq ($(filter /%,$(firstword $(template_dir))),)\n template_instdir = $(template_dir)\n else\n template_instdir = $(prefix)/$(template_dir)\n endif\n export template_instdir\n \n-ifeq ($(abspath $(gitexecdir)),$(gitexecdir))\n+ifneq ($(filter /%,$(firstword $(gitexecdir))),)\n gitexec_instdir = $(gitexecdir)\n else\n gitexec_instdir = $(prefix)/$(gitexecdir)\n-- \n1.6.1.2.382.gda69a\n"},{"id":"103324","messageId":"498ADB1B.7030300@viscovery.net","threadId":"17485","inReplyTo":"7vhc39guv2.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: fix misdetection of relative pathnames","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-05T12:27:07Z","receivedAt":"2009-02-05T12:27:07Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> -ifeq ($(abspath $(template_dir)),$(template_dir))\n> +ifneq ($(filter /%,$(firstword $(template_dir))),)\n\n> -ifeq ($(abspath $(gitexecdir)),$(gitexecdir))\n> +ifneq ($(filter /%,$(firstword $(gitexecdir))),)\n\nNo, this does not work on Windows for the use-case that this check is\nintended for, namely when the user specifies an *absolute* path for\ngitexecdir and/or template_dir in config.mak [*].\n\nNeither does the version that uses $(abspath ...)! Because $(abspath ...)\ndoes not work in our msysgit environment (that has GNU make 3.79.1).\n\nThat said, I don't think it's worth to cater for this use-case, precisely\nbecause we want to *avoid* absolute paths on Windows anyway, and apply the\nchange that you proposed here.\n\n[*] The reason it does not work is that we cannot use MSYS-style absolute\npaths /c/Foo/Bar because the paths will be interpreted by git, which does\nnot understand them; the user must specify drive-letter absolute paths\nc:/Foo/Bar, but the check does not catch them.\n\n-- Hannes\n"},{"id":"103355","messageId":"7v1vucg5ss.fsf@gitster.siamese.dyndns.org","threadId":"17485","inReplyTo":"498ADB1B.7030300@viscovery.net","subject":"Re: [PATCH] Makefile: fix misdetection of relative pathnames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-05T17:19:31Z","receivedAt":"2009-02-05T17:19:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> That said, I don't think it's worth to cater for this use-case, precisely\n> because we want to *avoid* absolute paths on Windows anyway, and apply the\n> change that you proposed here.\n>\n> [*] The reason it does not work is that we cannot use MSYS-style absolute\n> paths /c/Foo/Bar because the paths will be interpreted by git, which does\n> not understand them; the user must specify drive-letter absolute paths\n> c:/Foo/Bar, but the check does not catch them.\n\nHmm, not c:\\Program Files\\Git?\n\nIn any case, thanks for an Ack. It clears one remaining issue for -rc0\n(and there are certainly others I have forgot).\n"},{"id":"103401","messageId":"498B652D.8060708@gmail.com","threadId":"17485","inReplyTo":"7vhc39guv2.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: fix misdetection of relative pathnames","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2009-02-05T22:16:13Z","receivedAt":"2009-02-05T22:16:13Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> A Large Angry SCM writes:\n> \n>> In 026fa0d5ad9538ca76838070861531c037d7b9ba, \"Move computation of\n>> absolute paths from Makefile to runtime (in preparation for\n>> RUNTIME_PREFIX)\", the following change was made to the Makefile. The\n>> problem is that $(abspath names...) is a relatively recent addition to\n>> GNU make and when used in an older GNU make, the test always fails\n>> resulting incorrect installation dirs for the templates and commands.\n>>\n>> The new test is also wrong; (for *nix systems) in that it really wants\n>> to test if the first character is a '/'\n> \n> Ok, here is the final one from me that I am considering to commit, but it\n> would be nice to hear success/failure feedback from people who had trouble\n> with Steffen's changes. I'd also like to hear from people who have been\n> happy with Steffen's changes that this does not break things for them.\n> \n> -- >8 --\n> \n> The installation rules wanted to differentiate between a template_dir that\n> is given as an absolute path (e.g. /usr/share/git-core/templates) and a\n> relative one (e.g. share/git-core/templates) but it was done by checking\n> if $(abspath $(template_dir)) and $(template_dir) yield the same string.\n> \n> This was wrong in number of ways.\n> \n> * The user can give template_dir with a trailing slash from the command\n> line to invoke make, or in the included config.mak. A directory path\n> ought to mean the same thing with or without such a trailing slash but\n> use of $(abspath) means with an absolute path with a trailing slash\n> fails the test.\n> \n> * The compilation could be done inside /usr directory with a relative\n> pathname share/git-core/templates and it will be mistaken as an\n> absolute path.\n> \n> * Versions of GNU make older than 3.81 do not have $(abspath) to begin\n> with.\n> \n> This changes the detection logic to see if the given path begins with a\n> slash.\n> \n> Signed-off-by: Junio C Hamano \n\nTested-by: A Large Angry SCM \n"}]}