Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: cmd/go: allow replace directives in go.mod to work for standard libraries #35283

Open
zachwalton opened this issue Oct 31, 2019 · 9 comments
Labels
Milestone

Comments

@zachwalton
Copy link

@zachwalton zachwalton commented Oct 31, 2019

This may be completely incompatible with Go's architecture, but I'm putting it out there as a request for comments :)

Basically, make this work:

09:56 $ cat go.mod
module github.com/my/repo

go 1.13

replace (
	crypto/tls => ./third_party/crypto/tls
)

This would allow for forking standard libraries without requiring users to use a forked version of Go. In my case, I've implemented #22836 and would like to use it in my project without maintaining a full Go fork.

What version of Go are you using (go version)?

09:56 $ go version
go version go1.13 darwin/amd64
@gopherbot gopherbot added this to the Proposal milestone Oct 31, 2019
@gopherbot gopherbot added the Proposal label Oct 31, 2019
@networkimprov

This comment has been minimized.

Copy link

@networkimprov networkimprov commented Oct 31, 2019

Or you can patch your stdlib, which a number of projects do.

I'd liike to see Go support for patching stdlib & runtime, e.g. an env variable giving a patchfile or directory, and a way to amend the version string to "1.13.3_myproject-1.1"

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Nov 6, 2019

This is in general not possible. The standard library is not a collection of individual modules. It is more like a single module. That is, the packages in the standard library are very tightly coupled. You can't mix and match some from one version of Go and some from another, nor some hacked up and some not.

@rsc rsc changed the title Proposal: Allow replace directives in go.mod to work for standard libraries proposal: cmd/go: allow replace directives in go.mod to work for standard libraries Nov 6, 2019
@networkimprov

This comment has been minimized.

Copy link

@networkimprov networkimprov commented Nov 6, 2019

@rsc, what do you think about Go support for patching stdlib & runtime, e.g.

  1. a build-time env variable giving a patchfile or directory, and
  2. a way to append to the runtime.Version() result: "1.13.3_myproject-1.1"
@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Nov 6, 2019

To expand on @rsc's comment, replace directives in go.mod control modules, not packages. Modules are the unit of versioning and distribution, so it doesn't really make sense for them to apply to individual packages (and as with std, doing so would break many modules).

@networkimprov Why not just patch GOROOT directly and build a toolchain from there? That seems far simpler.

@networkimprov

This comment has been minimized.

Copy link

@networkimprov networkimprov commented Nov 6, 2019

@jayconrod $GOROOT contents may be discarded on upgrade. And any patches are invisible at build-time. You wouldn't want everything you build to carry patches necessary for a certain app.

I currently patch my $GOROOT directly, but runtime.Version() does not reflect this fact. I looked for a way to amend the .Version() result, but didn't come up with something straightforward. I forget why offhand...

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Nov 7, 2019

@networkimprov

$GOROOT contents may be discarded on upgrade.

There are a lot of ways to handle this. For example, as commits on a branch that gets rebased after upgrading. Or as a list of files applied by a bash script.

I looked for a way to amend the .Version() result, but didn't come up with something straightforward.

I think you can write to $GOROOT/VERSION before running make.bash. See findgoversion in src/cmd/dist/build.go for details.

In general though, anyone who's patching the standard library or toolchain in their project has a fairly specialized use case, and I wouldn't expect one solution to work for everyone.

This is a bit off-topic from the original issue in any case (replace on standard packages), so let's table this part of the discussion.

@zachwalton

This comment has been minimized.

Copy link
Author

@zachwalton zachwalton commented Nov 7, 2019

Thanks for the thoughtful replies. It sounds like the premise of my proposal was off-base, so more generally:

I think it'd be helpful to have a portable way to patch standard packages imported by a Go project, "portable" in this sense meaning being able to use a module that patches standard packages with nothing more than go get github.com/module/that/patches/stdlib.

Is there a potential place for some mechanism along those lines in Go?

@networkimprov

This comment has been minimized.

Copy link

@networkimprov networkimprov commented Nov 7, 2019

@jayconrod since the original suggestion isn't achievable, I'd like to hear from @rsc whether a patch-support proposal is worth investing time in. It would solve the stated problem.

Patching runtime or stdlib should not require a git-clone of the Go project, nor a toolchain build from source.

A number of Windows users currently duplicate stdlib code in order to patch it; see #34681.

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Nov 7, 2019

I wouldn't support this proposal. It would result in modules running slightly different, incompatible dialects of Go. That would prevent the ecosystem from scaling and would cause problems for large projects.

It's also not clear how this would be implemented. Would go build rebuild the toolchain and re-exec itself when the patches change?

I'm not familiar with the details of file flags in #34681. My (somewhat uninformed) opinion is that if a standard library package isn't working as desired, it should either be fixed (assuming it won't break other users) or forked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.