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.

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

Make sure that we only hash the env pairs. #4708

Merged
merged 1 commit into from Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 54 additions & 24 deletions cli/internal/run/global_hash.go
Expand Up @@ -24,8 +24,8 @@ var _defaultEnvVars = []string{
"VERCEL_ANALYTICS_ID",
}

// GlobalHashable represents all the things that we use to create the global hash
type GlobalHashable struct {
// GlobalHashableInputs represents all the things that we use to create the global hash
type GlobalHashableInputs struct {
globalFileHashMap map[turbopath.AnchoredUnixPath]string
rootExternalDepsHash string
envVars env.DetailedMap
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This envVars right here is actually the root cause of the problem.

Expand All @@ -35,9 +35,31 @@ type GlobalHashable struct {
envMode util.EnvMode
}

// This exists because the global hash used to have different fields. Changing
// to a new struct layout changes the global hash. We can remove this converter
// when we are going to have to update the global hash for something else.
type newGlobalHashable struct {
globalFileHashMap map[turbopath.AnchoredUnixPath]string
rootExternalDepsHash string
envVars env.EnvironmentVariablePairs
globalCacheKey string
pipeline fs.PristinePipeline
envVarPassthroughs []string
envMode util.EnvMode
}

// newGlobalHash is a transformation of GlobalHashableInputs.
// It's used for the situations where we have an `EnvMode` specified
// as that is not compatible with existing global hashes.
func newGlobalHash(full GlobalHashableInputs) (string, error) {
return fs.HashObject(newGlobalHashable{
globalFileHashMap: full.globalFileHashMap,
rootExternalDepsHash: full.rootExternalDepsHash,
envVars: full.envVars.All.ToHashable(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this line is the fix. Everything else is just plumbing.

globalCacheKey: full.globalCacheKey,
pipeline: full.pipeline,
envVarPassthroughs: full.envVarPassthroughs,
envMode: full.envMode,
})
}

type oldGlobalHashable struct {
globalFileHashMap map[turbopath.AnchoredUnixPath]string
rootExternalDepsHash string
Expand All @@ -46,44 +68,52 @@ type oldGlobalHashable struct {
pipeline fs.PristinePipeline
}

// calculateGlobalHashFromHashable returns a hash string from the globalHashable
func calculateGlobalHashFromHashable(full GlobalHashable) (string, error) {
// oldGlobalHash is a transformation of GlobalHashableInputs.
// This exists because the existing global hashes are still usable
// in some configurations that do not include a specified `EnvMode`.
// We can remove this whenever we want to migrate users.
nathanhammond marked this conversation as resolved.
Show resolved Hide resolved
func oldGlobalHash(full GlobalHashableInputs) (string, error) {
return fs.HashObject(oldGlobalHashable{
globalFileHashMap: full.globalFileHashMap,
rootExternalDepsHash: full.rootExternalDepsHash,
envVars: full.envVars.All.ToHashable(),
globalCacheKey: full.globalCacheKey,
pipeline: full.pipeline,
})
}

// calculateGlobalHashFromHashableInputs returns a hash string from the GlobalHashableInputs
func calculateGlobalHashFromHashableInputs(full GlobalHashableInputs) (string, error) {
switch full.envMode {
case util.Infer:
if full.envVarPassthroughs != nil {
// In infer mode, if there is any passThru config (even if it is an empty array)
// we'll hash the whole object, so we can detect changes to that config
// Further, resolve the envMode to the concrete value.
full.envMode = util.Strict
return fs.HashObject(full)
return newGlobalHash(full)
}

// If we're in infer mode, and there is no global pass through config,
// we use the old struct layout. this will be true for everyone not using the strict env
// feature, and we don't want to break their cache.
return fs.HashObject(oldGlobalHashable{
globalFileHashMap: full.globalFileHashMap,
rootExternalDepsHash: full.rootExternalDepsHash,
envVars: full.envVars.All.ToHashable(),
globalCacheKey: full.globalCacheKey,
pipeline: full.pipeline,
})
return oldGlobalHash(full)
case util.Loose:
// Remove the passthroughs from hash consideration if we're explicitly loose.
full.envVarPassthroughs = nil
return fs.HashObject(full)
return newGlobalHash(full)
case util.Strict:
// Collapse `nil` and `[]` in strict mode.
if full.envVarPassthroughs == nil {
full.envVarPassthroughs = make([]string, 0)
}
return fs.HashObject(full)
return newGlobalHash(full)
default:
panic("unimplemented environment mode")
}
}

func calculateGlobalHash(
func getGlobalHashInputs(
rootpath turbopath.AbsoluteSystemPath,
rootPackageJSON *fs.PackageJSON,
pipeline fs.Pipeline,
Expand All @@ -96,14 +126,14 @@ func calculateGlobalHash(
logger hclog.Logger,
ui cli.Ui,
isStructuredOutput bool,
) (GlobalHashable, error) {
) (GlobalHashableInputs, error) {
// Calculate env var dependencies
envVars := []string{}
envVars = append(envVars, envVarDependencies...)
envVars = append(envVars, _defaultEnvVars...)
globalHashableEnvVars, err := env.GetHashableEnvVars(envVars, []string{".*THASH.*"}, "")
if err != nil {
return GlobalHashable{}, err
return GlobalHashableInputs{}, err
}

// The only way we can add env vars into the hash via matching is via THASH,
Expand All @@ -121,12 +151,12 @@ func calculateGlobalHash(
if len(globalFileDependencies) > 0 {
ignores, err := packageManager.GetWorkspaceIgnores(rootpath)
if err != nil {
return GlobalHashable{}, err
return GlobalHashableInputs{}, err
}

f, err := globby.GlobFiles(rootpath.ToStringDuringMigration(), globalFileDependencies, ignores)
if err != nil {
return GlobalHashable{}, err
return GlobalHashableInputs{}, err
}

for _, val := range f {
Expand All @@ -149,10 +179,10 @@ func calculateGlobalHash(

globalFileHashMap, err := hashing.GetHashableDeps(rootpath, globalDepsPaths)
if err != nil {
return GlobalHashable{}, fmt.Errorf("error hashing files: %w", err)
return GlobalHashableInputs{}, fmt.Errorf("error hashing files: %w", err)
}

return GlobalHashable{
return GlobalHashableInputs{
globalFileHashMap: globalFileHashMap,
rootExternalDepsHash: rootPackageJSON.ExternalDepsHash,
envVars: globalHashableEnvVars,
Expand Down
18 changes: 9 additions & 9 deletions cli/internal/run/run.go
Expand Up @@ -245,7 +245,7 @@ func (r *run) run(ctx gocontext.Context, targets []string) error {
}
}

globalHashable, err := calculateGlobalHash(
globalHashInputs, err := getGlobalHashInputs(
r.base.RepoRoot,
rootPackageJSON,
pipeline,
Expand All @@ -264,7 +264,7 @@ func (r *run) run(ctx gocontext.Context, targets []string) error {
return fmt.Errorf("failed to collect global hash inputs: %v", err)
}

if globalHash, err := calculateGlobalHashFromHashable(globalHashable); err == nil {
if globalHash, err := calculateGlobalHashFromHashableInputs(globalHashInputs); err == nil {
r.base.Logger.Debug("global hash", "value", globalHash)
g.GlobalHash = globalHash
} else {
Expand Down Expand Up @@ -352,8 +352,8 @@ func (r *run) run(ctx gocontext.Context, targets []string) error {
}

var envVarPassthroughMap env.EnvironmentVariableMap
if globalHashable.envVarPassthroughs != nil {
if envVarPassthroughDetailedMap, err := env.GetHashableEnvVars(globalHashable.envVarPassthroughs, nil, ""); err == nil {
if globalHashInputs.envVarPassthroughs != nil {
if envVarPassthroughDetailedMap, err := env.GetHashableEnvVars(globalHashInputs.envVarPassthroughs, nil, ""); err == nil {
envVarPassthroughMap = envVarPassthroughDetailedMap.BySource.Explicit
}
}
Expand All @@ -376,12 +376,12 @@ func (r *run) run(ctx gocontext.Context, targets []string) error {
packagesInScope,
globalEnvMode,
runsummary.NewGlobalHashSummary(
globalHashable.globalFileHashMap,
globalHashable.rootExternalDepsHash,
globalHashable.envVars,
globalHashInputs.globalFileHashMap,
globalHashInputs.rootExternalDepsHash,
globalHashInputs.envVars,
envVarPassthroughMap,
globalHashable.globalCacheKey,
globalHashable.pipeline,
globalHashInputs.globalCacheKey,
globalHashInputs.pipeline,
),
rs.Opts.SynthesizeCommand(rs.Targets),
)
Expand Down