x/mod/modfile: IsDirectoryPath produces a false negative on some short clean directory paths

[dmitshur]

In #51448 (comment), Bryan wrote:

The argument . should trigger modfile.IsDirectoryPath here:
https://cs.opensource.google/go/go/+/ce427cf96128b545ae473983bafb6a0b80ecaa08:src/cmd/go/internal/modload/modfile.go;l=805-807

But it doesn't: this looks like a bug in IsDirectoryPath, which does not expect to ever need to refer to a module in the same directory (compare #34417).

For Go 1.19, we should probably fix up modfile.IsDirectoryPath. For 1.18, perhaps we should change the condition to if path == "." || modfile.IsDirectoryPath(path).

The Go 1.18 condition change happened in CL 389298. This is the tracking issue for the bug in modfile.IsDirectoryPath that we can still fix in Go 1.22 or later. (Or even in Go 1.21 if this bug fix is considered okay in the current pre-RC1 phase of the release freeze.)

I'll send a CL.

CC @bcmills, @matloob.

[gopherbot]

Change https://go.dev/cl/500335 mentions this issue: modfile: improve directory path detection and error text consistency

[gopherbot]

Change https://go.dev/cl/500355 mentions this issue: cmd: update vendored golang.org/x/mod

[gopherbot]

Change https://go.dev/cl/501016 mentions this issue: gopls: remove trailing slash from replace directory path

[dmitshur]

Writing down the compatibility notes for this bug fix.

There are known callers of IsDirectoryPath in x/mod itself and in cmd/go in the main repo, which is what I considered in scope below. For other possible callers, this is fixing a small edge case in the intended behavior (and documenting the actual behavior more accurately than before).

Compatibility

Fixing this bug doesn't change the meaning of any existing go.mod or go.work files. It makes it possible to explicitly start using relative paths as directory paths that were previously rejected, both in replace directives (in go.mod and go.work files) and in use directives (in go.work files). Doing that can only work in a Go toolchain that has the bug fix. So modules that wish to keep supporting older Go versions would need to wait before they start using the new directory paths. (CI testing with supported toolchains, e.g., see this change failing on Go 1.20.x and 1.19.x, is one of the ways users can know to avoid unintentionally breaking compatibility.)

The go mod commands don't remove trailing slashes or otherwise format directory paths: doing go mod edit -replace=example.com/module=[directory path] or go mod tidy keeps the directory path as is. The directory path today can stay arbitrary un-clean (e.g., ".././../target/dir"). The new shorter directory paths would need to be explicitly requested before showing up in a module.

The go work commands do try to do some lightweight formatting of incoming paths (via ToDirectoryPath) but only when it's not already a directory path or "." (the special case fixed only for go work in #51448). Since this fix is not decreasing the number of paths considered as directory paths, it continues to be possible to use directory paths that work today across all Go versions.

There's no effect on dependency modules because replace directives in go.mod files are ignored when a module is not main module. So even if a new version of a dependency module chooses to start using new directory paths sooner, all existing Go toolchains will not be affected when using that module as a dependency. (This is similar to how currently checking in a replace directive containing a Windows-style \ path separator makes that module not work on non-Windows systems when it's the main module, but it doesn't stop non-Windows systems using it as a dependency.)

So to me this seems like a backward compatible minor bug fix that doesn't need special treatment (i.e., teaching all supported Go versions to understand but not yet permit the new directory paths) to be rolled out.