[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu: Add kakoune
From: |
Carlo Zancanaro |
Subject: |
Re: [PATCH] gnu: Add kakoune |
Date: |
Sat, 31 Dec 2016 10:19:53 +1100 |
User-agent: |
mu4e 0.9.16; emacs 25.1.1 |
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
signature.asc
Description: PGP signature