Wildcard env #5017

nathanhammond · 2023-05-18T17:13:03Z

Add the ability to specify environment variables via wildcards anywhere in turbo.json where environment variables are accepted.

vercel · 2023-05-18T17:13:11Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
examples-kitchensink-blog	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	May 24, 2023 0:19am
examples-nonmonorepo	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	May 24, 2023 0:19am
examples-tailwind-web	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	May 24, 2023 0:19am
turbo-site	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 24, 2023 0:19am

7 Ignored Deployments

Name	Status	Preview	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2023 0:19am
examples-cra-web	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2023 0:19am
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2023 0:19am
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2023 0:19am
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2023 0:19am
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2023 0:19am
examples-vite-web	⬜️ Ignored (Inspect)	Visit Preview	May 24, 2023 0:19am

vercel · 2023-05-18T17:13:14Z

Someone is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

nathanhammond · 2023-05-19T16:23:32Z

cli/internal/env/env.go

+	// We need the computed literal env value because FOO= is meaningful.
+	var literalValue string
+	if !hasWildcard {
+		literalValue = strings.Join(literalString, "")
+		return &literalValue, strings.Join(regexString, "")


This is going to go away. This was introduced to prove that the new and old behaviors were identical; though wrong.

The fix to this is to add env and globalEnv into the correct hash positions.

Removed here: nathanhammond#10

tknickman

Looks good

cli/internal/inference/inference.go

tknickman · 2023-05-23T13:31:41Z

cli/internal/taskhash/taskhash.go

+			allEnvVarMap.Union(userEnvVarSet.Inclusions)
+			allEnvVarMap.Union(inferenceEnvVarMap)
+			allEnvVarMap.Difference(userEnvVarSet.Exclusions)
+
+			explicitEnvVarMap.Union(userEnvVarSet.Inclusions)
+			explicitEnvVarMap.Difference(userEnvVarSet.Exclusions)
+
+			matchingEnvVarMap.Union(inferenceEnvVarMap)
+			matchingEnvVarMap.Difference(userEnvVarSet.Exclusions)


Looks like we're doing this (almost identical) in two places, worth adding a method?

I considered it, I really did. The part that is missing from my calculation here was that, when originally authored, they were different because the global had to support *THASH*. Also, I want to figure out a way to drop _defaultEnvVars which would reduce the global one to just FromWildcards.

There was even a point where I considered leaving GetHashableEnvVars implemented via FromWildcards in the codebase, which was that abstraction layer:

bba955f#diff-79ab7e6ee57b9aa25276151ceb78d11d64a8496d30979203412789cfd8970eb5R277

I ultimately decided against that too. I decided that I already had evidence that these two things changed independently and that their current ~identical implementation is just (currently) convergent evolution.

tknickman · 2023-05-23T13:37:37Z

cli/internal/env/env_test.go

-			if err != nil {
-				t.Errorf("error setup failure: %s", err)
+			got, err := tt.self.FromWildcards(tt.wildcardPatterns)
+			if (err != nil) != tt.wantErr {


Looks like we don't have any tests with wantErr: true - could remove to simplify unless we're planning to add some more in the future.

I tried to create an error, but since I'm using QuoteMeta and only adding .* the only point of error (regexp.Compile) basically can't error unless I authored something wrong. Will remove.

gsoltis

Just a couple comment requests

gsoltis · 2023-05-22T21:37:56Z

cli/internal/env/env.go

+func GetEnvMap() EnvironmentVariableMap {
+	envMap := make(map[string]string)
+	for _, envVar := range os.Environ() {
+		if i := strings.Index(envVar, "="); i >= 0 {


What ends up in os.Environ() that doesn't have an =?

I didn't touch the implementation here, but since this ends up making a syscall and has platform specific behavior I really don't mind it being defensive:
https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/syscall/env_unix.go;l=34

gsoltis · 2023-05-22T21:45:52Z

cli/internal/env/env.go

+	for _, envVar := range os.Environ() {
+		if i := strings.Index(envVar, "="); i >= 0 {
+			parts := strings.SplitN(envVar, "=", 2)
+			envMap[parts[0]] = strings.Join(parts[1:], "")


I think this could use a comment. What I think is going on: = could be the last token, so we use slice syntax + Join to set an empty string.

AFAIK we shouldn't ever have more than 2 elements in parts.

This is a "break it you buy it" situation. I simply moved the function definition up in the file to make the file read more-reasonably top-to-bottom.

Looking at it in more detail I'm mostly sure I could replace it with this:

if i := strings.Index(envVar, "="); i >= 0 { key := envVar[0:i] value := envVar[i+1:] envMap[key] = value }

... but I've decided that I don't want to touch it in this PR. We know it works right now and that's unnecessary risk for something that doesn't have tests.

https://github.com/vercel/turbo/blob/main/cli/internal/env/env.go#L97-L107

turbo-orchestrator bot added the team: turborepo label May 18, 2023

nathanhammond force-pushed the wildcard-env branch from 2e9f14c to 5bdbc38 Compare May 19, 2023 16:15

nathanhammond marked this pull request as ready for review May 19, 2023 16:21

nathanhammond requested a review from a team as a code owner May 19, 2023 16:21

nathanhammond requested review from tknickman and chris-olszewski May 19, 2023 16:21

nathanhammond commented May 19, 2023

View reviewed changes

nathanhammond force-pushed the wildcard-env branch 2 times, most recently from ef5f95f to 904fea0 Compare May 22, 2023 07:47

nathanhammond mentioned this pull request May 22, 2023

Better env tracking nathanhammond/turbo#10

Open

tknickman approved these changes May 23, 2023

View reviewed changes

gsoltis approved these changes May 23, 2023

View reviewed changes

Nathan Hammond added 6 commits May 24, 2023 20:17

Wildcard calculation of env vars.

a22acc8

wildcards in most places

fd3e057

Works without changes.

0ae1321

Improve branching behavior.

8d74d78

Make linter happy.

11377ec

remove error testing

2af6030

nathanhammond force-pushed the wildcard-env branch from 2cd2dac to 2af6030 Compare May 24, 2023 12:18

vercel bot deployed to Preview – turbo-site May 24, 2023 12:19 View deployment

nathanhammond added the pr: automerge Kodiak will merge these automatically after checks pass label May 24, 2023

kodiakhq bot merged commit 9bbebb7 into vercel:main May 24, 2023

renovate bot mentioned this pull request Aug 28, 2023

chore(deps): update all non-major dev dependencies (minor) SocialGouv/anonymify#23

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wildcard env #5017

Wildcard env #5017

nathanhammond commented May 18, 2023 •

edited

Loading

vercel bot commented May 18, 2023 •

edited

Loading

vercel bot commented May 18, 2023

nathanhammond May 19, 2023

nathanhammond May 23, 2023

tknickman left a comment

tknickman May 23, 2023

nathanhammond May 24, 2023

tknickman May 23, 2023

nathanhammond May 24, 2023

gsoltis left a comment

gsoltis May 22, 2023

nathanhammond May 24, 2023

gsoltis May 22, 2023

nathanhammond May 24, 2023 •

edited

Loading

nathanhammond May 24, 2023 •

edited

Loading

nathanhammond May 24, 2023

Wildcard env #5017

Wildcard env #5017

Conversation

nathanhammond commented May 18, 2023 • edited Loading

vercel bot commented May 18, 2023 • edited Loading

vercel bot commented May 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tknickman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsoltis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathanhammond May 24, 2023 • edited Loading

Choose a reason for hiding this comment

nathanhammond May 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathanhammond commented May 18, 2023 •

edited

Loading

vercel bot commented May 18, 2023 •

edited

Loading

nathanhammond May 24, 2023 •

edited

Loading

nathanhammond May 24, 2023 •

edited

Loading