bash: Build new PATH the smart way #3096
Draft
+63
−19
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
**Summary** The goal with this PR is to ensure that any existing user-set PATH is respected to the widest extent possible. Any elements of the default PATH which have not already been added, will be added to the new PATH in (what I hope to be) a sane way. In the case of no user-set PATH, prepare a sane default PATH. I have left the debug statements in for now to provide visibility to the various test cases. Naturally, these debug statements will need to disappear before the commit is merged to main. Signed-off-by: Rune Morling <ermo@serpentos.com>
ermo
added
Topic: Platform Integration
silkeh
requested changes
Oct 24, 2024
There was a problem hiding this comment.
What I'm missing from the PR description is where the user's $PATH would be coming from. It's can't be set by the user's Bash config (see Bash Startup Files):
When Bash is invoked [...] it first reads and executes commands from the file /etc/profile, if that file exists.
The linked issue could be resolved as follows, which keeps this file much simpler and (importantly) more predictable for the user:
if [ -z ${PATH+x} ]
then
# Insert old implementation here
fiEdit: the issue describes that path would be overridden somewhere else though, so I think its probably worthwhile to figure out where.
| @@ -1,20 +1,64 @@ | |||
| # Begin /usr/share/defaults/etc/profile.d/10-path.sh | |||
There was a problem hiding this comment.
File should start with a shebang to clarify that this is a bash script:
Suggested change
| # Begin /usr/share/defaults/etc/profile.d/10-path.sh | |
| #!/usr/bin/env bash | |
| # Begin /usr/share/defaults/etc/profile.d/10-path.sh |
Additionally, please pass the file through shellcheck.
1 task
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.
Summary
The goal with this PR is to ensure that any existing user-set PATH is respected to the widest extent possible. Any elements of the default PATH which have not already been added, will be added to the new PATH in (what I hope to be) a sane way.
In the case of no user-set PATH, prepare a sane default PATH.
I have left the debug statements in for now to provide visibility to the various test cases. Naturally, these debug statements will need to disappear before the commit is merged to main.
Test Plan
Open a new virtual terminal with the new 10-path.sh file in place in /usr/share/defaults/etc/profile.d/
Checklist