proposal: cmd/go: pinning replacements in require

[jwebb]

Proposal Details

Motivation

In the go.mod for our large codebase we have about a dozen replace directives, for essentially three reasons:

1. Forks for bugfixes that will be upstreamed.
2. Permanent patchsets that will be continually rebased. For example, we had to fork x/crypto in order to add support for some nonstandard cipher modes. Those will never be upstreamed because they are technically dubious, but nevertheless certain 3rd party connections require us to use them.
3. Licensed closed-source dependencies, where the source location doesn't match the canonical name.

A recurring problem we have is that cases 1 and 2 here are both rather fragile. E.g. we might have:

require author1/package1 v1.0
require author2/package2 v1.2
replace author2/package2 => elsewhere/package2 v1.2-fix3

But once we upgrade package1, which depends on a newer version of package2, we might automatically get:

require author1/package1 v1.1
require author2/package2 v1.5
replace author2/package2 => elsewhere/package2 v1.2-fix3

Now it looks like we're using v1.5, but in fact we're still using our v1.2 fork, that package1 is apparently incompatible with in some unspecified way, that may or may not result in compile time failure. And since in practice the require and replace directives may be several pages apart in the file (the formatter explicitly encourages grouping them separately), this is not obvious during code review.

The replace directive supports a version number on its left-hand side, but that results in equally undesirable behaviour:

require author1/package1 v1.1
require author2/package2 v1.5
replace author2/package2 v1.2 => elsewhere/package2 v1.2-fix3

Now the replace is just ignored, so we upgraded to v1.5 and our fix is discarded.

Proposal

To add a combined syntax that explictly requires, pins and replaces a given version:

require author2/package2 v1.2 => elsewhere/package2 v1.2-fix3

In this form, any attempt to implicitly upgrade package2 should fail and require human intervention, because likely they need to go away and ensure that fix3 has been merged upstream or rebased as appropriate before proceeding.

I would expect this normally to be used only in main modules. To avoid surprises, it should be permitted in dependency modules only if either a) the exact same require appears in the main module or b) the package has a replace overriding it.

[gabyhelp]

Related Issues and Documentation

• Go Wiki: Go Modules > FAQs — Additional Control > When should I use the replace directive?
• proposal: cmd/go: permit replace in go.mod to act as path alias #35382 (closed)
• go.mod file reference > replace > Notes
• cmd/go: support easy way to install a remote binary while respecting remote 'replace' and 'exclude' directives #31173
• cmd/go: document best practice for “postdated” requirements #26150 (closed)
• Go Modules Reference > go.mod files > replace directive
• cmd/go: easier go.mod updates when packages are in the same repository #50698 (closed)
• proposal: cmd/go: support `local` directive in go.mod #51779 (closed)
• Proposal: Have replace statements in a separate file instead of go.mod #38775 (closed)
• proposal: cmd/go: `go get` automatically add `replace` in go.mod #39185

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ianlancetaylor]

CC @matloob

[matloob]

cc @samthanawalla

If I understand the proposal correctly, this combined require and replace directive makes upgrades of dependencies likely to cause builds to fail. I see why that's desirable for your project because you want to detect upgrades and update your forked version of the repo whenever that happens. But I think we'd want to be really careful before introducing a feature that does that because it could lead to surprises and user confusion. I think it would also encourage building modules that don't work properly as dependencies.

I also think mixing require and replace directives can lead to confusion since require directives apply for work and dependency modules while replace directives only apply to work modules.

One thing you could do to add similar functionality to your project without the feature being added to the module system is to add a test that ensures that versioned replacements in your go.mod file all apply in your build. Using the versioned replace directive (replace author2/package2 v1.2 => elsewhere/package2 v1.2-fix3), you could check that the actual resolved version of author2/package2 in your build graph is v1.2. Using the golang.org/x/mod/modfile package you could read all the replacements from your go.mod and build a set of expectations: the list of replace directives in the file. Then running go list -m all (you could use the -json flag to make tho output easier to parse) you could check that the selected version of the module is actually replaced.

[jwebb]

Certainly take your point about abuse by dependencies, and thanks for the suggestion to use a test to work around this.

I'm not particularly invested in the proposed syntax here, but I don't see our motivation as particularly niche. It seems to me that this fragility applies to most potential uses of replace. The only one that's reliable is my case 3, where we can supply no versions at all in replace, and have the required version passed through. Other examples in the docs, such as pointing to local checkouts, have the same potential problem - just a little less acutely given a shorter expected lifetime.

It's also at odds with the general Go design principle that code behaviour should be minimally dependent on its context. The version attached to a require can be an outright lie, depending on what's elsewhere in the file.

Another possible design would be simply to make the check you suggest as a test a standard behaviour. It's not clear to me why a replace directive for an unused version would ever be desirable, and it would be somewhat in line with the compiler's treatment of unused variables and imports.

[jwebb]

Actually I misremembered - my case 3 is also poorly supported since the replacement version is required if the path is not local - #28176 discusses this.

[matloob]

A colleague suggested an alternative to ensure that all versioned replacements in your go.mod file apply: if you have a replace directive with a version, you could add a second replace directive without a version that points to a dummy directory. A replace directive with a version takes precedence so the versioned replace would apply if the selected version of the module was the version being replaced. If the selected version changes, the versioned replace would not apply, and the dummy module would be used, causing the build to fail. This does have some issues: The error message wouldn't be totally intuitive (it would be a build failure saying a package doesn't exist in the dummy directory). Also if other versions of the replaced modules appear in the dependency graph, the dummy version would be selected, so those would need to be replaced too. But this is something to consider. I could provide more details about this if you wish.

I agree that replace directives can be fragile, but it's our intention that they're used as infrequently as possible, and only for short time frames. They're primarily there for the first use case you mentioned. The other main use case for replaces is for making changes across multiple modules. But both of those use cases are primarily there for temporary changes.

I hope that the second use case is an uncommon use case. It's a really hard place to be to plan to have a low level dependency replaced for the long term and I think it would be painful regardless of the level of support the go command has for it. Our hope is that users cooperate with lower level dependencies when possible. An example of Go's philosophy is in this blog post about the principles of Go's module system: https://research.swtch.com/vgo-principles#cooperation. (The example is different from what you're trying to do but I think the principle still applies).

[matloob]

Hi, could you let us know if the technique above could work for you? I would like to help improve your experience, though I think it's best to implement the requirements using mechanisms that already exist.

[gopherbot]

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)