Unbreak short-circuited binary builds (#1434235).

This commit is contained in:
Mark Wielaard 2017-03-27 10:37:53 +02:00
parent bc4decbc5f
commit 0277786e14
2 changed files with 176 additions and 1 deletions

View File

@ -0,0 +1,171 @@
commit eea78b023539875309b7d38e4c8924f647644924
Author: Panu Matilainen <pmatilai@redhat.com>
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 <mark@klomp.org>
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);
}
}

View File

@ -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 <mjw@redhat.com> - 4.13.0.1-15
- Unbreak short-circuited binary builds (#1434235).
* Tue Mar 21 2017 Mark Wielaard <mjw@redhat.com> - 4.13.0.1-14
- Add fix for off by one adding DW_FORM_string replacement (#1434347).