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

Add all candidates for reinstall to solver #2050

Merged

Conversation

j-mracek
Copy link
Contributor

@m-blaha
Copy link
Member

m-blaha commented Feb 15, 2024

The patch looks good.
But the original bugreport reported the issue with install command (which works correctly according to my attempts to reproduce it). This patch fixes incorrect reinstall behavior.

@j-mracek
Copy link
Contributor Author

The patch looks good. But the original bugreport reported the issue with install command (which works correctly according to my attempts to reproduce it). This patch fixes incorrect reinstall behavior.

Reporter incorrectly reported issue for install command, but https://issues.redhat.com/browse/RHEL-25005?focusedId=24167516&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-24167516 corrected the reproducer. The issue is not related to install command and even not to cost option. Setting cost option only shoved strange behavior but in background DNF send to libsolv available RPM with higher ID.

@m-blaha
Copy link
Member

m-blaha commented Feb 16, 2024

@j-mracek I see. We're good to go then.

@m-blaha
Copy link
Member

m-blaha commented Feb 16, 2024

Shouldn't we provide a test case for this?

@j-mracek
Copy link
Contributor Author

I can but I cannot ensure that the test will always fail without the patch, because the behavior without patch is not deterministic enough. Without patch the load of repository is driven by sorter in iterator.

    def items(self):
        """return repos sorted by priority"""
        return (item for item in sorted(super(RepoDict, self).items(),
                                        key=lambda x: (x[1].priority, x[1].cost)))

@j-mracek
Copy link
Contributor Author

I created a test - rpm-software-management/ci-dnf-stack#1451

@jan-kolarik jan-kolarik merged commit 96f8d79 into rpm-software-management:master Mar 1, 2024
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants