-
Notifications
You must be signed in to change notification settings - Fork 2k
fix(env): allow passThroughEnv
to negate built ins and globalPassThroughEnv
#9680
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewer's Guide
// See if we infer a framework | ||
if let Some(framework) = infer_framework(workspace, is_monorepo) { | ||
// See if we infer a framework | ||
let framework = do_framework_inference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously we had if do_framework_inference { if let Some(framework) = infer_framework() { A } else { B } } else { C }
where B
and C
were the same statements. We now merge these two outcomes to a single branch.
@@ -242,6 +242,7 @@ pub struct TaskHasher<'a> { | |||
hashes: HashMap<TaskId<'static>, String>, | |||
run_opts: &'a RunOpts, | |||
env_at_execution_start: &'a EnvironmentVariableMap, | |||
global_env: EnvironmentVariableMap, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
global_env
previously lived inside of the Visitor
structure. It is only used by TaskHasher
so we keep it there instead of passing it in every time we construct a task's environment.
passThroughEnv
to negate built ins and globalPassThroughEnv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ tests
Description
Fixes #9659
Fixing required a fair amount of code shuffling before it can be addressed. This PR is a collection of prework that I did before fixing the issue that will probably be easier to review in isolation. The final commit is the actual fix.
The important moves in the commit is moving some logic out of
TaskHasher
and intoturborepo-env
crate where we can easily test this logic without constructing a fullRun
.Each commit is a small incremental change to this codepath that shouldn't have any behavioral changes until the final commit.
Testing Instructions
Added unit tests for pass through env creation, existing integration tests that cover this behavior as well.
Quick manual test: