feat: support only updating existing BUILD files #1921
Conversation
| @@ -73,7 +86,7 @@ func (*Configurer) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) | |||
| func (*Configurer) CheckFlags(fs *flag.FlagSet, c *config.Config) error { return nil } | |||
|
|
|||
| func (*Configurer) KnownDirectives() []string { | |||
| return []string{"exclude", "follow", "ignore"} | |||
| return []string{"generation_mode", "exclude", "follow", "ignore"} | |||
There was a problem hiding this comment.
This is the directive named used by the Aspect plugins today. I think we need something better here that doesn't include the word "mode" which already has a meaning within gazelle.
Ideas?
There was a problem hiding this comment.
build_file_action?
create_build_files yes/no?
There was a problem hiding this comment.
I am going to try to this patch at Airtable. I am thinking instead of manually specifying generation_mode to isntead do something like (pseudocode) collectionOnly := f == nil !any(file.endswith('.go') for file in ents), since we have go code spread throughout the tree
There was a problem hiding this comment.
I think different repos will have different preferences and we can't really make any assumptions based on which files exist.
There was a problem hiding this comment.
Assuming you are in David's situation, could you still leverage generation_mode: update to simplify your language implementation or would you need to resort to create with the same hack mentioned in the linked issue?
I'm asking because adding a new directive comes with a complexity cost that may not be justified if it doesn't address many use cases. Do you see a way to simplify what a language would need to do to get this behavior without adding a new directive?
There was a problem hiding this comment.
I've changed my hack described in the issue to a patch that's essentially the same as this PR. The hack required manually walking the fs (in addition to gazelle walking it) which is bad in many ways, performance being a big one.
I think no matter what a new API is needed so gazelle can generate the language.GenerateArgs with the correct files. It could be an API in the Configure phase, but today there are no APIs there that change gazelle's behaviour and adding such an API sounds far more complex then adding a directive.
jbedard
force-pushed
the
1832-gen-mode
branch
from
c4a69da
to
bcf35e8
Compare
jbedard
force-pushed
the
1832-gen-mode
branch
4 times, most recently
from
496ec9e
to
fb71250
Compare
jbedard
force-pushed
the
1832-gen-mode
branch
from
fb71250
to
0e6a43f
Compare
jbedard
force-pushed
the
1832-gen-mode
branch
from
0e6a43f
to
3569d83
Compare
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
Close #1832
What type of PR is this?
Feature
What package or component does this PR mostly affect?
What does this PR do? Why is it needed?
Add an option to enable only updating existing BUILDs and not creating any new ones.
I think this is common among extension authors, normally implemented with a quick-exit within
Configureif the*rule.Fileisnil. However all the subdirs then need to be manually traversed to find files in subdirectories with no BUILD.I've always maintained a patch similar to this PR to avoid plugins having to do manual fs operations.
Which issues(s) does this PR fix?
Fixes #1832
Other notes for review