This also needs some documentation. The quick guide is:

1. Edit features.yaml (in the root of the repo)
2. Run make generate-features (in the root of the repo)
3. Run make doc (in the root of the repo)

Murdock results

✔️ PASSED

aa356d8 ci: add test that features_existing.inc.mk is up to date

Artifacts

This also needs some documentation. The quick guide is:

1. Edit `features.yaml` (in the root of the repo)

2. Run `make generate-features` (in the root of the repo)

3. Run `make doc` (in the root of the repo)

Would it make sense to have doc depend on generate-features then? And we probably should document that workflow right in features.yaml to prevent people changing that without running make generate-features afterwards (does YAML even have comments?).

This also needs some documentation.

I added the missing documentation now

Did you write all the descriptions anew or did you take them from Kconfig files?

A bit of both. The KConfig files did not have the grouping and since we disabled the feature check in the CI has started to bit-rot.

If the latter is the case, we have to be careful of having several sources of truth in the tree and should converge to one eventually. Probably worth discussing at the VMA next week (and maybe wait with this PR until then?).

We did so last VMA: https://forum.riot-os.org/t/virtual-maintainer-assembly-vma-2023-11/4058/6

There was a bit of misunderstanding whether we drop all of KConfig or just the dependency (e.g. features and modules modeling). But in either case, the KConfig features are just a legacy as of now and this tries to salvage what I can and wire this back up into the CI.

Would it make sense to have doc depend on generate-features then?

I've added calling the script to generate the markdown file to make doc.

Note: The target make generate-features is still there and will no only create the Makefile. The reason I don't like to add this to very build is CI time. I added a test to static-test that will check if features_existing.inc.mk is still up to date and fail if not. That is not super elegant, but it reduced the CI overhead a lot and should just work.

May I squash?

If the latter is the case, we have to be careful of having several sources of truth in the tree and should converge to one eventually. Probably worth discussing at the VMA next week (and maybe wait with this PR until then?).

We did so last VMA: https://forum.riot-os.org/t/virtual-maintainer-assembly-vma-2023-11/4058/6

There was a bit of misunderstanding whether we drop all of KConfig or just the dependency (e.g. features and modules modeling). But in either case, the KConfig features are just a legacy as of now and this tries to salvage what I can and wire this back up into the CI.

Okay, if we all agree that features are really part of the to-be-removed information of KConfig (I myself haven't worked enough with KConfig to judge this), then I would opt for removing the information that has been extracted into features.yaml from the KConfig files within the same PR to avoid confusion in the future.

Would it make sense to have doc depend on generate-features then?

I've added calling the script to generate the markdown file to make doc.

Note: The target make generate-features is still there and will no only create the Makefile. The reason I don't like to add this to very build is CI time. I added a test to static-test that will check if features_existing.inc.mk is still up to date and fail if not. That is not super elegant, but it reduced the CI overhead a lot and should just work.

Sounds sensible to me, but I'm far from an expert on our CI to be able to tell if that's a decent workaround.

May I squash?

Yes, from my side.

Two small suggestions for the structuring:

• Let's split word sizes and architectures. For further processing it's helpful if there are categories in which we'd know that their features are usually mutually exclusive. You could cherry-pick 1f01a7d.
• We could hoist catch-all features in a group out to the next level (with the alternative of introducing another level of nesting), as done in fd44f59. The CPU Grouping tree is one where features are not mutually exclusive (a CPU may have feature cpu_core_cortexm, spu_stm32 and cpu_stm32f4), but as long as those features are always in ordered combinations, there can be an easy to spot deepest feature.

(But that's just for if you choose to apply Kevin's suggestion, which I'm +0 about -- for if you do apply it, you could apply this before that to apply me the manual conflict resolution).

As for the specific format, maybe you want to consider @MrKevinWeiss proposal to have fixed keys (moving the title to the sub-structure)?

I'd vote for that, too. My main reason would be that it's easier to represent in Rust :). ... laze uses that scheme, too.

I'd vote for that, too. My main reason would be that it's easier to represent in Rust :). ... laze uses that scheme, too.

I'd even go further:

groups:
- title: CPU Features
  help: These features are related to CPU capabilities or just used to
        indicated which CPU family is used.
  groups:
    - title: CPU Capabilities
      help: These correspond to features/capabilities provided by certain CPUs
      features:
        - name: generic_cpu_feature
          help: A generic CPU feature that is not specific to any CPU family.

This maps cleanly to sth like:

struct FeatureYaml {
  groups: Vec<Group>,
}

struct Group {
  name: String,
  help: String,
  groups: Vec<Group>,
  features: Vec<Feature>,
}

struct Feature {
 name: String,
 help: String,
}

While not using the yaml map key as name and yaml map value as text gives up the free uniqueness checks, in practice that's trivial to add. And having explicit fields for e.g., "name" and "help" (instead of directly using key&value) allows adding fields.

it's easier to represent in Rust

btw, there's a serde yaml schema generator that makes generating a yaml (and json) schema from those structs trivial.

Rebased on top of #20408 to pass the CI, and squashed.

Now with the unused import dropped, all should be green

All green, should be get this in?

There's an implementation detail that could yet be improved but could also turn out to be a very deep time sink hole: Instead of (ab)using make with non-file-name targets to regenerate files, we could embrace make and use it to ensure that whenever the generated features_existing.inc.mk is out of date, it gets rebuilt. The reason I think this may be a huge time sink is that that could interact badly with having the file checked in, and the alternative to not having the file checked in is having a post-checkout step that generates it before forking off into thousands of build tasks, which is yet again complicated. So let's not slow things down here with that.

Given there is an ACK and that everything looks discussed and ready, pushing the button.

Yeah! Thx :)