ci: Fix default retry script usage #18798
Merged
Conversation
This file contains hidden or 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
maflcko
reviewed
Apr 28, 2020
| bash -c "export PATH=$BASE_SCRATCH_DIR/bins/:\$PATH && cd $P_CI_DIR && $*" | ||
| } | ||
| fi | ||
| export -f DOCKER_EXEC |
Member
There was a problem hiding this comment.
This part I don't understand. It was working "correctly" before (without retry).
Member
Author
There was a problem hiding this comment.
Besides Bash Reference Manual, there are more references:
- https://unix.stackexchange.com/questions/22796/can-i-export-functions-in-bash
- https://stackoverflow.com/questions/1885871/exporting-a-function-in-shell
If shell function retry is used, it runs a subshell, and cannot see definitions of other shell functions unless they are exported: "Functions may be exported so that subshells automatically have them defined with the -f option to the export builtin." (from Bash Reference Manual).
This could be verified by set -x.
Member
Author
|
The example of the fixed usage of the |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 29, 2020
45615de ci: Fix default retry script usage (Hennadii Stepanov) Pull request description: On master (5352d14) `CI_RETRY_EXE=${CI_RETRY_EXE:retry}` works as a [Substring Expansion](https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html), and that is wrong. If `CI_RETRY_EXE` variable was unset initially, its new value becomes an empty string, but not "retry" as one could expect. Consequently, the `${CI_RETRY_EXE} ...` command does _not_ use `ci/retry/retry` script. This PR makes for `CI_RETRY_EXE` variable a usual parameter expansion, i.e., `${parameter:-word}`. Reference: bitcoin#18735 (comment) Top commit has no ACKs. Tree-SHA512: 108173f6b2677979b9ddf2f9b9df4a6c56f5efa81c36543a1816bb3b984e42984bf3c83fe413ea3a5ca1e2317c4efb02fea7180a6b44863af7cfe6202e9cf94d
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
On master (5352d14)
CI_RETRY_EXE=${CI_RETRY_EXE:retry}works as a Substring Expansion, and that is wrong.If
CI_RETRY_EXEvariable was unset initially, its new value becomes an empty string, but not "retry" as one could expect. Consequently, the${CI_RETRY_EXE} ...command does not useci/retry/retryscript.This PR makes for
CI_RETRY_EXEvariable a usual parameter expansion, i.e.,${parameter:-word}.Reference: #18735 (comment)