Skip to content

fix: preserve conda-meta key order when updating requested_specs#2191

Open
Aman-Cool wants to merge 2 commits intoconda:mainfrom
Aman-Cool:fix/update-requested-specs-key-order
Open

fix: preserve conda-meta key order when updating requested_specs#2191
Aman-Cool wants to merge 2 commits intoconda:mainfrom
Aman-Cool:fix/update-requested-specs-key-order

Conversation

@Aman-Cool
Copy link
Contributor

Description

update_requested_specs_in_json was round-tripping conda-meta JSON through serde_json::Value, which uses BTreeMap internally (no preserve_order feature in this crate), so every rewrite came back with alphabetically-sorted keys.

That's a problem because MinimalPrefixRecord's parser stops early once it has seen both files and requested_specs. After the rewrite, sha256 ('s') ends up after requested_specs ('r') alphabetically and is never parsed. On the next pixi install, MinimalPrefixRecord.sha256 is None while RepoDataRecord.sha256 is Somedescribe_same_content sees a mismatch and forces a full PrefixRecord reload, permanently defeating the fast-path optimization. This repeats on every subsequent run.

Before (alphabetical key order after rewrite):

..., files, ..., requested_specs, sha256, ...
                                  ^^^^^^ parser already stopped here

After (struct-field order preserved):

..., sha256, ..., files, ..., requested_specs
     ^^^^^^ read correctly before early termination

Fix: replace the serde_json::Value round-trip with PrefixRecord::from_path + write_to_path. Serde serializes in struct-field order so key ordering is stable, and write_to_path uses an atomic temp-file rename — also fixing a secondary risk of file corruption on process kill.

Fixes #NA

How Has This Been Tested?

Manually verified by:

  1. Running pixi install on an environment with numpy/requests
  2. Confirming the rewritten conda-meta JSONs now preserve original key order (sha256 appears before requested_specs)
  3. Running pixi install a second time and confirming no full PrefixRecord reload is triggered (fast path holds)

AI Disclosure

  • This PR contains AI-generated content.
  • I have tested any AI-generated content in my PR.
  • I take responsibility for any AI-generated content in my PR.

Tools: Chatgpt ,Claude

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added sufficient tests to cover my changes.

@baszalmstra
Copy link
Collaborator

What is the downside of adding the preserve_order feature?

@Aman-Cool
Copy link
Contributor Author

@baszalmstra, I did consider that😄.

The main issue is preserve_order uses insertion order, so for files that don't already have requested_specs, it'd still get appended at the end (same problem). It also pulls in indexmap as a dep and since serde_json is workspace-level, it'd silently flip behavior for every crate using it, felt like too broad a side effect for a targeted fix.

Going through the actual PrefixRecord struct felt cleaner to me, serde handles the field order automatically and we also get the atomic write from write_to_path for free, which the old approach was missing. Fewer moving parts overall.

That said, happy to hear your thoughts if you think preserve_order is still the better path here!🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants