search

conda-forge / conda-smithy

The tool for managing conda-forge feedstocks.
https://conda-forge.org/
BSD 3-Clause "New" or "Revised" License
146 stars 170 forks source link

BUG: smithy ignores keys in migrator when affected by zipping logic #1911

Closed h-vetinari closed 2 months ago

h-vetinari commented 3 months ago

In https://github.com/conda-forge/conda-forge-pinning-feedstock/pull/5790, I'm updating some values that are part of a zip_keys, as well as another value (channel_sources). As far as I can tell (perhaps related to the overhaul for additional zip-keys in #1764 & #1769), as soon as the variant algebra finds some zip_keys, it will "forget" the other keys being updated.

There is another adjacent issue related to https://github.com/conda-forge/conda-smithy/pull/1752, in the sense that https://github.com/conda-forge/conda-smithy/blob/8a2eba45e40e53abd2e1744475942e133cf6bffe/conda_smithy/configure_feedstock.py#L976-L978 will only ever consider the first entry of migrated_combined_variant_spec, however, it is not a given that this only has one entry.

I have a draft fix, but opening this issue so that we can discuss the right approach (if it turns out I'm on the wrong track).

CC @beckermr @jaimergp @0xbe7a

h-vetinari commented 3 months ago

@conda-forge/core - this is a minimal fix that we need for the numpy 2.0 migration AFAICT (even aside from the question of what to do with python 3.8). Could someone comment here or review #1912 please?

h-vetinari commented 2 months ago

So the issue here was that variant_key_add doesn't know how to determine the "higher of the two" between conda-forge and conda-forge/label/numpy_rc,conda-forge. This can be solved by providing an ordering: to the migrator.

This now completely circumvents additional_zip_keys (which still may have some open corner cases, but if they exist and we hit them, we can open separate issues).

In the meantime, I'm closing this.