Skip to content

Commit 5062e64

Browse files
committed
* src/iracebin/irace.h. Better handling of paths in Windows. Fixes #79.
* tests/testthat/test-target-runner-dummy.R: Run more tests on Windows.
1 parent 58a91b5 commit 5062e64

File tree

3 files changed

+83
-33
lines changed

3 files changed

+83
-33
lines changed

NEWS.md

+2
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343

4444
## Fixes
4545

46+
* Fix #79: irace.exe did not encode the `libPath` correctly in R >= 4.0 on Windows, producing 'unrecognized escape in character string'. Fixed by replacing backslashes with forward slashes and quoting the arguments that may contain spaces.
47+
4648
* Fix #76: Recovery (`--recover-file`) is working again with a completely new implementation.
4749

4850
* Fixed documentation of `psRace()`.

src/iracebin/irace.h

+65-21
Original file line numberDiff line numberDiff line change
@@ -5,45 +5,89 @@
55
#include <unistd.h>
66
#include "whereami.h"
77

8-
#define max_path_length 1024
8+
#define MAX_PATH_LENGTH 1024
9+
910
char *get_install_path(const char * package_path)
1011
{
11-
char fullpath[max_path_length - 1] = "\0";
12-
int dirname_length = 0;
13-
int res = wai_getExecutablePath(fullpath, max_path_length, &dirname_length);
14-
if (res <= 0)
15-
return NULL;
16-
if (res <= dirname_length)
12+
char fullpath[MAX_PATH_LENGTH - 1] = "\0";
13+
int fullpath_length = 0;
14+
int res = wai_getExecutablePath(fullpath, MAX_PATH_LENGTH, &fullpath_length);
15+
if (res <= 0 || res <= fullpath_length)
1716
return NULL;
18-
dirname_length -= strlen(package_path);
19-
char * path = (char*) malloc(dirname_length+1);
20-
memcpy(path, fullpath, dirname_length);
21-
path[dirname_length] = '\0';
17+
// printf("fullpath: %s (%d)\n", fullpath, fullpath_length);
18+
#ifdef WIN32
19+
for (size_t i = 0; i < fullpath_length; ++i) {
20+
if (fullpath[i] == '\\') {
21+
fullpath[i] = '/';
22+
}
23+
}
24+
#endif
25+
// Remove package_path and everything after it from fullpath.
26+
size_t package_path_len = strlen(package_path);
27+
char *pos = strstr(fullpath, package_path);
28+
char * last_found = NULL;
29+
if (pos == NULL) {
30+
printf("Error: irace is installed at an unexpected path: %s\n", fullpath);
31+
exit(EXIT_FAILURE);
32+
}
33+
// Search the last occurrence in case fullpath is something like
34+
// /irace/bin/R/x86_64-pc-linux-gnu-library/4.1/irace/bin/irace .
35+
do {
36+
last_found = pos;
37+
pos = strstr(pos + package_path_len, package_path);
38+
} while (pos != NULL);
39+
*last_found = '\0';
40+
fullpath_length = strlen(fullpath);
41+
char * path = (char*) malloc((fullpath_length + 1) * sizeof(char));
42+
memcpy(path, fullpath, fullpath_length + 1);
43+
// printf("path: %s (%d)\n", fullpath, fullpath_length);
2244
return path;
2345
}
46+
#undef MAX_PATH_LENGTH
2447

2548
int exec_R(int argc, char *argv[], const char * user_code)
2649
{
50+
const char * path = get_install_path("/irace/bin/");
51+
if (path == NULL)
52+
path = "";
53+
54+
#define _libpath_str ".libPaths(c(r'{%s}', .libPaths()));%s"
2755
#ifdef WIN32
28-
# define EXE_EXT ".exe"
56+
// Windows execvp() does not pass command-line arguments correctly if they
57+
// contain space, tab, backslash, or double-quote characters.
58+
#define libpath_str "\"" _libpath_str "\""
2959
#else
30-
# define EXE_EXT ""
60+
#define libpath_str _libpath_str
3161
#endif
32-
const char file[] = "R" EXE_EXT;
33-
const char * path = get_install_path("/irace/bin");
34-
if (path == NULL) path = "";
35-
#define libpath_str ".libPaths(c('%s', .libPaths()));%s"
62+
// -4 because of two times %s.
3663
char * r_code = malloc(sizeof(char) * (strlen(libpath_str) - 4 + strlen(path) + strlen(user_code) + 1));
3764
sprintf(r_code, libpath_str, path, user_code);
38-
//printf("%s\n", libpath);
39-
65+
#undef _libpath_str
66+
#undef libpath_str
67+
// printf("%s\n", r_code);
68+
4069
char * const extra_argv[] = { "--vanilla", "--slave", "-e", r_code, "--args"};
4170
int extra_argc = sizeof(extra_argv) / sizeof(char *);
42-
char **cmd_args = calloc(extra_argc + argc + 1, sizeof(char *));
71+
char **cmd_args = malloc((extra_argc + argc + 1) * sizeof(char *));
4372
cmd_args[0] = argv[0];
4473
for (int i = 0; i < extra_argc; i++)
4574
cmd_args[i+1] = extra_argv[i];
4675
for (int i = 1; i < argc; i++)
4776
cmd_args[extra_argc + i] = argv[i];
48-
return execvp(file, cmd_args);
77+
cmd_args[extra_argc + argc] = NULL;
78+
// for (int i = 0; i < extra_argc + argc; i++)
79+
// printf("%d: %s\n", i, cmd_args[i]);
80+
81+
#ifdef WIN32
82+
# define EXE_EXT ".exe"
83+
#else
84+
# define EXE_EXT ""
85+
#endif
86+
const char file[] = "R" EXE_EXT;
87+
#undef EXE_EXT
88+
if (execvp(file, cmd_args) == -1) {
89+
perror("Error executing R");
90+
return EXIT_FAILURE;
91+
}
92+
return EXIT_SUCCESS;
4993
}

