From bf92476620d2a3fd62ffcdcfc395bbb99e662da3 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Thu, 8 Apr 2021 10:27:32 +0300 Subject: [PATCH] Revert strict buildroot content checks for now Turns out people are using %check in various "unexpected" ways, some of which are also tied to our long-standing %exclude semantics. We need need better %check which doesn't rely on or allow messing with actual buildroot contents, but there's simply too much plate for a too short a time-frame to try and do that. Revert to old buggy buildroot checking to give time to come up with better solutions to the %check issues. This reverts c1ca2e35025698328edcefa8dedee866d2ea0596 and 1110c280639b66c89da19a0f831af28845591997. --- build/files.c | 66 ++++++------------- doc/manual/spec.md | 14 ---- scripts/check-files | 2 +- tests/data/SPECS/hlinktest.spec | 8 --- .../data/SPECS/test-subpackages-exclude.spec | 11 ---- tests/rpmbuild.at | 39 ++--------- 6 files changed, 25 insertions(+), 115 deletions(-) diff --git a/build/files.c b/build/files.c index 4328e7eafb..9d54fa9aea 100644 --- a/build/files.c +++ b/build/files.c @@ -128,6 +128,9 @@ typedef struct AttrRec_s { mode_t ar_dmode; } * AttrRec; +/* list of files */ +static StringBuf check_fileList = NULL; + typedef struct FileEntry_s { rpmfileAttrs attrFlags; specfFlags specdFlags; @@ -1494,6 +1497,12 @@ static rpmRC addFile(FileList fl, const char * diskPath, if (fileGname == NULL) fileGname = rpmugGname(getgid()); + /* S_XXX macro must be consistent with type in find call at check-files script */ + if (check_fileList && (S_ISREG(fileMode) || S_ISLNK(fileMode))) { + appendStringBuf(check_fileList, diskPath); + appendStringBuf(check_fileList, "\n"); + } + /* Add to the file list */ if (fl->files.used == fl->files.alloced) { fl->files.alloced += 128; @@ -2790,61 +2799,21 @@ rpmRC processSourceFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags) /** * Check packaged file list against what's in the build root. * @param buildRoot path of build root - * @param packages spec package list + * @param fileList packaged file list * @return -1 if skipped, 0 on OK, 1 on error */ -static int checkFiles(const char *buildRoot, Package packages) +static int checkFiles(const char *buildRoot, StringBuf fileList) { static char * const av_ckfile[] = { "%{?__check_files}", NULL }; - StringBuf fileList = newStringBuf(); StringBuf sb_stdout = NULL; int rc = -1; char * s = rpmExpand(av_ckfile[0], NULL); - size_t brlen = strlen(buildRoot); - rpmstrPool dirs = rpmstrPoolCreate(); - int prevcount = 0; if (!(s && *s)) goto exit; rpmlog(RPMLOG_NOTICE, _("Checking for unpackaged file(s): %s\n"), s); - /* This needs to match with the find call in check-files script */ - for (Package pkg = packages; pkg != NULL; pkg = pkg->next) { - int fc = rpmfilesFC(pkg->cpioList); - int prevdix = -1; - for (int i = 0; i < fc; i++) { - const char *dp = pkg->dpaths[i]; - size_t dlen = strlen(dp); - int dix = rpmfilesDI(pkg->cpioList, i); - - /* Add the disk path itself to the manifest */ - appendLineStringBuf(fileList, dp); - - /* If dirname is the same as last one, move on */ - if (dix == prevdix) - continue; - - /* Remember all intermediate directories in added files */ - for (const char *s = dp + dlen; s >= dp + brlen; s--) { - if (*s == '/') { - rpmstrPoolIdn(dirs, dp, s-dp, 1); - - /* If we've already seen this dir, we can stop here */ - int n = rpmstrPoolNumStr(dirs); - if (n == prevcount) - break; - prevcount = n; - } - } - prevdix = dix; - } - } - - /* Add all intermediate directories to the manifest */ - for (rpmsid id = 1; id <= rpmstrPoolNumStr(dirs); id++) - appendLineStringBuf(fileList, rpmstrPoolStr(dirs, id)); - rc = rpmfcExec(av_ckfile, fileList, &sb_stdout, 0, buildRoot); if (rc < 0) goto exit; @@ -2862,8 +2831,6 @@ static int checkFiles(const char *buildRoot, Package packages) exit: freeStringBuf(sb_stdout); - freeStringBuf(fileList); - rpmstrPoolFree(dirs); free(s); return rc; } @@ -3168,6 +3135,7 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, #if HAVE_LIBDW elf_version (EV_CURRENT); #endif + check_fileList = newStringBuf(); genSourceRpmName(spec); buildroot = rpmGenPath(spec->rootDir, spec->buildRoot, NULL); @@ -3263,11 +3231,17 @@ rpmRC processBinaryFiles(rpmSpec spec, rpmBuildPkgFlags pkgFlags, } } - /* Check for missing and extraneous files */ - if (checkFiles(spec->buildRoot, spec->packages) > 0) { + /* Now we have in fileList list of files from all packages. + * We pass it to a script which does the work of finding missing + * and duplicated files. + */ + + + if (checkFiles(spec->buildRoot, check_fileList) > 0) { rc = RPMRC_FAIL; } exit: + check_fileList = freeStringBuf(check_fileList); _free(buildroot); _free(uniquearch); diff --git a/doc/manual/spec.md b/doc/manual/spec.md index bcad48c42e..f746f3bc07 100644 --- a/doc/manual/spec.md +++ b/doc/manual/spec.md @@ -507,7 +507,6 @@ places. For many simple packages this is just: ``` `%install` required for creating packages that contain any files. -All files installed in the buildroot must be packaged. ### %check @@ -638,19 +637,6 @@ For example: "/opt/bob\'s htdocs" ``` -With sub-packages it's common to end up with all but that one file in -a single package and the one in a sub-package. This can be handled -with `%exclude`, for example: - -``` - %files - %{_bindir}/* - %exclude %{_bindir}/that-one-file - - %files sub - %{_bindir}/that-one-file -``` - Names containing "%%" will be rpm macro expanded into "%". When trying to escape large number of file names, it is often best to create a file with the complete list of escaped file names. This is diff --git a/scripts/check-files b/scripts/check-files index a712d1af67..12bacfb0ed 100755 --- a/scripts/check-files +++ b/scripts/check-files @@ -27,6 +27,6 @@ trap "rm -f \"${FILES_DISK}\"" 0 2 3 5 10 13 15 # Find non-directory files in the build root and compare to the manifest. # TODO: regex chars in last sed(1) expression should be escaped -find "${RPM_BUILD_ROOT}" | LC_ALL=C sort > "${FILES_DISK}" +find "${RPM_BUILD_ROOT}" -type f -o -type l | LC_ALL=C sort > "${FILES_DISK}" LC_ALL=C sort | diff -d "${FILES_DISK}" - | sed -n 's!^\(-\|< \)'"${RPM_BUILD_ROOT}"'\(.*\)$! \2!gp' diff --git a/tests/data/SPECS/hlinktest.spec b/tests/data/SPECS/hlinktest.spec index 3f1437d89c..1f453524e8 100644 --- a/tests/data/SPECS/hlinktest.spec +++ b/tests/data/SPECS/hlinktest.spec @@ -1,6 +1,5 @@ %bcond_with unpackaged_dirs %bcond_with unpackaged_files -%bcond_with unpackaged_excludes %bcond_with owned_dir Summary: Testing hard link behavior @@ -38,16 +37,9 @@ mkdir -p $RPM_BUILD_ROOT/zoo/ touch $RPM_BUILD_ROOT/toot %endif -%if %{with unpackaged_excludes} -touch $RPM_BUILD_ROOT/teet -%endif - %files %defattr(-,root,root) %if %{with owned_dir} %dir /foo %endif /foo/* -%if %{with unpackaged_excludes} -%exclude /teet -%endif diff --git a/tests/data/SPECS/test-subpackages-exclude.spec b/tests/data/SPECS/test-subpackages-exclude.spec index 99e7920aa6..e93b10b70a 100644 --- a/tests/data/SPECS/test-subpackages-exclude.spec +++ b/tests/data/SPECS/test-subpackages-exclude.spec @@ -1,5 +1,3 @@ -%bcond_without test3 - Name: test Version: 1.0 Release: 1 @@ -16,10 +14,6 @@ Source: hello.c Summary: Test2. %description test2 -%package test3 -Summary: Test3. -%description test3 - %prep %autosetup -c -D -T cp -a %{S:0} . @@ -45,9 +39,4 @@ install -D -p -m 0755 -t %{buildroot}/bin hello3 /bin/hello2 %exclude /bin/hello3 -%if %{with test3} -%files test3 -/bin/hello3 -%endif - %changelog diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at index f0eef4eff0..09fbea1402 100644 --- a/tests/rpmbuild.at +++ b/tests/rpmbuild.at @@ -271,26 +271,10 @@ runroot rpmbuild \ ]) AT_CLEANUP -AT_SETUP([rpmbuild unpackaged excludes]) -AT_KEYWORDS([build]) -RPMDB_INIT -AT_CHECK([ -RPMDB_INIT - -runroot rpmbuild \ - -bb --quiet --with unpackaged_excludes /data/SPECS/hlinktest.spec -], -[1], -[], -[error: Installed (but unpackaged) file(s) found: - /teet - Installed (but unpackaged) file(s) found: - /teet -]) -AT_CLEANUP - +# rpm doesn't detect unpackaged directories but should, really AT_SETUP([rpmbuild unpackaged directories]) AT_KEYWORDS([build]) +AT_XFAIL_IF([test $RPM_XFAIL -ne 0]) RPMDB_INIT AT_CHECK([ RPMDB_INIT @@ -1474,7 +1458,7 @@ rundebug rpmbuild --quiet \ --define "_debuginfo_subpackages 1" \ -ba "${abs_srcdir}"/data/SPECS/test-subpackages-exclude.spec -# Check that there are 3 debuginfo packages. +# Check that there are 2 debuginfo packages. ls ${RPMTEST}/build/RPMS/*/*debuginfo*rpm | wc --lines # First contains hello.debug @@ -1510,27 +1494,12 @@ if test -f ./usr/lib/debug/bin/hello3*; then else echo "No hello3 debug" fi -# -# Third contains hello3.debug -rpm2cpio ${RPMTEST}/build/RPMS/*/test-test3-1.0-1.*.rpm \ - | cpio -diu --quiet -# Extract the debug name from the exe (.gnu_debuglink section, first string) -debug_name=$(readelf -p .gnu_debuglink ./bin/hello3 | grep hello | cut -c13-) - -rpm2cpio ${RPMTEST}/build/RPMS/*/test-test3-debuginfo-1.0-1.*.rpm \ - | cpio -diu --quiet -if test -f ./usr/lib/debug/bin/$debug_name; then - echo "hello3 debug exists" -else - echo "No hello3: $debug_name" -fi ], [0], -[3 +[2 hello debug exists hello2 debug exists No hello3 debug -hello3 debug exists ], [ignore]) AT_CLEANUP