Skip to content

Commit 0669f0f

Browse files
committed
Redirect stderr to stdout in subprocesses
This creates a better output. Also add tests for installing binaries.
1 parent cfceae3 commit 0669f0f

20 files changed

+332
-104
lines changed

R/files.R

-24
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,3 @@ mkdirp <- function(dir, msg = NULL) {
88
}
99
invisible(s)
1010
}
11-
12-
file_get_time <- function(path) {
13-
file.info(path)$mtime
14-
}
15-
16-
file_set_time <- function(path, time = Sys.time()) {
17-
assert_that(
18-
is_character(path),
19-
inherits(time, "POSIXct"))
20-
vlapply(path, Sys.setFileTime, time = time)
21-
}
22-
23-
## file.copy is buggy when to is a vector
24-
25-
file_copy_with_time <- function(from, to) {
26-
mkdirp(dirname(to))
27-
if (length(to) > 1) {
28-
mapply(file.copy, from, to,
29-
MoreArgs = list(overwrite = TRUE, copy.date = TRUE),
30-
USE.NAMES = FALSE)
31-
} else {
32-
file.copy(from, to, overwrite = TRUE, copy.date = TRUE)
33-
}
34-
}

R/git-auth.R

+4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11

2+
# nocov start
3+
24
gitcreds_get <- NULL
35
gitcreds_set <- NULL
46
gitcreds_delete <- NULL
@@ -812,3 +814,5 @@ read_file <- function(path, ...) {
812814

813815
environment()
814816
})
817+
818+
# nocov end

R/install-binary.R

+2-5
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,19 @@ install_binary <- function(filename, lib = .libPaths()[[1L]],
1111
is.null(quiet) || is_flag(quiet))
1212

1313
stdout <- ""
14-
stderr <- ""
1514

1615
px <- make_install_process(filename, lib = lib, metadata = metadata)
1716

