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

Modify solver_describe_decision to report cleaned (RhBug:1486749) #226

Closed
wants to merge 1 commit into from

Conversation

j-mracek
Copy link
Contributor

@j-mracek j-mracek commented Sep 8, 2017

It prioritize return SOLVER_REASON_CLEANDEPS_ERASE then SOLVER_REASON_UNIT_RULE.

In reported cases in bug report it provide confusing output.

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

@mlschroe
Copy link
Member

mlschroe commented Sep 8, 2017

I don't think that makes sense. If the decision was done because of a unit rule this should be reported. What you're basically doing is that you map all the unit rule decisions done because of a cleandeps erase to SOLVER_REASON_CLEANDEPS_ERASE. This deprives the library user of important information. If you really need something like this you should do it in the layer on top of libsolv.

(Furthermore your patch has a bug: it overwrites the content of i that is used later on...)

It prioritize return SOLVER_REASON_CLEANDEPS_ERASE then SOLVER_REASON_UNIT_RULE.

In reported cases in bug report it provide confusing output.

https://bugzilla.redhat.com/show_bug.cgi?id=1486749
@j-mracek
Copy link
Contributor Author

I try to distinguish between removal of unused dependencies and removal of packages that has to be removed due to removal of their dependency, but the current implementation provides confusing information in complex tasks. To get reason of removed packages we use libdnf/hy-goal.c function hy_goal_get_reason() that use int reason = solver_describe_decision(goal->solv, dnf_package_get_id(pkg), &info); and solver_ruleclass(goal->solv, info)
Example 1:
installed pkgA (requires pkgB), pkgB
I try to remove pkgB then:
pkgA reason = SOLVER_REASON_UNIT_RULE; solver_ruleclass = SOLVER_RULE_PKG
pkgB reason = SOLVER_REASON_UNIT_RULE; solver_ruleclass = SOLVER_RULE_JOB
This is fine, but see other example:

Example 2:
I installed in empty installroot brasero and I have there one userinstalled package and many dependenciies.
Then I try to remove brasero and I got funny result (3 types).
brasero reason = SOLVER_REASON_UNIT_RULE; solver_ruleclass = SOLVER_RULE_JOB (expexted)
atk reason = SOLVER_REASON_CLEANDEPS_ERASE; solver_ruleclass = SOLVER_RULE_UNKNOWN (expexted)
osinfo-db reason = SOLVER_REASON_UNIT_RULE; solver_ruleclass = SOLVER_RULE_PKG (confusing)

It means that by the present implementation I cannot distinguish between reason of removal of testB in first example and osinfo-db in second example. From my point of view the osinfo-db should be reported as SOLVER_REASON_CLEANDEPS_ERASE. Additional information in this case are really confusing (SOLVER_RULE_PKG).

Probably we use incorrect functionality in libsolv, or some part of code works incorrect, but the present implementation does not help us to get information that we requires. Thanks a lot for your help.

@mlschroe
Copy link
Member

The point that I'm trying to make is that the reason is no good indicator for the cleandeps status. Consider this testcase:

epo system 0 testtags <inline>
#>=Pkg: A 1 1 noarch
#>=Req: B
#>=Pkg: B 1 1 noarch
repo test 0 testtags <inline>
#>=Pkg: A 2 1 noarch
#>=Con: B
#>=Pkg: B 2 1 noarch
system unset rpm system
job update name A = 2-1 [cleandeps]
result transaction,problems,reason

This gives the following result:

erase B-1-1.noarch@system
reason A-1-1.noarch@system unit c455423d6427a1b3c65df4f4410cdee0
reason A-2-1.noarch@test update 498b1f4b52d78a9f6016ed0fc414c0ab
reason B-1-1.noarch@system unit b3b1851b907acf722a2db03ed73aa713
reason B-2-1.noarch@test unit aae7440f3f9a42298ab5c6f76f24d122
upgrade A-1-1.noarch@system A-2-1.noarch@test

As you can see, there is no cleandeps reason at all. But B is erased because of cleandeps.

IMHO libsolv should get a new interface to query cleandeps packages, e.g. solver_get_cleandeps(Solver *solv, Queue *q). It would fill the queue with all cleandeps packages.

mlschroe added a commit that referenced this pull request Sep 11, 2017
@mlschroe
Copy link
Member

I added that in commit 03fb95e

Please check if that meets your needs and reopen if not.

@mlschroe mlschroe closed this Sep 11, 2017
@j-mracek
Copy link
Contributor Author

@mlschroe Thanks a lot, it looks like that this is an alternative solution. I already created a patch that will use the functionality (rpm-software-management/libdnf#325).

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.

None yet

2 participants