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

Expand "Make" variables inside of values in the env dictionary on test rules #3088

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aaronsky
Copy link

@aaronsky aaronsky commented Sep 10, 2024

Fixes #3056

This PR implements a gap introduced when bazelbuild/rules_apple#2476 was introduced, where expandable env settings on test rules would not be expanded into the xcscheme during generation. There are a couple of things that reviewers should be aware of before proceeding with this change:

  • This change is not considerate of the rules_apple being used, meaning that it assumes the developer is using at least version 3.6.0 (when the breaking changes in Fix label encoding in incremental generators #2476 were introduced). This is not good and should be considered a blocker, but I don't know how to address this with the tools offered by the ruleset or Bazel.
  • This change as-implemented requires regeneration of the xcschemes if the configured value of the "Make" variable ever changes.

@brentleyjones
Copy link
Contributor

This change is not considerate of the rules_apple being used, meaning that it assumes the developer is using at least version 3.6.0 (when the breaking changes in Fix label encoding in incremental generators #2476 were introduced). This is not good and should be considered a blocker, but I don't know how to address this with the tools offered by the ruleset or Bazel.

I'm fine with bumping our minimum required version of rules_apple to 3.6.0. I agree there is no good way to detect this otherwise. Though, we could add a flag to go back to the old behavior? I would rather just bump our dep though.

This change as-implemented requires regeneration of the xcschemes if the configured value of the "Make" variable ever changes.

I don't think there is a better way?

@aaronsky
Copy link
Author

aaronsky commented Oct 28, 2024

I've increasingly come to regret the way I introduced support for this feature in rules_apple. I think the most flexible behavior from a UX standpoint is a way to map Bazel settings to project-level xcconfigs, so they're available to the scheme without having to regenerate them, but then you'd need to regenerate the xcconfig instead.

I can put up a new commit that bumps the minimum rules_apple to 3.6.0. Thank you for taking a look at this draft.

@congt
Copy link

congt commented Nov 14, 2024

Any progress on this? We encounter exactly the same problem.

@aaronsky aaronsky force-pushed the aaronsky/test_rule_expand_make_vars branch from e934fba to 26b2a71 Compare November 15, 2024 02:09
…t rules

Signed-off-by: Aaron Sky <aaronsky@skyaaron.com>
@aaronsky aaronsky force-pushed the aaronsky/test_rule_expand_make_vars branch from 26b2a71 to 62364ab Compare November 15, 2024 02:10
@aaronsky aaronsky marked this pull request as ready for review November 15, 2024 02:11
@aaronsky aaronsky requested review from a team as code owners November 15, 2024 02:11
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.

Bug: "Make" variable expansion in the env of test bundles does not work.
3 participants