[Buildd-tools-devel] [PATCH 17/22] Don't rollback non-run scripts
Jan-Marek Glogowski
glogow at fbihome.de
Thu Mar 26 21:13:55 UTC 2009
This just rolls back the scripts, which have run, excluding the
failed one. On failure scripts should cleanup before exit.
This helps especially with the killprocs script. If a second
loopback session was started, but mount failed (as expected),
killprocs killed the processes from the current running session.
---
sbuild/sbuild-run-parts.cc | 58 ++++++++++++++++------
sbuild/sbuild-run-parts.h | 16 ++++--
sbuild/sbuild-session.cc | 115 +++++++++++++++++++++++++++++++++++++++-----
sbuild/sbuild-session.h | 20 ++++++--
test/sbuild-run-parts.cc | 18 +++++--
5 files changed, 183 insertions(+), 44 deletions(-)
diff --git a/sbuild/sbuild-run-parts.cc b/sbuild/sbuild-run-parts.cc
index fb1696b..2ea20af 100644
--- a/sbuild/sbuild-run-parts.cc
+++ b/sbuild/sbuild-run-parts.cc
@@ -115,9 +115,17 @@ run_parts::set_reverse (bool reverse)
int
run_parts::run (string_list const& command,
- environment const& env)
+ environment const& env,
+ std::string & failed_program)
{
- int exit_status = 0;
+ int exit_status = EXIT_FAILURE;
+ bool found_offset = failed_program.empty();
+
+ if (found_offset && this->programs.empty())
+ return EXIT_SUCCESS;
+
+ string_list real_command = command;
+ real_command.insert( real_command.begin(), std::string() );
if (!this->reverse)
{
@@ -125,17 +133,26 @@ run_parts::run (string_list const& command,
pos != this->programs.end();
++pos)
{
- string_list real_command;
- real_command.push_back(*pos);
- for (string_list::const_iterator spos = command.begin();
- spos != command.end();
- ++spos)
- real_command.push_back(*spos);
+ if (!found_offset)
+ {
+ if (failed_program == *pos)
+ {
+ found_offset = true;
+ failed_program.clear();
+ exit_status = EXIT_SUCCESS;
+ }
+ continue;
+ }
+
+ *(real_command.begin()) = *pos;
exit_status = run_child(*pos, real_command, env);
if (exit_status && this->abort_on_error)
- return exit_status;
+ {
+ failed_program = *pos;
+ break;
+ }
}
}
else
@@ -144,17 +161,26 @@ run_parts::run (string_list const& command,
pos != this->programs.rend();
++pos)
{
- string_list real_command;
- real_command.push_back(*pos);
- for (string_list::const_iterator spos = command.begin();
- spos != command.end();
- ++spos)
- real_command.push_back(*spos);
+ if (!found_offset)
+ {
+ if (failed_program == *pos)
+ {
+ found_offset = true;
+ failed_program.clear();
+ exit_status = EXIT_SUCCESS;
+ }
+ continue;
+ }
+
+ *(real_command.begin()) = *pos;
exit_status = run_child(*pos, real_command, env);
if (exit_status && this->abort_on_error)
- return exit_status;
+ {
+ failed_program = *pos;
+ break;
+ }
}
}
diff --git a/sbuild/sbuild-run-parts.h b/sbuild/sbuild-run-parts.h
index 125d986..15927f2 100644
--- a/sbuild/sbuild-run-parts.h
+++ b/sbuild/sbuild-run-parts.h
@@ -104,17 +104,23 @@ namespace sbuild
set_reverse (bool reverse);
/**
- * Run all scripts in the specified directory. If abort_on_error
- * is true, execution will stop at the first script to fail.
+ * Run all scripts in the specified directory.
+ *
+ * If abort_on_error is true, execution will stop at the first script
+ * to fail. If failed_program is set, all program execution is skipped
+ * until after the specified program.
*
* @param command the command to run.
* @param env the environment to use.
- * @returns the exit status of the scripts. This will be 0 on
- * success, or the exit status of the last failing script.
+ * @param failed_program will be set, if abort_on_error is true
+ * @returns the exit status of the scripts. This will be 0 on success,
+ * or the exit status of the last failing script. If failed_program is
+ * set but not found, exit status is failed.
*/
int
run(string_list const& command,
- environment const& env);
+ environment const& env,
+ std::string & failed_program);
/**
* Output the environment to an ostream.
diff --git a/sbuild/sbuild-session.cc b/sbuild/sbuild-session.cc
index a6bbe8d..ce849e5 100644
--- a/sbuild/sbuild-session.cc
+++ b/sbuild/sbuild-session.cc
@@ -72,6 +72,7 @@ namespace
emap(session::CHILD_CORE, N_("Child dumped core")),
emap(session::CHILD_FAIL, N_("Child exited abnormally (reason unknown; not a signal or core dump)")),
emap(session::CHILD_FORK, N_("Failed to fork child")),
+ emap(session::CHILD_PIPE, N_("Failed to create pipe for child communication")),
// TRANSLATORS: %4% = signal name
emap(session::CHILD_SIGNAL, N_("Child terminated by signal '%4%'")),
emap(session::CHILD_WAIT, N_("Wait for child failed")),
@@ -1120,6 +1121,13 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
int exit_status = 0;
pid_t pid;
+ int pipefd[2];
+
+ if (pipe(pipefd) == -1)
+ {
+ this->chroot_status = false;
+ throw error(session_chroot->get_name(), CHILD_PIPE, strerror(errno));
+ }
if ((pid = fork()) == -1)
{
@@ -1128,12 +1136,19 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
}
else if (pid == 0)
{
+ int status = EXIT_FAILURE;
+
+ // Close read end
+ close(pipefd[0]);
+
try
{
// The setup scripts don't use our syslog fd.
closelog();
- chdir("/");
+ if (chdir ("/") != 0)
+ throw error("/", CHDIR, strerror(errno));
+
/* This is required to ensure the scripts run with uid=0 and gid=0,
otherwise setuid programs such as mount(8) will fail. This
should always succeed, because our euid=0 and egid=0.*/
@@ -1141,9 +1156,24 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
setgid(0);
initgroups("root", 0);
- int status = rp.run(arg_list, env);
-
- _exit (status);
+ status = rp.run(arg_list, env, this->failed_program);
+ if (status != EXIT_SUCCESS)
+ {
+ ssize_t offset = 0;
+ ssize_t left = this->failed_program.length();
+ while (left > 0)
+ {
+ ssize_t count = write(pipefd[1],
+ this->failed_program.data() + offset, left);
+ if (count == -1)
+ left = 0;
+ else if (count > 0)
+ {
+ left -= count;
+ offset += count;
+ }
+ }
+ }
}
catch (std::exception const& e)
{
@@ -1154,11 +1184,22 @@ session::setup_chroot (sbuild::chroot::ptr& session_chroot,
sbuild::log_error()
<< _("An unknown exception occurred") << std::endl;
}
- _exit(EXIT_FAILURE);
+
+ // Close write end
+ close(pipefd[1]);
+
+ _exit(status);
}
else
{
- wait_for_child(pid, exit_status);
+ // Close write end
+ close(pipefd[1]);
+
+ this->failed_program.clear();
+ wait_for_child(pid, exit_status, pipefd[0], &this->failed_program);
+
+ // Close read end
+ close(pipefd[0]);
}
try
@@ -1213,7 +1254,7 @@ session::run_child (sbuild::chroot::ptr& session_chroot)
<< session_chroot->get_persona()<< std::endl;
/* Enter the chroot */
- if (chdir (location.c_str()))
+ if (chdir (location.c_str()) != 0)
throw error(location, CHDIR, strerror(errno));
log_debug(DEBUG_NOTICE) << "Changed directory to " << location << std::endl;
if (::chroot (location.c_str()))
@@ -1246,7 +1287,7 @@ session::run_child (sbuild::chroot::ptr& session_chroot)
dpos != dlist.end();
++dpos)
{
- if (chdir ((*dpos).c_str()) < 0)
+ if (chdir ((*dpos).c_str()) != 0)
{
error e(*dpos, CHDIR, strerror(errno));
@@ -1312,14 +1353,43 @@ session::run_child (sbuild::chroot::ptr& session_chroot)
_exit(EXIT_FAILURE);
}
+#define BUFFER_SIZE 128
+
+static void
+flush_and_close (int read_fd,
+ std::string *data,
+ char *buffer)
+{
+ if (read_fd <= -1)
+ return;
+
+ while (1)
+ {
+ ssize_t count = read(read_fd, buffer, BUFFER_SIZE);
+ if (count <= 0)
+ break;
+ if (data)
+ data->append(buffer, count);
+ }
+
+ close(read_fd);
+}
+
void
-session::wait_for_child (pid_t pid,
- int& child_status)
+session::wait_for_child (pid_t pid,
+ int& child_status,
+ int read_fd,
+ std::string *data)
{
child_status = EXIT_FAILURE; // Default exit status
- int status;
+ int status, wait_status;
bool child_killed = false;
+ int wait_option = (read_fd > -1) ? WNOHANG : 0;
+ char buffer[BUFFER_SIZE];
+
+ if (data != NULL)
+ data->clear();
while (1)
{
@@ -1343,27 +1413,44 @@ session::wait_for_child (pid_t pid,
child_killed = true;
}
- if (waitpid(pid, &status, 0) == -1)
+ wait_status = waitpid(pid, &status, wait_option);
+ if (wait_status == -1)
{
if (errno == EINTR && (sighup_called || sigterm_called))
continue; // Kill child and wait again.
else
- throw error(CHILD_WAIT, strerror(errno));
+ {
+ flush_and_close(read_fd, data, buffer);
+ throw error(CHILD_WAIT, strerror(errno));
+ }
}
else if (sighup_called)
{
sighup_called = false;
+ flush_and_close(read_fd, data, buffer);
throw error(SIGNAL_CATCH, strsignal(SIGHUP));
}
else if (sigterm_called)
{
sigterm_called = false;
+ flush_and_close(read_fd, data, buffer);
throw error(SIGNAL_CATCH, strsignal(SIGTERM));
}
+ else if (wait_option)
+ {
+ if (wait_status == pid)
+ break;
+
+ ssize_t count = read(read_fd, buffer, BUFFER_SIZE);
+ if (data && (count > 0))
+ data->append(buffer, count);
+ }
else
break;
}
+ flush_and_close(read_fd, data, buffer);
+
if (!WIFEXITED(status))
{
if (WIFSIGNALED(status))
@@ -1377,6 +1464,8 @@ session::wait_for_child (pid_t pid,
child_status = WEXITSTATUS(status);
}
+#undef BUFFER_SIZE
+
void
session::run_chroot (sbuild::chroot::ptr& session_chroot)
{
diff --git a/sbuild/sbuild-session.h b/sbuild/sbuild-session.h
index 966cec2..4e11932 100644
--- a/sbuild/sbuild-session.h
+++ b/sbuild/sbuild-session.h
@@ -64,6 +64,7 @@ namespace sbuild
CHILD_CORE, ///< Child dumped core.
CHILD_FAIL, ///< Child exited abnormally (reason unknown)
CHILD_FORK, ///< Failed to fork child.
+ CHILD_PIPE, ///< Failed to create pipe for child communication.
CHILD_SIGNAL, ///< Child terminated by signal.
CHILD_WAIT, ///< Wait for child failed.
CHROOT, ///< Failed to change root to directory.
@@ -386,14 +387,21 @@ namespace sbuild
/**
* Wait for a child process to complete, and check its exit status.
*
+ * Optionally read data from a child pipe while waiting. If the data
+ * parameter is omitted, data will be read but tossed.
+ *
* An error will be thrown on failure.
*
* @param pid the pid to wait for.
* @param child_status the place to store the child exit status.
+ * @param read_fd readable pipe fd to get data from
+ * @param data pointer to a std:string to store the data.
*/
void
- wait_for_child (pid_t pid,
- int& child_status);
+ wait_for_child (pid_t pid,
+ int& child_status,
+ int read_fd = -1,
+ std::string *data = 0);
/**
* Set the SIGHUP handler.
@@ -456,7 +464,7 @@ namespace sbuild
/// The current chroot status.
int chroot_status;
/// Lock status for locks acquired during chroot setup.
- bool lock_status;
+ bool lock_status;
/// The child exit status.
int child_status;
/// The session operation to perform.
@@ -470,9 +478,11 @@ namespace sbuild
/// Signal saved while sigterm handler is set.
struct sigaction saved_sigterm_signal;
/// Saved terminal settings.
- struct termios saved_termios;
+ struct termios saved_termios;
/// Are the saved terminal settings valid?
- bool termios_ok;
+ bool termios_ok;
+ /// Last failed program / script
+ std::string failed_program;
protected:
/// Current working directory.
diff --git a/test/sbuild-run-parts.cc b/test/sbuild-run-parts.cc
index 208337c..8ba878d 100644
--- a/test/sbuild-run-parts.cc
+++ b/test/sbuild-run-parts.cc
@@ -80,22 +80,26 @@ public:
int status;
+ std::string failed_program;
sbuild::string_list command;
sbuild::environment env(environ);
command.push_back("ok");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_SUCCESS);
+ CPPUNIT_ASSERT(failed_program.empty());
command.clear();
command.push_back("fail");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_FAILURE);
+ CPPUNIT_ASSERT(failed_program == "20test2");
command.clear();
command.push_back("fail2");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_FAILURE);
+ CPPUNIT_ASSERT(failed_program == "30test3");
}
void test_run2()
@@ -104,12 +108,14 @@ public:
int status;
+ std::string failed_program;
sbuild::string_list command;
sbuild::environment env(environ);
command.push_back("ok");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_SUCCESS);
+ CPPUNIT_ASSERT(failed_program.empty());
}
void test_run3()
@@ -118,12 +124,14 @@ public:
int status;
+ std::string failed_program;
sbuild::string_list command;
sbuild::environment env(environ);
command.push_back("ok");
- status = rp.run(command, env);
+ status = rp.run(command, env, failed_program);
CPPUNIT_ASSERT(status == EXIT_FAILURE);
+ CPPUNIT_ASSERT(failed_program == "50invalid");
}
};
--
1.6.2.1
More information about the Buildd-tools-devel
mailing list