Skip to content

Commit d7e1192

Browse files
pkratochppisar
pkratoch
authored and
ppisar
committed
comps: Store comps xml files in groups and environments subdirectories
It is possible to have a group and an environment with the same id, in which case installing both attempts to save the xml definitions into the same file, which results in replacing the one that was created first. By having a separate subdirectories for groups and environments, there are no conflicts in the file names. This also adds a logic into fix_group_missing_xml to check if the file is in the parent directory first, and if so, move the file into the proper subdirectory.
1 parent 9fec3d6 commit d7e1192

File tree

4 files changed

+111
-61
lines changed

4 files changed

+111
-61
lines changed

‎dnf5.spec‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,8 @@ Package management library.
416416
%{_libdir}/libdnf5.so.2*
417417
%dir %{_prefix}/lib/sysimage/libdnf5
418418
%attr(0755, root, root) %ghost %dir %{_prefix}/lib/sysimage/libdnf5/comps_groups
419+
%attr(0755, root, root) %ghost %dir %{_prefix}/lib/sysimage/libdnf5/comps_groups/environments
420+
%attr(0755, root, root) %ghost %dir %{_prefix}/lib/sysimage/libdnf5/comps_groups/groups
419421
%verify(not md5 size mtime) %attr(0644, root, root) %ghost %{_prefix}/lib/sysimage/libdnf5/environments.toml
420422
%verify(not md5 size mtime) %attr(0644, root, root) %ghost %{_prefix}/lib/sysimage/libdnf5/groups.toml
421423
%verify(not md5 size mtime) %attr(0644, root, root) %ghost %{_prefix}/lib/sysimage/libdnf5/modules.toml

