patchperl: fix installing modules from contrib

Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules) removed a target that allowed Makefiles from contrib/ to get the correct install path. This introduces a new target for main Makefile and fixes installation for Mediawiki module.

Signed-off-by: Christian Hesse <mail@eworm.de>
---
 Makefile                   | 2 ++
 contrib/mw-to-git/Makefile | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

On Tue, Apr 03 2018, Christian Hesse wrote:

Thanks, I (obviously) missed that when getting rid of the perl/Makefile.

This fixes it up for now, but it seems we're going to need some solution to make this work with the in-flight RUNTIME_PREFIX Dan's been working on.

I think the best solution for that, not just for this but for most of contrib/ in general, is to simply move it into our main tree out of contrib/, and introduce some Makefile flags for whether or not you'd want to install such-and-such from contrib.

That would probably be easier than the current arrangement, and we could do things like say we always want to run tests for contrib/ stuff, even though we're not installing it.

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

This use of _SQ variant is fishy, isn't it? Judging from the output of

    $ git grep _SQ Makefile

e.g.

    Makefile:	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
    Makefile:	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
    Makefile:	$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'

I'd expect that any _SQ variant must be referenced inside a single quote pair. In fact, that is why a single quote (and nothing else) in the base variable is replaced with the magic "'\''" sequence, first stepping out of the current sq context, append a single sq (escaped with a backslash from the shell), and then stepping back into another sq context.

I think nobody saw breakage only because they do not have two consecutive SPs (or any single quote) in their path to $perllibdir. If we depend on such limitation, there is no point using _SQ variant, but we already have _SQ variant, let's use it correctly.

Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules) removed a target that allowed Makefiles from contrib/ to get the correct install path. This introduces a new target for main Makefile and fixes installation for Mediawiki module.

v2: Pass prefix as that can have influence as well, add single quotes
    for _SQ variant.

Signed-off-by: Christian Hesse <mail@eworm.de>
---
 Makefile                   | 2 ++
 contrib/mw-to-git/Makefile | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

Christian Hesse <mail@eworm.de> writes:

Sorry for not noticing it before, but as this rule will not create and update timestamp of a filesystem entity 'perllibdir', shouldn't we mark it with .PHONY? I'd call the target 'say-perllibdir' if I were doing this patch but that is merely a personal preference.

Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules) removed a target that allowed Makefiles from contrib/ to get the correct install path. This introduces a new target for main Makefile and fixes installation for Mediawiki module.

v2: Pass prefix as that can have influence as well, add single quotes
    for _SQ variant.
v3: Rename target, add to .PHONY.

Signed-off-by: Christian Hesse <mail@eworm.de>
---
 Makefile                   | 3 +++
 contrib/mw-to-git/Makefile | 5 +++--
 2 files changed, 6 insertions(+), 2 deletions(-)

Christian Hesse <mail@eworm.de> writes:

Thanks for rerolling. I should have made it a bit more clear that the say-* thing was merely a personal preference "I would be writing it that way if I were doing it", not a "You should write it this way when working on this project". I think .PHONY is still a good idea to have, even for only its documentation value (it is unlikely that anybody would create a file "perllibdir").

Let me queue this on top of the v2 queued in 'next' as an incremental update.

Thanks.

-- >8 --
From: Christian Hesse <mail@eworm.de>
SUbject: Makefile: mark perllibdir as a .PHONY target

This target should be marked as .PHONY, just like other targets that exist only for their side effects that do not create filesystem entities with the same name.

Signed-off-by: Christian Hesse <mail@eworm.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

Junio C Hamano <gitster@pobox.com> on Thu, 2018/04/19 06:44:

Well, it's you who maintains the code. So I am fine with whatever you prefer. ;)

Thanks a lot!