[pacman-dev] [PATCH] redirect scriptlet stderr through alpm

Jonathan Conder j at skurvy.no-ip.org
Sat May 15 11:41:39 EDT 2010


Fixes FS#18770, and hopefully an occasional deadlock in my frontend as well.
This introduces a new EVENT: PM_TRANS_EVT_SCRIPTLET_ERROR, and moves all
scriptlet-related callbacks into the parent process.

I modelled my work on the GLib function g_spawn_sync, but I am quite
unfamiliar with POSIX so there could be some issues with the implementation.
However, all the tests I have performed so far (against v3.3.3) seemed to
work fine (even the kernel installed and gave me the right warnings). This
patch might also make it easier to configure the shell used for install
scriptlets. I'm not 100% happy with the code structure at the moment, so
any suggestions would be welcome.

Signed-off-by: Jonathan Conder <j at skurvy.no-ip.org>
---
 lib/libalpm/alpm.h    |    4 +
 lib/libalpm/util.c    |  162 ++++++++++++++++++++++++++++++++++++++-----------
 src/pacman/callback.c |    3 +
 3 files changed, 133 insertions(+), 36 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 3329132..2692777 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -364,6 +364,10 @@ typedef enum _pmtransevt_t {
 	 * A line of text is passed to the callback.
 	 */
 	PM_TRANS_EVT_SCRIPTLET_INFO,
+	/** Scriptlet has printed information to stderr.
+	 * A line of text is passed to the callback.
+	 */
+	PM_TRANS_EVT_SCRIPTLET_ERROR,
 	/** Files will be downloaded from a repository.
 	 * The repository's tree name is passed to the callback.
 	 */
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index 32eaa44..1f76372 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -444,10 +444,52 @@ int _alpm_logaction(int usesyslog, FILE *f, const char *fmt, va_list args)
 	return(ret);
 }
 
