GNU bug report logs - #48849
[PATCH core-updates]: Add #:sh argument to wrap-qt-program

Previous Next

Full log

View this message in rfc822 format

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: M <maximedevos <at> telenet.be>
Cc: "48849 <at> debbugs.gnu.org" <48849 <at> debbugs.gnu.org>
Subject: [bug#48849] [PATCH core-updates]: Add #:sh argument to wrap-qt-program
Date: Sun, 21 Jan 2024 23:58:59 -0500

Hi Maxime,

M <maximedevos <at> telenet.be> writes:

>>> Subject: [PATCH 1/8] qt-utils: Allow overriding the shell interpreter in
>>>  'wrap-qt-program'.
>>>
>>> * guix/build/qt-utils.scm (wrap-qt-program): Introduce a #:sh keyword
>>>   argument and pass it to 'wrap-program'.
>>
>> If bash-minimal is added to inputs as we do for other packages making
>> use of wrap-program, we don't need to do more, no?  Why do we need to
>> explicit the argument here?
>
> Post-hoc reason (for the first patch): wrap-program has #:sh argument,
> wrap-qt-program doesn’t, which is inconsistent.

Good point.

> For the rest (to be clear I think the remaining patches can be removed):

OK, good.

> Right, technically we don’t. The reason is to make sure that it’s the
> bash from inputs instead of the bash native-inputs. Currently, at
> first it gets the (wrong) native bash, and later on this is fixed up
> by the patch-shebangs phase, IIRC.

I see.

> However, (IIRC) that behaviour is a bug – patch-shebangs is for
> /usr/bin/… -> /gnu/store/… stuff – if the code “make install” or the
> like already set a proper /gnu/store/… shebang, why automatically
> change it to something else? Presumably it set it to the right
> interpreter, and now patch-shebangs might autocorrupt it.

Ah!  Interesting.  I haven't seen any report for such bug.

> Another problem: there might not even be a patch-shebangs phase, uses
> of wrap-program, wrap-qt-program and the phase of the qt-build-system
> that uses wrap-qt-program (IIRC there exists such a phase) should be
> usable in isolation. Also, there is a hidden assumption that the uses
> of wrap-program are _before_ the shebang patching, whereas it might be
> run afterwards as wll.
>
> Instead, I think it’s better for the uses of ‘wrap-program’ to directly set it to the _right_ bash.
> That’s what the #:sh argument is for, but #:sh is set to by default
> (which “bash”), which is incorrect. Hence, #:sh needs to be set
> explicitly, and hence wrap-qt-program needs a #:sh argument or the
> like to pass on to wrap-program.
>
> That said, I don’t think all this explicit #:sh is appropriate either
> – it would need to be repeated for every single package definition
> refering to wrap-program, etc.. Instead, for the future, I’d propose
> to eliminate the argument list of phases, turning phase procedures in
> phase thunks and stuffing the old arguments in parameter objects
> instead.
>
> Then, the #:sh of ‘wrap-program’ could default to (search-input-file
> (inputs) “bin/inputs”) – automatically correct (without needing
> patch-shebangs) both for native and cross-compilation, and when
> cross-compiling without “bash” in (implicit) inputs, it automatically
> errors out (instead of doing the wrong thing as done currently).
>
> The phases would also be a bit less verbose to write – (lambda* (#:key
> this that #:allow-other-keys) (proc this) stuff …) could become
> (lambda () (proc) stuff …).
>
> (The ‘procedure’ syntax (inputs) for parameter objects might not be
> the best here, but that’s nothing some bikeshedding over the precise
> syntax can’t fix.)

Thanks for sharing your perspective on this.  It's an interesting idea
that you are proposing, but it'd entail a massive effort to port the
code base.

Cheers!  Enjoy whatever hack you are currently pursuing!

-- 
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.