From 0277786e14a849203e149f557f44637229d7e8ba Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Mon, 27 Mar 2017 10:37:53 +0200 Subject: [PATCH] Unbreak short-circuited binary builds (#1434235). --- 0022-unbreak-short-circuit.patch | 171 +++++++++++++++++++++++++++++++ rpm.spec | 6 +- 2 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 0022-unbreak-short-circuit.patch diff --git a/0022-unbreak-short-circuit.patch b/0022-unbreak-short-circuit.patch new file mode 100644 index 0000000..ca57624 --- /dev/null +++ b/0022-unbreak-short-circuit.patch @@ -0,0 +1,171 @@ +commit eea78b023539875309b7d38e4c8924f647644924 +Author: Panu Matilainen +Date: Thu Jan 5 13:47:28 2017 +0200 + + Unbreak short-circuited binary builds + + Commit bbfe1f86b2e4b5c0bd499d9f3dd9de9c9c20fff2 broke short-circuited + binary builds (which can be handy for testing when working on large + packages), eg: + rpmbuild -bi foo.spec; rpmbuild -bb --short-circuit foo.spec + + The problem is that in a short-circuited build all the links already + exist and point to the right place, but the code doesn't realize this + and creates new links instead, which leaves the old links unowned + in the buildroot which ultimately causes the build to fail with + "Installed (but unpackaged) file(s) found" for the previously created + build-id links. + + When checking for pre-existing links see if they already point to + the right file and in that case just reuse it instead of creating new ones. + Keep track of duplicate build-ids found by noticing existing links that + point to different targets. But don't do this for compat links, they should + just point to the last (duplicate) main build-id symlink found. + + Signed-off-by: Mark Wielaard + +diff --git a/build/files.c b/build/files.c +index cca14b9..93021d1 100644 +--- a/build/files.c ++++ b/build/files.c +@@ -1592,11 +1592,12 @@ exit: + + static int addNewIDSymlink(FileList fl, + char *targetpath, char *idlinkpath, +- int isDbg, int isCompat) ++ int isDbg, int *dups) + { + const char *linkerr = _("failed symlink"); + int rc = 0; + int nr = 0; ++ int exists = 0; + char *origpath, *linkpath; + + if (isDbg) +@@ -1606,6 +1607,26 @@ static int addNewIDSymlink(FileList fl, + origpath = linkpath; + + while (faccessat(AT_FDCWD, linkpath, F_OK, AT_SYMLINK_NOFOLLOW) == 0) { ++ /* We don't care about finding dups for compat links, they are ++ OK as is. Otherwise we will need to double check if ++ existing link points to the correct target. */ ++ if (dups == NULL) ++ { ++ exists = 1; ++ break; ++ } ++ ++ char ltarget[PATH_MAX]; ++ ssize_t llen; ++ /* In short-circuited builds the link might already exist */ ++ if ((llen = readlink(linkpath, ltarget, sizeof(ltarget)-1)) != -1) { ++ ltarget[llen] = '\0'; ++ if (rstreq(ltarget, targetpath)) { ++ exists = 1; ++ break; ++ } ++ } ++ + if (nr > 0) + free(linkpath); + nr++; +@@ -1613,21 +1634,16 @@ static int addNewIDSymlink(FileList fl, + isDbg ? ".debug" : ""); + } + +- char *symtarget = targetpath; +- if (nr > 0 && isCompat) +- rasprintf (&symtarget, "%s.%d", targetpath, nr); +- +- if (symlink(symtarget, linkpath) < 0) { ++ if (!exists && symlink(targetpath, linkpath) < 0) { + rc = 1; + rpmlog(RPMLOG_ERR, "%s: %s -> %s: %m\n", +- linkerr, linkpath, symtarget); ++ linkerr, linkpath, targetpath); + } else { + fl->cur.isDir = 0; + rc = addFile(fl, linkpath, NULL); + } + +- /* Don't warn (again) if this is a compat id-link, we retarget it. */ +- if (nr > 0 && !isCompat) { ++ if (nr > 0) { + /* Lets see why there are multiple build-ids. If the original + targets are hard linked, then it is OK, otherwise warn + something fishy is going on. Would be nice to call +@@ -1656,8 +1672,8 @@ static int addNewIDSymlink(FileList fl, + free(origpath); + if (nr > 0) + free(linkpath); +- if (nr > 0 && isCompat) +- free(symtarget); ++ if (dups != NULL) ++ *dups = nr; + + return rc; + } +@@ -1897,6 +1913,7 @@ static int generateBuildIDs(FileList fl) + || (rc = addFile(fl, buildidsubdir, NULL)) == 0) { + char *linkpattern, *targetpattern; + char *linkpath, *targetpath; ++ int dups = 0; + if (isDbg) { + linkpattern = "%s/%s"; + targetpattern = "../../../../..%s"; +@@ -1908,7 +1925,7 @@ static int generateBuildIDs(FileList fl) + buildidsubdir, &ids[i][2]); + rasprintf(&targetpath, targetpattern, paths[i]); + rc = addNewIDSymlink(fl, targetpath, linkpath, +- isDbg, 0); ++ isDbg, &dups); + + /* We might want to have a link from the debug + build_ids dir to the main one. We create it +@@ -1931,16 +1948,30 @@ static int generateBuildIDs(FileList fl) + && build_id_links == BUILD_IDS_COMPAT) { + /* buildidsubdir already points to the + debug buildid. We just need to setup +- the symlink to the main one. */ ++ the symlink to the main one. There ++ might be duplicate IDs, those are found ++ by the addNewIDSymlink above. Target ++ the last found duplicate, if any. */ + free(linkpath); + free(targetpath); +- rasprintf(&linkpath, "%s/%s", +- buildidsubdir, &ids[i][2]); +- rasprintf(&targetpath, +- "../../../.build-id%s/%s", +- subdir, &ids[i][2]); ++ if (dups == 0) ++ { ++ rasprintf(&linkpath, "%s/%s", ++ buildidsubdir, &ids[i][2]); ++ rasprintf(&targetpath, ++ "../../../.build-id%s/%s", ++ subdir, &ids[i][2]); ++ } ++ else ++ { ++ rasprintf(&linkpath, "%s/%s.%d", ++ buildidsubdir, &ids[i][2], dups); ++ rasprintf(&targetpath, ++ "../../../.build-id%s/%s.%d", ++ subdir, &ids[i][2], dups); ++ } + rc = addNewIDSymlink(fl, targetpath, linkpath, +- 0, 1); ++ 0, NULL); + } + + if (rc == 0 && isDbg +@@ -1978,7 +2009,7 @@ static int generateBuildIDs(FileList fl) + rasprintf(&targetpath, "../../../../..%s", + targetstr); + rc = addNewIDSymlink(fl, targetpath, +- linkpath, 0, 0); ++ linkpath, 0, &dups); + free(targetstr); + } + } diff --git a/rpm.spec b/rpm.spec index b9b1d09..c724031 100644 --- a/rpm.spec +++ b/rpm.spec @@ -29,7 +29,7 @@ Summary: The RPM package management system Name: rpm Version: %{rpmver} -Release: %{?snapver:0.%{snapver}.}14%{?dist} +Release: %{?snapver:0.%{snapver}.}15%{?dist} Group: System Environment/Base Url: http://www.rpm.org/ Source0: http://rpm.org/releases/%{srcdir}/%{name}-%{srcver}.tar.bz2 @@ -93,6 +93,7 @@ Patch267: 0018-update-build-id-endian.patch Patch268: 0019-fix-sed-build-id-match-test.patch Patch269: 0020-build-files-exec-build-id.patch Patch270: 0021-debugedit-Fix-off-by-one-adding-DW_FORM_string-repla.patch +Patch271: 0022-unbreak-short-circuit.patch # OpenSSL backend Patch300: 0001-Add-OpenSSL-support-for-digest-and-signatures.patch @@ -597,6 +598,9 @@ exit 0 %doc doc/librpm/html/* %changelog +* Mon Mar 27 2017 Mark Wielaard - 4.13.0.1-15 +- Unbreak short-circuited binary builds (#1434235). + * Tue Mar 21 2017 Mark Wielaard - 4.13.0.1-14 - Add fix for off by one adding DW_FORM_string replacement (#1434347).