Skip to content

Commit 1a61afc

Browse files
jlebonevan-goode
authored andcommitted
transaction: sort packages when SOURCE_DATE_EPOCH set
For reproducible builds that involve RPMs, the exact order in which packages are installed matters. This affects some files that are easy to normalize in postprocessing (e.g. `/etc/passwd` line order), but it also affects the rpmdb, where e.g. row order may change. See this comment and following: rpm-software-management/rpm#2219 (comment) This was fixed for the rpm-ostree use case by sorting the packages before adding them to the transaction and calling rpmtsOrder(): coreos/rpm-ostree#5469 This is the dnf equivalent. We only do this if SOURCE_DATE_EPOCH is defined for two reasons: 1. It reduces risk by limiting it to the group of people who actually care about these details. (Though ironically, this change itself will cause a rebuild of reproducible artifacts.) 2. It mitigates the concern raised in the linked discussions regarding well-defined reordering possibly masking bugs in RPMs that unknowingly rely on a specific order. Ideally, this functionality would live on the rpm side instead. For the same reasons as above, this would also likely have to be an opt-in flag. Once we have that though, the logic here could be simplified. Assisted-by: Claude Code / Sonnet 4.5
1 parent 0c30f11 commit 1a61afc

File tree

4 files changed

+99
-0
lines changed

4 files changed

+99
-0
lines changed

libdnf5/rpm/transaction.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include <sys/stat.h>
4040
#include <sys/types.h>
4141

42+
#include <algorithm>
4243
#include <map>
4344

4445

@@ -140,6 +141,15 @@ std::string Transaction::get_db_cookie() const {
140141
void Transaction::fill(const base::Transaction & transaction) {
141142
transaction_items = transaction.get_transaction_packages();
142143

144+
// If SOURCE_DATE_EPOCH is set, sort the package list for reproducibility
145+
if (std::getenv("SOURCE_DATE_EPOCH") != nullptr) {
146+
auto & logger = *base->get_logger();
147+
logger.debug("SOURCE_DATE_EPOCH detected; sorting {} packages for reproducibility", transaction_items.size());
148+
std::sort(transaction_items.begin(), transaction_items.end(), [](const auto & a, const auto & b) {
149+
return a.get_package().get_nevra() < b.get_package().get_nevra();
150+
});
151+
}
152+
143153
// Auxiliary map name->package with the latest versions of currently
144154
// installed installonly packages.
145155
// Used to detect installation of a installonly package with lower version
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
Name: three
2+
Epoch: 0
3+
Version: 1
4+
Release: 1
5+
Vendor: dnf5-test
6+
7+
License: Public Domain
8+
URL: http://example.com/
9+
10+
Summary: A dummy package
11+
BuildArch: noarch
12+
13+
%description
14+
A dummy package.
15+
16+
%files
17+
18+
%changelog

test/libdnf5/rpm/test_transaction.cpp

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ class PackageDownloadCallbacks : public libdnf5::repo::DownloadCallbacks {
7878
int mirror_failure_cnt{0};
7979
};
8080

81+
class OrderCapturingCallbacks : public libdnf5::rpm::TransactionCallbacks {
82+
public:
83+
explicit OrderCapturingCallbacks(std::vector<std::string> expected) : expected_order(std::move(expected)) {}
84+
85+
void install_start(const libdnf5::base::TransactionPackage & item, [[maybe_unused]] uint64_t total) override {
86+
auto nevra = item.get_package().get_nevra();
87+
if (current_index >= expected_order.size() || nevra != expected_order[current_index]) {
88+
order_correct = false;
89+
}
90+
current_index++;
91+
}
92+
93+
std::vector<std::string> expected_order;
94+
size_t current_index = 0;
95+
bool order_correct = true;
96+
};
97+
8198
} // namespace
8299

83100

@@ -161,3 +178,55 @@ void RpmTransactionTest::test_transaction_temp_files_cleanup() {
161178
CPPUNIT_ASSERT(!std::filesystem::exists(package_path));
162179
}
163180
}
181+
182+
void RpmTransactionTest::test_source_date_epoch_sorting() {
183+
add_repo_rpm("rpm-repo1", /* load */ false);
184+
add_repo_rpm("rpm-repo2", /* load */ false);
185+
add_repo_rpm("rpm-repo3", /* load */ true);
186+
187+
// setting a global var in a test is not great... no other tests right now
188+
// relate to SOURCE_DATE_EPOCH at least
189+
setenv("SOURCE_DATE_EPOCH", "1234567890", 1);
190+
191+
std::vector<std::string> packages = {"one", "two", "three"};
192+
// The exact final install order chosen by librpm doesn't matter. What
193+
// matters is that it's consistent across all permutations. An alternative
194+
// approach resilient to potential librpm changes is to capture the order on
195+
// the first iteration and then compare future iterations against that.
196+
std::vector<std::string> expected_order = {"two-2-2.noarch", "three-1-1.noarch", "one-2-1.noarch"};
197+
auto installroot = base.get_config().get_installroot_option().get_value();
198+
199+
std::sort(packages.begin(), packages.end());
200+
do {
201+
// recreate the installroot each time so we start from scratch
202+
std::filesystem::remove_all(installroot);
203+
std::filesystem::create_directory(installroot);
204+
205+
libdnf5::Goal goal(base);
206+
for (const auto & pkg : packages) {
207+
goal.add_rpm_install(pkg);
208+
}
209+
210+
auto transaction = goal.resolve();
211+
transaction.download();
212+
213+
auto callbacks = std::make_unique<OrderCapturingCallbacks>(expected_order);
214+
auto callbacks_ptr = callbacks.get();
215+
transaction.set_callbacks(std::move(callbacks));
216+
217+
// TODO(jkolarik): Temporarily disable the test to allow further investigation of issues on the RISC-V arch
218+
// See https://github.com/rpm-software-management/dnf5/issues/503
219+
auto res = transaction.run();
220+
if (res != libdnf5::base::Transaction::TransactionRunResult::SUCCESS) {
221+
std::cout << std::endl << "WARNING: Transaction was not successful" << std::endl;
222+
std::cout << libdnf5::utils::string::join(transaction.get_transaction_problems(), ", ") << std::endl;
223+
break; // no point in trying more permutations if arch is broken
224+
}
225+
226+
CPPUNIT_ASSERT(callbacks_ptr->order_correct);
227+
CPPUNIT_ASSERT_EQUAL(expected_order.size(), callbacks_ptr->current_index);
228+
229+
} while (std::next_permutation(packages.begin(), packages.end()));
230+
231+
unsetenv("SOURCE_DATE_EPOCH");
232+
}

test/libdnf5/rpm/test_transaction.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,13 @@ class RpmTransactionTest : public BaseTestCase {
3131
CPPUNIT_TEST_SUITE(RpmTransactionTest);
3232
CPPUNIT_TEST(test_transaction);
3333
CPPUNIT_TEST(test_transaction_temp_files_cleanup);
34+
CPPUNIT_TEST(test_source_date_epoch_sorting);
3435
CPPUNIT_TEST_SUITE_END();
3536

3637
public:
3738
void test_transaction();
3839
void test_transaction_temp_files_cleanup();
40+
void test_source_date_epoch_sorting();
3941
};
4042

4143
#endif

0 commit comments

Comments
 (0)