-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[ci] make shell scripts stricter #6266
Conversation
compiler: gcc | ||
r_version: 4.3 | ||
build_type: cran | ||
container: 'ubuntu:22.04' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rchk
job has been silently broken for over a year.
I'm working on fixing it in #6332 . See that PR for details.
In this PR, I'm proposing removing it entirely to save some CI time and to not have to introduce complexity to workaround its issues as a part of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks!
Proposes standardizing on 2 practices for shell scripts in this project:
#!/bin/bash
(notsh
or other shells)Benefits of these changes
Prevents situations where tests are silently failing... like the
rchk
tests have been for over a year (#6332).Reduces debugging time by ensuring that CI logs tend to end with the first thing that was broken. For example, no more
brew install
silently failing and then only finding out about it via cryptic errors fromR CMD check
.Notes for Reviewers
Per the
set
man page (link):I've kept the following scripts using
sh
, so they can be executed in the widest possible range of environments (including Windows), without needing to worry about setting upbash
(which tends to be less common on non-Linux platforms thansh
):build-python.sh
build-cran-package.sh
.ci/check_python_dists.sh