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

travis_buildtempl_header: Save full PIPESTATUS #528

Closed
wants to merge 2 commits into from

Conversation

Artoria2e5
Copy link

This commit saves the full PIPESTATUS[] array into a globally-accessible array called TRAVIS_PIPESTATUS[], and is an extension of the edits involved in infertux/bashcov#12.

Two doubts so far:

  • Shall we consider chances of the TRAVIS_PIPESTATUS stuff being exported? If we consider those, we may need to save it like a something-delimited strings like a ling of TRAVIS_PIPESTATUS=${TRAVIS_PIPESTATUS[*]} right after the result= line, and users can split them later like pstat=( $TRAVIS_PIPESTATUS ).
  • Is the bash version on travis new enough for negative indices to indicate the last elements? If not, we need to compute them using the length of the array ourselves.

This commit saves the full `PIPESTATUS[]` array into a globally-accessible array called `TRAVIS_PIPESTATUS[]`, and is an extension of the edits involved in travis-ci#12.

Two doubts so far:
* Shall we consider chances of the `TRAVIS_PIPESTATUS` stuff being exported? If we consider those, we may need to save it like a something-delimited strings like a ling of `TRAVIS_PIPESTATUS=${TRAVIS_PIPESTATUS[*]}` right after the `result=` line, and users can split them later like `pstat=( $TRAVIS_PIPESTATUS )`.
* Is the bash version on travis new enough for negative indices to indicate the last elements? If not, we need to compute them using the length of the array ourselves.
@Artoria2e5
Copy link
Author

I think I'll find out about doubt 2 after the CI check.

Oh it seems to work.

@Artoria2e5
Copy link
Author

Messing things up.. referenced the wrong issue in the first commit..

@@ -67,7 +67,7 @@ travis_time_finish() {
return $result
}

function travis_nanoseconds() {
travis_nanoseconds() {
local cmd="date"
local format="+%s%N"
local os=$(uname)

This comment was marked as spam.

@joshk
Copy link
Contributor

joshk commented Oct 14, 2015

Hi, thanks for the PR, could you please explain some more information on what the use case of this PR is?

@Artoria2e5
Copy link
Author

Forget it. travis-ci/travis-ci#3771 took me to some weird place and made me dizzy… Oh it might work still.


@joshk For pipe commands like foo | bar | baz, the $? would only consider the return value of baz which can be quite annoying.

Bash's $PIPESTATUS[] array will save all those three's return values, but like $? it will be modified immediately after another command ran (and result=$? simply ruins it..). To ease users who want to do extra testing around it, we should save it somewhere.

@BanzaiMan
Copy link
Contributor

For complex builds that requires the use of $?, I tend to think writing a custom script is a cleaner solution.

@Artoria2e5
Copy link
Author

You are right.

@Artoria2e5 Artoria2e5 closed this Oct 21, 2015
@Artoria2e5 Artoria2e5 deleted the patch-1 branch October 21, 2015 13:17
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.

3 participants