Skip to content
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

subdirectories are affected by load statements flipping gazelle into legacy mode #104

Closed
jmhodges opened this issue Jan 27, 2018 · 1 comment
Labels

Comments

@jmhodges
Copy link
Contributor

jmhodges commented Jan 27, 2018

So, if you have a load statement with the old go_proto_library statement like

  load("@io_bazel_rules_go//proto:go_proto_library.bzl", "go_proto_library")

in a parent BUILD.bazel file, the BUILD.bazel's in subdirectories will not have their proto targets updated even if that subdirectory has no old go_proto_library loads in it. This seems to be a automatic-legacy-mode thing triggering too much.

This also happens even if go_proto_library is not used in the parent BUILD.

I hit this when I left the load statement, unused but present, behind in the parent BUILD that I was moving the proto library out of (and trying to move it to a subdirectory as its own package).

@jmhodges jmhodges changed the title subdirectories are affected by load statements filipping gazelle into legacy mode subdirectories are affected by load statements flipping gazelle into legacy mode Jan 27, 2018
@jayconrod
Copy link
Contributor

Sorry, I meant to respond to this earlier.

This is mostly working as intended. If the proto mode isn't explicitly set, Gazelle attempts to infer it from a build file in the directory. The inferred proto mode is "inherited" by subdirectories. This is under the theory that if are using the legacy proto rules in one directory, you probably don't want to accidentally start using the new proto rules in a subdirectory.

The logic here ended up being more complicated and opaque than I like. However, I think migrating to new proto rules is probably too big a change to be automatic, and I didn't want to break people using the legacy proto rules by requiring a new option. At some point, we'll remove the legacy rules, and Gazelle won't generate filegroup rules for them anymore.

One thing that does not seem to be working is that if you explicitly set proto mode to default, it looks like it may drop down to legacy mode in a subdirectory. I should fix this so the inference does not override user intent.

@jayconrod jayconrod added the bug label Jan 30, 2018
jayconrod pushed a commit to jayconrod/bazel-gazelle that referenced this issue Jan 31, 2018
If the proto mode is explicitly set to the default mode (via the
command line or directive), it will no longer be overridden by an
inferred legacy or disabled mode. It can still be overridden by an
explicit directive in a subdirectory.

Fixes bazelbuild#104
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Jan 31, 2018
go_repository will no longer pass arguments to Gazelle on the command
line when the corresponding attributes are not explicitly set.

This gives Gazelle more control over the defaults. For example,
Gazelle allows an inferred proto mode to override the default mode,
but not when the default mode is explicitly requested.

Related bazelbuild/bazel-gazelle#104
jayconrod added a commit to bazelbuild/rules_go that referenced this issue Jan 31, 2018
go_repository will no longer pass arguments to Gazelle on the command
line when the corresponding attributes are not explicitly set.

This gives Gazelle more control over the defaults. For example,
Gazelle allows an inferred proto mode to override the default mode,
but not when the default mode is explicitly requested.

Related bazelbuild/bazel-gazelle#104
jayconrod added a commit that referenced this issue Feb 1, 2018
…109)

If the proto mode is explicitly set to the default mode (via the
command line or directive), it will no longer be overridden by an
inferred legacy or disabled mode. It can still be overridden by an
explicit directive in a subdirectory.

Fixes #104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants