git-annex: problematic shebangs in .git/hooks/pre-commit?

git-annex: problematic shebangs in .git/hooks/pre-commit?

[Kyle Meyer]

Hello,

Thanks for packaging git-annex, Tim!  I'm excited to see it in Guix.

I'm wondering whether the shebang patching in .git/hooks/pre-commit will
cause a problem.  Using the patched shellPath_portable, 'git annex init'
generates a hook like this:

    % cat .git/hooks/pre-commit
    #!/gnu/store/rbrandv7anzjxqkr40d7fkanzssslk4b-bash-minimal-4.4.19/bin/sh
    # automatically configured by git-annex
    git annex pre-commit .

But won't this break if that particular bash-mininimal is garbage collected?

Re: git-annex: problematic shebangs in .git/hooks/pre-commit?

[Timothy Sample]

Hi Kyle,

Kyle Meyer <kyle@kyleam.com> writes:

> Hello,
>
> Thanks for packaging git-annex, Tim!  I'm excited to see it in Guix.

You’re welcome!

> I'm wondering whether the shebang patching in .git/hooks/pre-commit will
> cause a problem.  Using the patched shellPath_portable, 'git annex init'
> generates a hook like this:
>
>     % cat .git/hooks/pre-commit
>     #!/gnu/store/rbrandv7anzjxqkr40d7fkanzssslk4b-bash-minimal-4.4.19/bin/sh
>     # automatically configured by git-annex
>     git annex pre-commit .
>
> But won't this break if that particular bash-mininimal is garbage collected?

My understanding is that since git-annex contains a reference to
bash-minimal, the garbage collector will leave it alone.  Indeed,
running “guix gc --references /gnu/store/…-git-annex-20180626” shows
that Guix knows that git-annex needs bash-minimal.  As long as git-annex
is available (a prerequisite to the pre-commit hook working)
bash-minimal will be available.


-- Tim

Re: git-annex: problematic shebangs in .git/hooks/pre-commit?

[Kyle Meyer]

Timothy Sample <samplet@ngyro.com> writes:

[...]

>> I'm wondering whether the shebang patching in .git/hooks/pre-commit will
>> cause a problem.  Using the patched shellPath_portable, 'git annex init'
>> generates a hook like this:
>>
>>     % cat .git/hooks/pre-commit
>>     #!/gnu/store/rbrandv7anzjxqkr40d7fkanzssslk4b-bash-minimal-4.4.19/bin/sh
>>     # automatically configured by git-annex
>>     git annex pre-commit .
>>
>> But won't this break if that particular bash-mininimal is garbage collected?
>
> My understanding is that since git-annex contains a reference to
> bash-minimal, the garbage collector will leave it alone.  Indeed,
> running “guix gc --references /gnu/store/…-git-annex-20180626” shows
> that Guix knows that git-annex needs bash-minimal.  As long as git-annex
> is available (a prerequisite to the pre-commit hook working)
> bash-minimal will be available.

Hmm, I'm not convinced that *that* bash-minimal will be available.

Perhaps I'm just missing something, but to spell out my concern a little
more: I have /gnu/store/A/git-annex and /gnu/store/X/bash-minimal.  I
set up a repo with 'git annex init', and
/gnu/store/X/bash-minimal/bin/sh is used in the shebang.

Some time later I am using /gnu/store/B/git-annex, which references
/gnu/store/Y/bash-minimal.  That repo's hook still uses the
X/bash-minimal/bin/sh.  But if /gnu/store/X/bash-minimal is no longer
referenced by any package, it could be garbage collected, leading to the
hook failing.

Re: git-annex: problematic shebangs in .git/hooks/pre-commit?

[Timothy Sample]

Hi Kyle,

Kyle Meyer <kyle@kyleam.com> writes:

> Timothy Sample <samplet@ngyro.com> writes:
>
> [...]
>
>>> I'm wondering whether the shebang patching in .git/hooks/pre-commit
>>> will
>>> cause a problem.  Using the patched shellPath_portable, 'git annex
>>> init'
>>> generates a hook like this:
>>>
>>>     % cat .git/hooks/pre-commit
>>>     #!/gnu/store/rbrandv7anzjxqkr40d7fkanzssslk4b-bash-minimal-4.4.19/bin/sh
>>>     # automatically configured by git-annex
>>>     git annex pre-commit .
>>>
>>> But won't this break if that particular bash-mininimal is garbage
>>> collected?
>>
>> My understanding is that since git-annex contains a reference to
>> bash-minimal, the garbage collector will leave it alone.  Indeed,
>> running “guix gc --references /gnu/store/…-git-annex-20180626” shows
>> that Guix knows that git-annex needs bash-minimal.  As long as
>> git-annex
>> is available (a prerequisite to the pre-commit hook working)
>> bash-minimal will be available.
>
> Hmm, I'm not convinced that *that* bash-minimal will be available.
>
> Perhaps I'm just missing something, but to spell out my concern a
> little
> more: I have /gnu/store/A/git-annex and /gnu/store/X/bash-minimal.  I
> set up a repo with 'git annex init', and
> /gnu/store/X/bash-minimal/bin/sh is used in the shebang.
>
> Some time later I am using /gnu/store/B/git-annex, which references
> /gnu/store/Y/bash-minimal.  That repo's hook still uses the
> X/bash-minimal/bin/sh.  But if /gnu/store/X/bash-minimal is no longer
> referenced by any package, it could be garbage collected, leading to
> the
> hook failing.

