diff --git a/go/config/BUILD.bazel b/go/config/BUILD.bazel index b95ba5cc60..b832742767 100644 --- a/go/config/BUILD.bazel +++ b/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, diff --git a/go/private/actions/link.bzl b/go/private/actions/link.bzl index de1a9ab33c..9139430203 100644 --- a/go/private/actions/link.bzl +++ b/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) diff --git a/go/private/context.bzl b/go/private/context.bzl index 9b182a2404..5fa52386ae 100644 --- a/go/private/context.bzl +++ b/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 diff --git a/go/tools/builders/importcfg.go b/go/tools/builders/importcfg.go index da80f53dcb..2cfaab1191 100644 --- a/go/tools/builders/importcfg.go +++ b/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) diff --git a/go/tools/builders/link.go b/go/tools/builders/link.go index 71f9559c7a..edb8bf52f0 100644 --- a/go/tools/builders/link.go +++ b/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 { diff --git a/tests/core/go_binary/package_conflict_test.go b/tests/core/go_binary/package_conflict_test.go index b51ab16508..9ec6340a89 100644 --- a/tests/core/go_binary/package_conflict_test.go +++ b/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") -}