-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: add a flag to ignore build constraints when listing packages #42504
Comments
The main counter-argument I see to this flag solution is that it could make However, I think this third mode would be fairly niche, and we could explain it rather simply: list packages while ignoring build constraints, hence not loading packages as if they were to be built. This helps us limit |
I support the idea here. I've often wanted to enumerate dependencies across all build configurations and it's awkward to do currently (I've built custom solutions to do it in the past). As for the name of the flag, how about |
The only reason I tried to avoid "all builds" as a flag name is because, to me, that implies "load and possibly build all build constraint combinations". As if |
For govendor, years ago, I had to re-build |
I hadn't thought about |
This would be useful. We already have some of the code for this in In As for the CLI, maybe overload This should be restricted to |
That would be fine for my use case. The packages I want to list will eventually be loaded in some build configuration, so the
Overloading |
To clarify in case anyone wonders - this kind of workaround about copying entire directories doesn't help solve the issue in this thread. For example, think packages which contain zero files matching the current build constraints, which wouldn't show up in the |
We will need a bit of care with import cycles, though. When ignoring build tags, it is possible to end up with import cycles that are otherwise rejected. |
It transforms the ouptut of go list from a DAG to a true graph, and there are real world cases of this. It would never be feasible to have a version of the go/packages API that sat on top of this for instance (which if it was just a special tag would end up happening) |
@ianthehat, FWIW I suspect that the existing |
To answer #42504 (comment), |
We briefly spoke about this on yesterday's tools call, and we seemed to agree that there is consensus that we should do this. We've found at least two good use cases: enumerating dependencies across all build environments, and support for build-tag-agnostic features in go/packages or gopls. Tentatively moving this to NeedsFix and milestoning for 1.17. |
Assuming this is exactly what tidy and vendor do (not quite "ignore all constraints" because we would still respect What to call the flag? Internally we make this work by setting a special tag |
I slightly worry that some users might confuse that with |
@mvdan, at least |
I lean towards |
Is this going to happen for 1.17? |
I don't believe anyone has started work on this, and as a new feature it seems like it should wait until 1.18. |
Change https://golang.org/cl/332571 mentions this issue: |
Any update on this? While we wait on this, we've planted a very slow workaround: repeat |
@bcmills, does
|
By providing a mechanism to list all files, gopls could theoretically implement package construction in a post-processing pass. This could facilitate several gopls features:
|
if since go1.17, |
As we're going to run this on a big tree of vendored packages, I anticipate this is going to be much slower than our current workaround, which is calling Would love to see the proper solution, as @rsc had suggested above |
These two functions together duplicated much of the functionality of modload.Lookup. Use that instead in modcmd.vendorPkg, and reduce the modload surface area. Updates #42504 Updates #40775 For #26904 Change-Id: Ib8aaac495d090178dd56971aef9e5aa44ffa818b Reviewed-on: https://go-review.googlesource.com/c/go/+/332571 Reviewed-by: Michael Matloob <[email protected]> Run-TryBot: Bryan Mills <[email protected]> Reviewed-by: David Chase <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Jay Conrod <[email protected]> Auto-Submit: Bryan Mills <[email protected]>
Sorry to see this pushed back again, given that the functionality is already there, from what had Russ mentioned above. We expect the result of this implemented should considerably improve developer's experience for monorepo cases, reducing scanning times of (big amounts of) vendored code. |
@yarikk this feature would indeed be very helpful. If you want it to be implemented faster, giving a thumbs up on the original post or volunteering as a code contributor are the two options that come to mind. Beyond that, I don't think that commenting regularly is going to help :) |
I started investigating this, and have a dirty hack that makes My first change was to overload the The next change is to make And the final change is to make the Problems here:
|
@matloob, @samthanawalla: if I recall correctly, this issue is mostly just waiting on a decision about how to specify the flag, and this would be a nice usability improvement for tools & scripts that use (Otherwise, those scripts have to try various package-specific combinations of build tags to get the complete set that will be considered by This may also be needed for |
I realise that the point of
go list
is to list packages, and I realise that one must generally obey build constraints when loading packages. For example, it makes no sense to try to type-check all the Go files in a package while ignoring build constraints, because there will likely be duplicate definition errors if declarations are split byGOOS
orGOARCH
.Having said that, it can be very useful to merely list or traverse a set of packages in a way that build constraints are completely ignored. The most common use case is: what packages are potentially imported by the current package, directly or indirectly, across all build configurations?
This question is valid, for example, if one wants to copy a package and all of its transitive package dependencies.
go list -deps <package>
does not work in general, because if I run the command on Linux, I would not be including imports which are only used on Windows.The closest thing we have right now is
go mod vendor
, which copies all transitively imported packages by all the packages in the current module into the vendor folder, across all build constraints. Socmd/go
already has the machinery to traverse a package dependency graph while ignoring build constraints, it seems.--
Here ends the problem statement, and begins my initial suggestion for a solution: a
go list
flag to ignore build constraints. Here are some example use cases:List all the packages potentially depended on by a given package:
go list -deps -newflag <package>
List all the packages under the current directory tree, not just for the current platform:
go list -newflag ./...
I'm not sure what this new flag could be called. Perhaps
-nobuild
or-anybuild
.The flag would also restrict what
go list
can do. For example, using-newflag
along with-export
or-compiled
would be an error, because it does not make sense to load/build packages when we're ignoring build constraints. Similarly,-newflag -json
would always omit some fields likeIgnoredGoFiles
, since they make sense only when following build constraints.This idea has been discussed briefly before, but as far as I know no problem statement or solution has been raised as an issue yet. I'm not using a proposal title and label; as far as I know that's only necessary if we need the proposal review committee to intervene.
cc @bcmills @jayconrod @matloob @rsc for cmd/go
cc @ianthehat @dominikh @myitcv @rogpeppe @kardianos for golang-tools and some previous issues
The text was updated successfully, but these errors were encountered: