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

Test all supported bazel versions in integration tests #1772

Merged
merged 6 commits into from
Jul 12, 2022

Conversation

k1nkreet
Copy link
Contributor

@k1nkreet k1nkreet commented Jul 8, 2022

This PR is an invitation to discuss which Bazel versions we are considering as supported, since we can explicitly specify all Bazel versions which would be tested on CI.
As a start I've added all major versions from the interval specified in haskell/private/versions.bzl

The second question is which nixpkgs Bazel versions we are going to test. Current implementation offers to specify a list of Nixpkgs packages to test, but it will use nixpkgs version pinned in rules_haskell/nixpkgs/default.nix (there is no bazel_5 package for ex.) which is a bit unfair to rules_haskell's users. As a suggestion maybe we should use pair (nixpkgs revision, bazel package) to identify bazel packages to test. Or just keeping rules_haskell/nixpkgs/default.nix up-to-date will be good enough?

@k1nkreet k1nkreet requested a review from aherrmann July 8, 2022 13:43
@aherrmann
Copy link
Member

As a start I've added all major versions from the interval specified in haskell/private/versions.bzl

IIUC, this corresponds to all minor versions in the range.
According to Bazel's versioning policy

  • A major release contains features that are not backward compatible with the previous release.
  • A minor release contains new backward-compatible features.

Perhaps a lower and upper bound for each major version would be enough? E.g. if we start relying on a new minor release feature, we may no longer be compatible with a previous minor of the same major release family.

@k1nkreet
Copy link
Contributor Author

k1nkreet commented Jul 8, 2022

Perhaps a lower and upper bound for each major version would be enough? E.g. if we start relying on a new minor release feature, we may no longer be compatible with a previous minor of the same major release family.

Agreed

Ilya Polyakovskiy added 2 commits July 8, 2022 16:26
use this values as a min and max bazel versions in
haskell/private/versions.bzl
@k1nkreet k1nkreet force-pushed the ip/test-supported-bazel-versions branch from f29ebe4 to 47307e5 Compare July 8, 2022 14:27
@aherrmann
Copy link
Member

Looks like CI is unhappy about something.

@k1nkreet k1nkreet force-pushed the ip/test-supported-bazel-versions branch from 535a787 to 433e3b6 Compare July 11, 2022 13:19
@k1nkreet
Copy link
Contributor Author

Should we test Bazel 5.x also? @aherrmann what do you think?

@aherrmann
Copy link
Member

Should we test Bazel 5.x also? @aherrmann what do you think?

Yes, though, feel free to do that in a separate PR if it needs fixing some compatibility issues.

Comment on lines +13 to +16
SUPPORTED_BAZEL_VERSIONS = [
"4.0.0",
"4.2.2",
]
Copy link
Member

Choose a reason for hiding this comment

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

IIUC these version numbers are duplicated between bazel_versions.bzl and haskell/private/versions.bzl. Is there a good reason for that? Otherwise, can this be avoided?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, the point was to deduplicate that, I just forgot to remove bazel_versions.bzl :)

Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

@aherrmann aherrmann added the merge-queue merge on green CI label Jul 12, 2022
@mergify mergify bot merged commit 4d9d65f into master Jul 12, 2022
@mergify mergify bot deleted the ip/test-supported-bazel-versions branch July 12, 2022 13:16
@mergify mergify bot removed the merge-queue merge on green CI label Jul 12, 2022
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.

None yet

2 participants