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

@@ -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),

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);
 	}
     }
 

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);
     }

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;

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;

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,

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}

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

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,16 +326,16 @@ 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=$?
 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):"