search

precice / nix-packages

Official preCICE adapters and solvers packaged with the Nix package manager. See https://precice.discourse.group/t/precice-goes-nix-os-improving-reproducibility-of-scientific-software/
5 stars 1 forks source link

Linting scripts with shellcheck #11

Open MakisH opened 1 year ago

MakisH commented 1 year ago

You may want to try shellcheck. For setup.sh, it only gives two issues, but it can be quite helpful in other cases:

~/repos/precice/nix-packages [master]$ shellcheck setup.sh 

In setup.sh line 17:
if [ "$(grep 'export PATH=$PATH:$HOME/.bin' "$HOME/.profile" -c)" -eq 0 ]; then
             ^----------------------------^ SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.

In setup.sh line 18:
  echo 'export PATH=$PATH:$HOME/.bin' >> "$HOME/.profile"
       ^----------------------------^ SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.

For more information:
  https://www.shellcheck.net/wiki/SC2016 -- Expressions don't expand in singl...
MakisH commented 1 year ago

Looking at this line, you may also want to prepend, instead of appending, as the system would then look at the Nix binaries first:

export PATH=$HOME/.bin:$PATH

cheriimoya commented 1 year ago

Here, we actually want the single quotes, as we really want to grep for this exact string without expanding it. But having shellcheck is probably a good idea anyway.

For the prepending part: I am unsure if this is a good idea. The way it is, it will first take a natively installed nix binary, if available and then fallback to the binary that was installed by using the setup.sh script. What are your thoughts on this?

MakisH commented 1 year ago

For the prepending part: I am unsure if this is a good idea. The way it is, it will first take a natively installed nix binary, if available and then fallback to the binary that was installed by using the setup.sh script. What are your thoughts on this?

I would assume that a user installation of nix has priority over the system installation. If the user needed to install a different version locally, there was probably a reason, such as a problem or missing feature of the system installation.