establish standard leading __ for virtual packages #8738
Conversation
tests/core/test_solve.py
Outdated
| @@ -249,46 +249,32 @@ def test_prune_1(): | |||
| specs_to_remove = MatchSpec("numbapro"), | |||
| with get_solver(specs_to_remove=specs_to_remove, prefix_records=final_state_1, | |||
| history_specs=specs) as solver: | |||
| unlink_precs, link_precs = solver.solve_for_diff(prune=False) | |||
There was a problem hiding this comment.
Between this change and the one on line 345, it looks like there are no longer any tests that verify correct behavior of prune at the unit test level and therefore the --prune flag for the CLI. Is this intentional?
There was a problem hiding this comment.
For context, the description of prune behavior is
Remove packages that have previously been brought into an environment to satisfy dependencies of user-requested packages, but are no longer needed.
There was a problem hiding this comment.
I think you mean to be commenting on #8689 - I got it wrong on my original comment above.
There was a problem hiding this comment.
Prune is the only behavior, as of #8689. Any --prune flag is superfluous. It's actually more of a hybrid, though. It has the --prune behavior for setting up specs_map, more or less, but it will add in other existing specs as long as they don't conflict with any of the explicit specs. Anything that conflicts will be removed. It should be possible, for example, to downgrade a py27 env to a py36 one, and have things like enum34 fall away.
There was a problem hiding this comment.
and have things like enum34 fall away
That would be more of a "forced" change, to satisfy the specs that the user is requesting in the current operation.
Actually, there are two cases here. Case number one is when enum34 is the traditional conda package tied to a specific python version. Case number two is when enum34 is a noarch python package and is "universal," in that it's designed to work with python =2.7|>=3.3.
The first case would "force" an enum34 removal when switching the environment from python 2.7 to python 3.6, since there would be no enum34 package for python 3.6. The second case could feasibly leave the enum34 package in the environment, even though it wouldn't be used or required by any package in the environment after the change. The solver behavior from 4.4 to 4.6 was designed to make the first case happen automatically, forcing the removal of enum34 if that is what was required to satisfy the specs the user is requesting in the current operation. The --prune flag in 4.4. to 4.6 was designed to address the second case, where the enum34 package would hang around sort of just through "hysteresis," but then provide a way to actually clean up superfluous packages that are no longer a requirement of the full history of user-requested specs.
There was a problem hiding this comment.
With the code as it was, since the entries in specs_map were derived from the prefix records, I found that enum34 couldn't be removed to do the update to python 3.6.
This is more fundamentally a change in how specs_map is constructed. Conflating it with --prune is unavoidable, because it has that effect, but that's not really the goal. The reason for doing this change, first and foremost, is to feed less to the solver. Feeding everything in the env in is bad. Feeding the explicitly requested stuff only is better. Feeding in the explicit stuff plus frozen specs for anything else that doesn't conflict with explicit specs + history is the best I could achieve.
There was a problem hiding this comment.
I see. So in both cases the enum34 package is removed, and in fact there is no hysteresis like we've had before. Fine with me. Hopefully that's not too much of an issue for the community at large. I think you'll run into an issue on release with conda removing far more packages than users expected (new tickets being filed to that effect). If you're up for handling those, then I like the change.
In fact I think the original solve code in 4.4 was along exactly these lines. We couldn't go with it on a release because very old history files didn't record user-requested specs. There was also an issue of constructor not creating a history file with conda and/or anaconda packages as the first entries for user-requested specs for the Miniconda and Anaconda installers. We might be enough farther along in time now though that those issues won't be as much of a bother. Hopefully that's the case.
There was a problem hiding this comment.
In this case I'd suggest adding SUPPRESS to the --prune flag so that it can one day be removed from the CLI, and also just removing the context.prune config parameter.
There was a problem hiding this comment.
Yes, you left good comments in the code for that, and I learned a lot from them. I don't know about really old history files, but constructor is at least doing the right thing now, and has been for a while.
Fingers crossed that there won't be too many unexpected broken envs from weird histories, but I think this bullet is worth biting.
tests/core/test_solve.py
Outdated
| # 'channel-1::pandas-0.11.0-np16py27_1', | ||
| # ) | ||
| # assert convert_to_dist_str(final_state_2) == order | ||
| # assert solver._r.environment_is_consistent(final_state_2) |
There was a problem hiding this comment.
Looks like we've removed a lot of behavior testing/enforcement here too. Just want to make sure that was intentional.
There was a problem hiding this comment.
Some of this can/should go back. I'll re-examine it before merging. Thanks for pointing it out.
tests/core/test_solve.py
Outdated
| assert convert_to_dist_str(unlink_precs) == unlink_order | ||
| assert convert_to_dist_str(link_precs) == link_order | ||
|
|
||
|
|
There was a problem hiding this comment.
Removing tests that ensure correct feature behavior I guess?
|
Hi there, thank you for your contribution to Conda! This pull request has been automatically locked since it has not had recent activity after it was closed. Please open a new issue or pull request if needed. |
CC @kalefranz @seibert