-
Notifications
You must be signed in to change notification settings - Fork 98
Make all shell scripts nounset-clean, enable "strict mode" #427
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
Conversation
kevinAlbs
left a comment
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, LGTM. Thank you for the cleanup.
The "create packages" script was moved out-of-line from config.yml and placed in a dedicated file to allow for braced shell substitutions without EVG expansions interfering.
That seems helpful. It makes it simpler to run the script locally as well.
rcsanchez97
left a comment
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.
LGTM, with the one minor nit about the indentation and with this additional change to README.md:
diff --git a/README.md b/README.md
index e6e3742..8892402 100644
--- a/README.md
+++ b/README.md
@@ -116,7 +116,7 @@ Do the following when releasing:
- Update CHANGELOG.md with any new changes and update the `[Unreleased]` text to the version being released.
- If this is a new minor release (e.g. `x.y.0`):
- Update the Linux distribution package installation instructions in the below sections to refer to the new version x.y.
- - Update the release branch references (i.e., `-DRELEASE_BRANCH_REF=origin/rx.y`) in the invocations of `GetVersion.cmake` (currently located in `CMakeLists.txt`, `.evergreen/debian_package_build.sh`, and `.evergreen/config.yml`) so that they refer to the new branch you are about to create
+ - Update the release branch references (i.e., `-DRELEASE_BRANCH_REF=origin/rx.y`) in the invocations of `GetVersion.cmake` (currently located in `CMakeLists.txt`, `.evergreen/debian_package_build.sh`, `.evergreen/create-packages-and-repos.sh`, and `.evergreen/config.yml`) so that they refer to the new branch you are about to create
- Commit these changes (on `master`) so that both the `master` branch and the new branch you are about to create refer to the new branch (note that this means you will commit changes to this file, `CHANGELOG.md`, and the files which contain invocations of `GetVersion.cmake`)
- Create a branch named `rx.y`.
- Update the [libmongocrypt-release](https://evergreen.mongodb.com/projects##libmongocrypt-release) Evergreen project to set `Branch Name` to `rx.y`.
| "PACKAGER_DISTRO=${packager_distro}" \ | ||
| "PACKAGER_ARCH=${packager_arch}" \ | ||
| ${compile_env|} \ | ||
| bash libmongocrypt/.evergreen/create-packages-and-repos.sh |
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.
Trivial: the indentation looks a little off here.
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.
The indentation is intentionally strange here because it's all a single env <vars> <cmd> command, and the whitespacing is meant to emphasize it.
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.
Well, it definitely jumped out at me, so mission accomplished! 🤣
| "PACKAGER_DISTRO=${packager_distro}" \ | ||
| "PACKAGER_ARCH=${packager_arch}" \ | ||
| ${compile_env|} \ | ||
| bash libmongocrypt/.evergreen/create-packages-and-repos.sh |
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.
Trivial: the indentation looks a little off here.
This changeset ensures most of the build shell scripts execute cleanly in Bash with "strict mode." Several environment variables used by the scripts are given default empty values in
setup-env.sh. The "create packages" script was moved out-of-line fromconfig.ymland placed in a dedicated file to allow for braced shell substitutions without EVG expansions interfering. The behavior of the scripts is otherwise unchanged.