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.

Sign up for GitHub

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

Jump to bottom

BuildPackages.sh --strict no longer works #4384

Closed
fingolfin opened this issue Apr 9, 2021 · 2 comments · Fixed by #4386
Closed

BuildPackages.sh --strict no longer works #4384

fingolfin opened this issue Apr 9, 2021 · 2 comments · Fixed by #4386
Labels
kind: bug Issues describing general bugs, and PRs fixing them topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ...

Comments

@fingolfin
Copy link
Member

fingolfin commented Apr 9, 2021 •
edited by wilfwilson
Loading

Previous title: CI: failure to compile IO, profiling should cause the relevant GitHub Actions step to fail

Right now, it doesn't: In https://github.com/gap-system/gap/pull/4381/checks?check_run_id=2305769116 it fails to compile io but just goes on. And that despite using BuildPackages.sh --strict which should give us a non-zero exit code. Perhaps that got broken, or perhaps we are not correctly passing it on, or...
@fingolfin fingolfin added the topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ... label Apr 9, 2021
@wilfwilson
Copy link
Member

wilfwilson commented Apr 9, 2021 •
edited
Loading

It is the fault of BuildPackages.sh. If I deliberately break the profiling package and then call

../bin/BuildPackages.sh --strict profiling* && echo $?

from my pkg dir, then the final line of output is 0. Here's the last few lines of output:

WARNING: Failed to build profiling

Packages which failed to build are in ./log/fail.log
0

Indeed, the if [[ $STRICT ]]; then exit 1; fi bit of code occurs immediately after the code that prints the WARNING: line, so that error code is not being caught; the script then goes on to print the "Packages which failed to build..." line and exit happily.

I'll see if I can fix it.

@wilfwilson
Copy link
Member

This was introduced in #3922, which (according to my very limited understanding of bash...) put the relevant bit in a subshell whose result was ignored - something like that.

@wilfwilson wilfwilson self-assigned this Apr 9, 2021
wilfwilson added a commit to wilfwilson/gap that referenced this issue Apr 9, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
be21e9e 
gap-system#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
@wilfwilson wilfwilson changed the title CI: failure to compile IO, profiling should cause the relevant GitHub Actions step to fail BuildPackages.sh --strict no longer works Apr 9, 2021
@wilfwilson wilfwilson added the kind: bug Issues describing general bugs, and PRs fixing them label Apr 9, 2021
@wilfwilson wilfwilson mentioned this issue Apr 9, 2021
Merged
wilfwilson added a commit to wilfwilson/gap that referenced this issue Apr 9, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
54228dc 
gap-system#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
wilfwilson added a commit to wilfwilson/gap that referenced this issue Apr 9, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
59c8eb6 
gap-system#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
wilfwilson added a commit to wilfwilson/gap that referenced this issue Apr 10, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
4778041 
gap-system#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
wilfwilson added a commit to wilfwilson/gap that referenced this issue Apr 11, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
8f260c9 
gap-system#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
wilfwilson added a commit to wilfwilson/gap that referenced this issue Apr 11, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
2b11fa4 
gap-system#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
@wilfwilson wilfwilson removed their assignment Apr 12, 2021
wilfwilson added a commit to wilfwilson/gap that referenced this issue Apr 14, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
d16fecb 
gap-system#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
@wilfwilson wilfwilson mentioned this issue Apr 19, 2021
Open
@wilfwilson wilfwilson linked a pull request Apr 19, 2021 that will close this issue
Fix the broken --strict option to BuildPackages.sh, but only if --parallel is not also specified #4386
Merged
wilfwilson added a commit that referenced this issue Apr 19, 2021
@wilfwilson
Restore some of --strict to BuildPackages script
27c4e1f 
#4384

The addition of the --parallel option to the bin/BuildPackages.sh
script unfortunately broke the operation of the --strict option.

This restores the functionality of --strict when --parallel is not used.

I am not 1337 enough to work out how to do catch the error codes
of parallel background processes!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind: bug Issues describing general bugs, and PRs fixing them topic: ci Anything related to GitHub Actions, Codecov, AppVeyor, Coveralls, Travis, ...
Projects
None yet
Milestone
No milestone
2 participants
@fingolfin @wilfwilson