feat(ci): Use metadata for builds

[Nicell]

Refactors the build workflow to use hardware metadata to accomplish the following:

• Nightly, build every single onboard MCU board and board+shield combo possible
• Whenever app/** is changed (minus app/boards), run a short, premade core builds list
• Whenever app/boards is changed, build every board+shield combo that's affected by the changes

Here's a nightly build: https://github.com/Nicell/zmk/actions/runs/1519228121
Here's a boards change run: https://github.com/Nicell/zmk/actions/runs/1517831312
Here's a core run: https://github.com/Nicell/zmk/actions/runs/1517789825

Todo:

• make the final compile-matrix output remove all duplicates includes.
• add cmake-args to artifact name in some fashion
• figure out 256 matrix job limit (do one job per board rather than one job per board + shield combo)
• get-changed-files seems to fail after squashes and PRs (alternative action with fixes: https://github.com/Ana06/get-changed-files)
• don't inline the core build list (use a separate .yml file)
• make sure to build setting-reset shield (update link to them on docs)

[joelspadin]

exit() is supposed to only be used from within the Python interactive interpreter from what I understand. You probably want sys.exit() instead.

[joelspadin]

IMO it would be simpler to just

print(exc)
sys.exit(1)

instead of having a common exit point. You've already hit an error, so you should probably bail immediately.

Alternatively, you could wrap all of this up in another function and have it re-raise the exceptions with added info, then have a single handler that prints the error and exits. It might look something like this:

class MetadataError(Exception):
    pass

def do_run(self, args, unknown_args):
    try:
        self.validate_files()
    except MetadataError as e:
        print(e)
        sys.exit(1)
        
def validate_files(self):
    # ...
    try:
        # ...
    except yaml.YAMLError as e:
        # Use .format() instead of f-strings if we care about Python < 3.6
        raise MetadataError(f'Failed loading YAML file "{file}": {e}') from e
    except ValidationError as e:
        raise MetadataError(f'Failed validation of file "{file}": {e}') from e

...which I kinda like because all the logic for printing an error is in one spot instead of spread across multiple exception handlers.

[petejohanson]

Oh, the point is to continue, so we can check all the files. Then if any fail... we return a non-zero exit code.

[petejohanson]

We specifically don't want to exit early.

[joelspadin]

ret appears unused?

[joelspadin]

Super minor: using "leverage" twice makes this read a bit awkwardly to me. Maybe just use "use" and/or "to be read by setup scripts" to keep things easy to understand?

(If you change this, Ctrl+F "leverage" since it's used in a few other places too.)

[joelspadin]

Whoops. I am on the wrong PR. Those comments apply to #883.

[petejohanson]

Seems like a good start. I think in general, the code could be cleaned up, even if the you can't declare functions in github script, you could define lambdas and use them, to make the code clearer, e.g. const mapBoard = (metadata) => foo.

[petejohanson]

I was assuming we'd tag these boards and shields somehow in the .zmk.yml file, or maybe with an extra .core-build empty file, or something, to detect these automatically. Thoughts?

[petejohanson]

For this, we could find any boards w/ the display feature, and pass this as an arg to build it, to determine this dynamically.

[petejohanson]

Ditto, use the features for this.

[petejohanson]

The purist in me says, do it functionally, darn it!

Suggested change

	let include = [];
	coreCoverage.boards.forEach(b => {
	coreCoverage.shields.forEach(s => {
	include.push({
	board: b,
	shield: s
	});
	});
	});
	let include = coreCoverage.boards.flatMap(board => coreCoverage.shields.map(shield => ({ board, shield }));

(I may have the JS syntax slightly wrong)

[petejohanson]

Similar grumblings about using forEach here instead of mapping, but this also works fine.

[petejohanson]

This seems really close! A few tweaks to make the experience a tad nicer needed before we merge.

[Nicell]

@petejohanson Improved the logging with groups/errors as suggested. Also simplified the combining code using your suggestion, thanks. Let me know what you think now. I still want to add settings-reset to the nightly build and also add cmake args to the artifacts, but I'm not sure the best way to do either if you have suggestions. Have an idea for the settings-reset at the very least.

[petejohanson]

@Nicell Perhaps for the cmake-args case, we expect you to also pass a build-nickname or build-name that's used, and if someone fails to provide that, just do something like (pseudo-code): build-args-${m5dsum(args-string)} to give it a unique name?

[Nicell]

@petejohanson I added nicknames, you can see it on the core build of this PR. Works well. I've simply removed the spaces from cmake-args as the alternate rather than doing a hash, since it's more readable this way and just a fallback.

I had some thoughts about settings-reset I think we need to address, so I'm not sure if I'm going to add that to this PR. I think this is mostly ready to go then from my point of view.

[petejohanson]

One minor tweak needed, then good to go. Thanks!

[petejohanson]

Awesome, thanks!