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.

Sign up for GitHub

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

Jump to bottom

Merge EnvironmentName and ResolvedEnvironmentAlias #16770

Closed
stuhood opened this issue Sep 3, 2022 · 2 comments · Fixed by #16786
Closed

Merge EnvironmentName and ResolvedEnvironmentAlias #16770

stuhood opened this issue Sep 3, 2022 · 2 comments · Fixed by #16786
Assignees
@Eric-Arellano

Comments

@stuhood
Copy link
Member

stuhood commented Sep 3, 2022

Post #16721, the EnvironmentName is a parameter throughout the graph. ResolvedEnvironmentAlias should now take the name/location of EnvironmentName.

To accomplish that, we will need to pre-compute an EnvironmentName as a bootstrap step, to inject when running @goal_rules, etc.
@stuhood
Copy link
Member Author

stuhood commented Sep 3, 2022 •
edited
Loading

@Eric-Arellano : Do you feel comfortable doing this one early next week, or should I do it while you work with @chrisjrn on the public API?

@Eric-Arellano
Copy link
Contributor

I think I can do it! Now that I have the command runner wired up, my tasks are:

  1. this issue
  2. figure out how docker_image should be set on Process in Python -- confirm it should be set for every single Process.
  3. add environment: str field to targets

Then modeling should be done, other than Chris's rework of using subsystems for defaults. The rest is getting things to actually work properly, especially #16754.

I'm only working for 2 days next week, so we'll see how far I get.

@Eric-Arellano Eric-Arellano self-assigned this Sep 3, 2022
@Eric-Arellano Eric-Arellano mentioned this issue Sep 6, 2022
Merged
Eric-Arellano added a commit that referenced this issue Sep 7, 2022
@Eric-Arellano
Pass the __local__ environment target to all goals (#16786)
c702901 
Closes #16770.

Now, we dynamically determine what environment target to use at the root of the graph, and inject the value. This will always be the `__local__` target, or `None` if no targets are defined. That is, it cannot be a `docker_environment` etc. Then, callers can change the value to be a different environment by using `Get()` to inject a different `EnvironmentName` `Param`.

To avoid a rule graph cycle, this adds `WrappedTargetForBootstrap`.

[ci skip-rust]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Eric-Arellano Eric-Arellano

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pass the __local__ environment target to all goals Eric-Arellano/pants
2 participants
@stuhood @Eric-Arellano