Add ability to declare a env key in each pipeline task

[mehulkar]

Adds the ability to read environment variable dependencies for each task from an env key. It supports both configs, but logs a deprecation message on the old config, and de-dupes. The de-duping feature also applies duplicate declarations in the existing dependsOn key. (Not sure if this was being de-duped somewhere downstream already)

Coming up after this PR:

• same ability for global env vars in a separate PR
• codemod feature for existing config files
• lint rule for turbo.json having the wrong things?

(See #1929 for some initial refactor work that added a few more test cases if interested!)

[vercel]

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

Name	Status	Preview	Updated
turbo-site	✅ Ready (Inspect)	Visit Preview	Sep 15, 2022 at 6:17AM (UTC)

[mehulkar]

Reverting docs out of here, so we can wait until a release to update the docs. See #1936 for the changes.

[nathanhammond]

There is also docs/schema.d.ts which needs to be updated. Was that part reverted out?

[mehulkar]

There is also docs/schema.d.ts which needs to be updated. Was that part reverted out?

I moved to #1936 since we don't want to update it until a release! (my understanding is that schema is used only for the $schema field for hints

[gsoltis]

I wouldn't include this warning. In general, people ignore warnings unless something breaks. If something breaks, they'll investigate. When something does break, we don't know that it will be related to this, or any other warning that we might include. In the worst case, we send them on a false trail because whatever broke is unrelated. The best help we can give them is to show what commands were invoked with what inputs. This is where something like a run summary comes into play.

For this warning in particular, users of this API will fall into two camps:

1. Users coming from the previous behavior of dependsOn.
2. Users new to turbo who have never seen the previous syntax

We expect going forward that group 2 will be the much larger group. They will have no reason to think they need a $ prefix. In the event of an unlikely subset who do intend to set an environment variable that starts with a $, they will then have a warning they can't get rid of.

Group 1 will have a codemod to move them to the proper syntax.

[gsoltis]

Also, if we do end up keeping this warning, we need to thread our cli.Ui instance in here, so that it can respect whatever output is configured. We shouldn't be using log directly.

[mehulkar]

ok, that's fair too. I've removed the warning

[nathanhammond]

@gsoltis the issue here is that somebody will add "$FOO" to the env, get no feedback, and just assume that it works.

Later, when their cache isn't segmented, they'll have no idea why and (at this time) have no way to inspect the build to understand what happened.

In other words, failing to use this API correctly will ruin your day. We should aim to reduce the incidence of these types of failure modes which is why I propose automatically removing the $.

[mehulkar]

Reached consensus in slack to throw a hard error for $ prefixed strings

[mehulkar]

I tested this locally with my simplest-turbo repo and I'm able to see the error as expected (even with two tasks that may be running in parallel?)

➜  simplest-turbo git:(main) ✗ ./scripts/run
> Keeping cache
> apps/a/package.json description changed from "mydescription-ts1663216545259" to "mydescription-ts1663216569031"
> Building CLI from /Users/mehulkar/dev/vercel-projects/turborepo/cli
> Running turbo run build from /Users/mehulkar/dev/vercel-projects/turborepo/cli/turbo
==========================
 ERROR  run failed: turbo.json: You specified "$A" in the "env" key. This is treated as a literal, you may want to remove the "$"

[nathanhammond]

Some proposed verbiage changes.

[nathanhammond]

This is fine for this case but we should (all) start to get into the habit of fmt.Errorf(%w, err) so that we can use error.Is.