Platform reuse rules

[ola-rozenfeld]

It would be nice to be able to reuse existing platforms for defining other platforms. For example, we're using remote execution properties to turn on docker features like enabling network usage, or root access, or use a specific machine type for particular targets (multipool). Sometimes, extra constraints are needed to make sure the modified platform gets selected for a rule that has exec_compatible_with it.

I think extending or modifying single parent platform would be a nice syntactic sugar to have, e.g:

platform_from_parent(
parent,
constraint_values=... / extra_constraint_values=...
remote_properties=... / extra_remote_properties=...
)

[katre]

I think the proper semantics is to add a new attribute to the existing platform rule, called "parent", and allow the newly-defined platform to inherit non-conflicting constraint values from the parent.

Straw man example:

platform(name = "base",
    constraint_values = [
      ":foo",
      ":bar",
    ])

platform(name = "extended",
    parent = ":base",
    constraint_values = [
      ":baz", // Has the same constraint_setting as ":bar", and so replaces it.
    ])

One question: how should remote_execution_properties be handled when a platform extends another? Replace? Add some type of macro that can be expanded to the parent's remote execution properties?

[ola-rozenfeld]

Wait, if we could expand from ":some_platform" to its constraints and execution properties, we wouldn't even need the parent attribute, we could just reuse these directly, right? It would be a tad bulkier, but still achieve the main goal of not having to, for example, copy/paste the name of the docker image all over the build files.

[katre]

There's no current mechanism to expand the attributes of a target in another target. I'm trying to figure out what's needed to handle the remote_execution_properties. The simplest option is that it's completely replaced (or, if not changed, kept intact), but I suspect what you actually need is a way to modify the remote execution properties in the child.

Is it enough to simply add new text to the end of the remote execution properties? Would the beginning be better? Note that Bazel itself doesn't do anything with these properties until it's time to send a remote execution request, when they get converted into a proto. Ideally, Bazel wants to do the smallest amount of processing for these properties, since they are very dependent on the specific remote execution system that processes them.

[ola-rozenfeld]

There's no current mechanism to expand the attributes of a target in another target.

Oh, so what did you mean by "Add some type of macro that can be expanded to the parent's remote execution properties?"

Wait, is it okay that we could never remove constraints from the parent using this approach? (I'm not sure if there's a use case for it anyway)
I would prefer to duplicate both attributes as extra_constraint_values and extra_remote_execution_properties to clarify that they are added to the parent's instead of replaced (with extra_constraint_values producing an error if it's the same constraint setting and different value). And yes, for adding we could just append it to the end (slightly better for us, as it keeps the status quo). The properties are not unique, for example we can specify a platform with multiple machine types.
WDYT?

[katre]

When I said "Add some type of macro that can be expanded to the parent's remote execution properties" what I was suggesting was that the platform rule define a special macro, say "{PARENT_REMOTE_EXECUTION_PROPETIES}", and when a platform has the parent attribute set, the remote_execution_properties has this expanded. This would be a one-off for just this property, but as an example, we could do:

platform(name = "base",
    remote_execution_properties = """
      field1: foo
      field2 : bar
    """)

platform(name = "extended",
    parent = ":base",
    remote_execution_properties = """
      field1: override
      {PARENT_REMOTE_EXECUTION_PROPERITES}
      field3 : baz
    """)

Having duplicate attributes is confusing to users and violates how other Bazel rules work. When you define a java_library, you don't use deps in the base library and extra_deps in a library that depends on it, you use the same attribute in both places.

The simplest way to handle remote_execution_properties is to not have special macro handling, and just append at the end. The benefit of special macro handling is that it allows the user to control whether the parent properties are copies to the child or not, so it's more flexible.

[ola-rozenfeld]

Got it. Sure, sounds good! Let's do the macro, it's more flexible and more explicit. Thank you!