go_binary: correctly apply x_defs to main package when linking

go/private/actions/link.bzl

@@ -132,7 +132,7 @@ def emit_link(
             builder_args.add("-Xstamp", "%s=%s" % (k, v[1:-1]))
             stamp_x_defs = True
         else:
-            tool_args.add("-X", "%s=%s" % (k, v))
+            builder_args.add("-X", "%s=%s" % (k, v))
 
     # Stamping support
     stamp_inputs = []
@@ -142,6 +142,7 @@ def emit_link(
 
     builder_args.add("-o", executable)
     builder_args.add("-main", archive.data.file)
+    builder_args.add("-p", archive.data.importmap)
     tool_args.add_all(gc_linkopts)
     tool_args.add_all(go.toolchain.flags.link)
 

go/tools/builders/link.go

@@ -37,16 +37,19 @@ func link(args []string) error {
 	builderArgs, toolArgs := splitArgs(args)
 	xstamps := multiFlag{}
 	stamps := multiFlag{}
+	xdefs := multiFlag{}
 	archives := linkArchiveMultiFlag{}
 	flags := flag.NewFlagSet("link", flag.ExitOnError)
 	goenv := envFlags(flags)
 	main := flags.String("main", "", "Path to the main archive.")
+	packagePath := flags.String("p", "", "Package path of the main archive.")
 	outFile := flags.String("o", "", "Path to output file.")
 	flags.Var(&archives, "arc", "Label, package path, and file name of a dependency, separated by '='")
 	packageList := flags.String("package_list", "", "The file containing the list of standard library packages")
 	buildmode := flags.String("buildmode", "", "Build mode used.")
+	flags.Var(&xdefs, "X", "A string variable to replace in the linked binary (repeated).")
+	flags.Var(&xstamps, "Xstamp", "Like -X but the values are looked up in the -stamp file.")
 	flags.Var(&stamps, "stamp", "The name of a file with stamping values.")
-	flags.Var(&xstamps, "Xstamp", "A link xdef that may need stamping.")
 	if err := flags.Parse(builderArgs); err != nil {
 		return err
 	}
@@ -65,7 +68,7 @@ func link(args []string) error {
 	*main = abs(*main)
 
 	// If we were given any stamp value files, read and parse them
-	stampmap := map[string]string{}
+	stampMap := map[string]string{}
 	for _, stampfile := range stamps {
 		stampbuf, err := ioutil.ReadFile(stampfile)
 		if err != nil {
@@ -79,10 +82,10 @@ func link(args []string) error {
 				// Nothing to do here
 			case 1:
 				// Map to the empty string
-				stampmap[line[0]] = ""
+				stampMap[line[0]] = ""
 			case 2:
 				// Key and value
-				stampmap[line[0]] = line[1]
+				stampMap[line[0]] = line[1]
 			}
 		}
 	}
@@ -97,16 +100,37 @@ func link(args []string) error {
 	// generate any additional link options we need
 	goargs := goenv.goTool("link")
 	goargs = append(goargs, "-importcfg", importcfgName)
+
+	parseXdef := func(xdef string) (pkg, name, value string, err error) {
+		eq := strings.IndexByte(xdef, '=')
+		if eq < 0 {
+			return "", "", "", fmt.Errorf("-X or -Xstamp flag does not contain '=': %s", xdef)
+		}
+		dot := strings.LastIndexByte(xdef[:eq], '.')
+		if dot < 0 {
+			return "", "", "", fmt.Errorf("-X or -Xstamp flag does not contain '.': %s", xdef)
+		}
+		pkg, name, value = xdef[:dot], xdef[dot+1:eq], xdef[eq+1:]
+		if pkg == *packagePath {
+			pkg = "main"
+		}
+		return pkg, name, value, nil
+	}
 	for _, xdef := range xstamps {
-		split := strings.SplitN(xdef, "=", 2)
-		if len(split) != 2 {
-			continue
+		pkg, name, key, err := parseXdef(xdef)
+		if err != nil {
+			return err
+		}
+		if value, ok := stampMap[key]; ok {
+			goargs = append(goargs, "-X", fmt.Sprintf("%s.%s=%s", pkg, name, value))
 		}
-		name := split[0]
-		key := split[1]
-		if value, found := stampmap[key]; found {
-			goargs = append(goargs, "-X", fmt.Sprintf("%s=%s", name, value))
+	}
+	for _, xdef := range xdefs {
+		pkg, name, value, err := parseXdef(xdef)
+		if err != nil {
+			return err
 		}
+		goargs = append(goargs, "-X", fmt.Sprintf("%s.%s=%s", pkg, name, value))
 	}
 
 	if *buildmode != "" {

tests/README.rst

@@ -6,10 +6,10 @@ Main test areas
 
 .. Child list start
 
-* `Go rules examples <examples/README.rst>`_
 * `Core Go rules tests <core/README.rst>`_
 * `Integration tests <integration/README.rst>`_
 * `Legacy tests <legacy/README.rst>`_
+* `Go rules examples <examples/README.rst>`_
 
 .. Child list end
 

tests/core/README.rst

@@ -8,23 +8,22 @@ Contents
 
 .. Child list start
 
-* `Basic go_path functionality <go_path/README.rst>`_
-* `Basic go_vet_test functionality <go_vet_test/README.rst>`_
-* `Basic -buildmode=plugin functionality <go_plugin/README.rst>`_
-* `Basic go_binary functionality <go_binary/README.rst>`_
 * `Basic go_library functionality <go_library/README.rst>`_
+* `output_groups functionality <output_groups/README.rst>`_
+* `Basic -buildmode=plugin functionality <go_plugin/README.rst>`_
+* `Core Go rules tests <nogo/README.rst>`_
+* `go_proto_library importmap <go_proto_library_importmap/README.rst>`_
 * `Cross compilation <cross/README.rst>`_
-* `coverage functionality <coverage/README.rst>`_
 * `Basic go_proto_library functionality <go_proto_library/README.rst>`_
-* `Core Go rules tests <nogo/README.rst>`_
 * `c-archive / c-shared linkmodes <c_linkmodes/README.rst>`_
-* `output_groups functionality <output_groups/README.rst>`_
-* `stdlib functionality <stdlib/README.rst>`_
-* `Import maps <importmap/README.rst>`_
 * `Basic go_test functionality <go_test/README.rst>`_
 * `Basic cgo functionality <cgo/README.rst>`_
 * `race instrumentation <race/README.rst>`_
-* `go_proto_library importmap <go_proto_library_importmap/README.rst>`_
+* `stdlib functionality <stdlib/README.rst>`_
+* `Basic go_binary functionality <go_binary/README.rst>`_
+* `coverage functionality <coverage/README.rst>`_
+* `Import maps <importmap/README.rst>`_
+* `Basic go_path functionality <go_path/README.rst>`_
 
 .. Child list end
 

tests/core/go_binary/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
 load(":many_deps.bzl", "many_deps")
 
 test_suite(name = "go_binary")
@@ -30,3 +30,40 @@ go_binary(
 )
 
 many_deps(name = "many_deps")
+
+go_test(
+    name = "stamp_test",
+    srcs = ["stamp_test.go"],
+    data = [":stamp_bin"],
+    rundir = ".",
+    deps = ["@io_bazel_rules_go//go/tools/bazel:go_default_library"],
+)
+
+go_binary(
+    name = "stamp_bin",
+    srcs = ["stamp_bin.go"],
+    embed = [":stamp_embed"],
+    x_defs = {
+        "Bin": "Bin",
+        "example.com/stamp_dep.DepBin": "DepBin",
+    },
+    deps = [":stamp_dep"],
+)
+
+go_library(
+    name = "stamp_embed",
+    srcs = ["stamp_embed.go"],
+    importpath = "example.com/stamp_embed",
+    x_defs = {
+        "Embed": "Embed",
+    },
+)
+
+go_library(
+    name = "stamp_dep",
+    srcs = ["stamp_dep.go"],
+    importpath = "example.com/stamp_dep",
+    x_defs = {
+        "DepSelf": "DepSelf",
+    },
+)

tests/core/go_binary/README.rst

@@ -30,3 +30,9 @@ many_deps
 Test that a `go_binary`_ with many imports with long names can be linked. This
 makes sure we don't exceed command-line length limits with -I and -L flags.
 Verifies #1637.
+
+stamp_test
+----------
+Test that the `go_binary`_ ``x_defs`` attribute works correctly, both in a
+binary and in an embedded library. Tests regular stamps and stamps that
+depend on values from the workspace status script. Verifies #2000.

tests/core/go_binary/stamp_bin.go

@@ -0,0 +1,16 @@
+package main
+
+import (
+	"fmt"
+
+	"example.com/stamp_dep"
+)
+
+var Bin = "redacted"
+
+func main() {
+	fmt.Printf("Bin=%s\n", Bin)
+	fmt.Printf("Embed=%s\n", Embed)
+	fmt.Printf("DepSelf=%s\n", stamp_dep.DepSelf)
+	fmt.Printf("DepBin=%s\n", stamp_dep.DepBin)
+}

tests/core/go_binary/stamp_dep.go

@@ -0,0 +1,6 @@
+package stamp_dep
+
+var (
+	DepSelf = "redacted"
+	DepBin  = "redacted"
+)

tests/core/go_binary/stamp_embed.go

@@ -0,0 +1,3 @@
+package main
+
+var Embed = "redacted"

tests/core/go_binary/stamp_test.go

@@ -0,0 +1,29 @@
+package main
+
+import (
+	"os/exec"
+	"strings"
+	"testing"
+
+	"github.com/bazelbuild/rules_go/go/tools/bazel"
+)
+
+func TestStamp(t *testing.T) {
+	bin, ok := bazel.FindBinary("tests/core/go_binary", "stamp_bin")
+	if !ok {
+		t.Error("could not find stamp_bin")
+	}
+	out, err := exec.Command(bin).Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	got := strings.TrimSpace(string(out))
+	want := `Bin=Bin
+Embed=Embed
+DepSelf=DepSelf
+DepBin=DepBin`
+	if got != want {
+		t.Errorf("got:\n%s\nwant:\n%s", got, want)
+	}
+}

tests/core/nogo/README.rst

@@ -8,10 +8,11 @@ Contents
 
 .. Child list start
 
-* `Nogo configuration <config/README.rst>`_
 * `Vet check <vet/README.rst>`_
+* `Nogo configuration <config/README.rst>`_
 * `nogo analyzers with dependencies <deps/README.rst>`_
 * `Custom nogo analyzers <custom/README.rst>`_
+* `nogo test with coverage <coverage/README.rst>`_
 
 .. Child list end