Fix installed groups loading to libsolv - take 2 #424
Conversation
The repo contains installed comps groups and should be also considered as a system repo by libsolv.
By default libxml2 prints errors (e.g. "permission denied") directly to stderr which interferes with dnf5 output. This patch collects all errors to make them part of the exception message later.
Reads one group definition from xml file.
Creates a minimal group solvable from information contained in the system state - groupid and a list of installed packages from the group.
In case the xml definition for an installed group does not exist, try to re-create it from available repositories, with fall-back to creating minimal group solvable using only data available in system state.
m-blaha
force-pushed
the
mblaha/group_missing_xml_minimal
branch
from
642d222 to
0cc48ce
Compare
libdnf/repo/solv_repo.cpp
Outdated
| } | ||
| logger.debug("Loading {} extension for system repo from \"{}\"", type_name, ext_fn.string()); | ||
| if ((res = repo_add_comps(comps_repo, ext_file.get(), 0)) == 0) { | ||
| if (read_group_solvable_from_xml(ext_fn)) { | ||
| comps_solvables_start = solvables_start; |
There was a problem hiding this comment.
This assigns the same value for each iteration right?
Could it be done just once above the loop?
| comps_repo == (*pool)->installed, "SolvRepo::create_group_solvable() call enabled only for @System repo."); | ||
|
|
||
| // create a new solvable for the group | ||
| auto group_solvable_id = repo_add_solvable(comps_repo); |
There was a problem hiding this comment.
I see the @system repo is keeping the comps_solvables_start and comps_solvables_end up to date even though I don't see any place where it uses it.
I think adding these new solvables makes the end invalid, I wonder if it could cause any problems in the future.
This is just a note, I am not sure what to do about it anyway.
There was a problem hiding this comment.
The comps_solvables_start and comps_solvables_end are used in write_ext() function to set range of solvables to be written to solvx file. But given that now comps data are stored in separate pool (and separate solv repo), I believe we can drop the range and just write all the solvables. Having invalid range could definitely cause problems once we start using *.solvx files for the system repo.
There was a problem hiding this comment.
I've added a commit with comps solvables range removal.
There was a problem hiding this comment.
Having invalid range could definitely cause problems once we start using *.solvx files for the system repo.
oh, I didn't realize that there will be *.solvx for system repo.
ok, great, removal of unneeded code is the best solution. 👍
libdnf/repo/solv_repo.cpp
Outdated
| logger.debug("Loading group extension for system repo from \"{}\"", path); | ||
| read_success = repo_add_comps(comps_repo, ext_file.get(), 0) == 0; | ||
| if (read_success) { | ||
| comps_solvables_end = pool->nsolvables; |
There was a problem hiding this comment.
Calling this from fix_group_missing_xml() could also mess up the comps_solvables_end if there were additional data in the @system repo after the group solvables.
Like I mention in the other comment this is currently not an issue.
Since now the comps metadata are loaded into separate libsolv pool (and thus also separate libsolv repo) we do not need to set solvable ranges for repowriter. We can directly write all the comps solvables.
This is alternative solution to #406
Advantage of this PR is that there is no change in system state, therefore no duplicated information.
Installed group xml is now taken from available repositories, with a fall-back to generating minimal solvable from system state information in case the group is not available in repos.
Fixes: #420