Skip to content

Commit 0f20982

Browse files
Preamble: continue if .pex is outside a Please repo (#293)
`get_plz_bin_path` is too aggressive about erroring if the preamble configuration contains an interpreter path prepended with `$PLZ_BIN_PATH` and `argv[0]` is located outside a Please repo - this shouldn't be an error case if there are other, valid interpreters in the list that could be executed. Don't immediately emit a fatal error if `$PLZ_BIN_PATH` can't be resolved - instead, just omit interpreter paths prepended with `$PLZ_BIN_PATH` from the search path and continue, only erroring if no interpreter paths could be resolved.
1 parent fe6ff98 commit 0f20982

File tree

2 files changed

+31
-10
lines changed

2 files changed

+31
-10
lines changed

tools/please_pex/preamble/path.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ err_t *get_pex_path(char **pex_path) {
7979
* directory (which is assumed to be the value of TMP_DIR in the environment, as created by Please).
8080
* Otherwise, get_plz_bin_path returns the path to the bin/ subdirectory within the plz-out/ directory
8181
* to which this .pex file belongs. If the .pex file does not exist within a plz-out/ directory
82-
* tree, get_plz_bin_path fails.
82+
* tree, get_plz_bin_path stores NULL in path.
8383
*/
8484
err_t *get_plz_bin_path(char **path) {
8585
char *pex_path = NULL;
@@ -90,6 +90,8 @@ err_t *get_plz_bin_path(char **path) {
9090
size_t tmp_dir_len = 0;
9191
err_t *err = NULL;
9292

93+
(*path) = NULL;
94+
9395
if ((err = get_pex_path(&pex_path)) != NULL) {
9496
err = err_wrap("get_pex_path", err);
9597
goto end;
@@ -141,8 +143,7 @@ err_t *get_plz_bin_path(char **path) {
141143
}
142144
if (STREQ(pex_dir, "/") || STREQ(pex_dir, ".")) {
143145
// If we reached the left-most component in the path, the .pex file doesn't exist within a
144-
// plz-out/ directory tree.
145-
err = err_from_str(".pex file is in neither a Please build environment nor a Please repo");
146+
// plz-out/ directory tree - return NULL.
146147
goto end;
147148
}
148149
MALLOC(*path, char, strlen(pex_dir) + strlen("/bin") + 1);

tools/please_pex/preamble/preamble.c

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <errno.h>
2+
#include <stdbool.h>
23
#include <unistd.h>
34

45
#include "cJSON.h"
@@ -211,9 +212,11 @@ static err_t *get_interpreter_args(const cJSON *config, int *len, strlist_t **ar
211212
*/
212213
static err_t *get_interpreters(const cJSON *config, strlist_t **interps) {
213214
strlist_elem_t *interp = NULL;
215+
bool tried_plz_bin_path = false;
214216
char *plz_bin_path = NULL;
215217
const cJSON *json_interps = NULL;
216218
const cJSON *json_interp = NULL;
219+
int added = 0;
217220
err_t *err = NULL;
218221

219222
json_interps = cJSON_GetObjectItemCaseSensitive(config, "interpreters");
@@ -235,30 +238,47 @@ static err_t *get_interpreters(const cJSON *config, strlist_t **interps) {
235238
goto end;
236239
}
237240

238-
STAILQ_ENTRY_NEW(interp, strlist_elem_t);
239-
240241
if (STRPREFIX(json_interp->valuestring, "$PLZ_BIN_PATH/")) {
241-
if (plz_bin_path == NULL) {
242+
if (!tried_plz_bin_path) {
243+
tried_plz_bin_path = true;
244+
242245
if ((err = get_plz_bin_path(&plz_bin_path)) != NULL) {
243-
err = err_wrap("PLZ_BIN_PATH resolution failure", err);
246+
err = err_wrap("$PLZ_BIN_PATH resolution failure", err);
244247
goto end;
245248
}
246-
log_debug("Resolved $PLZ_BIN_PATH to %s", plz_bin_path);
249+
250+
if (plz_bin_path == NULL) {
251+
log_warn(".pex file is not inside a Please repo; omitting interpreters prepended with '$PLZ_BIN_PATH'");
252+
} else {
253+
log_debug("Resolved $PLZ_BIN_PATH to %s", plz_bin_path);
254+
}
255+
}
256+
257+
if (plz_bin_path == NULL) {
258+
log_info("Omitting interpreter from search path: %s", json_interp->valuestring);
259+
continue;
247260
}
248261

249262
// Replace "$PLZ_BIN_PATH" with the path to Please's binary outputs directory at the
250263
// start of the interpreter path.
264+
STAILQ_ENTRY_NEW(interp, strlist_elem_t);
251265
MALLOC(interp->str, char, strlen(plz_bin_path) + JSON_STRLEN(json_interp) - strlen("$PLZ_BIN_PATH") + 1); // 1 extra byte for trailing null
252266
strncpy(interp->str, plz_bin_path, strlen(plz_bin_path));
253267
strcpy(interp->str + strlen(plz_bin_path), json_interp->valuestring + strlen("$PLZ_BIN_PATH"));
268+
STAILQ_INSERT_TAIL(*interps, interp, next);
254269
} else {
270+
STAILQ_ENTRY_NEW(interp, strlist_elem_t);
255271
MALLOC(interp->str, char, JSON_STRLEN(json_interp) + 1); // 1 extra byte for trailing null
256272
strcpy(interp->str, json_interp->valuestring);
273+
STAILQ_INSERT_TAIL(*interps, interp, next);
257274
}
258275

259-
STAILQ_INSERT_TAIL(*interps, interp, next);
260-
261276
log_debug("Added interpreter to search path: %s", interp->str);
277+
added++;
278+
}
279+
280+
if (added == 0) {
281+
err = err_from_str("interpreters list contains no resolvable paths");
262282
}
263283

264284
end:

0 commit comments

Comments
 (0)