Skip to content
This repository has been archived by the owner on Mar 3, 2021. It is now read-only.

Support running without command file #2

Closed
negz opened this issue Jun 20, 2018 · 5 comments
Closed

Support running without command file #2

negz opened this issue Jun 20, 2018 · 5 comments

Comments

@negz
Copy link

negz commented Jun 20, 2018

Hello,

I'd like to use this convenience buildozer rule, but wish it were a little less opinionated. Currently it seems to require Buildozer commands be fed in via a file while I would prefer to pass them in as an argument, for example bazel run //:buildozer -- somecommand.

The context here is that for historical reasons my Bazel monorepo has a lot of Go vendor directories. We're also using an older Go vendoring tool that helpfully generates go_test rules for said vendor directories. This means our BUILD file generation strategy looks something like:

$ bazel run //:gazelle -- fix services/myservice
$ buildozer delete //services/myservice/vendor/...:%go_test

I'm not a Bazel expert and could be wrong, but I don't believe there's any way to specify a label like //.../vendor/...:%go_test - if there were I'd be happy to throw such a command in //:buildozer_commands.txt and stick with the existing pattern.

@ash2k
Copy link
Contributor

ash2k commented Jun 21, 2018

I don't see how being able to pass commands via command line changes anything? It will still not be possible to specify labels like //.../vendor/...:%go_test.

@ash2k
Copy link
Contributor

ash2k commented Jun 21, 2018

Why cannot you put all your delete //services/myservice/vendor/...:%go_test commands into a file?

@negz
Copy link
Author

negz commented Jun 21, 2018

@ash2k Doing so would require us to continually keep our commands file updated with all the paths we wanted buildozer to manage. That's a lot of paths owned by a lot of teams so I think that pattern would break down pretty fast.

You're correct that the wildcard //.../vendor/...:%go_test still won't work when passed as an argument to buildozer directly, but supporting arguments in addition to the commands file would allow us to keep our current pattern, but using bazel run rather than an external buildozer binary.

i.e. Today our documentation instructs engineers to run the following to update their BUILD files for whatever path of the monorepo they're working on:

$ bazel run //:gazelle -- fix services/myservice
$ buildozer delete //services/myservice/vendor/...:%go_test

I'd like to be able to simply change that documentation to be:

$ bazel run //:gazelle -- fix services/myservice
$ bazel run //:buildozer -- delete //services/myservice/vendor/...:%go_test

I feel that this is a more sustainable/maintainable pattern than having a large group of people maintain a buildozer commands file and run buildozer against the entire monorepo regularly.

@ash2k
Copy link
Contributor

ash2k commented Jun 21, 2018

Ok, I understand your use case now, thank you. Feel free to submit a PR (don't forget to sign the CLA).
However, if I were you, I'd instead do these two things:

  • Open an issue (or a PR) with buildozer to add support for label wildcards like what you need //.../vendor/...:%go_test
  • Instead of recommending engineers to run buildozer delete //services/myservice/vendor/...:%go_test add a bazel target that generates the commands file with directories that you want fixed. Use something like find . -type d -name vendor in the script. And then have the buildozer rule depend on that generated commands file. That way your engineers would not need to maintain that pattern per service and only run bazel run //:buildozer to fix all vendor folders. This may require some experimentation with glob() to declare dependencies of that generator rule... I'm not 100% it will work nicely, but worth a try.

@ash2k
Copy link
Contributor

ash2k commented Oct 27, 2020

I think this was implemented in #107.

@ash2k ash2k closed this as completed Oct 27, 2020
rmohr pushed a commit to rmohr/bazel-tools that referenced this issue Mar 23, 2021
rmohr pushed a commit to rmohr/bazel-tools that referenced this issue Mar 23, 2021
fix: change atlassian path to ash2k path, fix atlassian#2
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@ash2k @negz and others