bazelembedded / modular_cc_toolchains

MIT License
3 stars 1 forks source link

Decide on mechanism for enable/deps and requires/implies #9

Open nathaniel-brough opened 1 year ago

nathaniel-brough commented 1 year ago

We may want to have a feature depend on another. At the moment, there is no strict dependency tracking that is integrated into the cc_toolchains API. e.g. say we have two features, A, B.

Currently, there are two separate mechanisms to describe the relationships between these two features. The of A "requires" B then A cannot be enabled without B. But if A "implies" B then B is automatically enabled when A is.

The question is; do we need both of theses mechanisms or are these mechansims already possible using basic Bazel configurations e.g. selects and config_settings.

Do we want "dependant" features to automatically enable there dependants or do we want them to be enabled/disabled independantly.

tpudlik commented 1 year ago

@trybka might have opinions about this!

tpudlik commented 1 year ago

We had some discussions about this and related issues in person yesterday. I thought I'd record some of my comments here for posterity:

  1. It seems intuitive that when a cc_feature has a dep on another cc_feature, it should "imply" it.
  2. However, the toolchain configuration must still explicitly list both of the features. This is because we rely on the (ordered) feature list to determine the order of the flags.
  3. In my opinion relying on the ordered feature list is better than using a depset (based on deps between cc_features) to order the flags.

    • The advantage of the depset is that it's a well-estabilished Bazel construct, and allows expressing ordering relationships explicitly and precisely (in an ordered list [A, B, C], are we saying A must be before both B and C, or only before B and we tagged C at the end? The depset is more precise about this).
    • But the advantages of the list are many:
      • It's easy to visualize the resulting flag ordering (the list of features and list of flags provided to an invoked tool are both paths, in the graph sense; the depset is a more complex graph that gets converted to a path), and it's easy to tweak it (just reorder the list; modifying the deps graph is trickier).
      • The ordering of the flags is determined at the stage of constructing the toolchain configuration by pulling together the features, rather than at the stage of defining the cc_features. This is critical if the "common" cc_features are to be reused by multiple projects that may require different flag ordering.
    • A problem with the feature list is that buildifier will by default order lists alphabetically for you. (I.e., although Starlark lists are in theory ordered, in practice lists in BUILD files are treated as unordered sets). One way to work around this is to define a macro that just returns the list of its arguments (def ordered_list(*args): return [*args]?), and use it here. The usage of this macro will document, both to humans and to buildifier, that the order of the entries is significant.