{"thread":{"id":"48207","subject":"[PATCH 1/1] perl: fix installing modules from contrib","startedAt":"2018-04-03T09:26:03Z","lastAt":"2018-04-19T21:44:36Z","messageCount":8,"participants":["Christian Hesse","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"343668","messageId":"20180403092008.26892-1-mail@eworm.de","threadId":"48207","inReplyTo":null,"subject":"[PATCH 1/1] perl: fix installing modules from contrib","fromName":"Christian Hesse","fromEmail":"mail@eworm.de","sentAt":"2018-04-03T09:20:08Z","receivedAt":"2018-04-03T09:26:03Z","isPatch":true,"sender":{"key":"mail@eworm.de","avatar":"https://avatars.githubusercontent.com/u/970853?v=4"},"body":"Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules)\nremoved a target that allowed Makefiles from contrib/ to get the correct\ninstall path. This introduces a new target for main Makefile and fixes\ninstallation for Mediawiki module.\n\nSigned-off-by: Christian Hesse <mail@eworm.de>\n---\n Makefile                   | 2 ++\n contrib/mw-to-git/Makefile | 4 ++--\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex a1d8775ad..bcaf50495 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2002,6 +2002,8 @@ GIT-PERL-DEFINES: FORCE\n \t\techo \"$$FLAGS\" >$@; \\\n \t    fi\n \n+perllibdir:\n+\t@echo $(perllibdir_SQ)\n \n .PHONY: gitweb\n gitweb:\ndiff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\nindex a4b6f7a2c..0a6e59579 100644\n--- a/contrib/mw-to-git/Makefile\n+++ b/contrib/mw-to-git/Makefile\n@@ -21,8 +21,8 @@ HERE=contrib/mw-to-git/\n INSTALL = install\n \n SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))\n-INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \\\n-                -s --no-print-directory instlibdir)\n+INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \\\n+                -s --no-print-directory perllibdir=$(perllibdir) perllibdir)\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n INSTLIBDIR_SQ = $(subst ','\\'',$(INSTLIBDIR))\n \n"},{"id":"343674","messageId":"87tvssva2c.fsf@evledraar.gmail.com","threadId":"48207","inReplyTo":"20180403092008.26892-1-mail@eworm.de","subject":"Re: [PATCH 1/1] perl: fix installing modules from contrib","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-03T10:49:47Z","receivedAt":"2018-04-03T10:50:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Apr 03 2018, Christian Hesse wrote:\n\n> Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules)\n> removed a target that allowed Makefiles from contrib/ to get the correct\n> install path. This introduces a new target for main Makefile and fixes\n> installation for Mediawiki module.\n>\n> Signed-off-by: Christian Hesse <mail@eworm.de>\n> ---\n>  Makefile                   | 2 ++\n>  contrib/mw-to-git/Makefile | 4 ++--\n>  2 files changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index a1d8775ad..bcaf50495 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2002,6 +2002,8 @@ GIT-PERL-DEFINES: FORCE\n>  \t\techo \"$$FLAGS\" >$@; \\\n>  \t    fi\n>\n> +perllibdir:\n> +\t@echo $(perllibdir_SQ)\n>\n>  .PHONY: gitweb\n>  gitweb:\n> diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\n> index a4b6f7a2c..0a6e59579 100644\n> --- a/contrib/mw-to-git/Makefile\n> +++ b/contrib/mw-to-git/Makefile\n> @@ -21,8 +21,8 @@ HERE=contrib/mw-to-git/\n>  INSTALL = install\n>\n>  SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))\n> -INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \\\n> -                -s --no-print-directory instlibdir)\n> +INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \\\n> +                -s --no-print-directory perllibdir=$(perllibdir) perllibdir)\n>  DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n>  INSTLIBDIR_SQ = $(subst ','\\'',$(INSTLIBDIR))\n\nThanks, I (obviously) missed that when getting rid of the perl/Makefile.\n\nThis fixes it up for now, but it seems we're going to need some solution\nto make this work with the in-flight RUNTIME_PREFIX Dan's been working\non.\n\nI think the best solution for that, not just for this but for most of\ncontrib/ in general, is to simply move it into our main tree out of\ncontrib/, and introduce some Makefile flags for whether or not you'd\nwant to install such-and-such from contrib.\n\nThat would probably be easier than the current arrangement, and we could\ndo things like say we always want to run tests for contrib/ stuff, even\nthough we're not installing it.\n"},{"id":"344154","messageId":"xmqqpo39569k.fsf@gitster-ct.c.googlers.com","threadId":"48207","inReplyTo":"87tvssva2c.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/1] perl: fix installing modules from contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-09T04:54:31Z","receivedAt":"2018-04-09T04:54:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>>\n>> +perllibdir:\n>> +\t@echo $(perllibdir_SQ)\n\nThis use of _SQ variant is fishy, isn't it?  Judging from the output\nof \n\n    $ git grep _SQ Makefile\n\ne.g.\n\n    Makefile:\t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n    Makefile:\t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n    Makefile:\t$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n\nI'd expect that any _SQ variant must be referenced inside a single\nquote pair.  In fact, that is why a single quote (and nothing else)\nin the base variable is replaced with the magic \"'\\''\" sequence,\nfirst stepping out of the current sq context, append a single sq\n(escaped with a backslash from the shell), and then stepping back\ninto another sq context.\n\nI think nobody saw breakage only because they do not have two\nconsecutive SPs (or any single quote) in their path to $perllibdir.\nIf we depend on such limitation, there is no point using _SQ\nvariant, but we already have _SQ variant, let's use it correctly.\n\n"},{"id":"344343","messageId":"20180410133641.18861-1-mail@eworm.de","threadId":"48207","inReplyTo":"xmqqpo39569k.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 1/1] perl: fix installing modules from contrib","fromName":"Christian Hesse","fromEmail":"mail@eworm.de","sentAt":"2018-04-10T13:36:41Z","receivedAt":"2018-04-10T13:37:05Z","isPatch":true,"sender":{"key":"mail@eworm.de","avatar":"https://avatars.githubusercontent.com/u/970853?v=4"},"body":"Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules)\nremoved a target that allowed Makefiles from contrib/ to get the correct\ninstall path. This introduces a new target for main Makefile and fixes\ninstallation for Mediawiki module.\n\nv2: Pass prefix as that can have influence as well, add single quotes\n    for _SQ variant.\n\nSigned-off-by: Christian Hesse <mail@eworm.de>\n---\n Makefile                   | 2 ++\n contrib/mw-to-git/Makefile | 5 +++--\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 96f6138f6..19ca5e8de 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2011,6 +2011,8 @@ GIT-PERL-DEFINES: FORCE\n \t\techo \"$$FLAGS\" >$@; \\\n \t    fi\n \n+perllibdir:\n+\t@echo '$(perllibdir_SQ)'\n \n .PHONY: gitweb\n gitweb:\ndiff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\nindex a4b6f7a2c..4e603512a 100644\n--- a/contrib/mw-to-git/Makefile\n+++ b/contrib/mw-to-git/Makefile\n@@ -21,8 +21,9 @@ HERE=contrib/mw-to-git/\n INSTALL = install\n \n SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))\n-INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \\\n-                -s --no-print-directory instlibdir)\n+INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \\\n+                -s --no-print-directory prefix=$(prefix) \\\n+                perllibdir=$(perllibdir) perllibdir)\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n INSTLIBDIR_SQ = $(subst ','\\'',$(INSTLIBDIR))\n \n"},{"id":"344409","messageId":"xmqqpo3620tz.fsf@gitster-ct.c.googlers.com","threadId":"48207","inReplyTo":"20180410133641.18861-1-mail@eworm.de","subject":"Re: [PATCH v2 1/1] perl: fix installing modules from contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-10T21:44:40Z","receivedAt":"2018-04-10T21:44:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Hesse <mail@eworm.de> writes:\n\n> Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules)\n> removed a target that allowed Makefiles from contrib/ to get the correct\n> install path. This introduces a new target for main Makefile and fixes\n> installation for Mediawiki module.\n>\n> v2: Pass prefix as that can have influence as well, add single quotes\n>     for _SQ variant.\n>\n> Signed-off-by: Christian Hesse <mail@eworm.de>\n> ---\n>  Makefile                   | 2 ++\n>  contrib/mw-to-git/Makefile | 5 +++--\n>  2 files changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 96f6138f6..19ca5e8de 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2011,6 +2011,8 @@ GIT-PERL-DEFINES: FORCE\n>  \t\techo \"$$FLAGS\" >$@; \\\n>  \t    fi\n>  \n> +perllibdir:\n> +\t@echo '$(perllibdir_SQ)'\n\nSorry for not noticing it before, but as this rule will not create\nand update timestamp of a filesystem entity 'perllibdir', shouldn't\nwe mark it with .PHONY?  I'd call the target 'say-perllibdir' if I\nwere doing this patch but that is merely a personal preference.\n\n>  .PHONY: gitweb\n>  gitweb:\n> diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\n> index a4b6f7a2c..4e603512a 100644\n> --- a/contrib/mw-to-git/Makefile\n> +++ b/contrib/mw-to-git/Makefile\n> @@ -21,8 +21,9 @@ HERE=contrib/mw-to-git/\n>  INSTALL = install\n>  \n>  SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))\n> -INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \\\n> -                -s --no-print-directory instlibdir)\n> +INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \\\n> +                -s --no-print-directory prefix=$(prefix) \\\n> +                perllibdir=$(perllibdir) perllibdir)\n>  DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n>  INSTLIBDIR_SQ = $(subst ','\\'',$(INSTLIBDIR))\n>  \n"},{"id":"344443","messageId":"20180411065120.9432-1-mail@eworm.de","threadId":"48207","inReplyTo":"xmqqpo3620tz.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v3 1/1] perl: fix installing modules from contrib","fromName":"Christian Hesse","fromEmail":"mail@eworm.de","sentAt":"2018-04-11T06:51:20Z","receivedAt":"2018-04-11T06:51:33Z","isPatch":true,"sender":{"key":"mail@eworm.de","avatar":"https://avatars.githubusercontent.com/u/970853?v=4"},"body":"Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules)\nremoved a target that allowed Makefiles from contrib/ to get the correct\ninstall path. This introduces a new target for main Makefile and fixes\ninstallation for Mediawiki module.\n\nv2: Pass prefix as that can have influence as well, add single quotes\n    for _SQ variant.\nv3: Rename target, add to .PHONY.\n\nSigned-off-by: Christian Hesse <mail@eworm.de>\n---\n Makefile                   | 3 +++\n contrib/mw-to-git/Makefile | 5 +++--\n 2 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex f18168725..5a06eddf2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2014,6 +2014,9 @@ GIT-PERL-DEFINES: FORCE\n \t\techo \"$$FLAGS\" >$@; \\\n \t    fi\n \n+.PHONY: say-perllibdir\n+say-perllibdir:\n+\t@echo '$(perllibdir_SQ)'\n \n .PHONY: gitweb\n gitweb:\ndiff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile\nindex a4b6f7a2c..e301a5b4e 100644\n--- a/contrib/mw-to-git/Makefile\n+++ b/contrib/mw-to-git/Makefile\n@@ -21,8 +21,9 @@ HERE=contrib/mw-to-git/\n INSTALL = install\n \n SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))\n-INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \\\n-                -s --no-print-directory instlibdir)\n+INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \\\n+                -s --no-print-directory prefix=$(prefix) \\\n+                perllibdir=$(perllibdir) say-perllibdir)\n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n INSTLIBDIR_SQ = $(subst ','\\'',$(INSTLIBDIR))\n \n"},{"id":"345027","messageId":"xmqq36zsgpfb.fsf@gitster-ct.c.googlers.com","threadId":"48207","inReplyTo":"20180411065120.9432-1-mail@eworm.de","subject":"Re: [PATCH v3 1/1] perl: fix installing modules from contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-18T21:44:40Z","receivedAt":"2018-04-18T21:44:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Hesse <mail@eworm.de> writes:\n\n> Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules)\n> removed a target that allowed Makefiles from contrib/ to get the correct\n> install path. This introduces a new target for main Makefile and fixes\n> installation for Mediawiki module.\n>\n> v2: Pass prefix as that can have influence as well, add single quotes\n>     for _SQ variant.\n> v3: Rename target, add to .PHONY.\n>\n> Signed-off-by: Christian Hesse <mail@eworm.de>\n> ---\n\nThanks for rerolling.  I should have made it a bit more clear that\nthe say-* thing was merely a personal preference \"I would be writing\nit that way if I were doing it\", not a \"You should write it this way\nwhen working on this project\".  I think .PHONY is still a good idea\nto have, even for only its documentation value (it is unlikely that\nanybody would create a file \"perllibdir\").\n\nLet me queue this on top of the v2 queued in 'next' as an\nincremental update.\n\nThanks.\n\n-- >8 --\nFrom: Christian Hesse <mail@eworm.de>\nSUbject: Makefile: mark perllibdir as a .PHONY target\n\nThis target should be marked as .PHONY, just like other targets that\nexist only for their side effects that do not create filesystem\nentities with the same name.\n\nSigned-off-by: Christian Hesse <mail@eworm.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 75b9ad3b48..b284eb20aa 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1973,6 +1973,7 @@ GIT-PERL-DEFINES: FORCE\n \t\techo \"$$FLAGS\" >$@; \\\n \t    fi\n \n+.PHONY: perllibdir\n perllibdir:\n \t@echo '$(perllibdir_SQ)'\n \n"},{"id":"345169","messageId":"20180419234420.0dd66dce@leda","threadId":"48207","inReplyTo":"xmqq36zsgpfb.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 1/1] perl: fix installing modules from contrib","fromName":"Christian Hesse","fromEmail":"mail@eworm.de","sentAt":"2018-04-19T21:44:20Z","receivedAt":"2018-04-19T21:44:36Z","isPatch":true,"sender":{"key":"mail@eworm.de","avatar":"https://avatars.githubusercontent.com/u/970853?v=4"},"body":"Junio C Hamano <gitster@pobox.com> on Thu, 2018/04/19 06:44:\n> Christian Hesse <mail@eworm.de> writes:\n> \n> > Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make\n> > rules) removed a target that allowed Makefiles from contrib/ to get the\n> > correct install path. This introduces a new target for main Makefile and\n> > fixes installation for Mediawiki module.\n> >\n> > v2: Pass prefix as that can have influence as well, add single quotes\n> >     for _SQ variant.\n> > v3: Rename target, add to .PHONY.\n> >\n> > Signed-off-by: Christian Hesse <mail@eworm.de>\n> > ---  \n> \n> Thanks for rerolling.  I should have made it a bit more clear that\n> the say-* thing was merely a personal preference \"I would be writing\n> it that way if I were doing it\", not a \"You should write it this way\n> when working on this project\". \n\nWell,  it's you who maintains the code. So I am fine with whatever you\nprefer. ;)\n\n> I think .PHONY is still a good idea\n> to have, even for only its documentation value (it is unlikely that\n> anybody would create a file \"perllibdir\").\n> \n> Let me queue this on top of the v2 queued in 'next' as an\n> incremental update.\n\nThanks a lot!\n-- \nBest regards,\nChris\n"}]}