public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
* [rpms/librepo] f44: Backport PR #385 to fix bug #384 breaking Fedora CI
@ 2026-09-05 15:18 Adam Williamson
  0 siblings, 0 replies; only message in thread
From: Adam Williamson @ 2026-09-05 15:18 UTC (permalink / raw)
  To: git-commits

A new commit has been pushed.

Repo   : rpms/librepo
Branch : f44
Commit : aedb26a58ded3df234a9f950e88ad8583edd7024
Author : Adam Williamson <adamwill@fedoraproject.org>
Date   : 2026-09-05T08:18:12-07:00
Stats  : +136/-1 in 2 file(s)
URL    : https://src.fedoraproject.org/rpms/librepo/c/aedb26a58ded3df234a9f950e88ad8583edd7024?branch=f44

Log:
Backport PR #385 to fix bug #384 breaking Fedora CI

---
diff --git a/385.patch b/385.patch
new file mode 100644
index 0000000..df617d7
--- /dev/null
+++ b/385.patch
@@ -0,0 +1,127 @@
+From a5094146f22b033cd493e1404c6b6a85087aa5ca Mon Sep 17 00:00:00 2001
+From: Marek Blaha <mblaha@redhat.com>
+Date: Fri, 4 Sep 2026 10:31:41 +0000
+Subject: [PATCH] Fix file corruption when re-downloading a completed file with
+ resume
+
+When resume was requested for a file that already existed complete on
+disk but no longer carried the librepo xattr (e.g. a package fully
+downloaded in a previous run - the xattr is removed once a download
+finishes), prepare_next_transfer() determined the offset by seeking to
+the end of the file and then, finding no xattr, truncated the file back
+to zero. The truncation left the stdio stream position at the old end of
+file, so the freshly downloaded data was written after a zero-filled
+hole, doubling the file size and corrupting it.
+
+Rewind the stream to the beginning after truncating so the data is
+written from offset 0.
+
+This manifested in dnf5 as a corrupted cached RPM on a second install of
+a command-line package given by URL.
+
+Fixes: https://github.com/rpm-software-management/librepo/issues/384
+
+Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
+Signed-off-by: Marek Blaha <mblaha@redhat.com>
+---
+ librepo/downloader.c                          |  6 ++
+ .../tests/test_yum_package_downloading.py     | 69 +++++++++++++++++++
+ 2 files changed, 75 insertions(+)
+
+diff --git a/librepo/downloader.c b/librepo/downloader.c
+index 25562a96..c63b7809 100644
+--- a/librepo/downloader.c
++++ b/librepo/downloader.c
+@@ -1613,6 +1613,12 @@ prepare_next_transfer(LrDownload *dd, gboolean *candidatefound, GError **err)
+                             "ftruncate() failed: %s", g_strerror(errno));
+                 goto fail;
+             }
++            // The stream position was moved to the end of the file while
++            // determining the offset above. After truncating the file back
++            // to zero, rewind the stream so the freshly downloaded data is
++            // written from the beginning instead of leaving a zero-filled
++            // hole (which would corrupt and double the size of the file).
++            fseek(target->f, 0L, SEEK_SET);
+             target->original_offset = 0;
+         } else {
+             gint64 used_offset = target->original_offset;
+diff --git a/tests/python/tests/test_yum_package_downloading.py b/tests/python/tests/test_yum_package_downloading.py
+index 6002f6c7..e95c72f4 100644
+--- a/tests/python/tests/test_yum_package_downloading.py
++++ b/tests/python/tests/test_yum_package_downloading.py
+@@ -814,6 +814,75 @@ def test_download_packages_resume_on_mirror_switch(self):
+         sha256 = hashlib.sha256(data).hexdigest()
+         self.assertEqual(sha256, config.PACKAGE_01_01_SHA256)
+ 
++    def test_download_packages_resume_complete_file_no_xattr(self):
++        # Regression test for
++        # https://github.com/rpm-software-management/librepo/issues/384
++        #
++        # A complete file already exists at the destination but it no longer
++        # carries the librepo xattr - e.g. a package fully downloaded during a
++        # previous run (the xattr is removed once a download finishes).
++        # Downloading it again with resume=True but without a known checksum
++        # or expected size (as libdnf5 does for a command-line package given
++        # by URL on a repeated install) must not corrupt the file. Because no
++        # checksum/size is known, librepo cannot short-circuit as "already
++        # downloaded" and actually re-fetches over the existing file.
++        # Previously the offset was determined by seeking to the end of the
++        # file, and after the file was truncated back to zero the stream
++        # position was left at the old end, so the fresh data was written
++        # after a zero-filled hole, doubling the file size and corrupting it.
++        h = librepo.Handle()
++        h.urls = ["%s%s" % (self.MOCKURL, config.REPO_YUM_01_PATH)]
++        h.repotype = librepo.LR_YUMREPO
++
++        # First download - a normal complete download. On success librepo
++        # removes the "downloadinprogress" xattr, leaving a complete file
++        # with no librepo xattr (the state after any finished download).
++        pkgs = [librepo.PackageTarget(config.PACKAGE_01_01,
++                                      handle=h,
++                                      dest=self.tmpdir,
++                                      checksum_type=librepo.SHA256,
++                                      checksum=config.PACKAGE_01_01_SHA256)]
++        librepo.download_packages(pkgs, failfast=True)
++        first = pkgs[0]
++        self.assertTrue(first.err is None)
++        self.assertTrue(os.path.isfile(first.local_path))
++        local_path = first.local_path
++        expected_size = os.path.getsize(local_path)
++
++        # Sanity check: the xattr must be gone after a finished download,
++        # otherwise the resume branch below would not be exercised.
++        try:
++            xattr.getxattr(local_path,
++                           "user.librepo.downloadinprogress".encode("utf-8"))
++            has_xattr = True
++        except IOError as err:
++            if err.errno == errno.EOPNOTSUPP:
++                self.skipTest('extended attributes are not supported')
++            has_xattr = False
++        except OSError:
++            has_xattr = False
++        self.assertFalse(has_xattr,
++                         "xattr should be removed after a finished download")
++
++        # Second download with resume=True but WITHOUT a checksum or expected
++        # size - librepo re-fetches over the existing complete file.
++        pkgs2 = [librepo.PackageTarget(config.PACKAGE_01_01,
++                                       handle=h,
++                                       dest=self.tmpdir,
++                                       resume=True)]
++        librepo.download_packages(pkgs2, failfast=True)
++        second = pkgs2[0]
++        self.assertTrue(second.err is None)
++        self.assertTrue(os.path.isfile(second.local_path))
++
++        # File must not be corrupted: same size (not doubled) and correct
++        # checksum (not prefixed with a zero-filled hole).
++        self.assertEqual(os.path.getsize(second.local_path), expected_size)
++        with open(second.local_path, 'rb') as f:
++            data = f.read()
++        sha256 = hashlib.sha256(data).hexdigest()
++        self.assertEqual(sha256, config.PACKAGE_01_01_SHA256)
++
+     def test_download_packages_mirror_penalization_01(self):
+ 
+         # This test is useful for mirror penalization testing

diff --git a/librepo.spec b/librepo.spec
index c15785d..dd208b5 100644
--- a/librepo.spec
+++ b/librepo.spec
@@ -31,12 +31,17 @@
 
 Name:           librepo
 Version:        1.21.0
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Repodata downloading library
 
 License:        LGPL-2.1-or-later
 URL:            https://github.com/rpm-software-management/librepo
 Source0:        %{url}/archive/%{version}/%{name}-%{version}.tar.gz
+# https://github.com/rpm-software-management/librepo/issues/384
+# https://github.com/rpm-software-management/librepo/pull/385
+# Fix file corruption when re-downloading a completed file with resume
+# This was heavily affecting Fedora CI and some other CI workflows
+Patch:          385.patch
 
 BuildRequires:  cmake
 BuildRequires:  gcc
@@ -140,6 +145,9 @@ Python 3 bindings for the librepo library.
 %{python3_sitearch}/%{name}/
 
 %changelog
+* Sat Sep 05 2026 Adam Williamson <adamwill@fedoraproject.org> - 1.21.0-2
+- Backport PR #385 to fix bug #384 breaking Fedora CI
+
 * Tue Sep 01 2026 Marek Blaha <mblaha@redhat.com> - 1.21.0-1
 - Update to version 1.21.0
 

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-09-05 15:18 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-05 15:18 [rpms/librepo] f44: Backport PR #385 to fix bug #384 breaking Fedora CI Adam Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox