Unified configuration format, store_build_artifacts part

[mgorny]

Checklist

• Added a news entry
• Regenerated schema JSON if schema altered (python -m conda_smithy.schema)

Introduce a unified configuration tree for more fine-grained settings of the CI setup. Currently, store_build_artifacts is implemented in the new format. The settings are rooted at workflow_settings top key, with its subkeys representing specific settings. The values can either represent a common value for all workflows, e.g.:

workflow_settings:
  store_build_artifacts: true

or a list of multiple values with conditions describing when they apply, for example:

workflow_settings:
  store_build_artifacts:
    - provider: github_actions
      value: true
    - platform: [win-64, linux-64]  # matched as an OR
      value: true

in which case the last value matching the given workflow is used.

Part of issue #2349.

[mgorny]

Okay, handled most of the requests here. Tomorrow I'll try to move the any() logic to Jinja.

[mgorny]

Okay, addressed all the comments. Also made script templates conditional to specific platforms.

I was thinking of doing something clever and setting a variable from inside the loop in the template, but decided against it. After all, we still need it in Python to decide whether to output the scripts.

[mgorny]

Okay, I think I've addressed all the review comments.

[jaimergp]

Suggested change

	forge_config.setdefault("workflow_settings_processed", {})[
	forge_config.setdefault("workflow_settings_per_provider", {})[

Is this more accurate? "processed" sounded a bit too generic to me.

[mgorny]

Well, it's more like per-os right now.

i.e. it's: workflow_settings_processed.store_build_artifacts: set[os_name]

[mgorny]

My assumption was that it will be just a generic key for variety of preprocessed data we need to pass. Perhaps that's a bad assumption, and something like global store_build_artifacts_on_os would be better.

[h-vetinari]

Could we do something like:

workflow_settings_granular[ized]
workflow_settings_per_ci
workflow_settings_...

This obviously depends (somewhat at least) on the choice of data structure

[h-vetinari]

set-of-dicts is not an obvious choice of structure to me. Could you explain why you chose that?

[mgorny]

What? It's a set of strings, i.e. OS names. We could technically do dict[str, bool], but that would require unnecessary redundancy (i.e. filing false values). I suppose a dict[str, Any] would make sense if we wanted all workflow-settings-per-os, but it's not clear to me if we will ever actually need any other of the settings preprocessed like that.

[h-vetinari]

ok, that was my bad, not least because I mixed up config["workflow_settings"]["store_build_artifacts"] with forge_config["workflow_settings_processed"]["store_build_artifacts"]. Still, a set is a bit of a strange choice to me, even though I can see how you ended up there.

I suppose a dict[str, Any] would make sense if we wanted all workflow-settings-per-os, but it's not clear to me if we will ever actually need any other of the settings preprocessed like that.

Isn't this a logical extension for other cases from #2349? E.g. when setting swapfiles, we need to provide a size per platform/provider, not just a boolean.

[mgorny]

No, actually I was thinking wrong. The only reason for these "preprocessed" booleans is to know if there's at least one value for the OS, so we'd know whether to output the template or not.

[mgorny]

Okay, here's an idea: to avoid all the confusion, I'll stop putting the "preprocessed" stuff back and just do it in pure Jinja. The downside is that the is-any-value-in-loop-true logic will be done twice now: once in Python to determine whether to include .sh/.bat script, and then again in Jinja to determine whether to include the extra template part. However, all of this will be kept in a single file, so there will be no confusion over what exactly the value is.

[h-vetinari]

could you add a test for this please?

[h-vetinari]

Could we do something like:

workflow_settings_granular[ized]
workflow_settings_per_ci
workflow_settings_...

This obviously depends (somewhat at least) on the choice of data structure

[mgorny]

Moved the store_build_artifacts to Jinja entirely now. Don't love that code, but it isn't the worst either.

[mgorny]

Changed all pfarchless into os, and removed unused platformset from GHA while at it.

[h-vetinari]

I didn't look too closely at utils.py yet, but the interface looks good; mainly I'm interested in seeing more tests.

[mgorny]

Ok, this will take some time but I've decided to go for proper test coverage. So far I'm thinking:

• GHA plain true/false tests for old/new setting
• Azure plain true/false tests for old/new setting
• GHA/Azure test for error when both are defined
• GHA/Azure test for more complex conditions (per-platform / per-os?)
• GHA/Azure test for error when overlapping conditions are defined

[mgorny]

@h-vetinari, broad set of tests added :-).

[h-vetinari]

Basically LGTM, just one missing test and perhaps a bit of explaining

[h-vetinari]

Thanks for the multiple iterations on this! 🙏