-
Notifications
You must be signed in to change notification settings - Fork 180
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
Adding expansion.bzl
for Fully Expanding Environment Variables
#486
base: main
Are you sure you want to change the base?
Commits on Jan 23, 2024
-
Adding
expansion.bzl
for Fully Expanding Environment VariablesThe built-in functionality (exposed Skylark methods on `ctx`) for expanding environment variables leaves a lot of implementation to do in `bzl` files. This PR adds in that missing functionality for easy reuse with a new `expansion` struct. `expansion` has various methods for expanding environment variables with a flexible API. The existing APIs handle only one piece at a time: * `ctx.expand_location()` only handles `$(location ...)` (and similar) expansion. It does not handle any "make variable" expansion (no expansion from `TemplateVariableInfo` or other providers via toolchains). * `ctx.expand_make_variables()` only handles make variables. If it encounters `$(location ...)` (or similar), it [errors out with no means of recovery](https://github.com/bazelbuild/bazel/blob/addf41bce1f26e8bc4bd0b09cb2fbffbb0446f25/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationMakeVariableContext.java#L136). This method is also marked as deprecated, with `ctx.var` listed as the preferred API. * `ctx.var` is a simple dictionary, which contains resolved mappings based on toolchains. However, being a simple data structure (not a function) leaves recursive functionality and/or integration with `ctx.expand_location()` to be implemented by hand (or in this PR). Many internal systems make use of functionality that fully resolves both make variables and `$(location ...)` (and does so recursively). This is done with `ruleContext.getExpander().withDataLocations()` ([example](https://github.com/bazelbuild/bazel/blob/addf41bce1f26e8bc4bd0b09cb2fbffbb0446f25/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java#L522)). However, this is never exposed to skylark. This means that built-in rules will have the `env` attribute fully expanded, but custom rules cannot (easily). The above mentioned methods have their own (somewhat duplicated) implementations ([`ctx.expand_location()`](https://github.com/bazelbuild/bazel/blob/addf41bce1f26e8bc4bd0b09cb2fbffbb0446f25/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java#L1011), [`ctx.expand_make_variables()`](https://github.com/bazelbuild/bazel/blob/addf41bce1f26e8bc4bd0b09cb2fbffbb0446f25/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java#L964), [`ctx.var`](https://github.com/bazelbuild/bazel/blob/addf41bce1f26e8bc4bd0b09cb2fbffbb0446f25/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java#L823)). Note the identical `ConfigurationMakeVariableContext` initialization [here](https://github.com/bazelbuild/bazel/blob/addf41bce1f26e8bc4bd0b09cb2fbffbb0446f25/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java#L949) and [here](https://github.com/bazelbuild/bazel/blob/36fa60b1805faa7da2c4b5330b4b186740f5f00d/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java#L978) -- identical except for the use of `additionalSubstitutionsMap`, which could have been delegated (similar to the impl of `LocationTemplateContext`). Also note the separate/duplicated parsing implementations in [`LocationTemplateContext`](https://github.com/bazelbuild/bazel/blob/addf41bce1f26e8bc4bd0b09cb2fbffbb0446f25/src/main/java/com/google/devtools/build/lib/analysis/stringtemplate/TemplateExpander.java#L77) and in [`LocationExpander`](https://github.com/bazelbuild/bazel/blob/36fa60b1805faa7da2c4b5330b4b186740f5f00d/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java#L179). This PR tries to avoid more duplicate implementations (which can't really happen anyway; as this is in external Skylark, not Java / Bazel runtime). These new methods make use of `ctx.expand_location()` and `ctx.var` to allow fully recursive variable expansion that incorporates both inputs (toolchains and `location`). The methods avoid copies/string mutations/extra loops when necessary to ensure that the functions can run quickly. The methods' API allows for manual/direct data structures (lists/dicts) as input, or for pulling values directly from `ctx`. `tests/expansion_tests.bzl` added to test all added methods. This PR is being submitted here so that it can be reused (instead of copied in many repos). It is also preferable to add functionality here in Skylib, instead of (or in addition to) any changes in the [Bazel repo](https://github.com/bazelbuild/bazel) so that it can be more easily pulled into projects with a pinned Bazel version. Please feel free to leave comments or suggestion on ways to improve this PR, or let me know if you have any questions. Thanks!
Configuration menu - View commit details
-
Copy full SHA for d6ce59b - Browse repository at this point
Copy the full SHA d6ce59bView commit details -
* Adding
bzl_library
target forexpansion.bzl
inlib/BUILD
.* Making `buildifier` happy.
Configuration menu - View commit details
-
Copy full SHA for d724543 - Browse repository at this point
Copy the full SHA d724543View commit details -
* Replacing platform dependent strings (from
$(location ...)
and si……milar) for consistent assertions.
Configuration menu - View commit details
-
Copy full SHA for 587f254 - Browse repository at this point
Copy the full SHA 587f254View commit details -
Configuration menu - View commit details
-
Copy full SHA for 416305e - Browse repository at this point
Copy the full SHA 416305eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 789a32f - Browse repository at this point
Copy the full SHA 789a32fView commit details
Commits on Jan 24, 2024
-
* Reducing some duplication (merging
_expand_tc_all_keys_in_str()
a……nd `_expand_tc_and_loc_all_keys_in_str()` into `_expand_all_keys_in_str()` with None-able arg). * Updating handling / init of optional `additional_lookup_dict` arg. * Cleaning up some doc comments. * Updating tests. * Adding demonstration where `additional_lookup_dict` overrides value from toolchains. * Adding demonstration where recursive expansion involves env dict and toolchain dict back and forth. * Adding a few more section separator comments.
Configuration menu - View commit details
-
Copy full SHA for c38d3e8 - Browse repository at this point
Copy the full SHA c38d3e8View commit details
Commits on Jan 28, 2024
-
* Adding `_validate_all_keys_expanded()` to validate there are no expandable keys in a given string. * Will either call `fail()` or return list of failures (depending on `fail_instead_of_return` parameter). * Failure messages will contain the parsed unexpanded variable and the whole string which contains it. * Adding optional `validate_expansion` parameter to allow automatic validation after the expansion process. * Adding exposed `validate_expansions` to allow the client to call the functionality directly. * Adding new tests for validation logic. * Parameterized over many different configurations. * All tests pass in <= 0.1s each. * Pulling out `_CONSIDERED_KEY_FORMATS` into file-scoped constant. * Pulling out "odd count dollar sign" logic into `_odd_count_dollar_sign_repeat()`. * Updating some variable names and adding more comments.
Configuration menu - View commit details
-
Copy full SHA for a823ff7 - Browse repository at this point
Copy the full SHA a823ff7View commit details -
Configuration menu - View commit details
-
Copy full SHA for 2ad568f - Browse repository at this point
Copy the full SHA 2ad568fView commit details
Commits on Feb 18, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 9e4779a - Browse repository at this point
Copy the full SHA 9e4779aView commit details
Commits on May 11, 2024
-
Configuration menu - View commit details
-
Copy full SHA for a23dbc4 - Browse repository at this point
Copy the full SHA a23dbc4View commit details -
Configuration menu - View commit details
-
Copy full SHA for f0cbec4 - Browse repository at this point
Copy the full SHA f0cbec4View commit details -
* Updating expected rlocation value to work with different versions o…
…f bazel used in testing.
Configuration menu - View commit details
-
Copy full SHA for 4fd0388 - Browse repository at this point
Copy the full SHA 4fd0388View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7e24e95 - Browse repository at this point
Copy the full SHA 7e24e95View commit details -
* Downgrading usage of
dict | dict
intests/expansion_tests.bzl
t……o use `dict(dict).update(dict)` to be backwards compatible with Bazel 5.x.
Configuration menu - View commit details
-
Copy full SHA for a2ff961 - Browse repository at this point
Copy the full SHA a2ff961View commit details
Commits on May 12, 2024
-
Configuration menu - View commit details
-
Copy full SHA for aa133b9 - Browse repository at this point
Copy the full SHA aa133b9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 826b30c - Browse repository at this point
Copy the full SHA 826b30cView commit details -
* Downgrading usage of
dict | dict
to usedict(dict, **dict)
to b……e backwards compatible with Bazel 5.x. * `dict(dict).update(dict)` is not a good replacement as it returns `NoneType`.
Configuration menu - View commit details
-
Copy full SHA for e5d9112 - Browse repository at this point
Copy the full SHA e5d9112View commit details
Commits on Jun 30, 2024
-
* Renaming all `expand_with_*` functions to `expand_dict_strings_with_*`. * Explicit that it acts on a dict of strings. * Adding `expand_list_strings_with_manual_dict` and `expand_list_strings_with_manual_dict_and_location` functions. * Similar to the existing `expand_dict_strings_with_manual_dict*` functions, these expand values but from a list of strings instead of dict of strings. * Note that these methods only perform recursion via the "manual dict" (the input datatype, `list`, is not an associative type). * Simpler API for just expanding a few string values (of any attr type, or not necessarily associated with any attr). * Renaming `expand_location` parameter(s) to `wrapped_expand_location`. * This makes it more clear that this is not `ctx.expand_location` directly, but rather a wrapped version of that function. * Adding `wrap_expand_location` method. * Offers a helper function for creating `wrapped_expand_location` (used by many expansion functions) from a given `ctx` and `deps`. * Updating doc string comments. * Updating `tests/bzl/expansion_tests.bzl`. * Renaming all `expand_with_*` tests to `expand_dict_strings_with_*`. * Adding `expand_list_strings_with_*` tests (for new functions). * Adding "no recursion" "expected resolved" dicts for new tests. * Adding `TOOLCHAIN_INDIRECT_ENV_VAR*` values to expand, which show recursion specifically among only toolchain values.
Configuration menu - View commit details
-
Copy full SHA for 5046fb0 - Browse repository at this point
Copy the full SHA 5046fb0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4d6d8e7 - Browse repository at this point
Copy the full SHA 4d6d8e7View commit details