From 3a510926449f1dd779a2933dfaa17de3d03a4ea4 Mon Sep 17 00:00:00 2001
Message-Id: <3a510926449f1dd779a2933dfaa17de3d03a4ea4.1566556207.git.pmatilai@redhat.com>
In-Reply-To: <ad4673589428db6e3b9fecd6f151eb899500336d.1566556207.git.pmatilai@redhat.com>
References: <ad4673589428db6e3b9fecd6f151eb899500336d.1566556207.git.pmatilai@redhat.com>
From: Panu Matilainen <pmatilai@redhat.com>
Date: Thu, 15 Aug 2019 14:10:07 +0300
Subject: [PATCH 2/3] Restore strict order of build scriptlet stdout/stderr
 output (#794)

Commit 18e8f4e9b2dd170d090843adf5b5084658d68cf7 and related changes
caused us to capture and re-emit stdout of all build scriptlets,
whether we actually use the output for anything or not. Besides doing
a whole bunch of work for nothing, this can disrupt the output of
build scriptlets by making the output jerky and out of order, at least
inside mock and other tools which in turn grab rpm output. This makes
troubleshooting failed builds unnecessarily hard for no good reason.

Handle the whole thing in a different way: on regular builds, don't
capture anything where we don't actually need to. This restores the
natural flow of output. We still need to somehow handle quiet builds
though, and we can't use redirect to /dev/null from %___build_pre like
we used to, because dynamic buildrequires need to provide output even
on quiet builds. So somewhat counter-intuitively, we need to capture
the output in order to discard it.

Closes: #794
---
 build/build.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/build/build.c b/build/build.c
index 3887457d3..dc196090f 100644
--- a/build/build.c
+++ b/build/build.c
@@ -60,7 +60,6 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name,
     int argc = 0;
     const char **argv = NULL;
     FILE * fp = NULL;
-    FILE * cmdOut = rpmIsVerbose() ? stdout : NULL;
 
     FD_t fd = NULL;
     rpmRC rc = RPMRC_FAIL; /* assume failure */
@@ -156,7 +155,7 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name,
 
     rpmlog(RPMLOG_NOTICE, _("Executing(%s): %s\n"), name, buildCmd);
     if (rpmfcExec((ARGV_const_t)argv, NULL, sb_stdoutp, 1,
-		  spec->buildSubdir, cmdOut)) {
+		  spec->buildSubdir, NULL)) {
 	rpmlog(RPMLOG_ERR, _("Bad exit status from %s (%s)\n"),
 		scriptName, name);
 	goto exit;
@@ -242,6 +241,9 @@ static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
     int missing_buildreqs = 0;
     int test = (what & RPMBUILD_NOBUILD);
     char *cookie = buildArgs->cookie ? xstrdup(buildArgs->cookie) : NULL;
+    /* handle quiet mode by capturing the output into a sink buffer */
+    StringBuf sink = NULL;
+    StringBuf *sbp = rpmIsVerbose() ? NULL : &sink;
 
     if (rpmExpandNumeric("%{?source_date_epoch_from_changelog}") &&
 	getenv("SOURCE_DATE_EPOCH") == NULL) {
@@ -292,7 +294,7 @@ static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
 
 	if ((what & RPMBUILD_PREP) &&
 	    (rc = doScript(spec, RPMBUILD_PREP, "%prep",
-			   getStringBuf(spec->prep), test, NULL)))
+			   getStringBuf(spec->prep), test, sbp)))
 		goto exit;
 
 	if (what & RPMBUILD_BUILDREQUIRES)
@@ -321,17 +323,17 @@ static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
 
 	if ((what & RPMBUILD_BUILD) &&
 	    (rc = doScript(spec, RPMBUILD_BUILD, "%build",
-			   getStringBuf(spec->build), test, NULL)))
+			   getStringBuf(spec->build), test, sbp)))
 		goto exit;
 
 	if ((what & RPMBUILD_INSTALL) &&
 	    (rc = doScript(spec, RPMBUILD_INSTALL, "%install",
-			   getStringBuf(spec->install), test, NULL)))
+			   getStringBuf(spec->install), test, sbp)))
 		goto exit;
 
 	if ((what & RPMBUILD_CHECK) &&
 	    (rc = doScript(spec, RPMBUILD_CHECK, "%check",
-			   getStringBuf(spec->check), test, NULL)))
+			   getStringBuf(spec->check), test, sbp)))
 		goto exit;
 
 	if ((what & RPMBUILD_PACKAGESOURCE) &&
@@ -358,11 +360,11 @@ static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
 	
 	if ((what & RPMBUILD_CLEAN) &&
 	    (rc = doScript(spec, RPMBUILD_CLEAN, "%clean",
-			   getStringBuf(spec->clean), test, NULL)))
+			   getStringBuf(spec->clean), test, sbp)))
 		goto exit;
 
 	if ((what & RPMBUILD_RMBUILD) &&
-	    (rc = doScript(spec, RPMBUILD_RMBUILD, "--clean", NULL, test, NULL)))
+	    (rc = doScript(spec, RPMBUILD_RMBUILD, "--clean", NULL, test, sbp)))
 		goto exit;
     }
 
@@ -373,6 +375,7 @@ static rpmRC buildSpec(rpmts ts, BTA_t buildArgs, rpmSpec spec, int what)
 	(void) unlink(spec->specFile);
 
 exit:
+    freeStringBuf(sink);
     free(cookie);
     spec->rootDir = NULL;
     if (rc != RPMRC_OK && rc != RPMRC_MISSINGBUILDREQUIRES &&
-- 
2.21.0