Ah yes!  That could be a problem indeed.  Good catch.  Following the
example of autoconf, it seems it should just be left alone.  I will fix
it shortly.  (Right now git-annex doesn’t even build, so I will fix that
first!)

Thanks!


-- Tim

Re: git-annex: problematic shebangs in .git/hooks/pre-commit?

[Kyle Meyer]

Timothy Sample <samplet@ngyro.com> writes:

> Kyle Meyer <kyle@kyleam.com> writes:

[...]

>> Perhaps I'm just missing something, but to spell out my concern a
>> little
>> more: I have /gnu/store/A/git-annex and /gnu/store/X/bash-minimal.  I
>> set up a repo with 'git annex init', and
>> /gnu/store/X/bash-minimal/bin/sh is used in the shebang.
>>
>> Some time later I am using /gnu/store/B/git-annex, which references
>> /gnu/store/Y/bash-minimal.  That repo's hook still uses the
>> X/bash-minimal/bin/sh.  But if /gnu/store/X/bash-minimal is no longer
>> referenced by any package, it could be garbage collected, leading to
>> the
>> hook failing.
>
> Ah yes!  That could be a problem indeed.  Good catch.  Following the
> example of autoconf, it seems it should just be left alone.

Revisiting this, I tried to simply remove the patch-shell phase (diff
below).  That worked as intended when I tested manually, but it caused
many test failures with messages like

    fatal: cannot run .git/hooks/pre-commit: No such file or directory

Although the error message isn't too clear, this happens because
/bin/sh, now used in the hook shebangs, isn't available to the tests
executed in the build environment.

Any suggestions on how to deal with this?

---
diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
index dc2abb0c71..56fe6f5e7a 100644
--- a/gnu/packages/version-control.scm
+++ b/gnu/packages/version-control.scm
@@ -2236,11 +2236,6 @@ (define-public git-annex
        '("--flags=-Android -Assistant -Pairing -S3 -Webapp -WebDAV")
        #:phases
        (modify-phases %standard-phases
-         (add-before 'configure 'patch-shell
-           (lambda _
-             (substitute* "Utility/Shell.hs"
-               (("/bin/sh") (which "sh")))
-             #t))
          (add-before 'configure 'factor-setup
            (lambda _
              ;; Factor out necessary build logic from the provided

Re: git-annex: problematic shebangs in .git/hooks/pre-commit?

[Timothy Sample]

Hi Kyle,

Kyle Meyer <kyle@kyleam.com> writes:

> Timothy Sample <samplet@ngyro.com> writes:
>
>> Kyle Meyer <kyle@kyleam.com> writes:
>
> [...]
>
>>> Perhaps I'm just missing something, but to spell out my concern a
>>> little
>>> more: I have /gnu/store/A/git-annex and /gnu/store/X/bash-minimal.  I
>>> set up a repo with 'git annex init', and
>>> /gnu/store/X/bash-minimal/bin/sh is used in the shebang.
>>>
>>> Some time later I am using /gnu/store/B/git-annex, which references
>>> /gnu/store/Y/bash-minimal.  That repo's hook still uses the
>>> X/bash-minimal/bin/sh.  But if /gnu/store/X/bash-minimal is no longer
>>> referenced by any package, it could be garbage collected, leading to
>>> the
>>> hook failing.
>>
>> Ah yes!  That could be a problem indeed.  Good catch.  Following the
>> example of autoconf, it seems it should just be left alone.
>
> Revisiting this, I tried to simply remove the patch-shell phase (diff
> below).  That worked as intended when I tested manually, but it caused
> many test failures with messages like
>
>     fatal: cannot run .git/hooks/pre-commit: No such file or directory
>
> Although the error message isn't too clear, this happens because
> /bin/sh, now used in the hook shebangs, isn't available to the tests
> executed in the build environment.

Thanks for giving this a try!

> Any suggestions on how to deal with this?

I am travelling at the moment, so I can’t dig into it properly.  I
crossed my fingers and tried symlinking /bin/sh to bash, but did not
have permission to make the /bin directory.  (I guessed the build user
would be able to do this from within the build environment, but
apparently not.)  It would be ideal to set up the build environment like
this, but I’m not sure how.

> ---
> diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
> index dc2abb0c71..56fe6f5e7a 100644
> --- a/gnu/packages/version-control.scm
> +++ b/gnu/packages/version-control.scm
> @@ -2236,11 +2236,6 @@ (define-public git-annex
>         '("--flags=-Android -Assistant -Pairing -S3 -Webapp -WebDAV")
>         #:phases
>         (modify-phases %standard-phases
> -         (add-before 'configure 'patch-shell
> -           (lambda _
> -             (substitute* "Utility/Shell.hs"
> -               (("/bin/sh") (which "sh")))
> -             #t))
>           (add-before 'configure 'factor-setup
>             (lambda _
>               ;; Factor out necessary build logic from the provided


-- Tim