Skip to content

Comments

fix: warn about no pinning strategy for unused features#4065

Merged
lucascolley merged 1 commit intoprefix-dev:mainfrom
kilian-hu:fix-issue-3245
Aug 23, 2025
Merged

fix: warn about no pinning strategy for unused features#4065
lucascolley merged 1 commit intoprefix-dev:mainfrom
kilian-hu:fix-issue-3245

Conversation

@kilian-hu
Copy link
Contributor

@kilian-hu kilian-hu commented Jul 1, 2025

This PR initially tried to fix #3245 where users noticed that the dependency version pinning strategy is not applied in features that are not used by any environment. This is because right now the solver does not even run for the dependencies in an unused feature because there is no environment in which the solve could happen. One could run a sanity check solve for such an unused feature's dependencies but that would require quite some changes. For now I'd at least improve the warning that is shown when there is an unused feature and inform the user that those dependencies are not being resolved at all.

@kilian-hu kilian-hu changed the title Fix ignored pin-strategy when adding dep to feature fix: apply pinning strategy consistently to features Jul 1, 2025
@kilian-hu kilian-hu marked this pull request as ready for review July 1, 2025 15:42
baszalmstra
baszalmstra previously approved these changes Jul 14, 2025
@lucascolley lucascolley added the configuration Issue related to configuration of workspace or global behavior label Jul 21, 2025
@lucascolley lucascolley added the bug Something isn't working label Jul 29, 2025
Copy link
Collaborator

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see open thread

@lucascolley
Copy link
Collaborator

hey @kilian-hu ! Not sure if this was closed deliberately, but would you be interested in implementing point (1) from #4065 (comment) ?

@kilian-hu kilian-hu reopened this Aug 23, 2025
@lucascolley lucascolley dismissed baszalmstra’s stale review August 23, 2025 15:13

significant change since

@lucascolley lucascolley self-requested a review August 23, 2025 15:13
@kilian-hu
Copy link
Contributor Author

@lucascolley Sorry, no that was not intentional. I just pushed what you suggested about making the warning message more informative.

I noticed that when running e.g. pixi add --feature dev numpy when there is no dev feature in the manifest yet, we will not even get this warning as it can only be emitted while reading the existing manifest AFAIU. So this kind of warning should actually be emitted after the manifest has been written to disk imo so we see it both if the feature was not already there and when it's already there. But I'm not fully aware of all the constraints here, so that's just an idea.

So I'd like to merge this as a first start so users are less confused when this happens. Does the updated warning message look alright to you?

And for the future it remains to maybe implement at least a sanity check solve for unused features and also change that this warning message is shown regardless of whether the feature already existed or not.

Copy link
Collaborator

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @kilian-hu, looks good to me! And your plan for potential follow-up also sounds good 👍

@lucascolley lucascolley changed the title fix: apply pinning strategy consistently to features fix: warn about no pinning strategy for unused features Aug 23, 2025
@lucascolley lucascolley added UX Related to the User Experience of pixi area:add Related to pixi add labels Aug 23, 2025
@lucascolley lucascolley enabled auto-merge (squash) August 23, 2025 15:36
@lucascolley lucascolley disabled auto-merge August 23, 2025 15:36
@lucascolley lucascolley enabled auto-merge (squash) August 23, 2025 15:36
@lucascolley lucascolley merged commit 4be243f into prefix-dev:main Aug 23, 2025
65 of 75 checks passed
@kilian-hu kilian-hu deleted the fix-issue-3245 branch August 23, 2025 15:50
tdejager pushed a commit to tdejager/pixi that referenced this pull request Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:add Related to pixi add bug Something isn't working configuration Issue related to configuration of workspace or global behavior UX Related to the User Experience of pixi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pinning-strategy is ignored for unused features

3 participants