Switch to POSIX sh for deployment scripts. #5
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
This option is unsupported in POSIX sh.
POSIX sh does not maintain the EUID variable.
This is not as robust as BASH_SOURCE but should work for our needs.
The suffix option to mktemp is a GNU extension to the standard. The main advantage of the suffix is the syntax highlighting when reviewing the script. We could drop it altogether.
These are not available in POSIX sh.
j-rivero
reviewed
Apr 14, 2023
|
|
||
| # Create a tempfile so the user has the opportunity to review the omnitruck script. | ||
| omnitruck_script="$(mktemp --suffix=.bash)" | ||
| omnitruck_script="$(mktemp)" |
There was a problem hiding this comment.
Suggested change
| omnitruck_script="$(mktemp)" | |
| omnitruck_script="$(mktemp XXXXXXXXXX.bash)" |
and you can probably remove the line below. If you have interest into keeping the bash file into the current directory:
omnitruck_script="$(mktemp -p . XXXXXXXXXX.bash)"but I think that for cleanness we can keep it the temp directory.
j-rivero
reviewed
Apr 14, 2023
| @@ -43,11 +45,12 @@ freshen_chef_repo() { | |||
| if [ ! -d chef_repo ]; then | |||
| git clone "$CHEF_REPO" -b "$CHEF_BRANCH" --recurse-submodules --remote-submodules chef_repo | |||
| else | |||
| pushd chef_repo | |||
| CURDIR="$(pwd)" | |||
| cd chef_repo | |||
| git fetch origin | |||
There was a problem hiding this comment.
Alternatively, git -C chef_repo <rest-of-the-command> should work here to save some code lines, right?
j-rivero
approved these changes
Apr 14, 2023
3 tasks
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.
There's got to be something in here about chickens coming home to roost. I generally write things in POSIX sh in order to make sure they continue to function correctly, but doing so in a bash world is a challenge and when we were first preparing this repository I decided I could relax this constraint and use bash in order to make contributing more accessible (also there's nothing as robust as the
BASH_SOURCEenvironment variable in POSIX sh).However, trying to use these scripts for the OSRF build farm's mac deployments reminded me that bash (and GNU userland utils) is still far from universal.