Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Avoid erroneous import cycle loop detection in ToReachMap() arising from tests #420

Closed
fabulous-gopher opened this issue Apr 21, 2017 · 1 comment

Comments

@fabulous-gopher
Copy link

From @sdboyer on March 6, 2017 20:48

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.

Copied from original issue: sdboyer/gps#179

@sdboyer
Copy link
Member

sdboyer commented Jul 25, 2017

this is likely going to involve changing exported APIs, so we have to finish it before 1.0/whatever

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants