-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add support for %bazel.version*% variables in bazelrc imports #27447
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
base: master
Are you sure you want to change the base?
Conversation
|
Does
|
|
Re: Note: Even though I put out a patch for this, I have little stake in this since I don't plan on using this feature. I'm happy to go with whatever the community decides (or what the official bazel maintainers decide). Here are some thoughts: I put a small example in the commit message for just returning the individual number with the I do see your point on how unlikely it is to need just the patch number by itself. To update your naming:
So in total, still only 4 variables, but more verbose names to be clearer what it evaluates to. Also, wanted to clarify in your example, because you have |
|
Sorry, I feel like I'm bikeshedding when talking about |
c0075e8 to
bc48f4c
Compare
|
Updated the code. This now does proper semantic version processing instead of the naive split by dots I was doing earlier. re: variables available - In the spirit of why not both, I just implemented the original proposal and the variant of your new proposal that I mentioned in the previous comment. So for the build label:
I think this should handle all cases that a user would want with enough flexibility, clarity, and convenience baked in. |
Follows the proposal: bazelbuild#24043 (comment) by mattyclarkson@ to add support for the following variables when importing additional bazelrc files. Variables are followed by their values for the hypothetical build label 9.4.2-pre.20251022.1: - %bazel.version% - 9.4.2-pre.20251022.1 - %bazel.version.major% - 9 - %bazel.version.minor% - 4 - %bazel.version.patch% - 2 - %bazel.version.prerelease% - pre.20251022.1 - %bazel.version.buildmetadata% - (empty) - %bazel.version.major.minor% - 9.4 - %bazel.version.major.minor.patch% - 9.4.2 Eg. If your bazel version was 8.4.2-pre.20251022.1 and your .bazelrc had the following: > try-import %bazel.version%.bazelrc > import %bazel.version.major%.%bazel.version.minor%-%bazel.version.patch%.bazelrc > try-import %bazel.version.major%.bazelrc It would be evaluated to: > try-import 8.4.2.bazelrc > import 8.4-2.bazelrc > try-import 8.bazelrc # Implementation details: ## Build label extraction: Before: The build label was extracted out only when calling `bazel --version` and when running in client server mode. After: Now we always extract the the build label out early in the start of main ## Piping of build label: The build label is stored in the OptionProcessor so that it can be passed to `RcFile` when parsing occurs. ## Parsing and mapping of variables: The build label is parsed during `RcFile::ParseFile` into its constituent parts (major, minor, patch). This occurs EACH TIME an import statement is found in a .rc file. This is wasteful, but the logic is simple enough that it shouldn't matter. Interpolation of the build label happens first, followed by `%workspace%`. Fixes: bazelbuild#24043
bc48f4c to
afa5d8d
Compare
|
I would suggest limiting this to just two variables, returning 8 and 8.4 respectively for Bazel version 8.4.2rc2. There's pretty much never a reason to use anything but the latest patch version for a given minor version anyway. Every new variable becomes a public API surface that users will end up (possibly inadvertently) depending on, so there should be at least some potential use case for each variable we add. |
The current use case/feature requested just requires these two. Per fmeum in bazelbuild#27447 (comment) > There's pretty much never a reason to use anything but the latest patch > version for a given minor version anyway. > > Every new variable becomes a public API surface that users will end up > (possibly inadvertently) depending on, so there should be at least some > potential use case for each variable we add.
|
Ok, reduce the possible variables to just two:
|
|
Can you also update the PR description with this change? |
Done |
Instead of evaluating the semantic version from the build_label for each import call, the build_label is parsed once, before being used in rc_file reading.
|
Added a minor change to stop reparsing |
Adapts the proposal:
#24043 (comment) by mattyclarkson@ to add support for bazel version variables when doing
importortry-importof additional.bazelrcfiles. The 2 variables added are:%bazel.version.major%- evaluates to8when build label is8.4.2%bazel.version.major.minor%- evaluates to8.4when build label is8.4.2Eg. If your bazel version was 8.4.2 and your .bazelrc had the following:
It would be evaluated to:
Implementation details:
Build label extraction:
Before: The build label was extracted out only when calling
bazel --versionand when running in client server mode.After: Now we always extract the the build label out early in the start of main
Piping of build label:
The build label is stored in the OptionProcessor so that it can be passed to
RcFilewhen parsing occurs.Parsing and mapping of variables:
The build label is parsed into a new
SemVerstruct beforeRcFile::ParseFile. Only if the build label is a proper semantic version, are the specific major and minor values of a semantic version extracted, otherwise, theSemVerstruct will containno_versionfor eachSemVervalue. Interpolation of the build label variables (%bazel.version.major%and%bazel.version.major.minor%) happens first, followed by the existing%workspace%variable.Fixes: #24043