diff --git a/packer/core.go b/packer/core.go index b8682d159..5d7337e61 100644 --- a/packer/core.go +++ b/packer/core.go @@ -2,6 +2,7 @@ package packer import ( "fmt" + "log" "regexp" "sort" "strings" @@ -399,14 +400,29 @@ func (c *Core) renderVarsRecursively() (*interpolate.Context, error) { // out the appropriate interpolations. repeatMap := make(map[string]string) + allKeys := make([]string, 0) + // load in template variables for k, v := range c.Template.Variables { repeatMap[k] = v.Default + allKeys = append(allKeys, k) } // overwrite template variables with command-line-read variables for k, v := range c.variables { repeatMap[k] = v + allKeys = append(allKeys, k) + } + + // sort map to force the following loop to be deterministic. + sort.Strings(allKeys) + type keyValue struct { + Key string + Value string + } + sortedMap := make([]keyValue, len(repeatMap)) + for _, k := range allKeys { + sortedMap = append(sortedMap, keyValue{k, repeatMap[k]}) } // Regex to exclude any build function variable or template variable @@ -416,25 +432,27 @@ func (c *Core) renderVarsRecursively() (*interpolate.Context, error) { for i := 0; i < 100; i++ { shouldRetry = false + changed = false + deleteKeys := []string{} // First, loop over the variables in the template - for k, v := range repeatMap { + for _, kv := range sortedMap { // Interpolate the default - renderedV, err := interpolate.RenderRegex(v, ctx, renderFilter) + renderedV, err := interpolate.RenderRegex(kv.Value, ctx, renderFilter) switch err.(type) { case nil: // We only get here if interpolation has succeeded, so something is // different in this loop than in the last one. changed = true - c.variables[k] = renderedV + c.variables[kv.Key] = renderedV ctx.UserVariables = c.variables // Remove fully-interpolated variables from the map, and flag // variables that still need interpolating for a repeat. - done, err := isDoneInterpolating(v) + done, err := isDoneInterpolating(kv.Value) if err != nil { return ctx, err } if done { - delete(repeatMap, k) + deleteKeys = append(deleteKeys, kv.Key) } else { shouldRetry = true } @@ -442,7 +460,7 @@ func (c *Core) renderVarsRecursively() (*interpolate.Context, error) { castError := err.(ttmp.ExecError) if strings.Contains(castError.Error(), interpolate.ErrVariableNotSetString) { shouldRetry = true - failedInterpolation = fmt.Sprintf(`"%s": "%s"; error: %s`, k, v, err) + failedInterpolation = fmt.Sprintf(`"%s": "%s"; error: %s`, kv.Key, kv.Value, err) } else { return ctx, err } @@ -450,12 +468,26 @@ func (c *Core) renderVarsRecursively() (*interpolate.Context, error) { return ctx, fmt.Errorf( // unexpected interpolation error: abort the run "error interpolating default value for '%s': %s", - k, err) + kv.Key, err) } } if !shouldRetry { break } + + // Clear completed vars from sortedMap before next loop. Do this one + // key at a time because the indices are gonna change ever time you + // delete from the map. + for _, k := range deleteKeys { + for ind, kv := range sortedMap { + if kv.Key == k { + log.Printf("Deleting kv.Value: %s", kv.Value) + sortedMap = append(sortedMap[:ind], sortedMap[ind+1:]...) + break + } + } + } + deleteKeys = []string{} } if !changed && shouldRetry { diff --git a/packer/core_test.go b/packer/core_test.go index 8982afbb2..17d76eb0c 100644 --- a/packer/core_test.go +++ b/packer/core_test.go @@ -540,7 +540,12 @@ func TestCore_InterpolateUserVars(t *testing.T) { }) if (err != nil) != tc.Err { - t.Fatalf("err: %s\n\n%s", tc.File, err) + if tc.Err == false { + t.Fatalf("Error interpolating %s: Expected no error, but got: %s", tc.File, err) + } else { + t.Fatalf("Error interpolating %s: Expected an error, but got: %s", tc.File, err) + } + } if !tc.Err { for k, v := range ccf.variables { @@ -727,49 +732,43 @@ func TestEnvAndFileVars(t *testing.T) { os.Setenv("INTERPOLATE_TEST_ENV_2", "5") os.Setenv("INTERPOLATE_TEST_ENV_4", "bananas") - // run the interpolations a bunch of times, because we have had issues - // where the behavior is different based on the order in which the - // vars are interpolated - for i := 0; i < 100; i++ { - f, err := os.Open(fixtureDir("complex-recursed-env-user-var-file.json")) - if err != nil { - t.Fatalf("err: %s", err) - } + f, err := os.Open(fixtureDir("complex-recursed-env-user-var-file.json")) + if err != nil { + t.Fatalf("err: %s", err) + } - tpl, err := template.Parse(f) - f.Close() - if err != nil { - t.Fatalf("err: %s\n\n%s", "complex-recursed-env-user-var-file.json", err) - } + tpl, err := template.Parse(f) + f.Close() + if err != nil { + t.Fatalf("err: %s\n\n%s", "complex-recursed-env-user-var-file.json", err) + } - ccf, err := NewCore(&CoreConfig{ - Template: tpl, - Version: "1.0.0", - Variables: map[string]string{ - "var_1": "partyparrot", - "var_2": "{{user `env_1`}}-{{user `env_2`}}{{user `env_3`}}-{{user `var_1`}}", - "final_var": "{{user `env_1`}}/{{user `env_2`}}/{{user `env_4`}}{{user `env_3`}}-{{user `var_1`}}/vmware/{{user `var_2`}}.vmx", - }, - }) - expected := map[string]string{ + ccf, err := NewCore(&CoreConfig{ + Template: tpl, + Version: "1.0.0", + Variables: map[string]string{ "var_1": "partyparrot", - "var_2": "bulbasaur-5/path/to/nowhere-partyparrot", - "final_var": "bulbasaur/5/bananas/path/to/nowhere-partyparrot/vmware/bulbasaur-5/path/to/nowhere-partyparrot.vmx", - "env_1": "bulbasaur", - "env_2": "5", - "env_3": "/path/to/nowhere", - "env_4": "bananas", - } - if err != nil { - t.Fatalf("err: %s\n\n%s", "complex-recursed-env-user-var-file.json", err) - } - for k, v := range ccf.variables { - if expected[k] != v { - t.Fatalf("Expected value %s for key %s but got %s", - expected[k], k, v) - } + "var_2": "{{user `env_1`}}-{{user `env_2`}}{{user `env_3`}}-{{user `var_1`}}", + "final_var": "{{user `env_1`}}/{{user `env_2`}}/{{user `env_4`}}{{user `env_3`}}-{{user `var_1`}}/vmware/{{user `var_2`}}.vmx", + }, + }) + expected := map[string]string{ + "var_1": "partyparrot", + "var_2": "bulbasaur-5/path/to/nowhere-partyparrot", + "final_var": "bulbasaur/5/bananas/path/to/nowhere-partyparrot/vmware/bulbasaur-5/path/to/nowhere-partyparrot.vmx", + "env_1": "bulbasaur", + "env_2": "5", + "env_3": "/path/to/nowhere", + "env_4": "bananas", + } + if err != nil { + t.Fatalf("err: %s\n\n%s", "complex-recursed-env-user-var-file.json", err) + } + for k, v := range ccf.variables { + if expected[k] != v { + t.Fatalf("Expected value %s for key %s but got %s", + expected[k], k, v) } - } // Clean up env vars