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: workspace-mode graph pruning needs to follow upgraded roots #49763

Open
bcmills opened this issue Nov 23, 2021 · 2 comments
Open

cmd/go: workspace-mode graph pruning needs to follow upgraded roots #49763

bcmills opened this issue Nov 23, 2021 · 2 comments

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented Nov 23, 2021

In CL 362754 we added a workspace graph-pruning mode, to fix a bug that we identified through code review (added in the work_prune test script in that CL).

I've been thinking some more about that scenario, and I've identified a related problem arising from the interaction between workspaces and graph pruning.

The module graph in that example has edges like:

example.com/a -> example.com/b v1.0.0 -> example.com/q v1.1.0
example.com/p -> example.com/q v1.0.0

But what if example.com/q itself has edges to new, not-previously-imported dependencies?

example.com/q v1.1.0 -> example.com/r v1.0.0

With current workspace pruning semantics, that edge will be pruned out: it is pruned out of a's dependency graph (because it is not relevant to any package imported by a), and it is not present in b's dependency graph (because b requires a different version of q entirely).

The result is that a by itself may be tidy, and p by itself may be tidy, but loading the dependencies of p from the combined module graph is missing a dependency.

@bcmills
Copy link
Member Author

@bcmills bcmills commented Nov 23, 2021

One possible fix is to change modload.updateWorkspaceRoots do do essentially the same iteration as the upgrade loop in modload.updatePrunedRoots.

However, that loop has a pretty unfortunate running time: any time a dependency is upgraded it reloads the full graph to recompute MVS results, and upgrades can potentially form long spirals. In a single-module setting, that's not terrible because we can record the result in the go.mod file for next time, but in workspace mode we don't have any obvious place to record it.

Loading

@bcmills bcmills closed this Nov 23, 2021
@bcmills bcmills changed the title cmd/go: workspace mode cmd/go: workspace-mode graph pruning needs to follow upgraded roots Nov 23, 2021
@bcmills
Copy link
Member Author

@bcmills bcmills commented Nov 23, 2021

Another possible fix is to change the workspace pruning mode to do the expansion itself: after each round of loading from the roots, re-enqueue the selected version of each root if that version's dependencies have not yet been loaded.
(That would take the form of an extra condition and perhaps another layer of looping in modload.readModGraph.)

If we end up encountering a spiral of upgrades, that doesn't allow us to prune out as many intermediate versions, but it does allow us to avoid reloading the graph multiple times — and upgrade spirals seem like they will be fairly short in practice and are relatively easy for the user to cut off by running go work sync.

CC @matloob

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant