From dba32b39136f8da55f758600dd2eebbcec46495f Mon Sep 17 00:00:00 2001 From: Nathan Hammond Date: Tue, 11 Apr 2023 11:34:38 +0000 Subject: [PATCH] Deprecate `THASH`. (#4526) This PR deprecates the use of `THASH` for cache partitioning. - The search string is `.*THASH.*` and can match things like `GITHASH`. Inferring env var inclusion based upon arbitrary substring inclusion regardless of position is too broad. (Should have been `^THASH_.+`.) - Squatting on an env var string not scoped to `^TURBO_.+` is (at least) not polite to the rest of the ecosystem. - Using this feature makes it more-difficult to share caches across machines and create reproducible builds. - Residual `THASH` variables from earlier arbitrary configuration can unnecessarily partition the cache. - The interaction between this feature and "strict" environment variable mode introduces complexity where a user can't easily reason about the behavior. Should the `THASH` variable be included into the global hash if unspecified? Should the `THASH` variable pushed into the execution environment? The original goal of this feature is to be able to quickly and dynamically change the hash key for a run. I find that the implicit nature is anathema to our goals. Previous use cases and recommendations: - Compare cache artifacts across multiple runs: use run summary or `--cache-dir`. - Make sure everything runs: use `--force`. - `SOMETHING_THASH=foo turbo run build` is just a hardcoded env var. Add it to `turbo.json` under `globalEnv` instead. --------- Co-authored-by: Mehul Kar --- cli/integration_tests/global_env.t | 18 +++++++++++++++++- cli/internal/run/global_hash.go | 12 ++++++++++++ cli/internal/run/run.go | 4 ++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/cli/integration_tests/global_env.t b/cli/integration_tests/global_env.t index cea0697386083..c548eb2656635 100644 --- a/cli/integration_tests/global_env.t +++ b/cli/integration_tests/global_env.t @@ -39,6 +39,7 @@ Setup # set env var with "THASH" and ensure cache miss $ SOMETHING_THASH_YES=hi ${TURBO} run build --filter=util --output-logs=hash-only + [DEPRECATED] Using .*THASH.* to specify an environment variable for inclusion into the hash is deprecated. You specified: SOMETHING_THASH_YES. \xe2\x80\xa2 Packages in scope: util (esc) \xe2\x80\xa2 Running build in 1 packages (esc) \xe2\x80\xa2 Remote caching disabled (esc) @@ -58,4 +59,19 @@ Setup Tasks: 1 successful, 1 total Cached: 0 cached, 1 total Time:\s*[\.0-9]+m?s (re) - \ No newline at end of file + +# THASH deprecation doesn't break --dry=json + $ SOMETHING_THASH_YES=hi ${TURBO} run build --filter=util --dry=json | jq -r '.tasks[0].environmentVariables.global[0]' + SOMETHING_THASH_YES=8f434346648f6b96df89dda901c5176b10a6d83961dd3c1ac88b59b2dc327aa4 + +# THASH deprecation doesn't break --graph + $ SOMETHING_THASH_YES=hi ${TURBO} run build --filter=util --graph + + digraph { + \tcompound = "true" (esc) + \tnewrank = "true" (esc) + \tsubgraph "root" { (esc) + \t\t"[root] util#build" -> "[root] ___ROOT___" (esc) + \t} (esc) + } + diff --git a/cli/internal/run/global_hash.go b/cli/internal/run/global_hash.go index d2092e57ee0ee..0775a91d0888d 100644 --- a/cli/internal/run/global_hash.go +++ b/cli/internal/run/global_hash.go @@ -3,8 +3,10 @@ package run import ( "fmt" "path/filepath" + "strings" "github.com/hashicorp/go-hclog" + "github.com/mitchellh/cli" "github.com/vercel/turbo/cli/internal/env" "github.com/vercel/turbo/cli/internal/fs" "github.com/vercel/turbo/cli/internal/globby" @@ -93,6 +95,8 @@ func calculateGlobalHash( envVarPassthroughs []string, envMode util.EnvMode, logger hclog.Logger, + ui cli.Ui, + isStructuredOutput bool, ) (GlobalHashable, error) { // Calculate env var dependencies envVars := []string{} @@ -103,6 +107,14 @@ func calculateGlobalHash( return GlobalHashable{}, err } + // The only way we can add env vars into the hash via matching is via THASH, + // so we only do a simple check here for entries in `BySource.Matching`. + // If we enable globalEnv to accept wildcard characters, we'll need to update this + // check. + if !isStructuredOutput && len(globalHashableEnvVars.BySource.Matching) > 0 { + ui.Warn(fmt.Sprintf("[DEPRECATED] Using .*THASH.* to specify an environment variable for inclusion into the hash is deprecated. You specified: %s.", strings.Join(globalHashableEnvVars.BySource.Matching.Names(), ", "))) + } + logger.Debug("global hash env vars", "vars", globalHashableEnvVars.All.Names()) // Calculate global file dependencies diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index bcae022e8dc3f..ee002829ed00b 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -159,6 +159,8 @@ func (r *run) run(ctx gocontext.Context, targets []string) error { return fmt.Errorf("failed to read package.json: %w", err) } + isStructuredOutput := r.opts.runOpts.GraphDot || r.opts.runOpts.DryRunJSON + var pkgDepGraph *context.Context if r.opts.runOpts.SinglePackage { pkgDepGraph, err = context.SinglePackageGraph(r.base.RepoRoot, rootPackageJSON) @@ -247,6 +249,8 @@ func (r *run) run(ctx gocontext.Context, targets []string) error { turboJSON.GlobalPassthroughEnv, r.opts.runOpts.EnvMode, r.base.Logger, + r.base.UI, + isStructuredOutput, ) if err != nil {