+static int _fddup2(int fds[2], int i, int fd)
+{
+	while(dup2(fds[i], fd) == -1) {
+		if(errno != EINTR) {
+			return(-1);
+		}
+	}
+	return(0);
+}
+
+static int _fdselect(int fdout, int fderr, fd_set *fds)
+{
+	int nfds = -1;
+	FD_ZERO(fds);
+
+	if(fdout >= 0) {
+		FD_SET(fdout, fds);
+		if(fdout > nfds) {
+			nfds = fdout;
+		}
+	}
+	if(fderr >= 0) {
+		FD_SET(fderr, fds);
+		if(fderr > nfds) {
+			nfds = fderr;
+		}
+	}
+	if(++nfds == 0) {
+		return(2);
+	}
+
+	if(select(nfds, fds, NULL, NULL, NULL) == -1) {
+		if(errno == EINTR) {
+			FD_ZERO(fds);
+		} else {
+			return(1);
+		}
+	}
+	return(0);
+}
+
 int _alpm_run_chroot(const char *root, const char *cmd)
 {
 	char cwd[PATH_MAX];
 	pid_t pid;
+	int fdout[2], fderr[2];
 	int restore_cwd = 0;
 	int retval = 0;
 
@@ -471,6 +513,13 @@ int _alpm_run_chroot(const char *root, const char *cmd)
 	/* Flush open fds before fork() to avoid cloning buffers */
 	fflush(NULL);
 
+	/* Create pipes to redirect stderr and stdout */
+	if(pipe(fdout) == -1 || pipe(fderr) == -1) {
+		_alpm_log(PM_LOG_ERROR, _("could not create a pipe (%s)\n"), strerror(errno));
+		retval = 1;
+		goto cleanup;
+	}
+
 	/* fork- parent and child each have seperate code blocks below */
 	pid = fork();
 	if(pid == -1) {
@@ -480,56 +529,97 @@ int _alpm_run_chroot(const char *root, const char *cmd)
 	}
 
 	if(pid == 0) {
-		FILE *pipe;
 		/* this code runs for the child only (the actual chroot/exec) */
-		_alpm_log(PM_LOG_DEBUG, "chrooting in %s\n", root);
+		close(fdout[0]);
+		close(fderr[0]);
+
+		/* redirect standard output and error */
+		_fddup2(fdout, 1, 1);
+		_fddup2(fderr, 1, 2);
+		close(fdout[1]);
+		close(fderr[1]);
+
 		if(chroot(root) != 0) {
-			_alpm_log(PM_LOG_ERROR, _("could not change the root directory (%s)\n"),
-					strerror(errno));
+			fprintf(stderr, _("could not change the root directory (%s)\n"), strerror(errno));
 			exit(1);
 		}
+
 		if(chdir("/") != 0) {
-			_alpm_log(PM_LOG_ERROR, _("could not change directory to / (%s)\n"),
-					strerror(errno));
+			fprintf(stderr, _("could not change directory to / (%s)\n"), strerror(errno));
 			exit(1);
 		}
+
+		/* run the scriptlet and exit */
 		umask(0022);
-		pipe = popen(cmd, "r");
-		if(!pipe) {
-			_alpm_log(PM_LOG_ERROR, _("call to popen failed (%s)\n"),
-					strerror(errno));
-			exit(1);
-		}
-		while(!feof(pipe)) {
-			char line[PATH_MAX];
-			if(fgets(line, PATH_MAX, pipe) == NULL)
-				break;
-			alpm_logaction("%s", line);
-			EVENT(handle->trans, PM_TRANS_EVT_SCRIPTLET_INFO, line, NULL);
-		}
-		retval = pclose(pipe);
-		exit(WEXITSTATUS(retval));
+		execl("/bin/sh", "sh", "-c", cmd, (char *) NULL);
+		fprintf(stderr, _("call to execl failed (%s)\n"), strerror(errno));
+		exit(1);
 	} else {
-		/* this code runs for the parent only (wait on the child) */
-		pid_t retpid;
+		/* this code runs for the parent only (process output from the child) */
+		fd_set fds;
+		FILE *fout, *ferr;
 		int status;
-		do {
-			retpid = waitpid(pid, &status, 0);
-		} while(retpid == -1 && errno == EINTR);
-		if(retpid == -1) {
-			_alpm_log(PM_LOG_ERROR, _("call to waitpid failed (%s)\n"),
-			          strerror(errno));
+
+		close(fdout[1]);
+		close(fderr[1]);
+
+		/* use buffered streams to read line by line */
+		if((fout = fdopen(fdout[0], "r")) == NULL || (ferr = fdopen(fderr[0], "r")) == NULL) {
 			retval = 1;
-			goto cleanup;
+			close(fdout[0]);
+			close(fderr[0]);
 		} else {
-			/* check the return status, make sure it is 0 (success) */
-			if(WIFEXITED(status)) {
-				_alpm_log(PM_LOG_DEBUG, "call to waitpid succeeded\n");
-				if(WEXITSTATUS(status) != 0) {
-					_alpm_log(PM_LOG_ERROR, _("command failed to execute correctly\n"));
-					retval = 1;
+			while((retval = _fdselect(fdout[0], fderr[0], &fds)) == 0) {
+				char line[PATH_MAX];
+
+				if(FD_ISSET(fdout[0], &fds)) {
+					if(fgets(line, PATH_MAX, fout) != NULL) {
+						alpm_logaction("%s", line);
+						EVENT(handle->trans, PM_TRANS_EVT_SCRIPTLET_INFO, line, NULL);
+					} else {
+						fdout[0] = -1;
+					}
+				}
+
+				if(FD_ISSET(fderr[0], &fds)) {
+					if(fgets(line, PATH_MAX, ferr) != NULL) {
+						alpm_logaction("%s", line);
+						EVENT(handle->trans, PM_TRANS_EVT_SCRIPTLET_ERROR, line, NULL);
+					} else {
+						fderr[0] = -1;
+					}
 				}
 			}
+
+			fclose(fout);
+			fclose(ferr);
+		}
+
+		while(waitpid(pid, &status, 0) == -1) {
+			if (errno != EINTR) {
+				_alpm_log(PM_LOG_ERROR, _("call to waitpid failed (%s)\n"),
+			            strerror(errno));
+				retval = 1;
+				goto cleanup;
+			}
+		}
+
+		/* report error from above after the child has exited */
+		if(retval == 1) {
+			_alpm_log(PM_LOG_ERROR, _("could not read from pipe (%s)\n"),
+			          strerror(errno));
+			goto cleanup;
+		} else {
+			retval = 0;
+		}
+
+		/* check the return status, make sure it is 0 (success) */
+		if(WIFEXITED(status)) {
+			_alpm_log(PM_LOG_DEBUG, "call to waitpid succeeded\n");
+			if(WEXITSTATUS(status) != 0) {
+				_alpm_log(PM_LOG_ERROR, _("command failed to execute correctly\n"));
+				retval = 1;
+			}
 		}
 	}
 
diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index f5bf17d..a33178e 100644
--- a/src/pacman/callback.c
+++ b/src/pacman/callback.c
@@ -226,6 +226,9 @@ void cb_trans_evt(pmtransevt_t event, void *data1, void *data2)
 		case PM_TRANS_EVT_SCRIPTLET_INFO:
 			printf("%s", (char*)data1);
 			break;
+		case PM_TRANS_EVT_SCRIPTLET_ERROR:
+			fprintf(stderr, "%s", (char*)data1);
+			break;
 		case PM_TRANS_EVT_RETRIEVE_START:
 			printf(_(":: Retrieving packages from %s...\n"), (char*)data1);
 			break;
-- 
1.7.1




More information about the pacman-dev mailing list