[PATCH core-updates]: Add #:sh argument to wrap-qt-program

Hi guix,

This patch series adds a #:sh keyword argument to wrap-qt-program

and adjusts callers to set this keyword argument appropriately.

Setting this argument appropriately is required for cross-compilation.

Otherwise, a bash for SYSTEM (as in --system=..) is used instead

of a bash for TARGET (as in --target=...).

I didn't test building some qt programs with this patch series yet,

as I'm currently waiting on the substitute servers to catch up

with recent changed to core-updates (to avoid building a tower

of rusts). I'll do that later.

Greetings,

Maxime

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

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?

--

Thanks,

Maxim

tags 48849 + moreinfo

quit

Post-hoc reason (for the first patch): wrap-program has #:sh argument, wrap-qt-program doesn’t, which is inconsistent.

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

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.

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.

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.)

Bst regards,

Maxime Devos

(p.s. I received the mails for the other patches but I’m not responding at the time – not active with Guix currently, and borrowing another computer because of repairs.)

Hi Maxime,

M <maximedevos@telenet.be> writes:

Good point.

OK, good.

I see.

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

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

tags 48849 - moreinfo

quit

Hi again,

Maxime Devos <maximedevos@telenet.be> writes:

I see this patch was already merged by Mathieu in

77b98bf1874aac8ed447e9f0b0ee0865a1d652ba; thus, I'm closing this ticket.

Thank you!

--

Maxim