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

[ci] make shell scripts stricter #6266

Merged
merged 35 commits into from
Apr 23, 2024
Merged

[ci] make shell scripts stricter #6266

merged 35 commits into from
Apr 23, 2024

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Jan 11, 2024

Proposes standardizing on 2 practices for shell scripts in this project:

  • using #!/bin/bash (not sh or other shells)
  • running shell scripts under stricter settings, to get earlier and louder errors

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 from R CMD check.

Notes for Reviewers

Per the set man page (link):

      -e  Exit immediately if a command exits with a non-zero status.
      -o option-name
          Set the variable corresponding to option-name:
              allexport    same as -a
              ...
              pipefail     the return value of a pipeline is the status of
                           the last command to exit with a non-zero status,
                           or zero if no command exited with a non-zero status
              ...
      -u  Treat unset variables as an error when substituting.

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 up bash (which tends to be less common on non-Linux platforms than sh):

  • build-python.sh
  • build-cran-package.sh
  • .ci/check_python_dists.sh

@jameslamb jameslamb changed the title WIP: [ci] make shell scripts stricter [ci] make shell scripts stricter Apr 22, 2024
compiler: gcc
r_version: 4.3
build_type: cran
container: 'ubuntu:22.04'
Copy link
Collaborator Author

@jameslamb jameslamb Apr 22, 2024

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.

@jameslamb jameslamb marked this pull request as ready for review April 22, 2024 18:53
.ci/check_python_dists.sh Show resolved Hide resolved
.ci/lint-cpp.sh Show resolved Hide resolved
build-cran-package.sh Outdated Show resolved Hide resolved
build-python.sh Outdated Show resolved Hide resolved
@jameslamb jameslamb requested a review from borchero April 22, 2024 22:19
.ci/lint-cpp.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@borchero borchero left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@jameslamb jameslamb merged commit 52441c4 into master Apr 23, 2024
41 checks passed
@jameslamb jameslamb deleted the ci/stricter-scripts branch April 23, 2024 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants