Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow only CLI specs as long as compatible with pins #294

Merged
merged 6 commits into from
Sep 26, 2023

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Sep 26, 2023

Description

Extends #289 to allow CLI specs compatible with pins. It will still reject incompatible ones, with the standard SpecsConfigurationConflictError.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Sep 26, 2023
@jaimergp jaimergp changed the base branch from main to pin-ms-match September 26, 2023 14:15
@jaimergp
Copy link
Contributor Author

Some profiling reveals that querying the index is not that much of an overhead (see index.py:search):

image

The big picture shows it's not a major problem:

image

@jaimergp jaimergp marked this pull request as ready for review September 26, 2023 14:45
@jaimergp jaimergp changed the title Allow only CLI specs as long as compatible with pins (#289 alternative) Allow only CLI specs as long as compatible with pins Sep 26, 2023
Comment on lines -760 to +717
if sis.requested[name].match(spec): # <-- this line is buggy!
if True: # compatible_specs(index, sis.requested[name], spec):
# assume compatible, we will raise later otherwise
Copy link
Member

Choose a reason for hiding this comment

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

that seems weird to have still around

@jaimergp jaimergp merged commit 57e216d into pin-ms-match Sep 26, 2023
75 checks passed
@jaimergp jaimergp deleted the pin-ms-match-2 branch September 26, 2023 17:30
jaimergp added a commit that referenced this pull request Sep 26, 2023
* add compatible_matchspecs (MatchSpec.match is for PackageRecords only)

* more useful exception

* add test

* add news

* pre-commit

* pre-commit

* fix boolean logic

* simplify

* commit progress so far

* remove test

* add new exception for request and pinned errors

* do not allow pinned and requested specs

* cleanup

* pre-commit

* add tests

* pre-commit

* add docs

* ensure pins are in SolverOutputState.specs even if not explicit

* format exception

* raise earlier, no index needed

* fix state tests

* pre-commit

* relax test constraints

* better error messages in unsolvable pins

* fix test

* pre-commit

* override channels

* amend news

* fix test

* override channels here too

* Apply suggestions from code review

Co-authored-by: Travis Hathaway <travis.j.hathaway@gmail.com>

* Apply suggestions from code review

* Allow only CLI specs as long as compatible with pins (#294)

* implement some compatibility checks

* clean up a bit and add more tests

* pre-commit

* be explicit about channels

* simplify error logic for installed pins

* pre-commit

---------

Co-authored-by: Travis Hathaway <travis.j.hathaway@gmail.com>
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Sep 26, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants