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

Use correct path when destdir option is set #1628

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Use correct path when destdir option is set #1628

merged 3 commits into from
Aug 16, 2024

Conversation

m-blaha
Copy link
Member

@m-blaha m-blaha commented Aug 14, 2024

Changes:

1e83b07 (Marek Blaha, 9 minutes ago)
repo: New Repo.get_packages_download_dir() method

Currently, the path where a package should be downloaded is computed in
several places. This patch introduces a single location for this
computation to reduce code duplication. The patch also fixes usage of
incorrect package path in Package::get_package_path() call - the destdir
config option was not taken into account.

07105d0 (Marek Blaha, 2 hours ago)
doc: Document destdir main config option

Resolves: #1620
Tests: rpm-software-management/ci-dnf-stack#1536

Currently, the path where a package should be downloaded is computed in
several places. This patch introduces a single location for this
computation to reduce code duplication.
The patch also fixes usage of incorrect package path in
Package::get_package_path() call - the destdir config option was not
taken into account.
@pkratoch pkratoch self-assigned this Aug 14, 2024
@pkratoch
Copy link
Contributor

Thanks for the patch! I noticed one test that looks related to the change broke ("skip downloading of already downloaded rpm file" in the dnf/download.feature).

@m-blaha
Copy link
Member Author

m-blaha commented Aug 14, 2024

Yeah, I was lazy to run the tests locally and that's the result... I'll check it out, meanwhile switching to draft state.

@m-blaha m-blaha marked this pull request as draft August 14, 2024 14:50
@m-blaha
Copy link
Member Author

m-blaha commented Aug 14, 2024

So the issue is not with this PR itself, but was introduced in commit 59538f3 which did not properly call download callbacks in case the source and destination of a downloaded file were the same. The problem did not surface before this PR, because the package was not correctly identified as locally available due to an incorrect path being used.

The download callbacks were not called in case the source and
destination of locally available downloaded file were the same.
@pkratoch
Copy link
Contributor

Thank you!

@pkratoch pkratoch added this pull request to the merge queue Aug 16, 2024
Merged via the queue into main with commit b52426e Aug 16, 2024
11 of 16 checks passed
@pkratoch pkratoch deleted the mblaha/destdir branch August 16, 2024 07:44
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.

dnf fails to install packages with destdir option set
2 participants