Add -known_import flag #667
Conversation
|
Can one of the admins verify this patch? |
|
Hi. I have been keeping this functionality on my own fork for a while and have rewritten it enough times while rebasing that I'm wondering if you'd consider for the mainline. This is a quick-and-dirty solution that I've been using for the past few months. My overarching objective is to be able to update the externalResolver's cache. If this implementation doesn't suit do you think there's another one that would? |
|
Thinking over lunch: this is actually two changes
Please consider these independently. I'm happy to take on #1 asap. |
There was a problem hiding this comment.
Thanks for sending this! I like this change, but I'd prefer it as a repeatable command line option instead of a directive in WORKSPACE, at least for now. We may move toward having more directives in the root BUILD file in the future, but that's not designed yet.
See go/tools/builders/flags.go for how we're handling repeated flags elsewhere. That can be copied into Gazelle's main package.
It would be good to have a test for this. TestSpecialCases in resolve_external_test.go is probably the right place.
go/tools/gazelle/resolve/resolve.go
Outdated
| @@ -77,6 +77,12 @@ func NewLabelResolver(c *config.Config) LabelResolver { | |||
| switch c.DepMode { | |||
| case config.ExternalMode: | |||
| e = newExternalResolver() | |||
| case config.KnownMode: | |||
| er := newExternalResolver() | |||
There was a problem hiding this comment.
It would be better to pass additional imports to newExternalResolver instead of modifying the cache directly here.
There was a problem hiding this comment.
If I'm adding KnownImports to config.Config{...} should I move the default known imports from resolve_external.go to config.go?
There was a problem hiding this comment.
That's a good question. Let's keep them in resolve_external.go for now. Config is created in a lot of places for tests, so that would make this change more complicated. I may refactor it later.
go/tools/gazelle/config/config.go
Outdated
| @@ -107,6 +144,12 @@ const ( | |||
| // (declared in WORKSPACE). | |||
| ExternalMode DependencyMode = iota | |||
|
|
|||
| // KnownMode indicates imports should be resolved via ExternalMode unless | |||
There was a problem hiding this comment.
There doesn't need to be an extra mode of this. KnownImports can just be empty if there aren't any.
|
Ok. I'll turn this into a repeatable command line option. Thanks! |
pmcalpine
changed the title
Private repositories can take a long time to resolve, and if the repo host does not properly support go-import meta tags then gazelle may not be able to resolve them at all. This change adds a flag that can be specified multiple times to append arbitrary import paths to the cache of known imports when using the external import resolver.
pmcalpine
force-pushed
the
rebase-20170728
branch
from
dccbc11
to
ed1fe98
Compare
|
PTAL. |
|
Thanks, looks good! Jenkins test this please. |
Private repositories can take a long time to resolve, and if the repo
host does not properly support go-import meta tags then gazelle may not
be able to resolve them at all.
This change allows the author of WORKSPACE files to add specially
crafted comments which instruct gazelle to update the external
resolver's repo cache. As a result, gazelle can be instructed to skip
resolution of certain imports.