[bug#31488] [PATCH] gnu: git: Add "credential-netrc" output.

[Tobias Geerinckx-Rice]

Pierre,

[Forgive my MUA's disdain for whitespace. I'm working on it.]

May 18, 2018 11:22 AM, "Pierre Neidhardt" <address@hidden> wrote:
> @@ -217,6 +218,8 @@ as well as the classic centralized workflow.")
> (("/bin/sh") (which "sh"))
> (("/usr/bin/perl") (which "perl"))
> (("/usr/bin/python") (which "python")))
> + (substitute* "contrib/credential/netrc/git-credential-netrc"
> + (("/usr/bin/perl") (which "perl")))
> #t))
> (add-after 'configure 'add-PM.stamp
> (lambda _

This phase is (still) called ‘patch-makefiles’. Could you tweak the name or 
move this to its own phase?

I've a preference for the latter, since it can then probably be moved before 
'install-credential-netrc below to keep things together, but it's a slight one.

All that typed: is this even needed? It doesn't seem to be. The only mention of 
perl is the shebang, and those are already handled by the build system.

> @@ -271,6 +274,13 @@ as well as the classic centralized workflow.")
> (copy-file "contrib/completion/git-completion.bash"
> (string-append completions "/git"))
> #t)))
> + (add-after 'install 'install-credential-netrc
> + (lambda* (#:key outputs #:allow-other-keys)
> + (let* ((netrc (assoc-ref outputs "credential-netrc"))
> + (bin (string-append netrc "/bin")))
> + (mkdir-p bin)

INSTALL-FILE already implies MKDIR-P. That's one of its main features over 
COPY-FILE.

> + (install-file "contrib/credential/netrc/git-credential-netrc" bin)
> + #t)))
> (add-after 'install 'split
> (lambda* (#:key inputs outputs #:allow-other-keys)
> ;; Split the binaries to the various outputs.

I'll let others comment on the multiple outputs. I'm their resident fetishist, 
but not sufficiently informed to judge the ‘clutter’ vs. 48 KiB argument.

Sorry to reduce your work to a potential one-liner. The rest looks good to me. 
;-)

Thanks!

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.