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 %builddir 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 %builddir on,
while preserving compatibility with both of the above scenarios.

We would prefer to use NVRA for the %builddir 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

@@ -110,7 +110,7 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name,
 	       const char *sb, int test, StringBuf * sb_stdoutp)
 {
     char *scriptName = NULL;
-    char * buildDir = rpmGenPath(spec->rootDir, "%{_builddir}", "");
+    char * buildDir = rpmGenPath(spec->rootDir, "%{builddir}", "");
     char * buildSubdir = rpmGetPath("%{?buildsubdir}", NULL);
     char * buildCmd = NULL;
     char * buildTemplate = NULL;
@@ -126,6 +126,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}";
@@ -195,18 +199,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);
 
@@ -308,6 +305,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;
@@ -377,13 +392,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/files.c

@@ -1655,7 +1655,7 @@ static rpmRC processMetadataFile(Package pkg, FileList fl,
 	fn = rpmGenPath(fl->buildRoot, NULL, fileName);
 	absolute = 1;
     } else
-	fn = rpmGenPath("%{_builddir}", "%{?buildsubdir}", fileName);
+	fn = rpmGenPath("%{builddir}", "%{?buildsubdir}", fileName);
 
     switch (tag) {
     default:
@@ -2245,7 +2245,7 @@ int readManifest(rpmSpec spec, const char *path, const char *descr, int flags,
     if (*path == '/') {
 	fn = rpmGetPath(path, NULL);
     } else {
-	fn = rpmGenPath("%{_builddir}", "%{?buildsubdir}", path);
+	fn = rpmGenPath("%{builddir}", "%{?buildsubdir}", path);
     }
     fd = fopen(fn, "r");
 
@@ -2389,7 +2389,7 @@ static void processSpecialDir(rpmSpec spec, Package pkg, FileList fl,
     char *mkdocdir = rpmExpand("%{__mkdir_p} $", sdenv, NULL);
     StringBuf docScript = newStringBuf();
     int count = sd->entriesCount;
-    char *basepath = rpmGenPath(spec->rootDir, "%{_builddir}", "%{?buildsubdir}");
+    char *basepath = rpmGenPath(spec->rootDir, "%{builddir}", "%{?buildsubdir}");
     ARGV_t *files = xmalloc(sizeof(*files) * count);
     int i, j;
 

build/parsePreamble.c

@@ -1267,28 +1267,31 @@ 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);
+	    /* Override toplevel _builddir for backwards compatibility */
+	    rpmPushMacro(spec->macros, "_builddir", NULL, spec->buildDir, RMIL_SPEC);
+
+	    /* A user-oriented, unambiguous name for the thing */
+	    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/policies.c

@@ -155,7 +155,7 @@ static ModuleRec newModule(const char *path, const char *name,
     ModuleRec mod;
     uint8_t *raw = NULL;
     ssize_t rawlen = 0;
-    const char *buildDir = "%{_builddir}/%{?buildsubdir}/";
+    const char *buildDir = "%{builddir}/%{?buildsubdir}/";
 
     if (!path) {
 	rpmlog(RPMLOG_ERR, _("%%semodule requires a file path\n"));

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;

docs/manual/buildprocess.md

@@ -10,6 +10,7 @@ title: rpm.org - Package Build Process
 * Parse the [spec](spec.md)
   * If  buildarch detected parse spec multiple times - once for each arch with `_target_cpu` macro set
   * Build will iterate over all the spec variants and build multiple versions
+* Execute internal `%builddir` script to create `%builddir`
 * Check static build requires
 * Execute present [build scriptlets](spec.md#build-scriptlets)
   * `%prep`

docs/manual/dynamic_specs.md

@@ -13,9 +13,8 @@ build results.
 
 The files need to be placed in the **%{specpartsdir}** (also available
 as **$RPM_SPECPARTS_DIR** in the build scripts) and have a
-**.specpart** postfix. The directory is created by **%setup**. Default
-location is **%{_builddir}/%{buildsubdir}-SPECPARTS** which is beside
-the **%{buildsubdir}**. Scripts must not create it themselves but must
+**.specpart** postfix. The directory is created by **%setup**.
+Scripts must not create it themselves but must
 either fail if it is not present or switch to an alternative that does
 not require the feature. They should give an error message that
 dynamic spec generation is not supported on the given RPM version when

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

@@ -165,7 +165,7 @@
     %{?_find_debuginfo_dwz_opts} \\\
     %{?_find_debuginfo_opts} \\\
     %{?_debugsource_packages:-S debugsourcefiles.list} \\\
-    "%{_builddir}/%{?buildsubdir}"\
+    "%{builddir}/%{?buildsubdir}"\
 %{nil}
 
 #	Template for debug information sub-package.
@@ -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
 
@@ -286,7 +277,7 @@ Supplements:   (%{name} = %{version}-%{release} and langpacks-%{1})\
 #	Macros that are initialized as a side effect of rpmrc and/or spec
 #	file parsing.
 #
-#	The sub-directory (relative to %{_builddir}) where sources are compiled.
+#	The sub-directory (relative to %{builddir}) where sources are compiled.
 #	This macro is set after processing %setup, either explicitly from the
 #	value given to -n or the default name-version.
 #
@@ -763,7 +754,7 @@ Supplements:   (%{name} = %{version}-%{release} and langpacks-%{1})\
 %___build_cmd		%{?_sudo:%{_sudo} }%{?_remsh:%{_remsh} %{_remhost} }%{?_remsudo:%{_remsudo} }%{?_remchroot:%{_remchroot} %{_remroot} }%{___build_shell} %{___build_args}
 %___build_pre_env \
   RPM_SOURCE_DIR=\"%{_sourcedir}\"\
-  RPM_BUILD_DIR=\"%{_builddir}\"\
+  RPM_BUILD_DIR=\"%{builddir}\"\
   RPM_OPT_FLAGS=\"%{optflags}\"\
   RPM_LD_FLAGS=\"%{?build_ldflags}\"\
   RPM_ARCH=\"%{_arch}\"\
@@ -793,7 +784,7 @@ Supplements:   (%{name} = %{version}-%{release} and langpacks-%{1})\
   %{___build_pre_env} \
   %[%{verbose}?"set -x":""]\
   umask 022\
-  cd \"%{_builddir}\"\
+  cd \"%{builddir}\"\
 
 
 #%___build_body		%{nil}
@@ -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/data/SPECS/simple.spec

@@ -1,16 +1,21 @@
+%bcond setup 0
+
 Name:           simple
 Version:        1.0
 Release:        1
 Summary:        Simple test package
 Group:		Testing
 License:        GPL
 BuildArch:	noarch
+Source:		source-noroot.tar.gz
 
 %description
 %{summary}
 
+%if %{with setup}
 %prep
-%setup -c -T
+%setup -C
+%endif
 
 %build
 cat << EOF > simple