autoinstall: Don't use snap env when invoking early and late commands #1811
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ogayot
requested changes
Sep 29, 2023
Chris-Peterson444
force-pushed
the
lp-2032961-clean-environment-commands
branch
from
a0a7bba
to
7261d6c
Compare
dbungert
reviewed
Oct 2, 2023
Chris-Peterson444
force-pushed
the
lp-2032961-clean-environment-commands
branch
2 times, most recently
from
10100ba
to
196667b
Compare
dbungert
reviewed
Oct 2, 2023
Chris-Peterson444
force-pushed
the
lp-2032961-clean-environment-commands
branch
2 times, most recently
from
b470bcb
to
fa20e09
Compare
dbungert
reviewed
Oct 3, 2023
Chris-Peterson444
force-pushed
the
lp-2032961-clean-environment-commands
branch
from
fa20e09
to
0d03eea
Compare
dbungert
approved these changes
Oct 3, 2023
|
I believe the requested changes from @ogayot are resolved - if that's in error let's fix that later, as he's not available right now to comment further. @Chris-Peterson444 - one last check - have you tried this in a VM and confirmed that doing |
|
I tested in a VM with |
|
I tried |
Chris-Peterson444
force-pushed
the
lp-2032961-clean-environment-commands
branch
from
0d03eea
to
8f28063
Compare
Merged
Chris-Peterson444
added a commit
to Chris-Peterson444/subiquity
that referenced
this pull request
Oct 25, 2023
Chris-Peterson444
added a commit
to Chris-Peterson444/subiquity
that referenced
this pull request
Oct 25, 2023
Revert "Merge pull request canonical#1811 from Chris-Peterson444/lp-2032961-clean-environment-commands" This reverts commit 85af88e, reversing changes made to 143d8e3. The fix proposed in this patch caused more issues than it fixed. We will have to revisit this in a more nuanced way in the future. In the meantime users can make use of `env` directly to strip/modify the subcommand environment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LP #2032961: We are not cleaning the environment provided to subprocesses correctly. When trying to invoke a program that should run outside of the snap context, the
LD_LIBRARY_PATHvariable is pointing to the linker on the snap, which causes errors like:We had a similar issue solved in #1300. This fix similarly uses
orig_environto clean the environment before running commands in the respective command controllers.I would especially appreciate some input if the license header on the new test file looks correct. It's an exact copy and paste from one of the other new tests files from this year, but I just want to be sure.