1817
repeat {
1918
px$poll_io(100)
2019
stdout <- paste0(stdout, px$read_output())
21-
stderr <- paste0(stderr, px$read_error())
22-
if (!px$is_alive() &&
23-
!px$is_incomplete_output() && !px$is_incomplete_error()) {
20+
if (!px$is_alive() && !px$is_incomplete_output()) {
2421
break
2522
}
2623
}
2724

2825
if (px$get_exit_status() != 0) {
29-
stop("Package installation failed\n", stderr)
26+
stop("Package installation failed\n", stdout)
3027
}
3128

3229
cli_alert_success(paste0("Installed ", filename))

R/install-plan.R

+8-30
Original file line numberDiff line numberDiff line change
@@ -168,17 +168,14 @@ make_start_state <- function(plan, config) {
168168
package_time = I(rep_list(nrow(plan), as.POSIXct(NA))),
169169
package_error = I(rep_list(nrow(plan), list())),
170170
package_stdout = I(rep_list(nrow(plan), character())),
171-
package_stderr = I(rep_list(nrow(plan), character())),
172171
build_done = (plan$type %in% c("deps", "installed")) | plan$binary,
173172
build_time = I(rep_list(nrow(plan), as.POSIXct(NA))),
174173
build_error = I(rep_list(nrow(plan), list())),
175174
build_stdout = I(rep_list(nrow(plan), character())),
176-
build_stderr = I(rep_list(nrow(plan), character())),
177175
install_done = plan$type %in% c("deps", "installed"),
178176
install_time = I(rep_list(nrow(plan), as.POSIXct(NA))),
179177
install_error = I(rep_list(nrow(plan), list())),
180178
install_stdout = I(rep_list(nrow(plan), character())),
181-
install_stderr = I(rep_list(nrow(plan), character())),
182179
worker_id = NA_character_
183180
)
184181
plan <- cbind(plan, install_cols)
@@ -221,22 +218,17 @@ handle_events <- function(state, events) {
221218
handle_event <- function(state, evidx) {
222219
proc <- state$workers[[evidx]]$process
223220

224-
## Read out stdout and stderr. If process is done, then read out all
221+
## Read out stdout. If process is done, then read out all
225222
if (proc$is_alive()) {
226223
state$workers[[evidx]]$stdout <-
227224
c(state$workers[[evidx]]$stdout, out <- proc$read_output(n = 10000))
228-
state$workers[[evidx]]$stderr <-
229-
c(state$workers[[evidx]]$stderr, err <- proc$read_error(n = 10000))
230225
} else {
231226
state$workers[[evidx]]$stdout <-
232227
c(state$workers[[evidx]]$stdout, out <- proc$read_all_output())
233-
state$workers[[evidx]]$stderr <-
234-
c(state$workers[[evidx]]$stderr, err <- proc$read_all_error())
235228
}
236229

237230
## If there is still output, then wait a bit more
238-
if (proc$is_alive() ||
239-
proc$is_incomplete_output() || proc$is_incomplete_error()) {
231+
if (proc$is_alive() || proc$is_incomplete_output()) {
240232
return(state)
241233
}
242234

@@ -247,9 +239,8 @@ handle_event <- function(state, evidx) {
247239
## Post-process, this will throw on error
248240
if (is.function(proc$get_result)) proc$get_result()
249241

250-
## Cut stdout and stderr to lines
242+
## Cut stdout to lines
251243
worker$stdout <- cut_into_lines(worker$stdout)
252-
worker$stderr <- cut_into_lines(worker$stderr)
253244

254245
## Record what was done
255246
stop_task(state, worker)
@@ -436,7 +427,7 @@ start_task_package_uncompress <- function(state, task) {
436427
task$args$phase <- "uncompress"
437428
px <- make_uncompress_process(path, task$args$tree_dir)
438429
worker <- list(id = get_worker_id(), task = task, process = px,
439-
stdout = character(), stderr = character())
430+
stdout = character())
440431
state$workers <- c(
441432
state$workers, structure(list(worker), names = worker$id))
442433
state$plan$worker_id[pkgidx] <- worker$id
@@ -465,7 +456,7 @@ start_task_package_build <- function(state, task) {
465456
needscompilation, binary = FALSE,
466457
cmd_args = NULL)
467458
worker <- list(id = get_worker_id(), task = task, process = px,
468-
stdout = character(), stderr = character())
459+
stdout = character())
469460
state$workers <- c(
470461
state$workers, structure(list(worker), names = worker$id))
471462
state$plan$worker_id[pkgidx] <- worker$id
@@ -496,7 +487,7 @@ start_task_build <- function(state, task) {
496487
px <- make_build_process(path, pkg, tmp_dir, lib, vignettes, needscompilation,
497488
binary = TRUE, cmd_args = cmd_args)
498489
worker <- list(id = get_worker_id(), task = task, process = px,
499-
stdout = character(), stderr = character())
490+
stdout = character())
500491
state$workers <- c(
501492
state$workers, structure(list(worker), names = worker$id))
502493
state$plan$worker_id[pkgidx] <- worker$id
@@ -517,7 +508,7 @@ start_task_install <- function(state, task) {
517508
px <- make_install_process(filename, lib = lib, metadata = metadata)
518509
worker <- list(
519510
id = get_worker_id(), task = task, process = px,
520-
stdout = character(), stderr = character())
511+
stdout = character())
521512

522513
state$workers <- c(
523514
state$workers, structure(list(worker), names = worker$id))
@@ -565,7 +556,6 @@ stop_task_package_uncompress <- function(state, worker) {
565556
state$plan$package_time[[pkgidx]] <- time
566557
state$plan$package_error[[pkgidx]] <- ! success
567558
state$plan$package_stdout[[pkgidx]] <- worker$stdout
568-
state$plan$package_stderr[[pkgidx]] <- worker$stderr
569559
state$plan$worker_id[[pkgidx]] <- NA_character_
570560

571561
throw(new_pkg_uncompress_error(
@@ -574,8 +564,7 @@ stop_task_package_uncompress <- function(state, worker) {
574564
package = pkg,
575565
version = version,
576566
time = time,
577-
stdout = worker$stdout,
578-
stderr = worker$stderr
567+
stdout = worker$stdout
579568
)
580569
))
581570
}
@@ -608,20 +597,13 @@ stop_task_package_build <- function(state, worker) {
608597
} else {
609598
alert("info", "Standard output is empty")
610599
}
611-
if (!identical(worker$stderr, "")) {
612-
cli::cli_h1("Standard error")
613-
cli::cli_verbatim(worker$stdout)
614-
} else {
615-
alert("info", "Standard error is empty")
616-
}
617600
}
618601
update_progress_bar(state, 1L)
619602

620603
state$plan$package_done[[pkgidx]] <- TRUE
621604
state$plan$package_time[[pkgidx]] <- time
622605
state$plan$package_error[[pkgidx]] <- ! success
623606
state$plan$package_stdout[[pkgidx]] <- worker$stdout
624-
state$plan$package_stderr[[pkgidx]] <- worker$stderr
625607
state$plan$worker_id[[pkgidx]] <- NA_character_
626608

627609
if (!success) {
@@ -632,7 +614,6 @@ stop_task_package_build <- function(state, worker) {
632614
package = pkg,
633615
version = version,
634616
stdout = worker$stdout,
635-
stderr = worker$stderr,
636617
time = time
637618
)
638619
))
@@ -685,7 +666,6 @@ stop_task_build <- function(state, worker) {
685666
state$plan$build_time[[pkgidx]] <- time
686667
state$plan$build_error[[pkgidx]] <- ! success
687668
state$plan$build_stdout[[pkgidx]] <- worker$stdout
688-
state$plan$build_stderr[[pkgidx]] <- worker$stderr
689669
state$plan$worker_id[[pkgidx]] <- NA_character_
690670

691671
if (!success) {
@@ -695,7 +675,6 @@ stop_task_build <- function(state, worker) {
695675
package = pkg,
696676
version = version,
697677
stdout = worker$stdout,
698-
stderr = worker$stderr, # empty, but anyway...
699678
time = time
700679
)
701680
))
@@ -777,7 +756,6 @@ stop_task_install <- function(state, worker) {
777756
state$plan$install_time[[pkgidx]] <- time
778757
state$plan$install_error[[pkgidx]] <- ! success
779758
state$plan$install_stdout[[pkgidx]] <- worker$stdout
780-
state$plan$install_stderr[[pkgidx]] <- worker$stderr
781759
state$plan$worker_id[[pkgidx]] <- NA_character_
782760

783761
if (!success) {

R/install-tar.R

+9-8
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
make_untar_process <- function(tarfile, files = NULL, exdir = ".",
2323
restore_times = TRUE, post_process = NULL,
24-
stdout = "|", stderr = "|", ...) {
24+
stdout = "|", stderr = "2>&1", ...) {
2525
internal <- need_internal_tar()
2626
if (internal) {
2727
r_untar_process$new(tarfile, files, exdir, restore_times,
@@ -106,7 +106,7 @@ external_untar_process <- R6::R6Class(
106106
restore_times = TRUE,
107107
tar = Sys.getenv("TAR", "tar"),
108108
stdout = "|",
109-
stderr = "|",
109+
stderr = "2>&1",
110110
post_process = NULL,
111111
...) {
112112

@@ -167,7 +167,7 @@ r_untar_process <- R6::R6Class(
167167

168168
initialize = function(tarfile, files = NULL, exdir = ".",
169169
restore_times = TRUE, post_process = NULL,
170-
stdout = "|", stderr = "|", ...) {
170+
stdout = "|", stderr = "2>&1", ...) {
171171
options <- list(
172172
tarfile = normalizePath(tarfile),
173173
files = files,
@@ -300,22 +300,23 @@ run_uncompress_process <- function(archive, exdir = ".", ...) {
300300
))
301301
}
302302

303+
stdout <- tempfile()
303304
if (type == "zip") {
304305
external_process(
305306
make_unzip_process,
306307
zipfile = archive,
307308
exdir = exdir,
308-
stdout = tempfile(),
309-
stderr = tempfile()
310-
)
309+
stdout = stdout,
310+
stderr = stdout
311+
)
311312

312313
} else {
313314
external_process(
314315
make_untar_process,
315316
tarfile = archive,
316317
exdir = exdir,
317-
stdout = tempfile(),
318-
stderr = tempfile()
318+
stdout = stdout,
319+
stderr = stdout
319320
)
320321
}
321322
}

R/install-utils.R

-4
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,6 @@ format.package_uncompress_error <- function(x, ...) {
5959
stdout <- last_stdout_lines(x$data$stdout, "", prefix = "O> ")[-(1:2)]
6060
out <- c(out, "", "Standard output:", stdout)
6161
}
62-
if (!is.null(x$data$stderr)) {
63-
stderr <- last_stdout_lines(x$data$stderr, "", prefix = "E> ")[-(1:2)]
64-
out <- c(out, "", "Standard error:", stderr)
65-
}
6662
out
6763
}
6864

R/install-zip.R

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
make_unzip_process <- function(zipfile, exdir = ".",
55
post_process = NULL, stdout = "|",
6-
stderr = "|", ...) {
6+
stderr = "2>&1", ...) {
77
up <- unzip_process()
88
up$new(zipfile, exdir = exdir, post_process = post_process,
99
stdout = stdout, stderr = stderr, ...)

0 commit comments

Comments
 (0)