gps
gps copied to clipboard
sdboyer
Avoid erroneous import cycle loop detection in ToReachMap() arising from tests
When tests are included as part of a ToReachMap() call, all import statements are collapsed together, regardless of whether they originate from the actual package or from tests in that package. As @adg and I observed on one larger project, this can create a situation where those test imports create the appearance of an import cycle. For example, if:
A -> B -> C
Then everything's fine. But, say that C has tests that import A; that's fine for a real build, because C's tests are a "sidecar" and not actually scoped-in to anything that imports C. If, however, we can't tell that the C -> A import link arises from a test in ToReachMap(), then we'll mistakenly identify it as an import cycle.
Right now, we've addressed this by not addressing it - cycle detection is simply skipped in ToReachMap().
An analogous delineation of "importable package" could also be made with build tag parameterization...but honestly, that situation seems pretty far out from where we are right now. It'd be hard to do, too. And, because it would be strictly additive if we were to support it later, we can cross that bridge when someone actually runs into the problem.
