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

cmd/go: do not modify the 'go.mod' or 'go.sum' files if the 'go' directive indicates a newer-than-supported version? #34262

Open
bcmills opened this issue Sep 12, 2019 · 12 comments

Comments

@bcmills
Copy link
Member

commented Sep 12, 2019

@jayconrod notes in #34217 (comment):

One problem with this: adding go 1.14 to go.mod does not prevent old versions of the Go command from adding +incompatible versions. So a module could be broken unintentionally in a mixed environment.

There is a more general version of this problem: older versions of the Go toolchain may not know how to correctly interpret a go.mod file intended for a newer version, and thus could make arbitrarily wrong edits.

Probably we should have the go command error out instead of making changes if it thinks a go.mod file generated by a newer toolchain is incomplete or inaccurate.

@mvdan

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

I agree that this is generally a good idea, but - will it work well in practice? I've seen a number of issues along the lines of "a command modified my go.mod when I didn't expect it to", or "mod tidy and test add and remove deps in circles". Would those bugs not be a problem, or is the plan to fix most/all of them for 1.14 too?

@bcmills

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

This change would not address most of those issues, because they occur independent of the version in the go directive. (Generally they are fixes for inconsistencies between go mod tidy and other operations.)

@bcmills

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

But yes: in general the plan is to remove bugs that introduce unnecessary/erroneous edits to the go.mod file as soon as they are reported.

@mvdan

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

This change would not address most of those issues

Apologies if I was unclear; I mean that those bugs could be much more annoying with this change in place. For example:

  1. I'm on 1.14, go.mod is on 1.13 as it needs to support that version
  2. I try to do an action (such as go test ./...) which, due to a bug, tries to add a couple of lines to go.mod
  3. I get an error, which I can't solve without bumping the version in go.mod
@bcmills

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

If you're on 1.14 and your go.mod file says go 1.13, you would not get an error anyway: the Go 1.14 toolchain will still know how to interpret go.mod files using go 1.13 semantics, so there is no need to flag the mismatch as a potential error.

@mvdan

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

The error I mean is that my version would try to make changes to go.mod due to a bug (such as adding unnecessary dependency lines), and so would refuse to make those changes and error.

I'll stop trying to explain this hypothetical scenario if I'm still not making sense :)

@bcmills

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

The proposal is to produce an error only for versions of the go.mod file that the go command in use does not know how to interpret.

So the problem you describe could only occur for a go.mod file starting at go 1.15, and only for cases in which the file would be correctly interpreted by the go 1.14 toolchain save for some bug that causes an erroneous edit.

Ideally, we we have all of those bugs fixed before go 1.14 is released anyway, so I'm not worried about that actually happening. 🙂

@bcmills

This comment has been minimized.

Copy link
Member Author

commented Sep 19, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Sep 19, 2019

Change https://golang.org/cl/196519 mentions this issue: cmd/go: do not modify an existing go.mod file if its 'go' version is unsupported

@jayconrod

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2019

We should be careful here -- I want to think about this a bit more before moving ahead.

We already assume there may be incompatible changes to the go.mod language. If we add a new kind of statement, an old version of the go command will reject it in the main module and ignore it in other modules. The assumption now is that module, require, and go statements won't change and may be interpreted safely by old versions of the go command.

This change makes that stricter: old versions of the go command can't modify a main go.mod file with a greater go version, even if the file appears syntactically correct. Maybe that's okay: a greater go version indicates newer Go language features may be used, and we shouldn't expect older toolchains to build code marked that way.

But I think a few questions need to be resolved:

  1. Should the go command be even stricter and error out if it sees a go statement with a newer version in the main module regardless of whether it needs to make changes? This is probably the right thing to do if we need to change the interpretation of an existing statement. However, this should rarely be necessary, and we should avoid doing it if we can.
  2. Is this necessary for #34217? That issues suggests that a go 1.14 statement would make +incompatible versions non-canonical, which is a change to the meaning of an existing statement. I'd like to understand whether we can solve the problems described in that issue without such a change (but let's discuss over there).
  3. Do we need this change outside of #34217? Before I checked on how we handle go.mod syntax compatibility, I was thinking yes, but now I'm not sure. Maybe the go command can just print better error messages when it sees new go.mod syntax it doesn't recognize together with a go statement with a newer version.
@bcmills

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2019

Should the go command be even stricter and error out if it sees a go statement with a newer version in the main module regardless of whether it needs to make changes?

No: it won't do that for the compile step, so it shouldn't do it for loading either.

Is this necessary for #34217?

Rendered moot by withdrawing #34217.

Do we need this change outside of #34217?

On CL 198319, you suggested that we avoid writing vendor/modules.txt if the go version in go.mod is unsupported, because it might not include information needed by higher Go versions.

@rsc has also brought up the possibility of adding redundant information to go.mod files in order to allow the module loader to prune out parts of the dependency graph in some situations. If that happens, the situation will be similar: older go tools may try to remove redundant information that newer versions would retain, even though both tools interpret that information in the same way.

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@jayconrod

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2019

I think there are two main issues here:

  1. We want to avoid automatic edits that an old version of the go command can't understand without an explicit opt-in.
  • So far, we haven't needed to make any backward incompatible changes to go.mod, go.sum, and vendor/modules.txt (but for example, forbidding or ignoring +incompatible versions in #34217 would have been incompatible). We should definitely gate any automatic incompatible change on the go version.
  • We've made a few backward compatible changes (adding go statement, adding more information in vendor/modules.txt), and we have more proposed. I can't think of anything that would cause problems for automatic edits by old versions of the go command.
  1. We want to avoid two versions of the go command making conflicting changes.
  • This is came up in #34217 (+incompatible versions being added and removed) and in #33848 (go mod vendor will produce different vendor/modules.txt files in 1.13 and 1.14).
  • I think the general solution here is what we did with CL 201257. If we have this kind of conflict, a new version of the go command should emit either old or new syntax depending on the go statement in go.mod. An old version of the go command should still be able to edit go.mod and other files.
  • The consequence of this is that conflicts are still possible, but only if authors have opted into the new features by updating the go version in go.mod.
  • The drawback is that some new features will be less discoverable because developers need to opt in. This seems like a better tradeoff than preventing old versions of the go command from making edits though.
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.