From b6cf728569212192543f768b475a2ad9fef47460 Mon Sep 17 00:00:00 2001 From: Panu Matilainen Date: Tue, 16 Jan 2024 13:49:16 +0200 Subject: [PATCH] Introduce an rpm-controlled per-build directory In our ancient wacko setup, %_builddir is shared by all your packages, under which a package may create %buildsubdir - if it uses %setup that is. In other words, there's no safe and sane place for rpm to place per-build material. Introduce a new intermediate directory between the two, created in a newly added (internal) %builddir build step, to give rpm that place. This opens up all manner of opportunities, to be explored in later commits. A new build-time macro %pkgbuilddir is added for the absolute path of this new directory, but in addition we override %_builddir to the same value for maximum compatibility with existing specs - a LOT of packages reference %{_builddir} for all sorts of (mostly bad) reasons and we don't want to deal with the carnage that would follow from breaking it. A further complication here is that defining %_builddir (along with sources etc) to current working directory is a common configuration (used eg by fedpkg and its variants) that we don't want to break either. So upon entry, we grab the pre-existing %_builddir and define %_top_builddir from it, which we can then base the %pkgbuilddir on, while preserving compatibility with both of the above scenarios. We would prefer to use NVRA for the %pkgbuilddir name, but this breaks a buildid no-recompute test due to assumptions made there. Leave that for some other day, and use N-V-A for the directory for now. This also moves SPECPARTS and BUILDROOT into the new directory, bringing both locations fully under rpm control. Remove the corresponding %specpartsdir and %buildroot entries from the main macros file, these are not user overridable. --- build/build.c | 38 +++++++++++++++++++++++--------- build/parsePreamble.c | 34 +++++++++++++++-------------- build/parseSpec.c | 15 +++++-------- build/rpmbuild_internal.h | 1 + build/spec.c | 4 +++- include/rpm/rpmbuild.h | 3 ++- macros.in | 17 +++++++-------- tests/rpmbuild.at | 46 +++++++++++++++++++++++---------------- tests/rpmspec.at | 7 +++--- tools/rpmbuild.c | 18 +++++---------- 10 files changed, 102 insertions(+), 81 deletions(-) diff --git a/build/build.c b/build/build.c index 0ac8bf6c93..f137f2b105 100644 --- a/build/build.c +++ b/build/build.c @@ -121,6 +121,10 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name, rpmRC rc = RPMRC_FAIL; /* assume failure */ switch (what) { + case RPMBUILD_BUILDDIR: + mTemplate = "%{__spec_builddir_template}"; + mPost = "%{__spec_builddir_post}"; + mCmd = "%{__spec_builddir_cmd}"; case RPMBUILD_PREP: mTemplate = "%{__spec_prep_template}"; mPost = "%{__spec_prep_post}"; @@ -190,18 +194,11 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name, (void) fputs(buildTemplate, fp); - if (what != RPMBUILD_PREP && what != RPMBUILD_RMBUILD && buildSubdir[0] != '\0') + if (what != RPMBUILD_BUILDDIR && what != RPMBUILD_PREP && what != RPMBUILD_RMBUILD && buildSubdir[0] != '\0') fprintf(fp, "cd '%s'\n", buildSubdir); if (what == RPMBUILD_RMBUILD) { - if (rpmMacroIsDefined(spec->macros, "specpartsdir")) { - char * buf = rpmExpand("%{specpartsdir}", NULL); - fprintf(fp, "rm -rf '%s'\n", buf); - free(buf); - } - if (buildSubdir[0] != '\0') - fprintf(fp, "rm -rf '%s' '%s.gemspec'\n", - buildSubdir, buildSubdir); + fprintf(fp, "rm -rf '%s'\n", spec->buildDir); } else if (sb != NULL) fprintf(fp, "%s", sb); @@ -303,6 +300,24 @@ static rpmRC doCheckBuildRequires(rpmts ts, rpmSpec spec, int test) return rc; } +static rpmRC doBuildDir(rpmSpec spec, int test, StringBuf *sbp) +{ + char *doDir = rstrscat(NULL, + "rm -rf ", spec->buildDir, "\n", + "mkdir -p ", spec->buildDir, "\n", + NULL); + + rpmRC rc = doScript(spec, RPMBUILD_BUILDDIR, "%builddir", + doDir, test, sbp); + if (rc) { + rpmlog(RPMLOG_ERR, + _("failed to create package build directory %s: %s\n"), + spec->buildDir, strerror(errno)); + } + free(doDir); + return rc; +} + static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what) { rpmRC rc = RPMRC_OK; @@ -372,13 +387,16 @@ static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what) if (!rpmSpecGetSection(spec, RPMBUILD_BUILDREQUIRES) && sourceOnly) { /* don't run prep if not needed for source build */ /* with(out) dynamic build requires*/ - what &= ~(RPMBUILD_PREP); + what &= ~(RPMBUILD_PREP|RPMBUILD_BUILDDIR); } if ((what & RPMBUILD_CHECKBUILDREQUIRES) && (rc = doCheckBuildRequires(ts, spec, test))) goto exit; + if ((what & RPMBUILD_BUILDDIR) && (rc = doBuildDir(spec, test, sbp))) + goto exit; + if ((what & RPMBUILD_PREP) && (rc = doScript(spec, RPMBUILD_PREP, "%prep", rpmSpecGetSection(spec, RPMBUILD_PREP), diff --git a/build/parsePreamble.c b/build/parsePreamble.c index a23e5a8f65..6893a7f4da 100644 --- a/build/parsePreamble.c +++ b/build/parsePreamble.c @@ -1267,28 +1267,30 @@ int parsePreamble(rpmSpec spec, int initialPackage) } } - /* - * Expand buildroot one more time to get %{version} and the like - * from the main package, validate sanity. The spec->buildRoot could - * still contain unexpanded macros but it cannot be empty or '/', and it - * can't be messed with by anything spec does beyond this point. - */ if (initialPackage) { if (checkForRequiredForBuild(pkg->header)) { goto exit; } - char *buildRoot = rpmGetPath(spec->buildRoot, NULL); - free(spec->buildRoot); - spec->buildRoot = buildRoot; - rpmPushMacro(spec->macros, "buildroot", NULL, spec->buildRoot, RMIL_SPEC); - if (*buildRoot == '\0') { - rpmlog(RPMLOG_ERR, _("%%{buildroot} couldn't be empty\n")); - goto exit; + /* Grab the top builddir on first entry as we'll override _builddir */ + if (!rpmMacroIsDefined(spec->macros, "_top_builddir")) { + rpmPushMacro(spec->macros, "_top_builddir", NULL, "%{_builddir}", + RMIL_GLOBAL); } - if (rstreq(buildRoot, "/")) { - rpmlog(RPMLOG_ERR, _("%%{buildroot} can not be \"/\"\n")); - goto exit; + + if (!spec->buildDir) { + /* Using release here causes a buildid no-recompute test to fail */ + spec->buildDir = rpmExpand("%{_top_builddir}/%{NAME}-%{VERSION}-%{_arch}", NULL); + /* a user-oriented, unambiguous name for the thing */ + rpmPushMacro(spec->macros, "pkgbuilddir", NULL, spec->buildDir, RMIL_SPEC); + rpmPushMacro(spec->macros, "_builddir", NULL, spec->buildDir, RMIL_SPEC); + + spec->buildRoot = rpmGetPath(spec->buildDir, "/BUILDROOT", NULL); + rpmPushMacro(spec->macros, "buildroot", NULL, spec->buildRoot, RMIL_SPEC +); + char *specparts = rpmGetPath(spec->buildDir, "/SPECPARTS", NULL); + rpmPushMacro(spec->macros, "specpartsdir", NULL, specparts, RMIL_SPEC); + free(specparts); } } diff --git a/build/parseSpec.c b/build/parseSpec.c index 9a0a1f8734..741d28ea25 100644 --- a/build/parseSpec.c +++ b/build/parseSpec.c @@ -994,7 +994,7 @@ static rpmRC parseBuildsystem(rpmSpec spec) } static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags, - const char *buildRoot, int recursing); + int recursing); static rpmRC parseSpecSection(rpmSpec *specptr, int secondary) { @@ -1124,7 +1124,7 @@ static rpmRC parseSpecSection(rpmSpec *specptr, int secondary) if (!rpmMachineScore(RPM_MACHTABLE_BUILDARCH, spec->BANames[x])) continue; rpmPushMacro(NULL, "_target_cpu", NULL, spec->BANames[x], RMIL_RPMRC); - spec->BASpecs[index] = parseSpec(spec->specFile, spec->flags, spec->buildRoot, 1); + spec->BASpecs[index] = parseSpec(spec->specFile, spec->flags, 1); if (spec->BASpecs[index] == NULL) { spec->BACount = index; goto errxit; @@ -1190,7 +1190,7 @@ static rpmRC parseSpecSection(rpmSpec *specptr, int secondary) static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags, - const char *buildRoot, int recursing) + int recursing) { rpmSpec spec; @@ -1199,12 +1199,7 @@ static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags, spec->specFile = rpmGetPath(specFile, NULL); pushOFI(spec, spec->specFile); - /* If buildRoot not specified, use default %{buildroot} */ - if (buildRoot) { - spec->buildRoot = xstrdup(buildRoot); - } else { - spec->buildRoot = rpmGetPath("%{?buildroot:%{buildroot}}", NULL); - } + rpmPushMacro(NULL, "_docdir", NULL, "%{_defaultdocdir}", RMIL_SPEC); rpmPushMacro(NULL, "_licensedir", NULL, "%{_defaultlicensedir}", RMIL_SPEC); spec->recursing = recursing; @@ -1292,7 +1287,7 @@ static rpmRC finalizeSpec(rpmSpec spec) rpmSpec rpmSpecParse(const char *specFile, rpmSpecFlags flags, const char *buildRoot) { - rpmSpec spec = parseSpec(specFile, flags, buildRoot, 0); + rpmSpec spec = parseSpec(specFile, flags, 0); if (spec && !(flags & RPMSPEC_NOFINALIZE)) { finalizeSpec(spec); } diff --git a/build/rpmbuild_internal.h b/build/rpmbuild_internal.h index e2d4f8d683..c6fe51c2c6 100644 --- a/build/rpmbuild_internal.h +++ b/build/rpmbuild_internal.h @@ -115,6 +115,7 @@ struct rpmSpec_s { char * specFile; /*!< Name of the spec file. */ char * buildRoot; + char * buildDir; const char * rootDir; struct OpenFileInfo * fileStack; diff --git a/build/spec.c b/build/spec.c index 6f8a6a1555..d26d9f7583 100644 --- a/build/spec.c +++ b/build/spec.c @@ -234,6 +234,7 @@ rpmSpec newSpec(void) spec->sourcePackage = NULL; spec->buildRoot = NULL; + spec->buildDir = NULL; spec->buildRestrictions = headerNew(); spec->BANames = NULL; @@ -260,6 +261,7 @@ rpmSpec rpmSpecFree(rpmSpec spec) freeStringBuf(spec->parsed); spec->buildRoot = _free(spec->buildRoot); + spec->buildDir = _free(spec->buildDir); spec->specFile = _free(spec->specFile); closeSpec(spec); @@ -298,7 +300,7 @@ rpmSpec rpmSpecFree(rpmSpec spec) spec->pool = rpmstrPoolFree(spec->pool); spec->buildHost = _free(spec->buildHost); - + spec = _free(spec); return spec; diff --git a/include/rpm/rpmbuild.h b/include/rpm/rpmbuild.h index b363ce6cf4..d379d02ad3 100644 --- a/include/rpm/rpmbuild.h +++ b/include/rpm/rpmbuild.h @@ -38,6 +38,7 @@ enum rpmBuildFlags_e { RPMBUILD_BUILDREQUIRES = (1 << 20), /*!< Execute %%buildrequires. */ RPMBUILD_DUMPBUILDREQUIRES = (1 << 21), /*!< Write buildrequires.nosrc.rpm. */ RPMBUILD_CONF = (1 << 22), /*!< Execute %%conf. */ + RPMBUILD_BUILDDIR = (1 << 23), /*!< Internal use only */ RPMBUILD_NOBUILD = (1 << 31) /*!< Don't execute or package. */ }; @@ -77,7 +78,7 @@ typedef struct rpmBuildArguments_s * BTA_t; * * @param specFile path to spec file * @param flags flags to control operation - * @param buildRoot buildRoot override or NULL for default + * @param buildRoot unused * @return new spec control structure */ rpmSpec rpmSpecParse(const char *specFile, rpmSpecFlags flags, diff --git a/macros.in b/macros.in index 88bc6b4e92..0d165561cf 100644 --- a/macros.in +++ b/macros.in @@ -266,15 +266,6 @@ Supplements: (%{name} = %{version}-%{release} and langpacks-%{1})\ # The directory where newly built source packages will be written. %_srcrpmdir %{_topdir}/SRPMS -# The directory where buildroots will be created. -%_buildrootdir %{_topdir}/BUILDROOT - -# Build root path, where %install installs the package during build. -%buildroot %{_buildrootdir}/%{NAME}-%{VERSION}-%{RELEASE}.%{_arch} - -# Path for spec file snippets generated during build -%specpartsdir %{_builddir}/%{buildsubdir}-SPECPARTS - # Directory where temporaray files can be created. %_tmppath %{_var}/tmp @@ -815,6 +806,14 @@ Supplements: (%{name} = %{version}-%{release} and langpacks-%{1})\ # ---- Scriptlet templates. # Macro(s) that expand to a command and script that is executed. # +%__spec_builddir_shell %{nil} +%__spec_builddir_args %{nil} +%__spec_builddir_cmd %{nil} +%__spec_builddir_pre %{nil} +%__spec_builddir_body %{%nil} +%__spec_builddir_post %{nil} +%__spec_builddir_template %{nil} + %__spec_prep_shell %{___build_shell} %__spec_prep_args %{___build_args} %__spec_prep_cmd %{___build_cmd} diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at index c47a532e9b..bd2f586bc0 100644 --- a/tests/rpmbuild.at +++ b/tests/rpmbuild.at @@ -92,7 +92,8 @@ RPMTEST_CHECK([ runroot rpmbuild -bp /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) ], []) @@ -108,7 +109,8 @@ RPMTEST_CHECK([ runroot rpmbuild -br /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) Executing(%generate_buildrequires) ], []) @@ -117,7 +119,8 @@ RPMTEST_CHECK([ runroot rpmbuild -bd /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) Executing(%generate_buildrequires) ], []) @@ -126,7 +129,8 @@ RPMTEST_CHECK([ runroot rpmbuild -bf /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) Executing(%generate_buildrequires) Executing(%conf) ], @@ -135,7 +139,8 @@ RPMTEST_CHECK([ runroot rpmbuild -bc /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) Executing(%generate_buildrequires) Executing(%conf) Executing(%build) @@ -146,7 +151,8 @@ RPMTEST_CHECK([ runroot rpmbuild -bi /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) Executing(%generate_buildrequires) Executing(%conf) Executing(%build) @@ -159,7 +165,8 @@ RPMTEST_CHECK([ runroot rpmbuild -bb /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) Executing(%generate_buildrequires) Executing(%conf) Executing(%build) @@ -174,7 +181,8 @@ RPMTEST_CHECK([ runroot rpmbuild -ba /data/SPECS/specstep.spec 2>&1|grep ^Executing|cut -d: -f1 ], [0], -[Executing(%prep) +[Executing(%builddir) +Executing(%prep) Executing(%generate_buildrequires) Executing(%conf) Executing(%build) @@ -2296,17 +2304,17 @@ runroot rpmbuild -bb --quiet /data/SPECS/filemiss.spec ], [1], [], -[error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/doc/filemisstest-1.0/CREDITS -error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/foo -error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/bar{a,b} -cp: cannot stat '/build/BUILD/INSTALL': No such file or directory -cp: cannot stat '/build/BUILD/README*': No such file or directory -error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/doc/filemisstest-1.0/INSTALL -error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/doc/filemisstest-1.0/README* -cp: cannot stat '/build/BUILD/LICENSE': No such file or directory -cp: cannot stat '/build/BUILD/OTHERLICENSE?': No such file or directory -error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/licenses/filemisstest-1.0/LICENSE -error: File not found: /build/BUILDROOT/filemisstest-1.0-1.x86_64/opt/share/licenses/filemisstest-1.0/OTHERLICENSE? +[error: File not found: /build/BUILD/filemisstest-1.0-x86_64/BUILDROOT/opt/share/doc/filemisstest-1.0/CREDITS +error: File not found: /build/BUILD/filemisstest-1.0-x86_64/BUILDROOT/opt/foo +error: File not found: /build/BUILD/filemisstest-1.0-x86_64/BUILDROOT/opt/bar{a,b} +cp: cannot stat '/build/BUILD/filemisstest-1.0-x86_64/INSTALL': No such file or directory +cp: cannot stat '/build/BUILD/filemisstest-1.0-x86_64/README*': No such file or directory +error: File not found: /build/BUILD/filemisstest-1.0-x86_64/BUILDROOT/opt/share/doc/filemisstest-1.0/INSTALL +error: File not found: /build/BUILD/filemisstest-1.0-x86_64/BUILDROOT/opt/share/doc/filemisstest-1.0/README* +cp: cannot stat '/build/BUILD/filemisstest-1.0-x86_64/LICENSE': No such file or directory +cp: cannot stat '/build/BUILD/filemisstest-1.0-x86_64/OTHERLICENSE?': No such file or directory +error: File not found: /build/BUILD/filemisstest-1.0-x86_64/BUILDROOT/opt/share/licenses/filemisstest-1.0/LICENSE +error: File not found: /build/BUILD/filemisstest-1.0-x86_64/BUILDROOT/opt/share/licenses/filemisstest-1.0/OTHERLICENSE? ], ) RPMTEST_CLEANUP diff --git a/tests/rpmspec.at b/tests/rpmspec.at index bb83599db2..8370f1339e 100644 --- a/tests/rpmspec.at +++ b/tests/rpmspec.at @@ -299,6 +299,7 @@ runroot rpmspec --parse \ --define "__patch /usr/bin/patch" \ --define "__chmod /usr/bin/chmod" \ --define "debug_package %{nil}" \ + --target x86_64 \ /data/SPECS/hello.spec ], [0], @@ -325,7 +326,7 @@ Prefix: /usr Simple rpm demonstration. %prep -cd '/build/BUILD' +cd '/build/BUILD/hello-1.0-x86_64' rm -rf 'hello-1.0' /usr/lib/rpm/rpmuncompress -x '/build/SOURCES/hello-1.0.tar.gz' STATUS=$? @@ -333,8 +334,8 @@ if [ $STATUS -ne 0 ]; then exit $STATUS fi cd 'hello-1.0' -rm -rf '/build/BUILD/hello-1.0-SPECPARTS' -/usr/bin/mkdir -p '/build/BUILD/hello-1.0-SPECPARTS' +rm -rf '/build/BUILD/hello-1.0-x86_64/SPECPARTS' +/usr/bin/mkdir -p '/build/BUILD/hello-1.0-x86_64/SPECPARTS' /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w . echo "Patch #0 (hello-1.0-modernize.patch):" diff --git a/tools/rpmbuild.c b/tools/rpmbuild.c index 724dee82ac..be5ac8c58c 100644 --- a/tools/rpmbuild.c +++ b/tools/rpmbuild.c @@ -21,7 +21,6 @@ static struct rpmBuildArguments_s rpmBTArgs; #define POPT_NOLANG -1012 #define POPT_RMSOURCE -1013 #define POPT_RMBUILD -1014 -#define POPT_BUILDROOT -1015 #define POPT_TARGETPLATFORM -1016 #define POPT_NOBUILD -1017 #define POPT_RMSPEC -1019 @@ -126,13 +125,6 @@ static void buildArgCallback( poptContext con, case POPT_RMSOURCE: rba->buildAmount |= RPMBUILD_RMSOURCE; break; case POPT_RMSPEC: rba->buildAmount |= RPMBUILD_RMSPEC; break; case POPT_RMBUILD: rba->buildAmount |= RPMBUILD_RMBUILD; break; - case POPT_BUILDROOT: - if (rba->buildRootOverride) { - rpmlog(RPMLOG_ERR, _("buildroot already specified, ignoring %s\n"), arg); - break; - } - rba->buildRootOverride = xstrdup(arg); - break; case POPT_TARGETPLATFORM: argvSplit(&build_targets, arg, ","); break; @@ -251,8 +243,6 @@ static struct poptOption rpmBuildPoptTable[] = { N_("build through %install (%prep, %build, then install) from "), N_("") }, - { "buildroot", '\0', POPT_ARG_STRING, 0, POPT_BUILDROOT, - N_("override build root"), "DIRECTORY" }, { "build-in-place", '\0', 0, 0, POPT_BUILDINPLACE, N_("run build in current directory"), NULL }, { "clean", '\0', 0, 0, POPT_RMBUILD, @@ -523,7 +513,7 @@ static int buildForTarget(rpmts ts, const char * arg, BTA_t ba) } /* Create build tree if necessary */ - if (rpmMkdirs(root, "%{_topdir}:%{_builddir}:%{_rpmdir}:%{_srcrpmdir}:%{_buildrootdir}")) + if (rpmMkdirs(root, "%{_topdir}:%{_builddir}:%{_rpmdir}:%{_srcrpmdir}")) goto exit; if ((rc = rpmSpecBuild(ts, spec, ba))) { @@ -676,6 +666,8 @@ int main(int argc, char *argv[]) if (!noDeps) { ba->buildAmount |= RPMBUILD_CHECKBUILDREQUIRES; } + if (!shortCircuit) + ba->buildAmount |= RPMBUILD_BUILDDIR; break; case 'l': ba->buildAmount |= RPMBUILD_FILECHECK; @@ -683,8 +675,10 @@ int main(int argc, char *argv[]) case 'r': /* fallthrough */ case 'd': - if (!shortCircuit) + if (!shortCircuit) { ba->buildAmount |= RPMBUILD_PREP; + ba->buildAmount |= RPMBUILD_BUILDDIR; + } ba->buildAmount |= RPMBUILD_BUILDREQUIRES; ba->buildAmount |= RPMBUILD_DUMPBUILDREQUIRES; if (!noDeps)