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

[Mamba POC] Bring in more conda logic #10983

Merged
merged 68 commits into from
Nov 30, 2021
Merged

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Oct 11, 2021

This PR implements a consolidated logic after studying the code in Solver._collect_all_metadata(), Solver._add_specs() and Solver._post_sat_handling. It's a reimplementation that doesn't use the SolverStateContainer features found in the Resolve instance they carry. More comments when needed, but it still needs to be documented thoroughly.

It's not perfect yet (conflict analysis is pending), but I am seeing more passing tests now.

This is an alternative implementation to the retry logic and history pinning found in #10946, #10947.

Let's see how many we can tackle now:

  • tests/test_create.py

    • test_allow_softlinks
    • test_clone_offline_multichannel_with_untracked
    • test_conda_pip_interop_conda_editable_package -- this passes locally! :/
    • test_conda_recovery_of_pip_inconsistent_env
    • test_install_features
    • test_install_force_reinstall_flag
    • test_install_mkdir
    • test_list_with_pip_wheel
    • test_neutering_of_historic_specs
    • test_noarch_python_package_reinstall_on_pyver_change
    • test_noarch_python_package_without_entry_points
    • test_offline_with_empty_index_cache
    • test_package_optional_pinning
    • test_packages_not_found
    • test_pinned_override_with_explicit_spec
    • test_remove_all
    • test_remove_features
    • test_update_with_pinned_packages
    • test_create_env_default_packages
  • tests/cli/test_cli_install.py

    • test_find_conflicts_called_once
  • tests/conda_env/test_cli.py

    • test_update
    • test_update_env_json_output
  • tests/core/test_solve.py

    • test_pinned_1
    • test_freeze_deps_1
    • test_prune_1
    • test_update_all_1
    • test_conda_downgrade
    • test_python2_update
    • test_fast_update_with_update_modifier_not_set
    • test_force_reinstall_1
    • test_channel_priority_churn_minimized
    • test_priority_1
    • test_features_solve_1
    • test_current_repodata_fallback

@anaconda-issue-bot anaconda-issue-bot added the cla-signed [bot] added once the contributor has signed the CLA label Oct 11, 2021
@jezdez jezdez added the type::poc indicates some proof of concept or MVP work label Oct 11, 2021
@chenghlee chenghlee self-assigned this Oct 12, 2021
@jaimergp
Copy link
Contributor Author

Down to 26 failing tests!

we do this by:

1) Translating conda update python to conda update python>={installed} if not conflicting. This forces conflicts we can neuter.
2) Letting conflicts grow a bit (up to 10 items) so we can get python
3) Down-prioritizing some dependencies that can conflict.
make sure conflicting packages that are not explicitly requested are also neutered if needed
@jaimergp jaimergp reopened this Nov 26, 2021
@jaimergp
Copy link
Contributor Author

jaimergp commented Nov 26, 2021

All green in both platforms, except four tiny issues on Windows:

Something about cleaning the index cache. They pass in isolation, but not with other tests in the same run. Fun stuff.

  • tests/test_create.py::IntegrationTests::test_clean_index_cache
  • tests/test_create.py::IntegrationTests::test_use_index_cache

Something about paths:

  • tests/test_create.py::IntegrationTests::test_install_tarball_from_local_channel. This tries to create a Unix-like path for a Windows path and fails; e.g. C:\Some\Dir is turned into /C:/Some/Dir. Unclear if this is on us or mamba.
  • tests/test_create.py::IntegrationTests::test_legacy_repodata. Fails with RuntimeError: Multi-download failed. and no more details. Probably path related as well. Unclear why it fails on Windows only, but this fake channel only has data for noarch so maybe Mamba is trying to download stuff from the win-64 directory and failing? Anyway, it says "legacy", we could skip :)

I will fix this first thing next week and merge for all green goodness.

@jaimergp
Copy link
Contributor Author

Also note that I needed to use restricted unicode on Windows for now. This is being fixed in libmamba as we speak and we will be able to undo that workaround in the future.

And I also took split test_solvers.py tests into legacy and libsolv for CI purposes. LibSolv is only tested in the experimental-resolver.sh script. That way the CI checks tell us immediately if something is failing on legacy or not.

@jaimergp
Copy link
Contributor Author

Looks like the index cache issue might be Windows leaving something open... Logs say:

DEBUG conda.gateways.disk.delete:unlink_or_rename_to_trash(136): renaming file path D:\Users\jrodriguez\Documents\conda\devenv\pkgs\cache\119df107.json to trash failed.  Output was: < empty >
WARNING conda.gateways.disk.delete:unlink_or_rename_to_trash(143): Could not remove or rename D:\Users\jrodriguez\Documents\conda\devenv\pkgs\cache\119df107.json.  Please remove this file manually (you may need to reboot to free file handles)

@jaimergp
Copy link
Contributor Author

Is this the first time everything's green? 🔥

@jezdez jezdez marked this pull request as ready for review November 30, 2021 11:44
@jezdez jezdez requested a review from a team as a code owner November 30, 2021 11:44
Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

This is ready. Thank you @jaimergp!

@jaimergp jaimergp merged commit e90caa8 into poc-libsolv Nov 30, 2021
@jaimergp jaimergp deleted the poc-libsolv-conda-logic branch November 30, 2021 12:07
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Dec 1, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 1, 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 type::poc indicates some proof of concept or MVP work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants