replace golang.org/x/tools with golang.org/x/tools/go/vcs@v0.1.0-deprecated

[malt3]

What type of PR is this?

Bug fix

What package or component does this PR mostly affect?

go dependencies

What does this PR do? Why is it needed?

golang/x/tools no longer ships the vcs package. Instead, there is a new module that now contains the same code.

Which issues(s) does this PR fix?

Fixes #1579

Other notes for review

[malt3]

I do not understand why gazelle wants to modify the build files in this way.
The same check works when I run it under bzlmod: bazel run //:gazelle_ci --enable_bzlmod
We clearly depend on golang.org/x/tools/go/vcs (and we even depend on a newer version of golang.org/x/tools that doesn't have the vcs package anymore).

$ bazel run //:gazelle_ci 
INFO: Analyzed target //:gazelle_ci (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:gazelle_ci up-to-date:
  bazel-bin/gazelle_ci-runner.bash
  bazel-bin/gazelle_ci
INFO: Elapsed time: 0.106s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/gazelle_ci
--- cmd/fetch_repo/BUILD.bazel  1970-01-01 00:00:00.000000001 +0000
+++ cmd/fetch_repo/BUILD.bazel  1970-01-01 00:00:00.000000001 +0000
@@ -9,7 +9,7 @@
     ],
     importpath = "github.com/bazelbuild/bazel-gazelle/cmd/fetch_repo",
     visibility = ["//visibility:private"],
-    deps = ["@org_golang_x_tools_go_vcs//:vcs"],
+    deps = ["@org_golang_x_tools//go/vcs"],
 )
 
 go_binary(
@@ -22,7 +22,7 @@
     name = "fetch_repo_test",
     srcs = ["fetch_repo_test.go"],
     embed = [":fetch_repo_lib"],
-    deps = ["@org_golang_x_tools_go_vcs//:vcs"],
+    deps = ["@org_golang_x_tools//go/vcs"],
 )
 
 filegroup(
--- language/go/BUILD.bazel     1970-01-01 00:00:00.000000001 +0000
+++ language/go/BUILD.bazel     1970-01-01 00:00:00.000000001 +0000
@@ -84,7 +84,7 @@
         "@com_github_bazelbuild_buildtools//build:go_default_library",
         "@com_github_google_go_cmp//cmp",
         "@io_bazel_rules_go//go/tools/bazel:go_default_library",
-        "@org_golang_x_tools_go_vcs//:vcs",
+        "@org_golang_x_tools//go/vcs",
     ],
 )
 
--- repo/BUILD.bazel    1970-01-01 00:00:00.000000001 +0000
+++ repo/BUILD.bazel    1970-01-01 00:00:00.000000001 +0000
@@ -13,7 +13,7 @@
         "//pathtools",
         "//rule",
         "@org_golang_x_mod//modfile",
-        "@org_golang_x_tools_go_vcs//:vcs",
+        "@org_golang_x_tools//go/vcs",
     ],
 )
 
@@ -29,7 +29,7 @@
         "//pathtools",
         "//rule",
         "//testtools",
-        "@org_golang_x_tools_go_vcs//:vcs",
+        "@org_golang_x_tools//go/vcs",
     ],
 )
 

[fmeum]

@malt3 You probably need to add a # gazelle:repository directive to the very end of WORKSPACE.

[tyler-french]

I tested this at Uber and it works (with a patch to buildtools). Could you update buildtools again to include bazelbuild/buildtools@691e01a ?

Separately and optionally: I took a different approach in #1615 (which has similar logic) to include a ghost import to x/tools: https://github.com/bazelbuild/bazel-gazelle/pull/1615/files#diff-fe5a1af491aaa37acf76bdff0f3bb47ddbca27356b676128e21298a4e58b46cb . This approach is a bit hacky, but what if instead we have a file (called nogodeps.go), which is just

package nogodeps

import (
	// These deps are required by @rules_go//go/tools/builders/nogo_main.go
	// These ghost imports force them to be to be included in the `go.mod`,
	// and therefore in deps.bzl and MODULE.bzl/go_deps
	_ "golang.org/x/tools/go/analysis"
	_ "golang.org/x/tools/go/gcexportdata"
	_ "golang.org/x/tools/internal/facts"
)

By including these deps in Go (rather than just Bazel), it also allows the bazel run //tools/releaser to update them automatically.

That being said, it is more hacky and therefore less "correct", which could be a drawback.

@fmeum, @linzhp Do you have an opinion on these approaches?

[malt3]

I tested this at Uber and it works (with a patch to buildtools). Could you update buildtools again to include bazelbuild/buildtools@691e01a ?

Will do!

EDIT: done

[malt3]

Separately and optionally: I took a different approach in #1615 (which has similar logic) to include a ghost import to x/tools

I personally would be fine with either solution. If you see a benefit in the other, feel free to close this PR and go with the ghost import :). It feels like we should implement bazelbuild/rules_go#3646 to completely remove this class of problems.

[tyler-french]

Separately and optionally: I took a different approach in #1615 (which has similar logic) to include a ghost import to x/tools

I personally would be fine with either solution. If you see a benefit in the other, feel free to close this PR and go with the ghost import :). It feels like we should implement bazelbuild/rules_go#3646 to completely remove this class of problems.

Sounds good 😄. Let me know if you think the approach in #1615 is sufficient. One benefit here is that the releaser will automatically update this dep for us, but this is the only real benefit I can think of.