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

Define Bazel version in one shared location #1252

Merged
merged 3 commits into from
Feb 25, 2020
Merged

Define Bazel version in one shared location #1252

merged 3 commits into from
Feb 25, 2020

Conversation

aherrmann
Copy link
Member

@aherrmann aherrmann commented Feb 24, 2020

Instead of each CI pipeline separately defining the Bazel version, we define it in one shared location .ci/bazelversion. Ideally, we would use .bazelversion in the top-level which would be compatible with bazelisk. Unfortunately, this is incompatible with the way Bazel is packaged in nixpkgs right now. See NixOS/nixpkgs#80950.

The pipelines that use nixpkgs to provision Bazel check that the Bazel version matches using .ci/check-bazel-version. This was already the case on Linux, but has been added on macOS. The pipelines that fetch a Bazel binary distribution use .ci/fetch-bazel-bindist which in turn consults the .ci/bazelversion file. This was already the case on buildkite, but has been added to netlify and Azure. To avoid issues with corrupted downloads we check the downloaded bindist against expected sha256 hashes defined in .ci/bazel-*.sha256.

The files .ci/bazelversion and .ci/bazel-*.sha256 need to be kept in sync. The script .ci/update-bazel-version helps with this. Given a Bazel version number it will update both files. The nixpkgs revision needs to be updated separately. The MAINTAINERS.md file has been updated accordingly.

@aherrmann aherrmann force-pushed the bazel-version branch 2 times, most recently from 2793e37 to 8bbc94a Compare February 24, 2020 16:02
# installation, see https://github.com/NixOS/nixpkgs/issues/80950.
VERSION_EXPECTED="bazel $(cat "$DIR/bazelversion")"
VERSION_ACTUAL=$(bazel version --gnu_format)
# nixpkgs Bazel version ends on '- (@non-git)'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we fix this in nixpkgs?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would also fix this issue, so that would be good. Though, I don't think we need to block this PR on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that’s already fixed by the changes in NixOS/nixpkgs#80739 (review)

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC that PR changes bazel-real for bazel-VERSION..., so it fixes NixOS/nixpkgs#80950, but doesn't change the - (@non-git) part. (Unless Bazel 2.1 does that, I haven't checked).

@aherrmann aherrmann added the merge-queue merge on green CI label Feb 25, 2020
Instead of each CI pipeline separately defining the Bazel version, we
define it in one shared location `.ci/bazelversion`. Ideally, we would
use `.bazelversion` in the top-level which would be compatible with
[bazelisk](https://github.com/bazelbuild/bazelisk). Unfortunately, this
is incompatible with the way Bazel is packaged in nixpkgs right now.
See, `.ci/check-bazel-version`.

The pipelines that use nixpkgs to provision Bazel check that the Bazel
version matches using `.ci/check-bazel-version`. The pipelines that
fetch a Bazel binary distribution use `.ci/fetch-bazel-bindist` which in
turn consults the `.ci/bazelversion` file. To avoid issues with
corrupted downloads we check the downloaded bindist against expected
sha256 hashes defined in `.ci/bazel-sha256`.

The files `.ci/bazelversion` and `.ci/bazel-sha256` need to be kept in
sync. The script `.ci/update-bazel-version` helps with this. Given a
Bazel version number it will update both files. The nixpkgs revision
needs to be updated separately. The `MAINTAINERS.md` file has been
updated accordingly.
To work around this we store separate `.sha256` files for each platform.
git was automatically converting `\n` lineendings to `\r\n`, which
confused `sha256sum`. By declaring `.sha256` files as binary
`.gitattributes` we avoid this issue.
@mergify mergify bot merged commit eaa8985 into master Feb 25, 2020
@mergify mergify bot deleted the bazel-version branch February 25, 2020 11:52
@mergify mergify bot removed the merge-queue merge on green CI label Feb 25, 2020
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.

2 participants