Hi, Panu sadi on irc he didn't like the duplication of code that parsed the spec file lists. So this updated patch extracts the setup and parsing loop in their own function and just calls them twice. I also reformatted the patch a little so the whitespace differences are minimal.
Cheers, Mark
From 739796798ac854f80ae2f0d677f74bca734055f7 Mon Sep 17 00:00:00 2001 From: Mark Wielaard <m...@klomp.org> Date: Wed, 21 Jun 2017 16:57:13 +0200 Subject: [PATCH] Use a file list to add build-id files to pkgList and explicitly set attrs. mkattr used "-" as default mode which would pick up the mode for files as they were on disk. This could cause files generated by rpmbuild to use a "non-standard" mode if umask was set by the user. Explitictly use 755 for directories and 644 for files to make builds independent of any umask settings. Change the generation of build-id files to a file list using ARGV_t. First go through the current package list and generate a files list. Then add those files using the defaults mode/attr settings as if they were part of the original package file list. https://bugzilla.redhat.com/show_bug.cgi?id=1452893 https://bugzilla.redhat.com/show_bug.cgi?id=1458839 Signed-off-by: Mark Wielaard <m...@klomp.org> --- build/files.c | 244 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 138 insertions(+), 106 deletions(-) diff --git a/build/files.c b/build/files.c index 4911162d1..a90af3bee 100644 --- a/build/files.c +++ b/build/files.c @@ -207,9 +207,9 @@ static char *mkattr(const char *fn) { char *s = NULL; if (fn) - rasprintf(&s, "%s(-,%s,%s) %s", "%attr", UID_0_USER, GID_0_GROUP, fn); + rasprintf(&s, "%s(755,%s,%s) %s", "%attr", UID_0_USER, GID_0_GROUP, fn); else - rasprintf(&s, "%s(-,%s,%s)", "%defattr", UID_0_USER, GID_0_GROUP); + rasprintf(&s, "%s(644,%s,%s,755)", "%defattr", UID_0_USER, GID_0_GROUP); return s; } @@ -1614,6 +1614,15 @@ exit: return rc; } +/* add a directory to the file list */ +static void argvAddDir(ARGV_t *filesp, const char *dir) +{ + char *line = NULL; + rasprintf(&line, "%%dir %s", dir); + argvAdd(filesp, line); + _free(line); +} + #if HAVE_LIBDW /* How build id links are generated. See macros.in for description. */ #define BUILD_IDS_NONE 0 @@ -1621,7 +1630,7 @@ exit: #define BUILD_IDS_SEPARATE 2 #define BUILD_IDS_COMPAT 3 -static int addNewIDSymlink(FileList fl, +static int addNewIDSymlink(ARGV_t *files, char *targetpath, char *idlinkpath, int isDbg, int *dups) { @@ -1670,8 +1679,7 @@ static int addNewIDSymlink(FileList fl, rpmlog(RPMLOG_ERR, "%s: %s -> %s: %m\n", linkerr, linkpath, targetpath); } else { - fl->cur.isDir = 0; - rc = addFile(fl, linkpath, NULL); + rc = argvAdd(files, linkpath); } if (nr > 0) { @@ -1709,7 +1717,7 @@ static int addNewIDSymlink(FileList fl, return rc; } -static int generateBuildIDs(FileList fl) +static int generateBuildIDs(FileList fl, ARGV_t *files) { int rc = 0; int i; @@ -1858,18 +1866,9 @@ static int generateBuildIDs(FileList fl) mainiddir = rpmGetPath(fl->buildRoot, BUILD_ID_DIR, NULL); debugiddir = rpmGetPath(fl->buildRoot, DEBUG_ID_DIR, NULL); - /* Make sure to reset all file flags to defaults. - Uses parseForAttr to reset ar, arFlags, and specdFlags. - Note that parseForAttr pokes at the attrstr, so we cannot - just pass a static string. */ - fl->cur.attrFlags = 0; - fl->def.attrFlags = 0; - fl->def.verifyFlags = RPMVERIFY_ALL; - fl->cur.verifyFlags = RPMVERIFY_ALL; - fl->def.specdFlags |= SPECD_VERIFY; - fl->cur.specdFlags |= SPECD_VERIFY; + /* Make sure to reset all file flags to defaults. */ attrstr = mkattr(NULL); - parseForAttr(fl->pool, attrstr, 1, &fl->def); + argvAdd(files, attrstr); free (attrstr); /* Supported, but questionable. */ @@ -1881,11 +1880,7 @@ static int generateBuildIDs(FileList fl) if ((rc = rpmioMkpath(mainiddir, 0755, -1, -1)) != 0) { rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, mainiddir); } else { - attrstr = mkattr(mainiddir); - parseForAttr(fl->pool, attrstr, 0, &fl->cur); - fl->cur.isDir = 1; - rc = addFile(fl, mainiddir, NULL); - free (attrstr); + argvAddDir(files, mainiddir); } } @@ -1893,11 +1888,7 @@ static int generateBuildIDs(FileList fl) if ((rc = rpmioMkpath(debugiddir, 0755, -1, -1)) != 0) { rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, debugiddir); } else { - attrstr = mkattr(debugiddir); - parseForAttr(fl->pool, attrstr, 0, &fl->cur); - fl->cur.isDir = 1; - rc = addFile(fl, debugiddir, NULL); - free (attrstr); + argvAddDir(files, debugiddir); } } } @@ -1936,9 +1927,9 @@ static int generateBuildIDs(FileList fl) && (rc = rpmioMkpath(buildidsubdir, 0755, -1, -1)) != 0) { rpmlog(RPMLOG_ERR, "%s %s: %m\n", errdir, buildidsubdir); } else { - fl->cur.isDir = 1; - if (!addsubdir - || (rc = addFile(fl, buildidsubdir, NULL)) == 0) { + if (addsubdir) + argvAddDir (files, buildidsubdir); + if (rc == 0) { char *linkpattern, *targetpattern; char *linkpath, *targetpath; int dups = 0; @@ -1952,7 +1943,7 @@ static int generateBuildIDs(FileList fl) rasprintf(&linkpath, linkpattern, buildidsubdir, &ids[i][2]); rasprintf(&targetpath, targetpattern, paths[i]); - rc = addNewIDSymlink(fl, targetpath, linkpath, + rc = addNewIDSymlink(files, targetpath, linkpath, isDbg, &dups); /* We might want to have a link from the debug @@ -1998,7 +1989,7 @@ static int generateBuildIDs(FileList fl) "../../../.build-id%s/%s.%d", subdir, &ids[i][2], dups); } - rc = addNewIDSymlink(fl, targetpath, linkpath, + rc = addNewIDSymlink(files, targetpath, linkpath, 0, NULL); } @@ -2036,7 +2027,7 @@ static int generateBuildIDs(FileList fl) buildidsubdir, &ids[i][2]); rasprintf(&targetpath, "../../../../..%s", targetstr); - rc = addNewIDSymlink(fl, targetpath, + rc = addNewIDSymlink(files, targetpath, linkpath, 0, &dups); free(targetstr); } @@ -2349,45 +2340,31 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl, freeStringBuf(docScript); free(mkdocdir); } - -static rpmRC processPackageFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, - Package pkg, int didInstall, int test) + +/* Resets the default settings for files in the package list. + Used in processPackageFiles whenever a new set of files is added. */ +static void resetPackageFilesDefaults (struct FileList_s *fl, + rpmBuildPkgFlags pkgFlags) { struct AttrRec_s root_ar = { 0, 0, 0, 0, 0, 0 }; - struct FileList_s fl; - ARGV_t fileNames = NULL; - specialDir specialDoc = NULL; - specialDir specialLic = NULL; - pkg->cpioList = NULL; + root_ar.ar_user = rpmstrPoolId(fl->pool, UID_0_USER, 1); + root_ar.ar_group = rpmstrPoolId(fl->pool, GID_0_GROUP, 1); + dupAttrRec(&root_ar, &fl->def.ar); /* XXX assume %defattr(-,root,root) */ - for (ARGV_const_t fp = pkg->fileFile; fp && *fp != NULL; fp++) { - if (readFilesManifest(spec, pkg, *fp)) - return RPMRC_FAIL; - } - /* Init the file list structure */ - memset(&fl, 0, sizeof(fl)); - - fl.pool = rpmstrPoolLink(spec->pool); - /* XXX spec->buildRoot == NULL, then xstrdup("") is returned */ - fl.buildRoot = rpmGenPath(spec->rootDir, spec->buildRoot, NULL); - fl.buildRootLen = strlen(fl.buildRoot); + fl->def.verifyFlags = RPMVERIFY_ALL; - root_ar.ar_user = rpmstrPoolId(fl.pool, UID_0_USER, 1); - root_ar.ar_group = rpmstrPoolId(fl.pool, GID_0_GROUP, 1); - dupAttrRec(&root_ar, &fl.def.ar); /* XXX assume %defattr(-,root,root) */ - - fl.def.verifyFlags = RPMVERIFY_ALL; - - fl.pkgFlags = pkgFlags; + fl->pkgFlags = pkgFlags; +} - { char *docs = rpmGetPath("%{?__docdir_path}", NULL); - argvSplit(&fl.docDirs, docs, ":"); - free(docs); - } - - for (ARGV_const_t fp = pkg->fileList; *fp != NULL; fp++) { +static void addPackageFileList (struct FileList_s *fl, Package pkg, + ARGV_t *fileList, + specialDir *specialDoc, specialDir *specialLic, + int fromSpecFileList) +{ + ARGV_t fileNames = NULL; + for (ARGV_const_t fp = *fileList; *fp != NULL; fp++) { char buf[strlen(*fp) + 1]; const char *s = *fp; SKIPSPACE(s); @@ -2397,41 +2374,63 @@ static rpmRC processPackageFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, rstrlcpy(buf, s, sizeof(buf)); /* Reset for a new line in %files */ - FileEntryFree(&fl.cur); + FileEntryFree(&fl->cur); /* turn explicit flags into %def'd ones (gosh this is hacky...) */ - fl.cur.specdFlags = ((unsigned)fl.def.specdFlags) >> 8; - fl.cur.verifyFlags = fl.def.verifyFlags; - - if (parseForVerify(buf, 0, &fl.cur) || - parseForVerify(buf, 1, &fl.def) || - parseForAttr(fl.pool, buf, 0, &fl.cur) || - parseForAttr(fl.pool, buf, 1, &fl.def) || - parseForDev(buf, &fl.cur) || - parseForConfig(buf, &fl.cur) || - parseForLang(buf, &fl.cur) || - parseForCaps(buf, &fl.cur) || - parseForSimple(buf, &fl.cur, &fileNames)) + fl->cur.specdFlags = ((unsigned)fl->def.specdFlags) >> 8; + fl->cur.verifyFlags = fl->def.verifyFlags; + + if (parseForVerify(buf, 0, &fl->cur) || + parseForVerify(buf, 1, &fl->def) || + parseForAttr(fl->pool, buf, 0, &fl->cur) || + parseForAttr(fl->pool, buf, 1, &fl->def) || + parseForDev(buf, &fl->cur) || + parseForConfig(buf, &fl->cur) || + parseForLang(buf, &fl->cur) || + parseForCaps(buf, &fl->cur) || + parseForSimple(buf, &fl->cur, &fileNames)) { - fl.processingFailed = 1; + fl->processingFailed = 1; continue; } for (ARGV_const_t fn = fileNames; fn && *fn; fn++) { - if (fl.cur.attrFlags & RPMFILE_SPECIALDIR) { - rpmFlags oattrs = (fl.cur.attrFlags & ~RPMFILE_SPECIALDIR); + + /* For file lists that don't come from a spec file list + processing is easy. There are no special files and the + file names don't need to be adjusted. */ + if (!fromSpecFileList) { + if (fl->cur.attrFlags & RPMFILE_SPECIALDIR + || fl->cur.attrFlags & RPMFILE_DOCDIR + || fl->cur.attrFlags & RPMFILE_PUBKEY) { + rpmlog(RPMLOG_ERR, + _("Special file in generated file list: %s\n"), + *fn); + fl->processingFailed = 1; + continue; + } + if (fl->cur.attrFlags & RPMFILE_DIR) + fl->cur.isDir = 1; + addFile(fl, *fn, NULL); + continue; + } + + /* File list does come from the spec, try to detect special + files and adjust the actual file names. */ + if (fl->cur.attrFlags & RPMFILE_SPECIALDIR) { + rpmFlags oattrs = (fl->cur.attrFlags & ~RPMFILE_SPECIALDIR); specialDir *sdp = NULL; if (oattrs == RPMFILE_DOC) { - sdp = &specialDoc; + sdp = specialDoc; } else if (oattrs == RPMFILE_LICENSE) { - sdp = &specialLic; + sdp = specialLic; } if (sdp == NULL || **fn == '/') { rpmlog(RPMLOG_ERR, _("Can't mix special %s with other forms: %s\n"), (oattrs & RPMFILE_DOC) ? "%doc" : "%license", *fn); - fl.processingFailed = 1; + fl->processingFailed = 1; continue; } @@ -2439,31 +2438,64 @@ static rpmRC processPackageFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, if (*sdp == NULL) { *sdp = specialDirNew(pkg->header, oattrs); } - addSpecialFile(*sdp, *fn, &fl.cur, &fl.def); + addSpecialFile(*sdp, *fn, &fl->cur, &fl->def); continue; } /* this is now an artificial limitation */ if (fn != fileNames) { rpmlog(RPMLOG_ERR, _("More than one file on a line: %s\n"),*fn); - fl.processingFailed = 1; + fl->processingFailed = 1; continue; } - if (fl.cur.attrFlags & RPMFILE_DOCDIR) { - argvAdd(&(fl.docDirs), *fn); - } else if (fl.cur.attrFlags & RPMFILE_PUBKEY) { - (void) processMetadataFile(pkg, &fl, *fn, RPMTAG_PUBKEYS); + if (fl->cur.attrFlags & RPMFILE_DOCDIR) { + argvAdd(&(fl->docDirs), *fn); + } else if (fl->cur.attrFlags & RPMFILE_PUBKEY) { + (void) processMetadataFile(pkg, fl, *fn, RPMTAG_PUBKEYS); } else { - if (fl.cur.attrFlags & RPMFILE_DIR) - fl.cur.isDir = 1; - (void) processBinaryFile(pkg, &fl, *fn); + if (fl->cur.attrFlags & RPMFILE_DIR) + fl->cur.isDir = 1; + (void) processBinaryFile(pkg, fl, *fn); } } - if (fl.cur.caps) - fl.haveCaps = 1; + if (fl->cur.caps) + fl->haveCaps = 1; } + argvFree(fileNames); +} + +static rpmRC processPackageFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, + Package pkg, int didInstall, int test) +{ + struct FileList_s fl; + specialDir specialDoc = NULL; + specialDir specialLic = NULL; + + pkg->cpioList = NULL; + + for (ARGV_const_t fp = pkg->fileFile; fp && *fp != NULL; fp++) { + if (readFilesManifest(spec, pkg, *fp)) + return RPMRC_FAIL; + } + /* Init the file list structure */ + memset(&fl, 0, sizeof(fl)); + + fl.pool = rpmstrPoolLink(spec->pool); + /* XXX spec->buildRoot == NULL, then xstrdup("") is returned */ + fl.buildRoot = rpmGenPath(spec->rootDir, spec->buildRoot, NULL); + fl.buildRootLen = strlen(fl.buildRoot); + + resetPackageFilesDefaults (&fl, pkgFlags); + + { char *docs = rpmGetPath("%{?__docdir_path}", NULL); + argvSplit(&fl.docDirs, docs, ":"); + free(docs); + } + + addPackageFileList (&fl, pkg, &pkg->fileList, + &specialDoc, &specialLic, 1); /* Now process special docs and licenses if present */ if (specialDoc) @@ -2478,11 +2510,21 @@ static rpmRC processPackageFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, /* Check build-ids and add build-ids links for files to package list. */ const char *arch = headerGetString(pkg->header, RPMTAG_ARCH); if (!rstreq(arch, "noarch")) { - if (generateBuildIDs (&fl) != 0) { + /* Go through the current package list and generate a files list. */ + ARGV_t idFiles = NULL; + if (generateBuildIDs (&fl, &idFiles) != 0) { rpmlog(RPMLOG_ERR, _("Generating build-id links failed\n")); fl.processingFailed = 1; goto exit; } + + if (idFiles != NULL) { + resetPackageFilesDefaults (&fl, pkgFlags); + addPackageFileList (&fl, pkg, &idFiles, NULL, NULL, 0); + } + + if (fl.processingFailed) + goto exit; } #endif @@ -2493,7 +2535,6 @@ static rpmRC processPackageFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, genCpioListAndHeader(&fl, pkg, 0); exit: - argvFree(fileNames); FileListFree(&fl); specialDirFree(specialDoc); specialDirFree(specialLic); @@ -2741,15 +2782,6 @@ static Package cloneDebuginfoPackage(rpmSpec spec, Package pkg, Package maindbg) return dbg; } -/* add a directory to the file list */ -static void argvAddDir(ARGV_t *filesp, const char *dir) -{ - char *line = NULL; - rasprintf(&line, "%%dir %s", dir); - argvAdd(filesp, line); - _free(line); -} - /* collect the debug files for package pkg and put them into * a (possibly new) debuginfo subpackage */ static void filterDebuginfoPackage(rpmSpec spec, Package pkg, -- 2.13.0
_______________________________________________ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