tests/testthat/test-target-runner-dummy.R

+16-12
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,25 @@ withr::with_output_sink("test-target-runner-dummy.Rout", {
3333
skip_if_not(nzchar(iraceexe), "Not run because 'irace' is not installed")
3434
expect_true(file.exists(iraceexe))
3535
# FIXME: For some reason, this does not generate any output on Windows
36-
output <- expect_silent(system2(iraceexe, "--help", stdout = TRUE, stderr = TRUE))
37-
skip_on_os("windows")
38-
expect_match(paste(collapse="", output),
39-
"irace: An implementation in R of.*called with: --help")
36+
output <- expect_silent(runexe(iraceexe, "--help"))
37+
## cat("irace --help\n")
38+
## print(output)
39+
expected_output <- if (system_os_is_windows()) "^$"
40+
else "irace: An implementation in R.*called with: --help"
41+
expect_match(paste(collapse="", output), expected_output)
4042
})
4143

4244
test_that("ablation exe works", {
4345
ablationexe <- get_executable("ablation")
4446
skip_if_not(nzchar(ablationexe), "Not run because 'ablation' is not installed")
4547
expect_true(file.exists(ablationexe))
4648
# FIXME: For some reason, this does not generate any output on Windows
47-
output <- expect_silent(system2(ablationexe, "--help", stdout = TRUE, stderr = TRUE))
48-
skip_on_os("windows")
49-
expect_match(paste(collapse="", output),
50-
"ablation: An implementation in R of Ablation Analysis.*called with: --help")
49+
output <- expect_silent(runexe(ablationexe, "--help"))
50+
## cat("ablation --help\n")
51+
## print(output)
52+
expected_output <- if (system_os_is_windows()) "^$"
53+
else "ablation: An implementation in R of Ablation Analysis.*called with: --help"
54+
expect_match(paste(collapse="", output), expected_output)
5155
})
5256

5357
run_cmdline <- function(parameters, args) {
@@ -65,7 +69,7 @@ withr::with_output_sink("test-target-runner-dummy.Rout", {
6569
target_runner_dummy <- get_executable("target-runner-dummy")
6670
skip_if_not(nzchar(target_runner_dummy), "Not run because 'target-runner-dummy' is not installed")
6771
expect_true(file.exists(target_runner_dummy))
68-
72+
6973
test_that("--check", {
7074
expect_warning(
7175
run_cmdline(paste0('p_int "--p_int " i (1, 10)\n',
@@ -91,7 +95,7 @@ withr::with_output_sink("test-target-runner-dummy.Rout", {
9195
'capping "--opt-time " c (1)\n'),
9296
'--max-time 1000 --bound-max 100 --capping 1'),
9397
"is too large"),
94-
"No scenario file given")
98+
"No scenario file given")
9599
})
96100
test_that("--capping", {
97101
expect_warning(
@@ -120,7 +124,7 @@ withr::with_output_sink("test-target-runner-dummy.Rout", {
120124
'capping "--opt-time " c (1)\n'),
121125
'--max-time 500 --bound-max 1 --capping 1 '),
122126
"With the current settings and estimated time per run"),
123-
"No scenario file given")
127+
"No scenario file given")
124128
})
125129
test_that("Error cost time", {
126130
expect_error(
@@ -151,5 +155,5 @@ withr::with_output_sink("test-target-runner-dummy.Rout", {
151155
'--min-experiments 50'),
152156
"No scenario file given"))
153157
})
154-
158+
155159
}) # withr::with_output_sink()

0 commit comments

Comments
 (0)