Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shell script changes for POSIX compliance and portability #217

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dcantrell
Copy link

Here's a batch of shell scripts I modified to work with a wider range of shells. Past experience has shown me that bash versions can vary from time to time. I made changes and then ran these through shellcheck as well.

If these are not of interest, let me know. Otherwise I will continue making changes to the other scripts I find in the tree.

This removes the use of bash-isms and simplifies some of the shell
syntax constructs.  I use ShellCheck as well to validate my changes.
The resulting script runs under bash, but also shells like dash and
zsh.

# This container builds with koji into $RPMDIR starting from a pagure PR

set -xeo pipefail
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this removal intentional?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that line is not portable shell.

@bgoncalv
Copy link
Collaborator

bgoncalv commented Apr 6, 2020

[test]

@bgoncalv
Copy link
Collaborator

bgoncalv commented Apr 6, 2020

@dcantrell the stage trigger CI job fails with:

+ ./create-containers.sh
Require oc but it's not installed.  Aborting.

Do you think it is related to your changes?
oc seems to be installed in our Jenkins master...

sh-4.2$ which oc                                                                                                                                                                                                                   
/bin/oc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants