From 8b64af1e8dff44e05d4d35da0235cbd4b1f71040 Mon Sep 17 00:00:00 2001
From: Panu Matilainen <pmatilai@redhat.com>
Date: Wed, 17 Jun 2020 13:54:25 +0300
Subject: [PATCH] Always fail build on dependency generator failures (#1183)

Previously only the legacy external dependency generator listened to
exit codes from the generator, and even that only for provides.
Anybody building packages will want to know if generators barf up
for one reason or another. Let them.

Always call rpmfcExec() with failnonzero set, pass errors around, add tests.

Fixes: #1183
---
 build/rpmfc.c     | 32 ++++++++++++++++++++------------
 tests/rpmbuild.at | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/build/rpmfc.c b/build/rpmfc.c
index 6fb67fdddc..a74c828242 100644
--- a/build/rpmfc.c
+++ b/build/rpmfc.c
@@ -509,7 +509,7 @@ static void rpmfcAddFileDep(rpmfcFileDeps *fileDeps, rpmds ds, int ix)
 }
 
 static ARGV_t runCmd(const char *cmd,
-		     const char *buildRoot, const char *fn)
+		     const char *buildRoot, const char *fn, int *rcp)
 {
     ARGV_t output = NULL;
     ARGV_t av = NULL;
@@ -518,8 +518,10 @@ static ARGV_t runCmd(const char *cmd,
     argvAdd(&av, cmd);
 
     appendLineStringBuf(sb_stdin, fn);
-    if (rpmfcExec(av, sb_stdin, &sb_stdout, 0, buildRoot) == 0) {
+    if (rpmfcExec(av, sb_stdin, &sb_stdout, 1, buildRoot) == 0) {
 	argvSplit(&output, getStringBuf(sb_stdout), "\n\r");
+    } else {
+	*rcp = -1;
     }
 
     argvFree(av);
@@ -530,19 +532,21 @@ static ARGV_t runCmd(const char *cmd,
 }
 
 static ARGV_t runCall(const char *cmd,
-		     const char *buildRoot, const char *fn)
+		     const char *buildRoot, const char *fn, int *rcp)
 {
     ARGV_t output = NULL;
+    char *exp = NULL;
 
     if (_rpmfc_debug)
 	rpmlog(RPMLOG_DEBUG, "Calling %s() on %s\n", cmd, fn);
 
     /* Hack to pass in the path as what looks like a macro argument */
     rpmPushMacroFlags(NULL, "1", NULL, fn, 1, RPMMACRO_LITERAL);
-    char *exp = rpmExpand(cmd, NULL);
-    rpmPopMacro(NULL, "1");
-    if (*exp)
+    if (rpmExpandMacros(NULL, cmd, &exp, 0) < 0)
+	*rcp = -1;
+    else
 	argvSplit(&output, exp, "\n\r");
+    rpmPopMacro(NULL, "1");
     free(exp);
 
     return output;
@@ -609,12 +613,12 @@ static int rpmfcHelper(rpmfc fc, int ix, const struct exclreg_s *excl,
 	goto exit;
 
     if (callable) {
-	pav = runCall(cmd, fc->buildRoot, fn);
+	pav = runCall(cmd, fc->buildRoot, fn, &rc);
     } else {
-	pav = runCmd(cmd, fc->buildRoot, fn);
+	pav = runCmd(cmd, fc->buildRoot, fn, &rc);
     }
 
-    if (pav == NULL)
+    if (pav == NULL || rc < 0)
 	goto exit;
 
     pac = argvCount(pav);
@@ -629,9 +633,14 @@ static int rpmfcHelper(rpmfc fc, int ix, const struct exclreg_s *excl,
 	    rc++;
     }
 
+exit:
     argvFree(pav);
 
-exit:
+    if (rc < 0) {
+	rpmlog(RPMLOG_ERR, _("%s generator %s failed: %s\n"),
+		rpmTagGetName(tagN), cmd, fn);
+    }
+
     return rc;
 }
 
@@ -1464,7 +1473,6 @@ static rpmRC rpmfcApplyExternal(rpmfc fc)
 	rpmsenseFlags tagflags;
 	char * s = NULL;
 	StringBuf sb_stdout = NULL;
-	int failnonzero = (tag == RPMTAG_PROVIDEFLAGS);
 
 	switch (tag) {
 	case RPMTAG_PROVIDEFLAGS:
@@ -1494,7 +1502,7 @@ static rpmRC rpmfcApplyExternal(rpmfc fc)
 	free(s);
 
 	if (rpmfcExec(dm->argv, sb_stdin, &sb_stdout,
-			failnonzero, fc->buildRoot) == -1)
+			1, fc->buildRoot) == -1)
 	    continue;
 
 	if (sb_stdout == NULL) {
diff --git a/tests/rpmbuild.at b/tests/rpmbuild.at
index faca79da29..db35680fd1 100644
--- a/tests/rpmbuild.at
+++ b/tests/rpmbuild.at
@@ -482,6 +482,8 @@ AT_CLEANUP
 
 AT_SETUP([Dependency generation 3])
 AT_KEYWORDS([build])
+RPMDB_INIT
+
 AT_CHECK([
 RPMDB_INIT
 
@@ -500,6 +502,40 @@ runroot rpmbuild -bb --quiet \
 [error: Illegal char '*' (0x2a) in: *
     Illegal char '*' (0x2a) in: *
 ])
+
+AT_CHECK([
+RPMDB_INIT
+
+cat << EOF > "${RPMTEST}"/tmp/fail.req
+#!/bin/sh
+exit 1
+EOF
+chmod a+x "${RPMTEST}"/tmp/fail.req
+
+runroot rpmbuild -bb --quiet \
+		--define "__script_requires /tmp/fail.req" \
+		/data/SPECS/shebang.spec
+],
+[1],
+[],
+[error: Requirename generator /tmp/fail.req  failed: /build/BUILDROOT/shebang-0.1-1.x86_64/bin/shebang
+    Requirename generator /tmp/fail.req  failed: /build/BUILDROOT/shebang-0.1-1.x86_64/bin/shebang
+])
+
+AT_CHECK([
+RPMDB_INIT
+
+runroot rpmbuild -bb --quiet \
+		--define "__script_provides() %{error:bad %{basename:%1} stuff}" \
+		/data/SPECS/shebang.spec
+],
+[1],
+[],
+[error: bad shebang stuff
+error: Providename generator %{__script_provides %{?__script_provides_opts}} failed: /build/BUILDROOT/shebang-0.1-1.x86_64/bin/shebang
+    bad shebang stuff
+    Providename generator %{__script_provides %{?__script_provides_opts}} failed: /build/BUILDROOT/shebang-0.1-1.x86_64/bin/shebang
+])
 AT_CLEANUP
 
 AT_SETUP([Dependency generation 4])