diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl index 71f20502cc..6499caecfc 100644 --- a/go/private/rules/transition.bzl +++ b/go/private/rules/transition.bzl @@ -420,12 +420,22 @@ def _non_go_transition_impl(settings, attr): for key, original_key in _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.items(): original_value = settings[original_key] if original_value: - # Reset to the original value and clear it. + # Reset to the original value of the setting before go_transition. new_settings[key] = json.decode(original_value) - new_settings[original_key] = "" else: new_settings[key] = settings[key] - new_settings[original_key] = settings[original_key] + + # Reset the value of the helper setting to its default for two reasons: + # 1. Performance: This ensures that the Go settings of non-Go + # dependencies have the same values as before the go_transition, + # which can prevent unnecessary rebuilds caused by configuration + # changes. + # 2. Correctness in edge cases: If there is a path in the build graph + # from a go_binary's non-Go dependency to a go_library that does not + # pass through another go_binary (e.g., through a custom rule + # replacement for go_binary), this transition could be applied again + # and cause incorrect Go setting values. + new_settings[original_key] = "" return new_settings diff --git a/tests/core/transition/hermeticity_test.go b/tests/core/transition/hermeticity_test.go index 35aa0c1fb3..136d5fdb4d 100644 --- a/tests/core/transition/hermeticity_test.go +++ b/tests/core/transition/hermeticity_test.go @@ -16,6 +16,7 @@ package hermeticity_test import ( "bytes" + "encoding/json" "fmt" "strings" "testing" @@ -199,6 +200,7 @@ func assertDependsCleanlyOnWithFlags(t *testing.T, targetA, targetB string, flag []string{ "cquery", "--transitions=full", + "--output=jsonproto", query, }, flags..., @@ -207,7 +209,7 @@ func assertDependsCleanlyOnWithFlags(t *testing.T, targetA, targetB string, flag if err != nil { t.Fatalf("bazel cquery '%s': %v", query, err) } - cqueryOut := string(bytes.TrimSpace(out)) + cqueryOut := bytes.TrimSpace(out) configHashes := extractConfigHashes(t, cqueryOut) if len(configHashes) != 1 { differingGoOptions := getGoOptions(t, configHashes...) @@ -231,36 +233,57 @@ func assertDependsCleanlyOnWithFlags(t *testing.T, targetA, targetB string, flag } } -func extractConfigHashes(t *testing.T, cqueryOut string) []string { - lines := strings.Split(cqueryOut, "\n") +func extractConfigHashes(t *testing.T, rawJsonOut []byte) []string { + var jsonOut bazelCqueryOutput + err := json.Unmarshal(rawJsonOut, &jsonOut) + if err != nil { + t.Fatalf("Failed to decode bazel config JSON output %v: %q", err, string(rawJsonOut)) + } var hashes []string - for _, line := range lines { - openingParens := strings.Index(line, "(") - closingParens := strings.Index(line, ")") - if openingParens == -1 || closingParens <= openingParens { - t.Fatalf("failed to find config hash in cquery out line: %s", line) - } - hashes = append(hashes, line[openingParens+1:closingParens]) + for _, result := range jsonOut.Results { + hashes = append(hashes, result.Configuration.Checksum) } return hashes } func getGoOptions(t *testing.T, hashes ...string) []string { - out, err := bazel_testing.BazelOutput(append([]string{"config"}, hashes...)...) + out, err := bazel_testing.BazelOutput(append([]string{"config", "--output=json"}, hashes...)...) if err != nil { t.Fatalf("bazel config %s: %v", strings.Join(hashes, " "), err) } - lines := strings.Split(string(bytes.TrimSpace(out)), "\n") - differingGoOptions := make([]string, 0) - for _, line := range lines { - // Lines with configuration options are indented - if !strings.HasPrefix(line, " ") { + rawJsonOut := bytes.TrimSpace(out) + var jsonOut bazelConfigOutput + err = json.Unmarshal(rawJsonOut, &jsonOut) + if err != nil { + t.Fatalf("Failed to decode bazel config JSON output %v: %q", err, string(rawJsonOut)) + } + var differingGoOptions []string + for _, fragment := range jsonOut.Fragments { + if fragment.Name != starlarkOptionsFragment { continue } - optionAndValue := strings.TrimLeft(line, " ") - if strings.HasPrefix(optionAndValue, "@io_bazel_rules_go//") { - differingGoOptions = append(differingGoOptions, optionAndValue) + for key, value := range fragment.Options { + if strings.HasPrefix(key, "@io_bazel_rules_go//") { + differingGoOptions = append(differingGoOptions, fmt.Sprintf("%s=%s", key, value)) + } } } return differingGoOptions } + +const starlarkOptionsFragment = "user-defined" + +type bazelConfigOutput struct { + Fragments []struct { + Name string `json:"name"` + Options map[string]string `json:"options"` + } `json:"fragments"` +} + +type bazelCqueryOutput struct { + Results []struct { + Configuration struct { + Checksum string `json:"checksum"` + } `json:"configuration"` + } `json:"results"` +}