link: make package conflicts into errors instead of warnings

go/config/BUILD.bazel

@@ -10,13 +10,6 @@ load(
     "LINKMODE_NORMAL",
 )
 
-bool_flag(
-    name = "incompatible_package_conflict_is_error",
-    # TODO(#1374): Flip in v0.25.
-    build_setting_default = False,
-    visibility = ["//visibility:public"],
-)
-
 bool_flag(
     name = "static",
     build_setting_default = False,

go/private/actions/link.bzl

@@ -153,11 +153,8 @@ def emit_link(
     tool_args.add_joined("-extldflags", extldflags, join_with = " ")
 
     conflict_err = _check_conflicts(arcs)
-    if go._package_conflict_is_error:
-        builder_args.add("-package_conflict_is_error")
     if conflict_err:
-        # Error is reported if -package_conflict_is_error was set, or if the
-        # linker command fails.
+        # Report package conflict errors in execution instead of analysis.
         # We could call fail() with this message, but Bazel prints a stack
         # that doesn't give useful information.
         builder_args.add("-conflict_err", conflict_err)

go/private/context.bzl

@@ -490,8 +490,6 @@ def go_context(ctx, attr = None):
         # Private
         # TODO: All uses of this should be removed
         _ctx = ctx,
-        # TODO(#1374): Remove in v0.25.
-        _package_conflict_is_error = go_config_info._package_conflict_is_error if go_config_info else True,
     )
 
 def _go_context_data_impl(ctx):
@@ -767,9 +765,6 @@ def _go_config_impl(ctx):
         linkmode = ctx.attr.linkmode[BuildSettingInfo].value,
         tags = ctx.attr.gotags[BuildSettingInfo].value,
         stamp = ctx.attr.stamp,
-
-        # TODO(#1374): Remove in v0.25.
-        _package_conflict_is_error = ctx.attr._package_conflict_is_error[BuildSettingInfo].value,
     )]
 
 go_config = rule(
@@ -808,9 +803,6 @@ go_config = rule(
             providers = [BuildSettingInfo],
         ),
         "stamp": attr.bool(mandatory = True),
-        "_package_conflict_is_error": attr.label(
-            default = "//go/config:incompatible_package_conflict_is_error",
-        ),
     },
     provides = [GoConfigInfo],
     doc = """Collects information about build settings in the current

go/tools/builders/importcfg.go

@@ -172,11 +172,7 @@ func buildImportcfgFileForLink(archives []archive, stdPackageListPath, installSu
 	depsSeen := map[string]string{}
 	for _, arc := range archives {
 		if _, ok := depsSeen[arc.packagePath]; ok {
-			// If this is detected during analysis, the -conflict_err flag will be set.
-			// We'll report that error if -package_conflict_is_error is set or if
-			// the link command fails.
-			// TODO(#1374): This should always be an error. Panic.
-			continue
+			return "", fmt.Errorf("internal error: package %s provided multiple times. This should have been detected during analysis.", arc.packagePath)
 		}
 		depsSeen[arc.packagePath] = arc.label
 		fmt.Fprintf(buf, "packagefile %s=%s\n", arc.packagePath, arc.file)

go/tools/builders/link.go

@@ -51,7 +51,6 @@ func link(args []string) error {
 	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.")
-	packageConflictIsError := flags.Bool("package_conflict_is_error", false, "Whether importpath conflicts are errors.")
 	conflictErrMsg := flags.String("conflict_err", "", "Error message about conflicts to report if there's a link error.")
 	if err := flags.Parse(builderArgs); err != nil {
 		return err
@@ -60,7 +59,7 @@ func link(args []string) error {
 		return err
 	}
 
-	if *packageConflictIsError && *conflictErrMsg != "" {
+	if *conflictErrMsg != "" {
 		return errors.New(*conflictErrMsg)
 	}
 
@@ -149,16 +148,8 @@ func link(args []string) error {
 	goargs = append(goargs, toolArgs...)
 	goargs = append(goargs, *main)
 	if err := goenv.runCommand(goargs); err != nil {
-		if *conflictErrMsg != "" {
-			// TODO(#1374): this should always be reported above.
-			err = fmt.Errorf("%v\n%s", err, *conflictErrMsg)
-		}
 		return err
 	}
-	if *conflictErrMsg != "" {
-		// TODO(#1374): this should always be reported above.
-		fmt.Fprintf(os.Stderr, "%s\nThis will be an error in the future.\n", *conflictErrMsg)
-	}
 
 	if *buildmode == "c-archive" {
 		if err := stripArMetadata(*outFile); err != nil {

tests/core/go_binary/package_conflict_test.go

@@ -113,30 +113,8 @@ func main() {
 	})
 }
 
-func runTest(t *testing.T, expectError bool, extraArgs ...string) {
-	args := append([]string{"build", "//:main"}, extraArgs...)
-
-	err := bazel_testing.RunBazel(args...)
-	if expectError {
-		if err == nil {
-			t.Fatal("Expected error")
-		}
-	} else {
-		if err != nil {
-			t.Fatal(err)
-		}
+func TestPackageConflict(t *testing.T) {
+	if err := bazel_testing.RunBazel("build", "//:main"); err == nil {
+		t.Fatal("Expected error")
 	}
 }
-
-func TestDefaultBehaviour(t *testing.T) {
-	// TODO(#1374): Default to `true`.
-	runTest(t, false)
-}
-
-func TestPackageConflictIsWarning(t *testing.T) {
-	runTest(t, false, "--@io_bazel_rules_go//go/config:incompatible_package_conflict_is_error=False")
-}
-
-func TestPackageConflictIsError(t *testing.T) {
-	runTest(t, true, "--@io_bazel_rules_go//go/config:incompatible_package_conflict_is_error=True")
-}