fix(conda): include channel in lock identity#9984
Conversation
Greptile SummaryThis PR fixes a lockfile identity gap for the Conda backend: the effective channel (e.g.
Confidence Score: 5/5Safe to merge — the change is additive, follows an established pattern, and is covered by a unit test. The new code is a straightforward addition of a method that already existed implicitly (channel was resolved at install time but not captured in lock identity). The implementation is consistent with every other backend that overrides resolve_lockfile_options, and the fallback to the global setting via channel_name() is the same logic already used during install. No existing behavior is altered. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix(conda): include channel in lock iden..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review
This pull request introduces the InstallManifest tool option source, allowing options to be persisted and used from installation manifests. It updates BackendArg to track the source of options, implements lockfile option resolution for the Conda backend, and ensures proper precedence where configuration overrides manifest options. I have no feedback to provide.
|
CI note: This comment was generated by an AI coding assistant. |
00b0e59 to
15425d2
Compare
📝 WalkthroughWalkthroughThe PR adds lockfile options support to the Conda backend. ChangesConda Lockfile Options
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend/conda.rs (1)
78-81: ⚡ Quick winConda lockfile
channelidentity currently uses the raw string (not canonicalized)
src/backend/conda.rsbuildslockfile_options()with("channel", self.channel_name()), i.e. the rawchanneloption value, not the parsed/typedChannelcanonical name. This is also enforced by tests (conda_lockfile_options_include_channel) expecting exact string passthrough (e.g."bioconda"stays"bioconda").Normalizing via
channel()would change lock-option behavior (and test expectations) and requires defining the canonical serialization plus parse-error handling. If stable identity across equivalent spellings is the goal, wirechannel()intolockfile_options()and update tests; otherwise leaving it as-is is consistent with other backends that generally emit raw identity-affecting option values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/conda.rs` around lines 78 - 81, The lockfile_options currently emits the raw channel string via lockfile_options() -> ("channel", self.channel_name()); change this to use the canonicalized channel identity from self.channel() instead: call self.channel(), map the resulting Channel to its canonical string form (e.g. Channel::to_string() or canonical_name()) and put that value into the BTreeMap; if parsing fails, fall back to self.channel_name() (and optionally log or warn) so we don't panic. Update the test conda_lockfile_options_include_channel (and any expectations) to expect the canonicalized form instead of the raw input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/backend/conda.rs`:
- Around line 78-81: The lockfile_options currently emits the raw channel string
via lockfile_options() -> ("channel", self.channel_name()); change this to use
the canonicalized channel identity from self.channel() instead: call
self.channel(), map the resulting Channel to its canonical string form (e.g.
Channel::to_string() or canonical_name()) and put that value into the BTreeMap;
if parsing fails, fall back to self.channel_name() (and optionally log or warn)
so we don't panic. Update the test conda_lockfile_options_include_channel (and
any expectations) to expect the canonicalized form instead of the raw input.
Summary
Why this belongs in lock identity
The Conda channel is part of the package universe being solved. The same tool name and version can resolve to different package builds, dependency URLs, and checksums on
conda-forge,bioconda, or a private channel. If the channel is not part of the lock identity,mise lockcan reuse solve metadata from one channel while the config asks for another, which breaks reproducibility and can install the wrong artifact set.Verification
cargo fmt --checkcargo test conda_lockfile_options_include_channelSummary by CodeRabbit