Trim hidden packages from (root) PackageTrees #1271
Conversation
This has general utility, independent of ListPackages() being improved to avoid unnecessary disk reads in typically-hidden directories.
This consolidates calls to parse the root project's code tree into a single place, so as to avoid confusion about the standard, expected way that dep's root project tree is to be created. It may not be fully suitable for all status cases, but standardization is crucial for init and ensure.
These should never have been global failures within ListPackages(), as they are entirely package-specific information.
sdboyer
requested review from
carolynvs,
darkowlzz and
ibrasho
as code owners
sdboyer
force-pushed
the
pkgtree-hidden-trimming
branch
from
cc07279
to
0951d5d
Compare
|
i think this is just down to needing some tests specifically for |
We only perform a single allocation for all []string, and are careful to copy all possible error values via reflection.
While this would be possible to implement, it would require writing entirely new entry logic into wmToReach(). That seems like a sufficiently strong indication of breaking domain assumptions that it's preferable just to drop the parameter, which was questionable in the first place.
|
i went back and forth on a however, going some fairly exhaustive combinatorial input tests tests demonstrated that there are semi-absurd edge cases where it would be impossible to |
What does this do / why do we need it?
This deals with a few interrelated issues:
-voutput is including packages that aren't actually being considered, which is misleading.pkgtree.ListPackages()aborting entirely.dep statusinvokesPackageTree.ToReachMap(), vs.dep initanddep ensure.This doesn't quite fully fix the fourth item, but it gets us close.
The crux of the fix here is the introduction of
PackageTree.TrimHiddenPackages(), which generates a newPackageTreethat has hidden, unreachable and optionally ignored packages removed from it. This is not the most efficient way of doing it, as it meansListPackages()is still doing analysis of directories that we may end up throwing away - but it's much cleaner to bolt on top this way, and this isn't mutually exclusive with adding that logic later anyway.It may also be appropriate to move the
checkErrors()function currently in cmd/dep/ensure.go over into the pkgtree package, as its behavior is sufficiently general.What should your reviewer look out for in this PR?
nothing in particular
Which issue(s) does this PR fix?
Honestly, it's kinda amazing our CI works right now - dep can't run on itself.