unused_deps: Add exported dependencies before removing #1169
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| @@ -291,6 +291,8 @@ func printCommands(label string, deps map[string]bool) (anyCommandPrinted bool) | |||
| if hasRuntimeComment(str) { | |||
| fmt.Printf("buildozer 'move deps runtime_deps %s' %s\n", str.Value, label) | |||
| } else { | |||
| // add dep's exported dependencies to label before removing dep | |||
| fmt.Printf("buildozer \"add deps $(%s query 'labels(exports, %s)' | tr '\\n' ' ')\" %s\n", *buildTool, str.Value, label) | |||
There was a problem hiding this comment.
I tried to locate a utility function for obtaining a label's attribute but was unable to find one, so I used a bazel query invocation here. It'd be better to avoid the query. I'd appreciate any pointers on this in case I missed something.
|
Do I understand correctly that you're adding deps unconditionally even if it doesn't contain exports? Or will the command be no-op if there are no exports? |
In the case where there are no exports, the command becomes |
A dependency that is marked for removal might contain exports. Since exports are treated as if the parent explicitly depended on them, removal of such a dependency can cause issues since the exported dependencies will no longer be available to the parent.
This PR proposes to fix this by first adding the exports explicitly to the parent's deps before removing the dependency.