Remove tests for go_path's link mode

`link` mode produces non-hermetically dangling symlinks by design, which means that it is broken with remote caching or execution and cannot be guaranteed to work in other situations. In the feature, we may be able to support it again based on `ctx.declare_symlink` and improved unresolved symlink support with remote caching and execution.
bazel-contrib · May 19, 2023 · dbc315b · dbc315b
1 parent 5933b6e
commit dbc315b
Show file tree

Hide file tree

Showing 6 changed files with 7 additions and 31 deletions.
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -13,11 +13,6 @@ tasks:
       - tests/core/cgo/generate_imported_dylib.sh
     build_targets:
     - "//..."
-    test_flags:
-      # Temporary rollback to fix //tests/core/go_path builds
-      # https://github.com/bazelbuild/continuous-integration/commit/a95a916098d3015bb4ea20b7e33bc7d27d00bffc
-      - "--remote_download_outputs=all"
-      - "--build_runfile_links"
     test_targets:
     - "//..."
   ubuntu2004:
@@ -88,8 +83,6 @@ tasks:
     # TODO(#1787): There is a cc_import target in //tests/core/cgo that
     # doesn't set the interface_library attribute. This causes an
     # analysis failure.
-    # TODO(#1789): go_path tests that require symlinks fail. These should
-    # be skipped automatically.
     # TODO(#1790): Tests that require data should use bazel.Runfile.
     # TODO(#2516): Tests that require protoc fail when protoc is built with mingw-gcc.
     - "--"
@@ -188,7 +181,6 @@ tasks:
     - "-//tests/core/go_path:go_path"
     - "-//tests/core/go_path:go_path_test"
     - "-//tests/core/go_path:nodata_path"
-    - "-//tests/core/go_path:link_path"
     - "-//tests/core/go_path:copy_path"
     - "-//tests/core/go_path:archive_path"
     - "-//tests/core/go_path/pkg/lib:vendored"
@@ -293,7 +285,6 @@ tasks:
     - "-//tests/extras/go_embed_data:go_default_test"
     - "-//tests/core/go_path:go_path"
     - "-//tests/core/go_path:go_path_test"
-    - "-//tests/core/go_path:link_path"
     - "-//tests/core/go_path/pkg/lib:embed_test"
     - "-//tests/core/go_path/pkg/lib:go_default_test"
     - "-//tests/core/go_plugin:go_plugin"

diff --git a/.bazelrc b/.bazelrc
@@ -1,11 +1,5 @@
 common --enable_platform_specific_config
 
-# //tests/core/go_path is incompatible with BwoB, which is enabled in Bazel CI.
-# https://github.com/bazelbuild/continuous-integration/commit/a95a916098d3015bb4ea20b7e33bc7d27d00bffc
-build --remote_download_outputs=all
-build --build_runfile_links
-build --experimental_remote_download_regex=.*tests/core/go_path/.*
-
 # Go requires a C toolchain that accepts options and emits errors like
 # gcc or clang. The Go SDK does not support MSVC.
 build:windows --cpu=x64_windows

