Address further comments

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
 

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"`
+}