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 expanding solvable provides for dependency matching #762

Merged
merged 4 commits into from Jan 27, 2020
Merged

Add expanding solvable provides for dependency matching #762

merged 4 commits into from Jan 27, 2020

Conversation

lukash
Copy link
Contributor

@lukash lukash commented Jul 15, 2019

This is meant to fix a bug where rich dependencies would match a package even if the require specified a different version range than doesn't match the version of the provide.

Uses a new libsolv function pool_whatmatchessolvable(). Further description is with the commits, along with some shortcomings this currently has.

dnf PR:
rpm-software-management/dnf#1432

Tests are here:
rpm-software-management/ci-dnf-stack#577

https://bugzilla.redhat.com/show_bug.cgi?id=1534123
https://bugzilla.redhat.com/show_bug.cgi?id=1698034

@dmach
Copy link

dmach commented Aug 21, 2019

@lukash could you check if you're able to build it locally using tito?
When I tried that, the build failed on unit tests.
Packit build also did not finish.

@lukash
Copy link
Contributor Author

lukash commented Aug 21, 2019

@dmach sorry, missed that. I've added a commit fixing the test, although it should probably be squashed with the one introducing the change, I can do that later.

@packit-as-a-service
Copy link

There was an error while running a copr build. You can re-trigger copr build by adding a comment (/packit copr-build) into this pull request.

@dmach dmach removed their assignment Sep 24, 2019
@j-mracek j-mracek self-assigned this Sep 24, 2019
@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably 6993c4b) made this pull request unmergeable. Please resolve the merge conflicts.

@Conan-Kudo
Copy link
Member

@rh-atomic-bot r+

@rh-atomic-bot
Copy link

📌 Commit 13911cb has been approved by Conan-Kudo

@lukash
Copy link
Contributor Author

lukash commented Nov 7, 2019

@rh-atomic-bot r-

@Conan-Kudo
Copy link
Member

@lukash the r- flag doesn't actually work, as far as I know.

@Conan-Kudo
Copy link
Member

@rh-atomic-bot r-

@lukash
Copy link
Contributor Author

lukash commented Nov 7, 2019

@Conan-Kudo oops :( I wanted @j-mracek to look at this and the related PRs before it gets merged... I do think it's what we want but there's a chance it may not be.

@Conan-Kudo
Copy link
Member

@lukash for what it's worth, your code changes made sense to me, and it didn't seem broken when I looked at it.

@lukash
Copy link
Contributor Author

lukash commented Nov 7, 2019

@Conan-Kudo ok. The changes here are most probably fine, the dnf part has probably more potential to be problematic... Hopefully this will prompt @j-mracek to do the review 🙂

libdnf/sack/query.cpp Outdated Show resolved Hide resolved
@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably db4b61a) made this pull request unmergeable. Please resolve the merge conflicts.

@j-mracek
Copy link
Member

@lukash Please could you also rebase it?

@packit-as-a-service
Copy link

Congratulations! The build has finished successfully. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/rpm-software-management-libdnf-762
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@lukash
Copy link
Contributor Author

lukash commented Dec 13, 2019

@j-mracek I've switched from Queue to IdQueue and rebased this as well as the other two PRs.

@rh-atomic-bot
Copy link

☔ The latest upstream changes (presumably d3eb5c9) made this pull request unmergeable. Please resolve the merge conflicts.

@j-mracek
Copy link
Member

@lukash Please could you rebase it?

Join the HY_PKG_CONFLICTS switch branch with the rest of the
dependencies and remove the matchtype assert. The assert is done in the
filterRcoReldep() function and the rest is the same.
@j-mracek
Copy link
Member

@lukash I would like to ask you to enhance python binging in query-py.cpp. The new code supports query but not list of packages.

assert(f.getMatches().size() == 1);

dnf_sack_make_provides_ready(sack);
Pool *pool = dnf_sack_get_pool(sack);
Copy link
Member

Choose a reason for hiding this comment

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

Please Pool * pool

Copy link
Member

Choose a reason for hiding this comment

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

Also in all other new line please use a middle position for *.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There were just a couple, I think I got them all.


o = hawkey.Query(self.sack).filter(obsoletes=q)
self.assertLength(o, 1)
self.assertEqual(str(o[0]), "fool-1-5.noarch")

def test_requires_with_package_list(self):
q = hawkey.Query(self.sack).filter(name="fool")
o = hawkey.Query(self.sack).filter(requires=q)
Copy link
Member

Choose a reason for hiding this comment

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

Please could you also add requires=q.run()

Lukáš Hrázký added 3 commits January 14, 2020 13:49
This allows to filter all dependencies (requires, conflicts, recommends,
etc.) by a list of solvables that match the dependency.

Can be used in the dnf repoquery command instead of a low-level piece of
code which was expanding packages to their provides and then matching the
dependencies as reldeps.

https://bugzilla.redhat.com/show_bug.cgi?id=1534123
https://bugzilla.redhat.com/show_bug.cgi?id=1698034
Fixes tests after allowing to pass a query (or an iterable of packages)
as an argument to dependency filters (requires, suggests, etc.) in
"Query: add a dependency by solvable filter".

Drops the exception check and adds a simple test for the new
functionality.
In addition to supporting a query as an argument to the dependency
filters (which then gets resolved to a list of packages), add support
for passing a sequence of packages directly.
@lukash
Copy link
Contributor Author

lukash commented Jan 14, 2020

@lukash I would like to ask you to enhance python binging in query-py.cpp. The new code supports query but not list of packages.

I'd also welcome if you were more specific with these requests, especially since it's been some time since the details were fresh in my head. Anyway, I think I figured out what you're asking for in the end and I've added a commit which adds support for a sequence of packages and the corresponding test with query.run().

@j-mracek j-mracek merged commit 367cf8a into rpm-software-management:master Jan 27, 2020
@lukash lukash deleted the rich-deps branch October 22, 2020 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants