bazelbuild / buildtools

A bazel BUILD file formatter and editor
Apache License 2.0
1.02k stars 416 forks source link

Migrating attributes between base-valued and string-valued #192

Open creachadair opened 6 years ago

creachadair commented 6 years ago

The motivation for this request is the recent change in the go_test rule defined by https://github.com/bazelbuild/rules_go, which deprecates

 library = "libname"

and replaces it with

 embed = [":libname"]

I tried various ways to script this with buildozer, and was unable to find one that worked. It seems to me there are various approaches one could take:

  1. Allow changing the "type" of an attribute (e.g., string to list-of-string and vice versa).
  2. Allow moving single-valued attributes into list-valued ones (currently this does not work, see [Note A]).
  3. Allow referencing values of other attributes in rewrite rules (probably complicated to implement).

I only really need to do the "forward" transformation once, to update our library= attributes to embed=. But when we import our project we have to rewrite them back, since the native rules don't support the new thing yet.

So far, the only solution I've found that might work would be to patch in a replacement Skylark macro, that pretends to the same interface modulo the deprecated thing. But that seems unduly fragile, given that the Go rules are changing frequently and there is no mechanism for testing against the changes.

Is this kind of rewrite something Buildozer could be modified to support? Offhand it seems like making the behaviour of move and add a little more forgiving might be the least complicated solution, but there may be other approaches I haven't thought of—and ultimately I don't care too much about the form, only the outcome.

[Note A]: move library embed leaves it as a string. Doing add embed NONCEVALUE turns the value into embed = "oldvalue" + ["NONCEVALUE"]. I had hoped to add a nonce to coerce it to a list then delete the nonce to leave the old value, but that did not work.

creachadair commented 6 years ago

It's also worth noting that going from list to string is actually the harder direction (and therefore probably the interesting one to consider first).

In writing our own Skylark rules, I've found shuttling back and forth between "string" and "string_list" to be common enough that you could also make a case for this to be its own fix category.

laurentlb commented 6 years ago

This should work, right?

remove library
add embed libname

To get the name libname, use print.

shahms commented 6 years ago

The suggestion is not something which can be automated given the current tools

shahms commented 6 years ago

(The use case here being the automated translation from open source BUILD files to Google-internal ones)

creachadair commented 6 years ago

To get the name libname, use print.

Unfortunately (in many respects) we are using copybara to do our imports. I don't know of any way to script this within the buildozer invocations in a copybara file. We switched to using copybara to get away from separate bash script workarounds—so I was hoping to avoid recreating that mess.