‎libdnf5/base/transaction.cpp‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,10 +1170,13 @@ Transaction::TransactionRunResult Transaction::Impl::_run(
11701170

11711171
// Set correct system state for groups in the transaction
11721172
auto comps_xml_dir = system_state.get_group_xml_dir();
1173-
std::filesystem::create_directories(comps_xml_dir);
1173+
auto comps_xml_dir_groups = comps_xml_dir / "groups";
1174+
auto comps_xml_dir_environments = comps_xml_dir / "environments";
1175+
std::filesystem::create_directories(comps_xml_dir_groups);
1176+
std::filesystem::create_directories(comps_xml_dir_environments);
11741177
for (const auto & tsgroup : groups) {
11751178
auto group = tsgroup.get_group();
1176-
auto group_xml_path = comps_xml_dir / (group.get_groupid() + ".xml");
1179+
auto group_xml_path = comps_xml_dir_groups / (group.get_groupid() + ".xml");
11771180
if (transaction_item_action_is_inbound(tsgroup.get_action())) {
11781181
libdnf5::system::GroupState state;
11791182
state.userinstalled = tsgroup.get_reason() == transaction::TransactionItemReason::USER;
@@ -1214,7 +1217,7 @@ Transaction::TransactionRunResult Transaction::Impl::_run(
12141217
// Set correct system state for environmental groups in the transaction
12151218
for (const auto & tsenvironment : environments) {
12161219
auto environment = tsenvironment.get_environment();
1217-
auto environment_xml_path = comps_xml_dir / (environment.get_environmentid() + ".xml");
1220+
auto environment_xml_path = comps_xml_dir_environments / (environment.get_environmentid() + ".xml");
12181221
if (transaction_item_action_is_inbound(tsenvironment.get_action())) {
12191222
libdnf5::system::EnvironmentState state;
12201223
// Remember groups installed by this environmental group
@@ -1497,7 +1500,7 @@ std::string Transaction::serialize(
14971500
group_replay.package_types = group.get_package_types();
14981501

14991502
if (!comps_path.empty()) {
1500-
group_replay.group_path = build_comps_xml_path(comps_path, xml_group.get_groupid());
1503+
group_replay.group_path = build_comps_xml_path(comps_path / "groups", xml_group.get_groupid());
15011504
}
15021505

15031506
transaction_replay.groups.push_back(group_replay);
@@ -1518,7 +1521,8 @@ std::string Transaction::serialize(
15181521
}
15191522

15201523
if (!comps_path.empty()) {
1521-
environment_replay.environment_path = build_comps_xml_path(comps_path, xml_environment.get_environmentid());
1524+
environment_replay.environment_path =
1525+
build_comps_xml_path(comps_path / "environments", xml_environment.get_environmentid());
15221526
}
15231527

15241528
transaction_replay.environments.push_back(environment_replay);
@@ -1532,18 +1536,22 @@ std::string Transaction::serialize(
15321536
void Transaction::store_comps(const std::filesystem::path & comps_path) const {
15331537
auto groups = get_transaction_groups();
15341538
auto envs = get_transaction_environments();
1535-
if (!groups.empty() || !envs.empty()) {
1536-
std::filesystem::create_directories(comps_path);
1537-
}
1538-
1539-
for (const auto & group : groups) {
1540-
auto xml_group = group.get_group();
1541-
xml_group.serialize(build_comps_xml_path(comps_path, xml_group.get_groupid()));
1539+
if (!groups.empty()) {
1540+
auto comps_xml_dir_groups = comps_path / "groups";
1541+
std::filesystem::create_directories(comps_xml_dir_groups);
1542+
for (const auto & group : groups) {
1543+
auto xml_group = group.get_group();
1544+
xml_group.serialize(build_comps_xml_path(comps_xml_dir_groups, xml_group.get_groupid()));
1545+
}
15421546
}
1543-
1544-
for (const auto & environment : envs) {
1545-
auto xml_environment = environment.get_environment();
1546-
xml_environment.serialize(build_comps_xml_path(comps_path, xml_environment.get_environmentid()));
1547+
if (!envs.empty()) {
1548+
auto comps_xml_dir_environments = comps_path / "environments";
1549+
std::filesystem::create_directories(comps_xml_dir_environments);
1550+
for (const auto & environment : envs) {
1551+
auto xml_environment = environment.get_environment();
1552+
xml_environment.serialize(
1553+
build_comps_xml_path(comps_xml_dir_environments, xml_environment.get_environmentid()));
1554+
}
15471555
}
15481556
}
15491557

‎libdnf5/repo/repo_sack.cpp‎

Lines changed: 83 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -922,44 +922,67 @@ void RepoSack::Impl::fix_group_missing_xml() {
922922
auto & logger = *base->get_logger();
923923
auto & system_state = base->p_impl->get_system_state();
924924
auto comps_xml_dir = system_state.get_group_xml_dir();
925+
auto comps_xml_dir_groups = comps_xml_dir / "groups";
926+
auto comps_xml_dir_environments = comps_xml_dir / "environments";
925927
bool directory_exists = true;
926928
std::error_code ec;
927-
std::filesystem::create_directories(comps_xml_dir, ec);
929+
std::filesystem::create_directories(comps_xml_dir_groups, ec);
928930
if (ec) {
929-
logger.debug("Failed to create directory \"{}\": {}", comps_xml_dir.string(), ec.message());
931+
logger.debug("Failed to create directory \"{}\": {}", comps_xml_dir_groups.string(), ec.message());
932+
directory_exists = false;
933+
}
934+
std::filesystem::create_directories(comps_xml_dir_environments, ec);
935+
if (ec) {
936+
logger.debug("Failed to create directory \"{}\": {}", comps_xml_dir_environments.string(), ec.message());
930937
directory_exists = false;
931938
}
932939
if (!group_missing_xml.empty()) {
933940
libdnf5::comps::GroupQuery available_groups(base);
934941
available_groups.filter_installed(false);
935942
for (const auto & group_id : group_missing_xml) {
936-
bool xml_saved = false;
943+
bool group_loaded = false;
937944
if (directory_exists) {
938-
// try to find the group id in availables
939-
libdnf5::comps::GroupQuery group_query(available_groups);
940-
group_query.filter_groupid(group_id);
941-
if (group_query.size() == 1) {
942-
// GroupQuery is basically a set thus iterators and `.get()` method
943-
// return `const Group` objects.
944-
// To call non-const serialize method we need to make a copy here.
945-
libdnf5::comps::Group group = group_query.get();
946-
auto xml_file_name = comps_xml_dir / (group_id + ".xml");
947-
logger.debug(
948-
"Re-creating installed group \"{}\" definition to file \"{}\".",
949-
group_id,
950-
xml_file_name.string());
951-
try {
952-
group.serialize(xml_file_name);
953-
xml_saved = true;
954-
} catch (utils::xml::XMLSaveError & ex) {
955-
logger.debug(ex.what());
945+
auto xml_file_name = comps_xml_dir_groups / (group_id + ".xml");
946+
auto xml_file_name_in_comps_dir = comps_xml_dir / (group_id + ".xml");
947+
// Try to copy the xml file from the comps directory where it used to be stored.
948+
if (solv_repo.read_group_solvable_from_xml(xml_file_name_in_comps_dir)) {
949+
group_loaded = true;
950+
std::filesystem::rename(xml_file_name_in_comps_dir, xml_file_name, ec);
951+
if (ec) {
952+
logger.debug(
953+
"Failed to move \"{}\" into directory \"{}\": {}",
954+
xml_file_name_in_comps_dir.string(),
955+
comps_xml_dir_groups.string(),
956+
ec.message());
956957
}
957-
if (xml_saved) {
958-
solv_repo.read_group_solvable_from_xml(xml_file_name);
958+
} else {
959+
// try to find the group id in availables
960+
libdnf5::comps::GroupQuery group_query(available_groups);
961+
group_query.filter_groupid(group_id);
962+
if (group_query.size() == 1) {
963+
// GroupQuery is basically a set thus iterators and `.get()` method
964+
// return `const Group` objects.
965+
// To call non-const serialize method we need to make a copy here.
966+
libdnf5::comps::Group group = group_query.get();
967+
logger.debug(
968+
"Re-creating installed group \"{}\" definition to file \"{}\".",
969+
group_id,
970+
xml_file_name.string());
971+
bool xml_saved = false;
972+
try {
973+
group.serialize(xml_file_name);
974+
xml_saved = true;
975+
} catch (utils::xml::XMLSaveError & ex) {
976+
logger.debug(ex.what());
977+
}
978+
if (xml_saved) {
979+
solv_repo.read_group_solvable_from_xml(xml_file_name);
980+
group_loaded = true;
981+
}
959982
}
960983
}
961984
}
962-
if (!xml_saved) {
985+
if (!group_loaded) {
963986
// fall-back to creating solvables only from system state
964987
solv_repo.create_group_solvable(group_id, system_state.get_group_state(group_id));
965988
}
@@ -969,30 +992,47 @@ void RepoSack::Impl::fix_group_missing_xml() {
969992
libdnf5::comps::EnvironmentQuery available_environments(base);
970993
available_environments.filter_installed(false);
971994
for (const auto & environment_id : environments_missing_xml) {
972-
bool xml_saved = false;
995+
bool environment_loaded = false;
973996
if (directory_exists) {
974-
// try to find the environment id in availables
975-
libdnf5::comps::EnvironmentQuery environment_query(available_environments);
976-
environment_query.filter_environmentid(environment_id);
977-
if (environment_query.size() == 1) {
978-
libdnf5::comps::Environment environment = environment_query.get();
979-
auto xml_file_name = comps_xml_dir / (environment_id + ".xml");
980-
logger.debug(
981-
"Re-creating installed environmental group \"{}\" definition to file \"{}\".",
982-
environment_id,
983-
xml_file_name.string());
984-
try {
985-
environment.serialize(xml_file_name);
986-
xml_saved = true;
987-
} catch (utils::xml::XMLSaveError & ex) {
988-
logger.debug(ex.what());
997+
auto xml_file_name = comps_xml_dir_groups / (environment_id + ".xml");
998+
auto xml_file_name_in_comps_dir = comps_xml_dir / (environment_id + ".xml");
999+
// Try to copy the xml file from the comps directory where it used to be stored.
1000+
if (solv_repo.read_group_solvable_from_xml(xml_file_name_in_comps_dir)) {
1001+
environment_loaded = true;
1002+
std::filesystem::rename(xml_file_name_in_comps_dir, xml_file_name, ec);
1003+
if (ec) {
1004+
logger.debug(
1005+
"Failed to move \"{}\" into directory \"{}\": {}",
1006+
xml_file_name_in_comps_dir.string(),
1007+
comps_xml_dir_groups.string(),
1008+
ec.message());
9891009
}
990-
if (xml_saved) {
991-
solv_repo.read_group_solvable_from_xml(xml_file_name);
1010+
} else {
1011+
// try to find the environment id in availables
1012+
libdnf5::comps::EnvironmentQuery environment_query(available_environments);
1013+
environment_query.filter_environmentid(environment_id);
1014+
if (environment_query.size() == 1) {
1015+
libdnf5::comps::Environment environment = environment_query.get();
1016+
auto xml_file_name = comps_xml_dir_environments / (environment_id + ".xml");
1017+
logger.debug(
1018+
"Re-creating installed environmental group \"{}\" definition to file \"{}\".",
1019+
environment_id,
1020+
xml_file_name.string());
1021+
bool xml_saved = false;
1022+
try {
1023+
environment.serialize(xml_file_name);
1024+
xml_saved = true;
1025+
} catch (utils::xml::XMLSaveError & ex) {
1026+
logger.debug(ex.what());
1027+
}
1028+
if (xml_saved) {
1029+
solv_repo.read_group_solvable_from_xml(xml_file_name);
1030+
environment_loaded = true;
1031+
}
9921032
}
9931033
}
9941034
}
995-
if (!xml_saved) {
1035+
if (!environment_loaded) {
9961036
// fall-back to creating solvables only from system state
9971037
solv_repo.create_environment_solvable(
9981038
environment_id, system_state.get_environment_state(environment_id));

‎libdnf5/repo/solv_repo.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,15 +298,15 @@ void SolvRepo::load_system_repo_ext(RepodataType type) {
298298
auto & system_state = base->p_impl->get_system_state();
299299
auto comps_dir = system_state.get_group_xml_dir();
300300
for (auto & group_id : system_state.get_installed_groups()) {
301-
auto ext_fn = comps_dir / (group_id + ".xml");
301+
auto ext_fn = comps_dir / "groups" / (group_id + ".xml");
302302
if (!read_group_solvable_from_xml(ext_fn)) {
303303
// The group xml file either not exists or is not parseable by
304304
// libsolv.
305305
groups_missing_xml.push_back(std::move(group_id));
306306
}
307307
}
308308
for (auto & environment_id : system_state.get_installed_environments()) {
309-
auto ext_fn = comps_dir / (environment_id + ".xml");
309+
auto ext_fn = comps_dir / "environments" / (environment_id + ".xml");
310310
if (!read_group_solvable_from_xml(ext_fn)) {
311311
// The environment xml file either not exists or is not parseable by
312312
// libsolv.

0 commit comments

Comments
 (0)