diff --git a/docs/go/core/rules.md b/docs/go/core/rules.md
@@ -286,7 +286,7 @@ go_path(<a href="#go_path-name">name</a>, <a href="#go_path-data">data</a>, <a h
 | <a id="go_path-include_data"></a>include_data |  When true, data files referenced by libraries, binaries, and tests will be             included in the output directory. Files listed in the <code>data</code> attribute             for this rule will be included regardless of this attribute.   | Boolean | optional | True |
 | <a id="go_path-include_pkg"></a>include_pkg |  When true, a <code>pkg</code> subdirectory containing the compiled libraries will be created in the             generated <code>GOPATH</code> containing compiled libraries.   | Boolean | optional | False |
 | <a id="go_path-include_transitive"></a>include_transitive |  When true, the transitive dependency graph will be included in the generated <code>GOPATH</code>. This is             the default behaviour. When false, only the direct dependencies will be included in the             generated <code>GOPATH</code>.   | Boolean | optional | True |
-| <a id="go_path-mode"></a>mode |  Determines how the generated directory is provided. May be one of:             <ul>                 <li><code>"archive"</code>: The generated directory is packaged as a single .zip file.</li>                 <li><code>"copy"</code>: The generated directory is a single tree artifact. Source files                 are copied into the tree.</li>                 <li><code>"link"</code>: Source files are symlinked into the tree. All of the symlink                 files are provided as separate output files.</li>             </ul>              ***Note:*** In <code>"copy"</code> mode, when a <code>GoPath</code> is consumed as a set of input             files or run files, Bazel may provide symbolic links instead of regular files.             Any program that consumes these files should dereference links, e.g., if you             run <code>tar</code>, use the <code>--dereference</code> flag.   | String | optional | "copy" |
+| <a id="go_path-mode"></a>mode |  Determines how the generated directory is provided. May be one of:             <ul>                 <li><code>"archive"</code>: The generated directory is packaged as a single .zip file.</li>                 <li><code>"copy"</code>: The generated directory is a single tree artifact. Source files                 are copied into the tree.</li>                 <li><code>"link"</code>: <b>Unmaintained due to correctness issues</b>. Source files                 are symlinked into the tree. All of the symlink files are provided as separate output                 files.</li>             </ul>              ***Note:*** In <code>"copy"</code> mode, when a <code>GoPath</code> is consumed as a set of input             files or run files, Bazel may provide symbolic links instead of regular files.             Any program that consumes these files should dereference links, e.g., if you             run <code>tar</code>, use the <code>--dereference</code> flag.   | String | optional | "copy" |
 
 
 

diff --git a/go/private/tools/path.bzl b/go/private/tools/path.bzl
@@ -210,8 +210,9 @@ go_path = rule(
                 <li><code>"archive"</code>: The generated directory is packaged as a single .zip file.</li>
                 <li><code>"copy"</code>: The generated directory is a single tree artifact. Source files
                 are copied into the tree.</li>
-                <li><code>"link"</code>: Source files are symlinked into the tree. All of the symlink
-                files are provided as separate output files.</li>
+                <li><code>"link"</code>: <b>Unmaintained due to correctness issues</b>. Source files
+                are symlinked into the tree. All of the symlink files are provided as separate output
+                files.</li>
             </ul>
 
             ***Note:*** In <code>"copy"</code> mode, when a <code>GoPath</code> is consumed as a set of input

diff --git a/tests/core/go_path/BUILD.bazel b/tests/core/go_path/BUILD.bazel
@@ -16,7 +16,7 @@ test_suite(name = "go_path")
         "//tests/core/go_path/pkg/lib:go_default_test",
         "//tests/core/go_path/pkg/lib:vendored",
     ],
-) for mode in ("archive", "copy", "link")]
+) for mode in ("archive", "copy")]
 
 go_path(
     name = "transition_path",
@@ -62,7 +62,6 @@ go_test(
     args = [
         "-archive_path=$(location :archive_path)",
         "-copy_path=$(location :copy_path)",
-        "-link_path=tests/core/go_path/link_path",  # can't use location; not a single file
         "-nodata_path=$(location :nodata_path)",
         "-embed_path=$(location :embed_path)",
         "-embed_no_srcs_path=$(location :embed_no_srcs_path)",
@@ -73,7 +72,6 @@ go_test(
         ":copy_path",
         ":embed_no_srcs_path",
         ":embed_path",
-        ":link_path",
         ":nodata_path",
         ":notransitive_path",
         ":transition_path",

diff --git a/tests/core/go_path/go_path_test.go b/tests/core/go_path/go_path_test.go
@@ -29,7 +29,7 @@ import (
 	"github.com/bazelbuild/rules_go/go/tools/bazel"
 )
 
-var copyPath, embedPath, embedNoSrcsPath, linkPath, archivePath, nodataPath, notransitivePath string
+var copyPath, embedPath, embedNoSrcsPath, archivePath, nodataPath, notransitivePath string
 
 var defaultMode = runtime.GOOS + "_" + runtime.GOARCH
 
@@ -57,7 +57,6 @@ var files = []string{
 
 func TestMain(m *testing.M) {
 	flag.StringVar(&copyPath, "copy_path", "", "path to copied go_path")
-	flag.StringVar(&linkPath, "link_path", "", "path to symlinked go_path")
 	flag.StringVar(&archivePath, "archive_path", "", "path to archive go_path")
 	flag.StringVar(&nodataPath, "nodata_path", "", "path to go_path without data")
 	flag.StringVar(&embedPath, "embed_path", "", "path to go_path with embedsrcs")
@@ -74,13 +73,6 @@ func TestCopyPath(t *testing.T) {
 	checkPath(t, copyPath, files)
 }
 
-func TestLinkPath(t *testing.T) {
-	if linkPath == "" {
-		t.Fatal("-link_path not set")
-	}
-	checkPath(t, linkPath, files)
-}
-
 func TestEmbedPath(t *testing.T) {
 	if embedPath == "" {
 		t.Fatal("-embed_path not set")
@@ -187,7 +179,7 @@ func checkPath(t *testing.T, dir string, files []string) {
 			wantAbsent = true
 		}
 		path := filepath.Join(dir, filepath.FromSlash(f))
-		st, err := os.Lstat(path)
+		st, err := os.Stat(path)
 		if wantAbsent {
 			if err == nil {
 				t.Errorf("found %s: should not be present", path)