Add -to_macro option to update-repos #480

blico · 2019-03-13T01:44:40Z

Addresses #301

The to_macro option tells Gazelle to write repository rules into a .bzl macro function rather than the WORKSPACE file. The user can then load the macro into their WORKSPACE and call it from there. This helps with the maintainability of the WORKSPACE file, especially for projects with many go_repository rules.

The format of the option is -to_macro=macroFile%defName

jayconrod

Thanks for working on this. This is really cool.

I have a few requests. Primarily, I think we should be able to generate a macro file from scratch.

jayconrod · 2019-03-13T17:03:37Z

README.rst

@@ -409,6 +413,10 @@ The following flags are accepted:
 |                                                                                                                                                         |
 | Gazelle will not process packages outside this directory.                                                                                               |
 +----------------------------------------------------------------------------------------------------------+----------------------------------------------+
+| :flag:`-to_macro macroFile%defName`                                                                      |                                              |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
+| Tells Gazelle to write repository rules into a .bzl macro function rather than the WORKSPACE file.                                                      |


It should probably be pointed out here that Gazelle won't see repositories declared in the macro when updating build files. That means that it won't use custom repository names, and it will go out to the network when trying to determine repository roots.

This is a solvable problem of course, but let's leave it for another time.

Hmmm, this was an oversight on my part. We are pretty dependent on the functionality you mentioned, so I would like to get this done sooner than later.

From a quick glance, it looks like one way to get this done would be to add a new flag named something like -go_repositories_macro=path. If the flag is set we call LoadMacroFile from newFixUpdateConfiguration. We then call FixWorkspace and FixLoads for both the macro and workspace files, and also pass both files to repo.ListRepositories.

WDYT?

#132 describes a # gazelle:repository directive that would declare this information in WORKSPACE without requiring command line arguments. That would be a useful escape hatch. I just opened #483, describing a more complete solution. In WORKSPACE, we could have a directive that tells Gazelle to read a macro in a separate .bzl file. Bonus points if we can update existing rules in that macro without needing the -to_macro flag.

Until those are implemented, we should mention this problem here, since the problem won't be obvious to people trying this out.

I think we can simplify this problem a bit to start with. We can have a default bzl file like //go_repositories.bzl unless it's overridden by some the Gazelle directive proposed in #483. Then --to_macro becomes a boolean flag, and write go_repository rules to //go_repositories.bzl when the flag is set. update and fix will first check if there is a //go_repositories.bzl. If so, read those rules first. Then we should allow people to override rules in //go_repositories.bzl in WORKSPACE files. For example, if they want to apply a patch to some go_repository, they can redefine the rule with the same name in WORKSPACE. Then //go_repositories.bzl contains no manual edits.

I can imagine large repositories needing multiple macros for different sets of dependencies. When language extensions eventually support update-repos functionality, it should be possible to update macros for non-Go dependencies, too. Basically, I want to make sure we stay flexible here.

rule/rule.go

cmd/gazelle/update-repos.go

jayconrod · 2019-03-13T17:31:36Z

cmd/gazelle/integration_test.go

+load("@bazel_gazelle//:deps.bzl", "go_repository")
+
+def go_repositories():
+    go_repository(name = "org_golang_x_net", build_file_generation = "off", commit = "66aacef3dd8a676686c7ae3716979581e8b03c47", importpath = "golang.org/x/net")


I think the output is pretty difficult to read with all of the attributes on one line. In Rule.sync, could you set the ForceMultiLine flag on rules with more than one attribute?

It looks like it is not currently possible to print out the attributes on more than one line :/. I created a PR to fix this issue here: bazelbuild/buildtools#580

Thanks for looking into this. I saw you set the ForceMultiline flag. So that's getting ignored? Hmm, that's unfortunate.

This change has been merged into buildtools. So we will need to update buildtools within Gazelle sometime soon.

jayconrod · 2019-03-15T22:24:16Z

README.rst

@@ -409,6 +413,10 @@ The following flags are accepted:
 |                                                                                                                                                         |
 | Gazelle will not process packages outside this directory.                                                                                               |
 +----------------------------------------------------------------------------------------------------------+----------------------------------------------+
