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

Option "disable_multithreading", repo::RepoSack ctors and repo::Repo::fetch_metadata method private, fix/adjust unit tests #273

Conversation

jrohel
Copy link
Contributor

@jrohel jrohel commented Feb 8, 2023

  • New configuration option "disable_multithreading".
  • RepoSack::update_and_load_repos: Added single-threaded mode (used when multi-threaded mode is disabled
    by configuration option "disable_multithreading".
  • Makes repo::RepoSack ctors and repo::Repo::fetch_metadata method private.
  • Unit tests: Use RepoSack::update_and_load_repos method instead of repo::Repo::fetch_metadata and repo::Repo::load
  • Unit tests: Fix: Use "base.get_repo_sack" in all places

repo->load();
libdnf::repo::RepoQuery repos(base);
repos.filter_id(repoid);
repo_sack->update_and_load_repos(repos);
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a bad feeling about possibility to load subset of repositories using update_and_load_repos(repos). This is not a problem of the PR but our API. The reason is modularity, where it is strongly recommended to have one point of application of modular filtering. The PR is going into right directions.

@j-mracek j-mracek self-assigned this Feb 8, 2023
j-mracek
j-mracek previously approved these changes Feb 8, 2023
Copy link
Contributor

@j-mracek j-mracek left a comment

Choose a reason for hiding this comment

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

LGTM

@j-mracek
Copy link
Contributor

j-mracek commented Feb 8, 2023

@jrohel May I ask you for checking unit tests, they are failing.

@j-mracek j-mracek dismissed their stale review February 8, 2023 16:01

Failing tests

@jrohel jrohel force-pushed the RepoSack_ctor_fetch_metadata_private branch from 8f2a738 to 353f676 Compare February 9, 2023 14:22
@jrohel
Copy link
Contributor Author

jrohel commented Feb 14, 2023

@j-mracek Yes. Unit tests failed. But I hope the problem is not caused by this PR.
In log is:

2023-02-14T12:11:06+0000 [6695] TRACE [rpm] entering chroot /tmp/libdnf5_unittest.wkCbFF/installroot/
2023-02-14T12:11:06+0000 [6695] ERROR [rpm] Unable to change root directory: Operation not permitted

I think the problem is the persmission change in podman (missing CAP_SYS_CHROOT) and librpm fails during chroot.

This PR is a patch for CI tests: rpm-software-management/ci-dnf-stack#1220
And I think we need fix for unit test infrastructure. librpm chroots during unit tests.

@jrohel jrohel force-pushed the RepoSack_ctor_fetch_metadata_private branch 3 times, most recently from fcc85a3 to a2b9c2b Compare February 17, 2023 08:19
@jrohel jrohel force-pushed the RepoSack_ctor_fetch_metadata_private branch 6 times, most recently from bb204ba to 07c6b16 Compare February 22, 2023 05:36
@jrohel jrohel force-pushed the RepoSack_ctor_fetch_metadata_private branch from 07c6b16 to 7ce73f6 Compare March 1, 2023 10:26
It will be used to disable multithreading in libdnf.
@jrohel jrohel force-pushed the RepoSack_ctor_fetch_metadata_private branch from 7ce73f6 to 3e37e98 Compare March 7, 2023 09:47
@jrohel jrohel changed the title Make repo::RepoSack ctors and repo::Repo::fetch_metadata method private, fix and adjust unit tests Option "disable_multithreading", repo::RepoSack ctors and repo::Repo::fetch_metadata method private, fix/adjust unit tests Mar 7, 2023
jrohel added 5 commits March 7, 2023 15:44
Single-threaded mode is used when multi-threaded mode is disabled
(configuration option "disable_multithreading").
Perl test created a new repo_sack instead of getting the one from base.
So for example repo::RepoQuery wouldn't work.
@jrohel jrohel force-pushed the RepoSack_ctor_fetch_metadata_private branch from 3e37e98 to 6145ecc Compare March 7, 2023 14:45
@j-mracek
Copy link
Contributor

j-mracek commented Mar 8, 2023

LGTM

@j-mracek j-mracek merged commit 6c9bc08 into rpm-software-management:main Mar 8, 2023
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