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: The modules feature acts in surprising ways #34025

Closed
aarondl opened this issue Sep 2, 2019 · 6 comments
Closed

cmd/go: The modules feature acts in surprising ways #34025

aarondl opened this issue Sep 2, 2019 · 6 comments
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@aarondl
Copy link

aarondl commented Sep 2, 2019

Experience Report

I was trying to use go mod why to determine why there was a dependency included in our build gave the answer (main module does not need package github.com/apache/thrift@v0.12.0). This is expected, as this command rarely produces any useful answers and you have to resort to go mod list or go mod graph with grep to figure out why.

Following that I tried to run a few commands and noticed many internet requests and re-downloads of packages it already had (gopkg.in/DATA-DOG/go-sqlmock.v1 is downloaded every time go mod why or go mod list or go mod tidy is run, but not go build, it's imported 0 times in our project and of course none of the introspection commands including go mod why can tell me why this is happening).

This led me to look into the many opened issues against go modules and the list is fairly expansive. I can be quoted as saying I will not move my open source libraries to Go modules until the feature has worked out the major kinks and I'm sad to report I still have not been able to move them. I think mostly the amount of surprise and inconsistency generated by the go command in module mode is the main problem. This is very unlike the rest of my Go experience, especially my experience with using the Go command ever since 1.0. It's been consistent, unsurprising, fast and generally an excellent tool. It does exactly what I tell it and expect in all circumstances until modules came along.

Problem

The go command has become surprising, inconsistent and sometimes slow when people run it in module mode. Commands that seem to be obvious introspection only commands (like go mod why or go list) will actually create changes to your project's go.mod file. Different subcommands (build vs list) will operate on different subsets of modules. Packages will be downloaded when you did not expect them to be, and it is very difficult to find out why a particular module is being downloaded or imported. Finally, when something does go wrong, the error messages are quite difficult to understand or debug which leads most users down a path of trial and error of replacing the now many environment variables and re-running the commands or deleting module caches which feels unsatisfactory as a resolution.

So why create this issue? I want to highlight the inconsistency and surprise, attempt to get the developers to think of it as a systemic problem and not just a bug here or there. We need some overall rules about how the go command should work, what sub-commands are blessed with the ability to reach out to the internet and download source, and which should NEVER attempt to do this (as an aside -mod=readonly probably shouldn't need to exist). We need better error messages, introspection and debug tools, and problematic features like replace need to be better thought out and refined.

Mostly, it is an attempt to have people re-evaluate the Go command in module mode from a user perspective, and see how the current implementation of modules inside it seems to be violating the principle of least surprise at the very least. Is there a version of the Go command that could be a better user experience? I just want to ask the question. Is it possible to step back and take a look at how it's working today and find a better way? Or are we stuck with the decisions we've made because of technical reasons (a requirement on full module graph resolution for every command which requires downloading of zips with go.mods in them)?

Examples

Read-only expected, but modify/talk to internet

Many commands (list, why, tidy) will inexplicably modify go.mod/go.sum (probably also downloading source as they do it). This is not expected and comes as a surprise to many users (myself included in Experience Report section).

Confusing/Insufficient errors

Lack of good introspection

Other surprises

Conclusion

There's not much actionable on this issue as it's more of a meta issue, I would hope a few of the developers read it before closing it and maybe have a hard think about how the Go tool is behaving these days (v1.12-v1.13) and if it's a desirable state.

On the positive side: I really appreciate the Go team's efforts on the module feature. In general I believe the design behind MVS, the Proxy/Sum/Index are all great and appear to work well (had the git.apache.org/thrift missing issue resolved by using proxy.golang.org as a stop-gap last night and I thought it was really great it worked exactly as intended to prevent disappearing packages (I realize their import path has changed)). Cheers to the Go team and other contributors for all the hard work so far.

@ALTree ALTree added the modules label Sep 2, 2019
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 2, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.14 milestone Sep 2, 2019
@ianlancetaylor
Copy link
Contributor

CC @bcmills @jayconrod

@bcmills
Copy link
Contributor

bcmills commented Sep 3, 2019

Thanks for the thoughtful feedback, but as you note, there isn't much actionable on this issue.

Nearly every point of confusion that you've raised already has an issue filed, and as we resolve each of those issues we are certainly doing our best to “step back and take a look […] and find a better way” to improve consistency and comprehensibility overall.

@bcmills bcmills closed this as completed Sep 3, 2019
@aarondl
Copy link
Author

aarondl commented Sep 3, 2019

@bcmills I think that there's been an unwillingness in the issues to consider a big change to the way it functions given the responses on the issues I've highlighted so it sounds like as an example many operations will continue to write to the disk and connect to the internet when they should perhaps not.

Have we thought about making these commands error out citing insufficient data? Have we thought about going more bundler/cargo/npm route with a dedicated "get stuff from the internet" vs all this automatic resolution at any cost (downloading source zip files to do go list)? These are the types of big design questions I was hoping this issue would help people start thinking about. Like I said it's a systematic problem throughout the tool, as each subcommand gains more and more power and less and less does one thing and does it well. As an example: go fmt talks to the internet... I mean surely you see that as being a big problem, no? I think the reason it comes up over and over again (there are many duplicates) is because it's surprising and undesirable behavior from a user's perspective.

Although this specific issue has no actions to carry out, your response is underwhelming and to hand-wave it away with "there are issues for all the issues you pointed out" is the exact problem I was trying to highlight: if we look at each of these in isolation it doesn't prompt big design changes, it makes them all look like individual issues but they're not, they're all part of a larger whole. If any of the responses in the issues hinted at "maybe we need some changes to the way this works" then I never would have created this issue in the first place. Had your response included hints that these sorts of "big picture" design/implementations have been carefully thought out and re-thought in the face of all these issues I never would have even replied to this issue's closing. But in the absence of all of that I felt compelled to reply once as a last ditch attempt. I hope that if not the issue itself, this response prompts the reconsideration of the design at large as I was hoping for even if the outcome is "this is the way we want it to be". Thanks for at least taking the time to read and reply, and again all your work on the project.

Edit: took out some of the passionate fire, apologies

@bcmills
Copy link
Contributor

bcmills commented Sep 3, 2019

Have we thought about making these commands error out citing insufficient data?

Yes. We have thought about it many times in the past (see the lengthy discussion on #27643), and are continuing to think about it (for example, in #32027, which you've already cited).

Have we thought about going more bundler/cargo/npm route with a dedicated "get stuff from the internet" vs all this automatic resolution at any cost (downloading source zip files to do go list)?

Yes, we are continuing to think about that as part of #29452.

It is not a matter of not having considered the alternatives, or of having rejected the alternatives. There are open issues for these things because they remain unresolved, and an appropriate resolution necessarily requires us to consider both small/tactical and large/strategic changes.

@thepudds
Copy link
Contributor

thepudds commented Sep 5, 2019

@aarondl FWIW, I also thought this was very thoughtful feedback.

You made many points, but I am only making two very narrow comments here:

1. You might want to consider linking to this issue from the "Experience Reports" wiki page. That can help other people find your commentary, and Experience Reports I think are one way the Go team is trying to organize broader feedback that not might be a specific bug report or specific feature change. See the top of the wiki page for a bit more details, and they have also been discussed in some of the GopherCon keynotes if you are interested in more of the philosophy behind asking the broader community to provide them.


2. If you are not already, you might want to try doing go mod why -m foo (note the -m), rather than or in addition to go mod why foo (without the -m). The -m indicates the argument is a module path, which usually casts a broader net and hence more often comes back with an answer. (Also, people are often doing go mod why in response to an error message, where module-related error messages often include module paths and hence makes sense to from a module path in an error message to go mod why -m with the -m).

In my personal experience, -m vs. no -m is more often the explanation for a main module does not need foo response from go mod why, and more rarely is that response due to something that is arguably a bug or missing feature like #27900.

That said, perhaps this could suggest that maybe the default behavior of go mod why should change, so that the default is a module path, or so that it suggests re-running with -m if no results, or some other tweak that helps people get an answer more frequently.

@aarondl
Copy link
Author

aarondl commented Sep 5, 2019

@thepudds Thank you for the feedback. I'm not sure it qualifies as a -real- experience report. It's missing a bit of detail that I'd expect from one, and I'd obviously be a bit too biased to correctly judge whether or not it should be included. I'll leave it to others to make the decision :)

As for go mod why -m vs no -m although I never state it I basically always include the -m because I'm generally looking for why modules in go.mod are imported. Whenever it doesn't turn up something I want I try the package version (omitting the -m). Even so, thanks for the tip, if I had missed that it would have been quite helpful. I don't disagree with changing it's behavior (why is it called go mod why when it operates on packages by default?). Perhaps we should create a new issue for that?

@golang golang locked and limited conversation to collaborators Sep 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants