On 01/05/2017 02:50 PM, Mark Wielaard wrote:
On Thu, Jan 05, 2017 at 01:47:28PM +0200, Panu Matilainen wrote:
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.
Sorry about that. I didn't even know rpmbuild --short-circuit existed.
Looks like there are lots of subtle ways this can get broken. Might be
good to add a testcase for it.
Yup, testcase wouldn't hurt. OTOH this is the first time ever something
broke it :) It's also such a crazy misfeature... I mean, it creates
deliberately broken packages because its strictly for non-production use
only. I think today was the first time I ever had an actual use-case for
it personally, while debugging an unrelated problem.
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.
@@ -1572,6 +1573,16 @@ static int addNewIDSymlink(FileList fl,
origpath = linkpath;
while (faccessat(AT_FDCWD, linkpath, F_OK, AT_SYMLINK_NOFOLLOW) == 0) {
+ 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;
+ }
+ }
Allocating PATH_MAX size on the stack might be a bit much
(and on some systems PATH_MAX might not exist). Might be
good to use lstat first to see if it is a symbolic link
and use the st_size i+1 as buffer size. I would malloc/free
it to not accidentially blow up the stack.
Yeah, using PATH_MAX is broken but then rpm is getting away with it (on
stack and all) in many other places so one more here and there... but
good point using lstat() there, will change it to use that, thanks.
Other than that - does it look like doing the right thing to you? I'm
really not familiar with the debuginfo business at all :)
- Panu -
Cheers,
Mark
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
