The following commit
breaks useq of paludis because IFS is being changed. portage is not affected because it sets IFS=$' \t\n'. The question to ask here is where a package manager is supposed to set IFS.
Created attachment 359810 [details]
Log of failing resolve
Same issue here.
We should probably specify in PMS whether the PM should deal with strange values in IFS and other variables. Portage is fairly random in where it does and doesn't set its own IFS, which makes me think this has been one of those "work around it as bugs show up" problems, which is of course the wrong thing to do.
I've changed the way splitting is done to avoid changing IFS. This should work-around the issue for Paludis users.
Re-assigning to Paludis maintainer, since it is still a bug in Paludis. Since PMS doesn't say a thing about IFS, it is incorrect for Paludis helpers to rely on any particular value of it.
We should be deciding this properly and speccing it, and for all helper functions. Portage is rather arbitrary about where it does and doesn't sanitise IFS in helpers. Also, this was a fairly recent Portage change, so it's not something that's "been that way all along".
(In reply to Ciaran McCreesh from comment #5)
> We should be deciding this properly and speccing it, and for all helper
But what should we specify? IFS only? I'd guess that also some of the shopt or set options can induce breakage.
Yeah, that's the annoying bit. PATH could also screw with things in some cases too. From a spec perspective, it might be easiest to just say "if ebuilds screw around with stuff, they have to undo it for helper calls"...
And we get to the point when we provide users with opaque helper bits which can rely on almost anything being untouched, and we basically gain nothing.
Yeah, it's that or we specify eeeeeverything and keep going back and checking carefully each time we change bash versions.
...or we expect PM authors to sanitize the environment inside helper functions as necessary.
How do we define "as necessary"? Are we including PATH, for example? What about weird shopts?
(In reply to Ciaran McCreesh from comment #11)
> How do we define "as necessary"? Are we including PATH, for example? What
> about weird shopts?
Sure, whatever is necessary. PM has to set it all up for the build process anyway, there's no reason it couldn't reset it locally within the helper. Most of it would be solved if helpers were actually external executables rather than functions.
As for PATH, the current wording makes helpers reside in PATH defined by the PM. Therefore, users can't clear it from the default.
Not sure if there's even an effective way to sanitise the shell's state, given that there are creative ways to redefine shell builtins or define aliases.
(In reply to Ulrich Müller from comment #13)
> Not sure if there's even an effective way to sanitise the shell's state,
> given that there are creative ways to redefine shell builtins or define
I think the most effective way is to run an external helper. Unless I'm missing something, this shouldn't pass anything through except for exported environment variables.
Don't we specify some of these to be builtins? Pretty sure we had to in the old days because external commands couldn't die.
(In reply to Ciaran McCreesh from comment #15)
> Don't we specify some of these to be builtins?
We do. Some of them cannot be external commands because they modify environment variables.
I tend to go with comment #7, because restoring the state is much easier in the caller than in the called function. Also, calling random functions with changed IFS _is_ bad style.
OTOH, the spec is quite clear about the "use" command:
"Returns shell true (0) if the first argument (a USE flag name) is enabled, false otherwise. If the flag name is prefixed with !, returns true if the flag is disabled, and false if it is enabled."
So the PM should make sure that its behaviour for word splitting e.g. of IUSE_EFFECTIVE is sane, and should sanitise IFS if necessary.