From 822df19165c051fe091f1219f3f866b0423837a0 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 23 Jun 2020 11:17:24 +0300 Subject: [PATCH] Fix (work around) FA_TOUCH not scoping correctly over hardlinks The existing FSM code doesn't correctly handle FA_TOUCH on hardlinked file sets, which causes the hardlink set to break on at least some upgrade scenarios when minimize_writes is enabled. Only enable FA_TOUCH on non-hardlinked files to work around the issue for now. While at it, rearrange the conditionals around min_writes to make it a bit clearer. Testcase adopted from original reproducer by Fabian Vogt, thanks! Fixes: #1278 --- lib/transaction.c | 17 ++++++++++------- tests/Makefile.am | 1 + tests/data/SPECS/hlbreak.spec | 22 ++++++++++++++++++++++ tests/rpmverify.at | 24 +++++++++++++++++++++++- 4 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 tests/data/SPECS/hlbreak.spec diff --git a/lib/transaction.c b/lib/transaction.c index 1156bb9f5b..9fea7b06bd 100644 --- a/lib/transaction.c +++ b/lib/transaction.c @@ -492,13 +492,6 @@ static void handleInstInstalledFile(const rpmts ts, rpmte p, rpmfiles fi, int fx rpmfsSetAction(fs, fx, action); } - /* Skip already existing files - if 'minimize_writes' is set. */ - if ((!isCfgFile) && (rpmfsGetAction(fs, fx) == FA_UNKNOWN) && ts->min_writes) { - if (rpmfileContentsEqual(otherFi, ofx, fi, fx)) { - rpmfsSetAction(fs, fx, FA_TOUCH); - } - } - otherFileSize = rpmfilesFSize(otherFi, ofx); /* Only account for the last file of a hardlink set */ @@ -508,6 +501,16 @@ static void handleInstInstalledFile(const rpmts ts, rpmte p, rpmfiles fi, int fx /* Add one to make sure the size is not zero */ rpmfilesSetFReplacedSize(fi, fx, otherFileSize + 1); + + /* Just touch already existing files if minimize_writes is enabled */ + if (ts->min_writes) { + if ((!isCfgFile) && (rpmfsGetAction(fs, fx) == FA_UNKNOWN)) { + /* XXX fsm can't handle FA_TOUCH of hardlinked files */ + int nolinks = (nlink == 1 && rpmfilesFNlink(fi, fx) == 1); + if (nolinks && rpmfileContentsEqual(otherFi, ofx, fi, fx)) + rpmfsSetAction(fs, fx, FA_TOUCH); + } + } } /** diff --git a/tests/Makefile.am b/tests/Makefile.am index 3c153dbc69..f7f63a91ae 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -59,6 +59,7 @@ EXTRA_DIST += data/SPECS/configtest.spec EXTRA_DIST += data/SPECS/filedep.spec EXTRA_DIST += data/SPECS/flangtest.spec EXTRA_DIST += data/SPECS/hlinktest.spec +EXTRA_DIST += data/SPECS/hlbreak.spec EXTRA_DIST += data/SPECS/iftest.spec EXTRA_DIST += data/SPECS/ifmultiline.spec EXTRA_DIST += data/SPECS/eliftest.spec diff --git a/tests/data/SPECS/hlbreak.spec b/tests/data/SPECS/hlbreak.spec new file mode 100644 index 0000000000..d96d17261d --- /dev/null +++ b/tests/data/SPECS/hlbreak.spec @@ -0,0 +1,22 @@ +Name: hlbreak +Version: %{ver} +Release: 0 +License: GPL +Summary: Testing changing hardlink behavior +BuildArch: noarch + +%description +%{summary} + +%install +mkdir -p %{buildroot}/opt +echo "content" > %{buildroot}/opt/file2 +%if %{ver} == 1 +ln %{buildroot}/opt/file2 %{buildroot}/opt/file1 +%endif + +%files +%if %{ver} == 1 +/opt/file1 +%endif +/opt/file2 diff --git a/tests/rpmverify.at b/tests/rpmverify.at index 43fa3e2337..02b0b8bea1 100644 --- a/tests/rpmverify.at +++ b/tests/rpmverify.at @@ -405,7 +405,29 @@ fox []) AT_CLEANUP -AT_SETUP([minimize writes (links)]) +AT_SETUP([minimize writes (hardlinks)]) +AT_KEYWORDS([upgrade verify min_writes]) +RPMDB_INIT +for v in "0" "1"; do + runroot rpmbuild --quiet -bb --define "ver ${v}" /data/SPECS/hlbreak.spec +done +AT_CHECK([ +RPMDB_INIT +runroot rpm -U --define "_minimize_writes 1" /build/RPMS/noarch/hlbreak-0-0.noarch.rpm +runroot rpm -Vav --nouser --nogroup +runroot rpm -U --define "_minimize_writes 1" /build/RPMS/noarch/hlbreak-1-0.noarch.rpm +runroot rpm -Vav --nouser --nogroup +], +[0], +[......... /opt/file2 +......... /opt/file1 +......... /opt/file2 +], +) +AT_CLEANUP + + +AT_SETUP([minimize writes (symlinks)]) AT_KEYWORDS([upgrade verify min_writes]) RPMDB_INIT for v in "1.0" "2.0"; do