Skip to content

Commit 6c7cab6

Browse files
committed
[BuildSystem] Add execution queue delegate handler for process errors.
1 parent 0e8334b commit 6c7cab6

File tree

4 files changed

+53
-14
lines changed

4 files changed

+53
-14
lines changed

include/llbuild/BuildSystem/BuildExecutionQueue.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,15 @@ class BuildExecutionQueueDelegate {
178178
/// provided to the \see commandProcessFinished() call.
179179
virtual void commandProcessStarted(Command*, ProcessHandle handle) = 0;
180180

181+
/// Called to report an error in the management of a command process.
182+
///
183+
/// \param handle - The process handle.
184+
/// \param message - The error message.
185+
//
186+
// FIXME: Need to move to more structured error handling.
187+
virtual void commandProcessHadError(Command*, ProcessHandle handle,
188+
const Twine& message) = 0;
189+
181190
/// Called to report a command processes' (merged) standard output and error.
182191
///
183192
/// \param handle - The process handle.
@@ -190,7 +199,8 @@ class BuildExecutionQueueDelegate {
190199
/// \param handle - The handle used to identify the process. This handle will
191200
/// become invalid as soon as the client returns from this API call.
192201
///
193-
/// \param exitStatus - The exit status of the process.
202+
/// \param exitStatus - The exit status of the process, or -1 if an error was
203+
/// encountered.
194204
//
195205
// FIXME: Need to include additional information on the status here, e.g., the
196206
// signal status, and the process output (if buffering).

include/llbuild/BuildSystem/BuildSystemFrontend.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,15 @@ class BuildSystemFrontendDelegate : public BuildSystemDelegate {
166166
/// provided to the \see commandProcessFinished() call.
167167
virtual void commandProcessStarted(Command*, ProcessHandle handle);
168168

169+
/// Called to report an error in the management of a command process.
170+
///
171+
/// \param handle - The process handle.
172+
/// \param message - The error message.
173+
//
174+
// FIXME: Need to move to more structured error handling.
175+
virtual void commandProcessHadError(Command*, ProcessHandle handle,
176+
const Twine& message);
177+
169178
/// Called to report a command processes' (merged) standard output and error.
170179
///
171180
/// \param handle - The process handle.

lib/BuildSystem/BuildSystemFrontend.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,14 @@ class BuildSystemFrontendExecutionQueueDelegate
155155
command, BuildSystemFrontendDelegate::ProcessHandle { handle.id });
156156
}
157157

158+
virtual void commandProcessHadError(Command* command, ProcessHandle handle,
159+
const Twine& message) override {
160+
static_cast<BuildSystemFrontendDelegate*>(&getSystem().getDelegate())->
161+
commandProcessHadError(
162+
command, BuildSystemFrontendDelegate::ProcessHandle { handle.id },
163+
message);
164+
}
165+
158166
virtual void commandProcessHadOutput(Command* command, ProcessHandle handle,
159167
StringRef data) override {
160168
static_cast<BuildSystemFrontendDelegate*>(&getSystem().getDelegate())->
@@ -352,6 +360,17 @@ void BuildSystemFrontendDelegate::commandProcessStarted(Command* command,
352360
fflush(stdout);
353361
}
354362

363+
void BuildSystemFrontendDelegate::
364+
commandProcessHadError(Command* command, ProcessHandle handle,
365+
const Twine& message) {
366+
SmallString<256> buffer;
367+
auto str = message.toStringRef(buffer);
368+
369+
// FIXME: Design the logging and status output APIs.
370+
fwrite(str.data(), str.size(), 1, stderr);
371+
fflush(stderr);
372+
}
373+
355374
void BuildSystemFrontendDelegate::
356375
commandProcessHadOutput(Command* command, ProcessHandle handle,
357376
StringRef data) {

lib/BuildSystem/LaneBasedExecutionQueue.cpp

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,9 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
191191
int outputPipe[2]{ -1, -1 };
192192
if (shouldCaptureOutput) {
193193
if (::pipe(outputPipe) < 0) {
194-
// FIXME: Error handling.
195-
fprintf(stderr, "error: unable to spawn process (%s)\n",
196-
strerror(errno));
194+
getDelegate().commandProcessHadError(
195+
context.job.getForCommand(), handle,
196+
Twine("unable to open output pipe (") + strerror(errno) + ")");
197197
getDelegate().commandProcessFinished(context.job.getForCommand(),
198198
handle, -1);
199199
return false;
@@ -240,9 +240,9 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
240240
if (posix_spawn(&pid, args[0], /*file_actions=*/&fileActions,
241241
/*attrp=*/&attributes, const_cast<char**>(args.data()),
242242
::environ) != 0) {
243-
// FIXME: Error handling.
244-
fprintf(stderr, "error: unable to spawn process (%s)\n", strerror(errno));
245-
// FIXME: Communicate error status appropriately.
243+
getDelegate().commandProcessHadError(
244+
context.job.getForCommand(), handle,
245+
Twine("unable to spawn process (") + strerror(errno) + ")");
246246
getDelegate().commandProcessFinished(context.job.getForCommand(), handle,
247247
-1);
248248
return false;
@@ -262,9 +262,9 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
262262
char buf[4096];
263263
ssize_t numBytes = read(outputPipe[0], buf, sizeof(buf));
264264
if (numBytes < 0) {
265-
// FIXME: Error handling.
266-
fprintf(stderr, "error: unable to read from output pipe (%s)\n",
267-
strerror(errno));
265+
getDelegate().commandProcessHadError(
266+
context.job.getForCommand(), handle,
267+
Twine("unable to read process output (") + strerror(errno) + ")");
268268
break;
269269
}
270270

@@ -283,10 +283,9 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
283283
while (result == -1 && errno == EINTR)
284284
result = waitpid(pid, &status, 0);
285285
if (result == -1) {
286-
// FIXME: Error handling.
287-
fprintf(stderr, "error: unable to wait for process (%s)\n",
288-
strerror(errno));
289-
// FIXME: Communicate error status appropriately.
286+
getDelegate().commandProcessHadError(
287+
context.job.getForCommand(), handle,
288+
Twine("unable to wait for process (") + strerror(errno) + ")");
290289
getDelegate().commandProcessFinished(context.job.getForCommand(), handle,
291290
-1);
292291
return false;
@@ -299,6 +298,8 @@ class LaneBasedExecutionQueue : public BuildExecutionQueue {
299298
}
300299

301300
// Notify of the process completion.
301+
//
302+
// FIXME: Need to communicate more information on the process exit status.
302303
getDelegate().commandProcessFinished(context.job.getForCommand(), handle,
303304
status);
304305
return (status == 0);

0 commit comments

Comments
 (0)