refactor!: back pixi's Config by rattler_config's ConfigBase<PixiConfig>#6531
Draft
wolfv wants to merge 3 commits into
Draft
refactor!: back pixi's Config by rattler_config's ConfigBase<PixiConfig>#6531wolfv wants to merge 3 commits into
wolfv wants to merge 3 commits into
Conversation
pixi's `Config` is no longer a parallel struct that duplicates the shared rattler configuration keys: it is now a thin `#[serde(transparent)]` wrapper around `rattler_config::config::ConfigBase<PixiConfig>` from the redesigned (unreleased) rattler_config, where `PixiConfig` is pixi's tool-specific extension. This proves out the shared-config extension mechanism end to end. What moved where: - Shared keys (default-channels, authentication-override-file, tls-no-verify, tls-root-certs, mirrors, build, channel-config, repodata-config, concurrency, proxy-config, s3-options, index-config, run-post-link-scripts, allow-symbolic/hard/ref-links, loaded_from) now live in the upstream `CommonConfig` / `ConfigBase` and are reached through `Deref` (`config.mirrors`, `config.concurrency`, ...). - Pixi-specific keys (pinning-strategy, pypi-config, detached-environments, shell, experimental, tool-platform, cache, and the deprecated change-ps1/force-activate) moved into the new `PixiConfig` extension (`config.extensions.*`), with all serde attributes (kebab-case, snake_case aliases, skip_serializing_if guards) carried over so the on-disk TOML format is unchanged. New accessors `shell()`, `experimental()`, `cache()`, `pinning_strategy()` keep call sites terse; existing accessors keep their signatures. - `tls_root_certs` is promoted to the upstream `rattler_config::config::tls::TlsRootCerts` (Webpki/System). The legacy "native"/"all" spellings deserialize as aliases of System, and pixi keeps its load-time deprecation warnings by inspecting the raw TOML document (`Config::from_toml`) and via a clap value parser for `--tls-root-certs` / `PIXI_TLS_ROOT_CERTS`. pixi_utils' feature-aware fallback logic (native-tls vs rustls) is unchanged; the LegacyNative/All match arms collapse into System. - allow-symbolic-links/allow-hard-links/allow-ref-links use the upstream CommonConfig fields (same flat key names, same format). Extensibility proof points (each covered by tests): - `Config::from_toml` uses `ConfigBase::<PixiConfig>::from_toml_str`, whose two-pass serde_ignored deserialization reports keys unknown to BOTH the common config and the extension, so the "Ignoring '...' in file" warnings keep working — including for typos of extension keys (`pinning-strateggy`) and unknown keys nested in known tables (`shell.chnge-ps1`, `pypi-config.index-urll`). New test: test_from_toml_reports_unknown_extension_keys. - `Config::set` now special-cases only the deprecated change-ps1/force-activate hard errors, detached-environments bool/path parsing (~-expansion), and the cache.* re-validation; the former ~400-line match is deleted and everything else — including extension keys like `pinning-strategy` — delegates to the upstream generic `ConfigBase::set` (TOML round-trip editor). Unknown keys produce an error listing all supported keys (shared + extension) from the `Config` trait's `keys()`. New test: test_generic_set_handles_extension_keys; also verified against the built CLI: `pixi config set pinning-strategy semver` writes `pinning-strategy = "semver"`. - `ProxyConfig::default()` no longer reads proxy env vars upstream, so pixi_config's tests are hermetic: the whole suite (including test_config_merge_multiple) now passes with and without HTTPS_PROXY/HTTP_PROXY set. Dependency patching: a `[patch.crates-io]` block redirects every rattler crate pixi consumes to the `claude/rattler-shared-config-design-m8d2gb` branch of conda/rattler (single copy of each crate, verified with `cargo tree -i rattler_conda_types`). TODO: remove once rattler_config 0.6 is released (see conda/rattler#2557) — this PR must not merge until then. Behavior changes (intentional, small): - `default_channels` is `Option<Vec<NamedChannelOrUrl>>` upstream; `Config::default_channels()` keeps returning the configured list or the consts::DEFAULT_CHANNELS fallback, and from_toml normalizes an explicitly empty list to None to preserve the old merge semantics. - `mirrors` is an `IndexMap` (was HashMap): deterministic ordering; `mirror_map()` signature updated. - `loaded_from` now lists files in load order (lowest precedence first); it previously listed them in reverse. - Editing a subkey of a non-existent s3-options bucket used to be a silent no-op; the generic editor rejects it (S3Options requires all three fields). Setting the whole bucket first works as before. - `pixi config set concurrency '<json>'` used to (buggily) write pypi-config; the generic editor handles it correctly. - PyPIConfig::is_default now also checks allow-insecure-host, so a pypi-config containing only allow-insecure-host is no longer dropped on save. - A file containing `tls-root-certs = "native"`/`"all"` still loads (with the same deprecation warning), but re-saving it writes the canonical `"system"`. Snapshot diff (config_merge_multiple): Debug output is now nested as ConfigBase { common, extensions, loaded_from }; default_channels prints as Some([...]); the (empty, serde-skipped) upstream index_config section appears; loaded_from order flipped as described above. All values are identical to the old snapshot. Tests: cargo test -p pixi_config (55 passed, with and without proxy env), cargo test -p pixi_utils (72 passed), cargo check --workspace --all-targets, cargo clippy -p pixi_config/pixi_utils/pixi_cli/ pixi_core/pixi_api/pixi_uv_context --all-targets clean, cargo fmt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
… schema
The pixi-side mirror of rattler_config's tests/compat.rs: that catalog
pins the contract of the shared keys; this one pins the whole pixi
schema (shared CommonConfig keys plus the PixiConfig extension) so
upgrading the shared config crate can never silently break a user's
existing config.toml.
New: crates/pixi_config/tests/compat.rs + test-data/compat/ fixtures
(kitchen-sink, snake-case-aliases, legacy-config, typos,
override-layer) + insta snapshots.
Coverage:
1. Parsing permutations — every fixture parses; (unused keys, parsed
Config) snapshotted per fixture (channel_config.root_dir normalized
so snapshots are machine-independent). snake_case aliases parse
equal to canonical with zero warnings; a realistic old pixi config
(change-ps1, force-activate, tls-root-certs = "native",
detached-environments = true, repodata-config.disable-jlap) loads
with warnings only, and the deprecated values still take effect
through the modern accessors (change_ps1()/force_activate() see the
folded shell.* values, "native" resolves to the system store);
typos of shared AND extension keys, top-level and nested, are all
reported as unused while valid neighbors survive.
2. Round-trip stability — load -> to_toml -> load is lossless and
idempotent for all five fixtures (including the legacy one: the
deprecated keys re-serialize and re-fold identically), and
serialization never invents unknown keys.
3. Editing matrix — 42 entries through the public Config::set (the
`pixi config set` code path): every shared key family plus
pinning-strategy, detached-environments (true/false/path),
tool-platform, pypi-config.*, shell.*,
experimental.use-environment-activation-cache and all cache.* keys
(absolute paths, since set re-validates). Each entry: set succeeds
on a fully populated config, the result round-trips, and set+unset
on a pristine config restores the default. Deprecated
change-ps1/force-activate stay hard errors pointing at shell.*, and
unknown keys (shared and extension typos) are rejected in both set
and unset direction.
4. Merge semantics — kitchen-sink + override layer through
Config::merge_config: scalars replace, mirrors/s3-options extend,
repodata merges field-wise (default and per-channel), pypi
index-url replaces while extra-index-urls/allow-insecure-host
extend, shell/experimental/cache merge field-wise, and the
long-standing tool-platform LOWER-layer-wins quirk is pinned with a
comment so changing it becomes a conscious decision. Merged output
snapshotted.
The catalog immediately caught a real bug: the pinned rattler commit
(33a4909) still had the repodata merge direction inverted (the lower
layer's disable-bzip2 survived an override). conda/rattler already
fixed this on the same branch in 5ded1a9 ("fix repodata merge
direction") — this commit bumps Cargo.lock to that rev, which makes
the merge test pass. None of the pre-existing pixi tests noticed the
inverted direction, which is exactly the gap this catalog closes.
Tests: cargo test -p pixi_config (55 unit + 9 compat) passes both with
proxy env scrubbed and with HTTPS_PROXY/HTTP_PROXY set; cargo check
--workspace --all-targets; clippy clean on pixi_config; cargo fmt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
6 tasks
wolfv
changed the title
The parse snapshots embedded the concurrency.solves default, which is the local CPU count, so fixtures that don't set [concurrency] (e.g. snake-case-aliases.toml) produced snapshots that only passed on the machine that generated them. Normalize solves unconditionally in the snapshot helper — a conditional "only when equal to the default" would flip whenever an explicitly configured value happens to match the CPU count — and assert the explicitly configured values in code instead, where they are deterministic. Verified by running the suite pinned to a different CPU count (taskset). Same fix applied to the rattler_config catalog upstream (conda/rattler#2557). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzYs4p6XJDj9QBcNB4ipR7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #6528 (which merged the leaf-type migration from #6144). This PR contains the two commits that were pushed to the branch just after #6528 was merged and therefore never showed up there: the final form of the shared-config design, plus a compatibility test catalog.
Commit 1 — the final form (requires unreleased rattler_config 0.6, see below).
pixi_config::Configis no longer a parallel struct — it is a#[serde(transparent)]wrapper aroundrattler_config::ConfigBase<PixiConfig>(net −630 lines):PixiConfigis the tool extension the design was made for:pinning-strategy,pypi-config,detached-environments,shell,experimental,tool-platform,cache, and the deprecatedchange-ps1/force-activate— all with their original serde attributes, so the on-disk format is unchanged. All shared keys come from upstreamCommonConfig(reached viaDeref, soconfig.mirrorsetc. keep working).tls_root_certsand theallow-*-linkstrio are promoted upstream (the follow-ups flagged in refactor: switch to rattler_config #6144). Legacy"native"/"all"spellings keep parsing (upstream aliases) and keep their deprecation warnings.#[serde(flatten)]used to break):pixi config listwarnsIgnoring 'pinning-strateggy, shell.chnge-ps1' in …/config.toml— typos in extension keys and keys nested in known tables are both caught.pixi config setdelegates to the upstream generic editor:pixi config set pinning-strategy semver(an extension key) works with zero per-key code; the old ~400-linesetmatch is deleted, keeping only 3 pixi-specific special cases (deprecated-key hard errors with identical error text,detached-environmentsbool/path parsing,cache.*expand+validate side effects).test_config_merge_multiplenow passes withHTTPS_PROXYset — the upstream fix forProxyConfig::default()leaking the environment into saved configs, proven from pixi's own test suite.Commit 2 — compatibility test catalog (mirrors the one added to rattler_config in conda/rattler#2557), so future changes can't silently break existing user configs:
change-ps1,force-activate,tls-root-certs = "native", booleandetached-environments, removeddisable-jlap— must parse with warnings, never hard errors), typos (top-level and nested), and a merge override layer.Config::set(all shared families plus every pixi extension key, incl. all threedetached-environmentsforms and all 9cache.*keys) with set+unset restoring defaults, and snapshotted merge semantics (thetool-platformlower-layer-wins quirk is pinned deliberately with a comment).This PR carries a
[patch.crates-io]section pointing the rattler crates at the branch of conda/rattler#2557 (markedTODO: remove once rattler_config 0.6 is released). Once that releases, the patch section is dropped and versions bumped, and this becomes mergeable.Intentional small behavior changes (all documented in the commit messages):
mirrorsis anIndexMap(order-preserving); emptydefault-channels = []normalizes to unset;s3-options.<bucket>.<field>on a nonexistent bucket now errors instead of silently no-oping; a latent bug whereset("concurrency", …)wrotepypi_configis fixed by the generic path; re-saving a config with legacy"native"/"all"writes canonical"system".How Has This Been Tested?
cargo check --workspace --all-targetspasses; clippy clean on all touched crates;cargo fmt --checkclean.pixi config listwarnings,pixi config seton extension keys and typos).cargo tree -i rattler_conda_typesconfirms a single (git) copy of every rattler crate under the patch.AI Disclosure
Tools: Claude Code
Checklist:
schema/model.py(no schema impact — serialized config format unchanged)Generated by Claude Code