+| :flag:`-to_macro macroFile%defName`                                                                      |                                              |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
+| Tells Gazelle to write repository rules into a .bzl macro function rather than the WORKSPACE file.                                                      |


#132 describes a # gazelle:repository directive that would declare this information in WORKSPACE without requiring command line arguments. That would be a useful escape hatch. I just opened #483, describing a more complete solution. In WORKSPACE, we could have a directive that tells Gazelle to read a macro in a separate .bzl file. Bonus points if we can update existing rules in that macro without needing the -to_macro flag.

Until those are implemented, we should mention this problem here, since the problem won't be obvious to people trying this out.

rule/rule.go

jayconrod · 2019-03-15T22:56:50Z

cmd/gazelle/integration_test.go

+load("@bazel_gazelle//:deps.bzl", "go_repository")
+
+def go_repositories():
+    go_repository(name = "org_golang_x_net", build_file_generation = "off", commit = "66aacef3dd8a676686c7ae3716979581e8b03c47", importpath = "golang.org/x/net")


Thanks for looking into this. I saw you set the ForceMultiline flag. So that's getting ignored? Hmm, that's unfortunate.

blico · 2019-03-25T23:21:09Z

Sorry for the delay, I was out sick most of last week.

jayconrod

Nice! I tried this out, and it seems to work pretty well. Thanks again for working on this.

I'll merge this and update buildtools on master. It doesn't look like we'll need any other change to get multi-line printing.

jayconrod · 2019-03-27T13:13:28Z

README.rst

@@ -409,6 +413,10 @@ The following flags are accepted:
 |                                                                                                                                                         |
 | Gazelle will not process packages outside this directory.                                                                                               |
 +----------------------------------------------------------------------------------------------------------+----------------------------------------------+
+| :flag:`-to_macro macroFile%defName`                                                                      |                                              |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
+| Tells Gazelle to write repository rules into a .bzl macro function rather than the WORKSPACE file.                                                      |


I can imagine large repositories needing multiple macros for different sets of dependencies. When language extensions eventually support update-repos functionality, it should be possible to update macros for non-Go dependencies, too. Basically, I want to make sure we stay flexible here.

jayconrod · 2019-03-27T18:24:43Z

It looks like there have been some breaking changes in buildtools since we last updated, and updating to the latest version will be non-trivial. I don't have time to do it right now, maybe this weekend. Filed #489 to track the update.

Add -to_macro option to update-repos

97de491

blico requested review from ianthehat and jayconrod as code owners March 13, 2019 01:44

jayconrod reviewed Mar 13, 2019

View reviewed changes

PR feedback

21aad1f

jayconrod mentioned this pull request Mar 15, 2019

Directive to reference macros from WORKSPACE #483

Closed

jayconrod reviewed Mar 15, 2019

View reviewed changes

PR feedback v2

e29c199

jayconrod approved these changes Mar 27, 2019

View reviewed changes

jayconrod merged commit 44bce61 into bazel-contrib:master Mar 27, 2019

achew22 mentioned this pull request Jun 19, 2019

Auto-generate go_repository rules. grpc-ecosystem/grpc-gateway#957

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add -to_macro option to update-repos #480

Add -to_macro option to update-repos #480

blico commented Mar 13, 2019

jayconrod left a comment

jayconrod Mar 13, 2019

blico Mar 15, 2019

jayconrod Mar 15, 2019

linzhp Mar 22, 2019

jayconrod Mar 27, 2019

jayconrod Mar 13, 2019

blico Mar 15, 2019 •

edited

Loading

jayconrod Mar 15, 2019

blico Mar 25, 2019

jayconrod Mar 15, 2019

jayconrod Mar 15, 2019

blico commented Mar 25, 2019

jayconrod left a comment

jayconrod Mar 27, 2019

jayconrod commented Mar 27, 2019

Add -to_macro option to update-repos #480

Add -to_macro option to update-repos #480

Conversation

blico commented Mar 13, 2019

jayconrod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blico Mar 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blico commented Mar 25, 2019

jayconrod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayconrod commented Mar 27, 2019

blico Mar 15, 2019 •

edited

Loading