Schniz
Suggestion for Modifying PATH Export in install.sh to Enhance Compatibility and Safety
Hello Team,
I've been reviewing the installation script .ci/install.sh and noticed that the current method of setting the PATH environment variable is done by prepending the installation directory to $PATH. This is the line: echo ' export PATH="'"$INSTALL_DIR"':$PATH"'. I have some concerns and suggestions regarding this approach.
Concerns: Conflict with Preexisting Tooling: Prepending the installation directory to $PATH might lead to potential conflicts with already installed tools. This is because it prioritizes the newly installed tools over the existing ones in the system, which could unintentionally overshadow tools with the same name.
Safety and Reliability: The current method might not be the safest way to modify the PATH. It does not account for the scenario where $PATH is initially unset. As a result, the installation directory would be the only thing in PATH, which could cause issues with system-wide tool availability.
Suggestions:
Appending to $PATH: I propose appending the installation directory to $PATH instead of prepending. This approach ensures that any existing tools with the same name are not overshadowed by the newly installed ones. It would maintain the expected behavior of used system tools.
More Robust $PATH Modification: To enhance safety, especially in scenarios where $PATH might be unset initially, a more robust method of modifying $PATH can be used. The method I suggest is:
echo ' export PATH="${PATH:+${PATH}:} '"$INSTALL_DIR"'"'
This approach ensures that the installation directory is safely added to $PATH, regardless of whether $PATH was initially set or not. It's a more bullet-proof method, as discussed in this StackExchange post.
Proposed Changes: I would like to implement these changes and submit a PR. The changes would include:
1. Modifying the install.sh script to append the installation directory to $PATH. 2. Implementing a more robust method for modifying $PATH to ensure safety and reliability.
I realize that this might seem like nitpicking and ultimately, the impact of these changes could be very minimal. I completely understand if you decide to disregard this suggestion. I just wanted to bring it to your attention as a potential improvement for the script's robustness and safety.
