Re: [PATCH] gnu: Add kakoune

[Carlo Zancanaro <carlo@zancanaro.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2658 bytes --]


On Fri, Dec 30 2016, Leo Famulari wrote
>> diff --git a/gnu/packages/patches/kakoune-fix-bash-path.patch b/gnu/packages/patches/kakoune-fix-bash-path.patch
>> diff --git a/gnu/packages/patches/kakoune-leave-manpages-uncompressed.patch b/gnu/packages/patches/kakoune-leave-manpages-uncompressed.patch
>
> Patches require some commentary explaining what problem they are solving
> and, if the approach they take is unclear, how they do it. If they are
> copied from somewhere else, they need attribution.
>
> In this case, we need to explain why upstream's approach of finding a
> POSIX shell doesn't work.

Perhaps you can give me a hint on this. I've patched it myself because
the original code uses confstr with _CS_PATH, but within the build
environment (and presumably outside of it, too) it gives back
"/bin:/usr/bin". Within the build environment this is invalid (as
neither directory exists), so the tests all fail. There is a bug[1]
about this already, but it was closed without fixing the issue (due to
some complexity/ambiguity in doing so, and no immediate need to fix it).

I can try to come up with a better way to patch this, but I don't know
if I should actually use the PATH environment variable. The patch that
changed this code to use _CS_PATH instead of PATH was made fifteen days
ago and was apparently to "use a POSIX guaranteed way of getting the
shell path"[2].

I could package the commit from just before the patch instead (which
would make this problem go away for now), or we can try to come up with
a real solution to the problem.

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=23112.
[2]: https://github.com/mawww/kakoune/commit/9a879262a272bd4c6458fcfa07a9289ee41d7220

>> +           (add-before 'configure 'patch-bash-path
>> +             (lambda* (#:key inputs #:allow-other-keys)
>> +               (substitute* "src/shell_manager.cc"
>> +                 (("__BASH_PATH__")
>> +                  (assoc-ref inputs "bash")))))
>
> I wonder if we can do this change all in the same place. It's sort of
> complicated to use a patch and a substitution.
>
> Also, using a patch to get half-way there will cause the source code
> given by `guix build --source` to be unbuildable, right? So, I think we
> should find another way to fix whatever is wrong with the shell-finding
> logic. Perhaps it will require cooperation with the upstream
> maintainers, I don't know.

I hadn't thought about the effect this would have on `guix build
--source`. That's a fairly big problem.

Everything else that I haven't responded to is fine. I'll fix them up
and send a new patch once I have some direction on the _CS_PATH issue.

Carlo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]