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

Automatically apply buildozer commands from warnings #18

Closed
ittaiz opened this issue Jul 28, 2017 · 13 comments
Closed

Automatically apply buildozer commands from warnings #18

ittaiz opened this issue Jul 28, 2017 · 13 comments

Comments

@ittaiz
Copy link
Member

ittaiz commented Jul 28, 2017

Hi,
I've heard that the internal version of Bazel-watcher (iBazel) knows to watch for log warnings in Bazel outputs which are buildozer commands and to automatically apply them.
The need for this (for me) is to be able to automatically apply strict java/scala deps warnings.
This means a developer can use code from transitive targets, the rule will warn against it with a buildozer command (saying which target needs to be added explicitly where) and ibazel will automatically apply it.
Is there an interest to extend ibazel to that area?

@achew22
Copy link
Member

achew22 commented Jul 31, 2017

Huh, this is a very interesting idea. I don't think I've yet run into that in iBazel but it doesn't mean that it doesn't do it. Do you have any ideas on how this would work? Are there any rules_ that are emitting buildozer commands to execute in case of failure?

In the specific case of Gazelle it might be reasonable to just blaze run //:gazelle whenever there is an import error but I'm not sure how to actually detect that. WDYT?

@ittaiz
Copy link
Member Author

ittaiz commented Jul 31, 2017

rules_scala does this
What is Gazelle?
BTW, @cgrushko is the one who told me about this feature in internal ibazel so maybe he has more info

@cgrushko
Copy link

IIUC it's nothing more than ibazel looking at the output of Bazel and running some commands when it sees something it recognizes (i.e., a regex --> command map).

@mprobst
Copy link
Contributor

mprobst commented Jul 31, 2017 via email

@ittaiz
Copy link
Member Author

ittaiz commented Sep 6, 2017 via email

@achew22
Copy link
Member

achew22 commented Sep 6, 2017

That seems like an extremely reasonable thing to do. If you wanted to implement it with a whitelist of regexps (to prevent running something scary) scanning the output from Bazel I think you could implement that without a huge amount of effort.

Why don't you take a whack at it? If you run into problems, feel free to comment on here.

@achew22
Copy link
Member

achew22 commented Sep 23, 2017

Useful notes to future implementers:

Go will give you access to the commands stdout pipe with cmdReader, err := cmd.StdoutPipe(). Rather than making the command's pipe be stdout you can intercept it line by line and print it and search for matching patterns.

A more complete example can be found at https://nathanleclaire.com/blog/2014/12/29/shelled-out-commands-in-golang/

@jjudd
Copy link

jjudd commented Mar 23, 2018

I have a rough and rudimentary version of this working. I'm working on this in conjunction with unused_deps for rules_scala. I'm going to keep working on it and polish it up as we get unused_deps working. When I'm confident it works well I'll open a PR for the issue here.

@jjudd
Copy link

jjudd commented Apr 27, 2018

Quick update on this: we have this working internally and are currently rolling ibazel + this change out to all of engineering. We plan to create a PR soon (1-2 weeks). If this is blocking anyone please let me know and I can try to get the PR ready faster than that.

@ittaiz
Copy link
Member Author

ittaiz commented Apr 27, 2018 via email

@jjudd
Copy link

jjudd commented Apr 27, 2018

At the moment, it is hardcoded to buildozer. The goal before merging this is to have there be a config file of some sort that provides regexes with capturing groups indicating the command and arguments.

If we wanted to extend it to do more than that, we could add a blacklist regex.

This should cover 2. As for 1, the blacklist should probably cover that. I'm not sure how it works with maven_jar. We have a fork of bazel-deps, which uses java_import_external and scala_import_external, so we can use strict/unused deps with maven dependencies. We're going to open a PR for that soon as well. It's made dealing with third party dependencies quite nice.

@ittaiz
Copy link
Member Author

ittaiz commented Apr 27, 2018 via email

@jjudd
Copy link

jjudd commented Apr 27, 2018

True. We're on a branch which has a jars_to_labels provider, so things are working for scala_import_external internally with buildozer suggestions. We'll work on getting PRs for all these open over the next couple weeks.

Sorry to drag this thread off topic 😄

borkaehw pushed a commit to lucidsoftware/bazel-watcher that referenced this issue Jul 30, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Jul 30, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw pushed a commit to lucidsoftware/bazel-watcher that referenced this issue Jul 30, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Jul 30, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Jul 31, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Jul 31, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 2, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 2, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 2, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 2, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 3, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 9, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 14, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

Fixes: bazelbuild#18
borkaehw added a commit to lucidsoftware/bazel-watcher that referenced this issue Aug 14, 2018
iBazel now watches the log warning from Bazel outputs. Users can watch for
specific commands and apply them automatically. The command matching is
implemented by regex and it is configurable.

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

No branches or pull requests

5 participants