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

ENH make sure packages in current repo data w/ features have versions without features #3957

Merged
merged 11 commits into from
Jun 5, 2020
Merged

Conversation

beckermr
Copy link
Contributor

@beckermr beckermr commented May 21, 2020

This PR makes sure that packages in the current repo data which have features also have version without features. This change makes sure that feature minimization works.

cc @isuruf @jjhelmus

@cla-bot cla-bot bot added the cla-signed [bot] added once the contributor has signed the CLA label May 21, 2020
@beckermr
Copy link
Contributor Author

So the osx test that failed passes locally for me and the windows failures appear unrelated but I don't quite follow them.

@marcelotrevisani do you know what is going on here?

@beckermr beckermr closed this May 22, 2020
@beckermr beckermr reopened this May 22, 2020
@beckermr
Copy link
Contributor Author

Ah it appears all of these tests are failing on master too. So I think this one is ready to go!

conda_build/index.py Outdated Show resolved Hide resolved
conda_build/index.py Outdated Show resolved Hide resolved
conda_build/index.py Outdated Show resolved Hide resolved
@beckermr
Copy link
Contributor Author

Bump here! I fixed one bug in the pr and it appears to be working now in my tests of the new sysroot stuff for conda forge.

@jjhelmus
Copy link
Contributor

I'm not certain excluding packages with features entirely from current_repodata.json is a good idea. Excluding packages from current_repodata could result in packages without features having un-met dependencies because the dependency has a feature.

I think a better solution is to treat features similar to "pins". That is, if package with features is included in current_repodata the newest version package without features should also be include, provided that one exists.

@beckermr
Copy link
Contributor Author

I'm not certain excluding packages with features entirely from current_repodata.json is a good idea. Excluding packages from current_repodata could result in packages without features having un-met dependencies because the dependency has a feature.

So, in this case, the solver will fall back to the full repodata, but that is a performance hit. Is the performance the concern here?

@beckermr
Copy link
Contributor Author

I think a better solution is to treat features similar to "pins". That is, if package with features is included in current_repodata the newest version package without features should also be include, provided that one exists.

This should work.

@jjhelmus
Copy link
Contributor

So, in this case, the solver will fall back to the full repodata, but that is a performance hit. Is the performance the concern here?

I'm not worried about performance, more my worry is that if all of the packages for a particular name have a feature then that entire package is excluded current_repodata which may cause the solver to prefer the solution from current_repodata over something that is more logical.

As an example: Currently conda create -n test blas=*=openblas numpy will give a solution using current_repodata. With this proposed change the blas-1.0-openblas package would not be included in current_repodata and conda should fall back to the full repodata.

My concern is that there will be some other solution that conda does find in current_repodata for these edge cases that will make no sense. That said, I cannot provide a non-contrived concrete example. Expanding, current_repodata to always include a non-feature package seems like a lower risk than removing all feature packages.

@beckermr
Copy link
Contributor Author

Ahhhhh thank you! That makes sense to me!

@beckermr
Copy link
Contributor Author

ok @jjhelmus this is ready for another look!

conda_build/index.py Outdated Show resolved Hide resolved
conda_build/index.py Outdated Show resolved Hide resolved
@beckermr beckermr changed the title ENH ignore packages with features in the current repo data ENH make sure packages in current repo data w/ features have versions without features May 29, 2020
@beckermr
Copy link
Contributor Author

beckermr commented Jun 2, 2020

Bump here @jjhelmus

@beckermr
Copy link
Contributor Author

beckermr commented Jun 4, 2020

Bump here

@jjhelmus
Copy link
Contributor

jjhelmus commented Jun 5, 2020

LGTM thanks again for putting this together @beckermr

@jjhelmus jjhelmus merged commit 61624e6 into conda:master Jun 5, 2020
@beckermr beckermr deleted the patch-1 branch June 5, 2020 15:05
@github-actions
Copy link

Hi there, thank you for your contribution!

This pull request has been automatically locked because it has not had recent activity after being closed.

Please open a new issue or pull request if needed.

Thanks!

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Mar 11, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2022
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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants