Author: Andy Green Date: Sat Oct 03 15:24:08 2026 +0100 builder: let platforms set up the env their builds see Builds don't get the builder's own environment, but a fixed base set, so there was no way to put eg a toolchain on the PATH short of the build scripts knowing about the builder. The conf has long parsed a platform "env" array, but only logged it. Make it real: each item, either "NAME=value", { "NAME": "value" }, or a bare "NAME" to pass on the builder's own value of NAME, is applied in order on top of the base set for that platform's step scripts and sai-shells. $NAME and ${NAME} are expanded by sai-builder itself, against the base set and the items before, so it's the same on every platform, eg "PATH=/opt/cross/bin:$PATH". On Windows the base set stays the builder's own environment, which the child needs to find the toolchain, now copied so the conf items can be applied to it. The unix step scripts no longer set PATH themselves: on macOS that replaced the whole PATH, which would have discarded the conf's. The macOS default PATH moves into the base set instead. Values may be secrets, so only names are logged, and they are not sent to sai-server. Co-Authored-By: Claude Opus 5.5 diff --git a/README.md b/README.md index 784c509..9d5211c 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,13 @@ See [READMEs/README-sai-push.md](READMEs/README-sai-push.md) for how it decides what to push, setting it up, and all of its conf options with an example. +## The environment builds run in + +Builds don't see the builder's own environment, but a small base set, which +each platform in the builder conf can add to with an `env` array, eg to put a +toolchain on the `PATH`. See +[READMEs/README-builder-env.md](READMEs/README-builder-env.md). + ## Build flow and support for embedded ![build flow](READMEs/sai-build-test-flow.png) diff --git a/READMEs/README-builder-env.md b/READMEs/README-builder-env.md new file mode 100644 index 0000000..12c402b --- /dev/null +++ b/READMEs/README-builder-env.md @@ -0,0 +1,66 @@ +# The environment builds run in + +The processes sai-builder starts for a task (the step scripts, and the +sai-shell a viewer can open on a task) do not see the builder's own +environment. Instead they start with a small base set: + +|Platform|Base environment| +|---|---| +|Linux, BSDs|`PATH=/usr/local/bin:/usr/bin:/bin`, `LANG=en_US.UTF-8`, `TERM=xterm-256color`| +|macOS|as above, but `PATH=/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/sbin:/usr/sbin`| +|Windows|the builder's own environment, since without `SystemRoot` and the Visual Studio variables a child can't even find `cl` or `nmake`| + +The step scripts then add sai's own variables on top, like `HOME`, `CI`, +`SAI_PROJECT`, `SAI_PARALLEL`, `SAI_LOGPROXY` etc; those always have sai's +values. + +## Adding to it from the builder conf + +Each platform in the builder conf can have an `env` array, applied in order +on top of the base set, to everything started for that platform's tasks. An +item is either a string, or an object of one or more names and values: + +``` + "platforms": [ + { + "name": "linux-debian13/x86_64-amd/gcc", + "env": [ + "PATH=/opt/cross/bin:$PATH", + "PKG_CONFIG_PATH=/opt/cross/lib/pkgconfig", + { "SAI_ARCH": "x86_64", "SAI_CROSS_BASE": "/opt/cross" }, + "CCACHE_DIR=/var/cache/ccache-${SAI_ARCH}", + "http_proxy", + "https_proxy" + ], + "servers": [ "wss://libwebsockets.org:4444/sai/builder" ] + } + ] +``` + + - `"NAME=value"`, or `{ "NAME": "value" }`, sets NAME, replacing any value + it already had. + + - A bare `"NAME"` passes on the builder's own value of NAME, if it has one + (and does nothing if not). That's the way to opt in to specific things + from the builder's environment, like proxy settings, without passing all + of it. The value is used as it is, with no expansion. On Windows this is + a no-op, as the base set is already the builder's environment. + + - `$NAME` and `${NAME}` in a value are replaced with the value NAME has at + that point, ie, from the base set plus the items before this one. So + `"PATH=/opt/cross/bin:$PATH"` prepends to the base PATH, and later items + can build on earlier ones. sai's own variables like `HOME` aren't set + yet, so they can't be used here. A NAME that isn't set expands to nothing. `$$` + is a literal `$`, as is a `$` that isn't followed by a name. The `${NAME}` + form allows any characters in the name apart from `}` and `=`, eg + `${ProgramFiles(x86)}` on Windows. + +The expansion is done by sai-builder itself, not a shell, so it's the same on +every platform, including Windows (where `%NAME%` is not expanded, and the +`PATH` separator is still `;`, eg `"PATH=C:\\tools;$PATH"`). Windows names +are matched without regard to case, so `PATH` there replaces `Path`. + +The values may be secrets, eg a token for a private package registry, so +sai-builder only logs the names, not the values. They are not sent to +sai-server. But of course, anything the build does may show them in the +task's logs, which may be public. diff --git a/etc-sai-EXAMPLE/builder/conf b/etc-sai-EXAMPLE/builder/conf index 38d1461..4f68367 100644 --- a/etc-sai-EXAMPLE/builder/conf +++ b/etc-sai-EXAMPLE/builder/conf @@ -37,7 +37,11 @@ { "name": "freertos-linkit/arm32-m4-mt7697-usi/gcc", "instances": 1, + + # optional: set up the environment the platform's builds + # see, see READMEs/README-builder-env.md "env": [ + "PATH=/opt/linkit/bin:$PATH", { "SAI_CROSS_BASE": "/opt/linkit" } ], "servers": [ "wss://libwebsockets.org:4444/sai/builder" ] diff --git a/src/builder/CMakeLists.txt b/src/builder/CMakeLists.txt index e542037..67c80d9 100644 --- a/src/builder/CMakeLists.txt +++ b/src/builder/CMakeLists.txt @@ -10,6 +10,7 @@ set(SRCS b-task.c b-artifacts.c b-pool.c + b-env.c b-logproxy.c b-refproxy.c b-load.c diff --git a/src/builder/b-conf.c b/src/builder/b-conf.c index 7b03fcb..730a67e 100644 --- a/src/builder/b-conf.c +++ b/src/builder/b-conf.c @@ -114,7 +114,7 @@ saib_conf_cb(struct lejp_ctx *ctx, char reason) struct jpargs *a = (struct jpargs *)ctx->user; sai_plat_server_ref_t *mref; struct lws_ss_handle *h; - const char **pp; + const char **pp, *eq; char temp[65]; int n; @@ -201,9 +201,21 @@ saib_conf_cb(struct lejp_ctx *ctx, char reason) switch (ctx->path_match - 1) { case LEJPM_PLATFORMS_ENV: - lwsl_notice("env %s %s\n", ctx->path, ctx->buf); - return 0; - // break; + /* "NAME=value", or just "NAME" to pass on the builder's own */ + eq = strchr(ctx->su.fp, '='); + + return saib_env_add(a->sai_plat, &a->builder->conf_head, + ctx->su.fp, + eq ? lws_ptr_diff_size_t(eq, ctx->su.fp) : + strlen(ctx->su.fp), + eq ? eq + 1 : NULL) ? -1 : 0; + + case LEJPM_PLATFORMS_ENV_ITEM: + /* { "NAME": "value" }, the wildcard is the NAME part */ + return saib_env_add(a->sai_plat, &a->builder->conf_head, + ctx->path + ctx->wild[0], + strlen(ctx->path + ctx->wild[0]), + ctx->su.fp) ? -1 : 0; case LEJPM_PLATFORMS_NAME: n = lws_snprintf(temp, sizeof(temp), "%s.%.*s", diff --git a/src/builder/b-env.c b/src/builder/b-env.c new file mode 100644 index 0000000..e4d42d6 --- /dev/null +++ b/src/builder/b-env.c @@ -0,0 +1,296 @@ +/* + * sai-builder env.c + * + * Copyright (C) 2019 - 2026 Andy Green + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation: + * version 2.1 of the License. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + * + * The environment the builder's children (step scripts, sai-shell) start + * with. On unix that's a small fixed set, not the builder's own + * environment; on Windows the child can't even find the toolchain without + * SystemRoot and the Visual Studio vars, so it starts from ours. + * + * On top of that, the platform's "env" items from the builder conf are + * applied in order, see READMEs/README-builder-env.md + */ + +#include +#include +#include + +#include "b-private.h" + +#if !defined(WIN32) +static const char * const env_base[] = { +#if defined(__APPLE__) + "PATH=/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/sbin:/usr/sbin", +#else + "PATH=/usr/local/bin:/usr/bin:/bin", +#endif + "LANG=en_US.UTF-8", + "TERM=xterm-256color", +}; +#endif + +/* + * Add a conf env item to the platform. value NULL means pass on the + * builder's own value of name, if it has one. value must already be in + * the conf lwsac, name is copied into it. + */ + +int +saib_env_add(sai_plat_t *sp, struct lwsac **ac, const char *name, + size_t nlen, const char *value) +{ + saib_env_t *e; + char *p; + + if (!nlen || memchr(name, '=', nlen)) { + lwsl_err("%s: invalid env var name '%.*s'\n", __func__, + (int)nlen, name); + return 1; + } + + e = lwsac_use_zero(ac, sizeof(*e), 512); + p = lwsac_use(ac, nlen + 1, 512); + if (!e || !p) + return 1; + + memcpy(p, name, nlen); + p[nlen] = '\0'; + e->name = p; + e->value = value; + + lws_dll2_add_tail(&e->list, &sp->env_head); + + /* the value may be a secret, so we don't log it */ + lwsl_notice("%s: env %s%s\n", __func__, e->name, + value ? "" : " (from builder)"); + + return 0; +} + +static int +env_name_match(const char *e, const char *name, size_t nlen) +{ + size_t n; + + for (n = 0; n < nlen; n++) { +#if defined(WIN32) + /* Windows env var names are case-insensitive */ + char c1 = e[n], c2 = name[n]; + + if (c1 >= 'A' && c1 <= 'Z') + c1 = (char)(c1 + ('a' - 'A')); + if (c2 >= 'A' && c2 <= 'Z') + c2 = (char)(c2 + ('a' - 'A')); + if (!c1 || c1 != c2) + return 0; +#else + if (!e[n] || e[n] != name[n]) + return 0; +#endif + } + + return e[nlen] == '='; +} + +static int +env_find(const char **env, int count, const char *name, size_t nlen) +{ + int n; + + for (n = 0; n < count; n++) + if (env_name_match(env[n], name, nlen)) + return n; + + return -1; +} + +static int +env_name_char(char c, int first) +{ + return c == '_' || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || + (!first && c >= '0' && c <= '9'); +} + +/* + * Expand $NAME and ${NAME} in "in" against the env being built. $$ is a + * literal $, and so is a $ not followed by a name. An unset var expands to + * nothing. With out NULL, it just measures. Returns the expanded length, + * not including the terminating NUL that's appended when out is given. + */ + +static size_t +env_expand(const char **env, int count, const char *in, char *out) +{ + const char *s, *v; + size_t len = 0, nl, vl; + int brace, n; + + while (*in) { + if (*in != '$' || in[1] == '$') { + if (out) + out[len] = *in; + len++; + in += *in == '$' ? 2 : 1; + continue; + } + + brace = in[1] == '{'; + s = in + 1 + brace; + nl = 0; + if (brace) + /* allows Windows names like ProgramFiles(x86) */ + while (s[nl] && s[nl] != '}' && s[nl] != '=') + nl++; + else + while (env_name_char(s[nl], !nl)) + nl++; + + if (!nl || (brace && s[nl] != '}')) { + /* not a reference, the $ is literal */ + if (out) + out[len] = '$'; + len++; + in++; + continue; + } + + in = s + nl + brace; + + n = env_find(env, count, s, nl); + if (n < 0) + continue; + + v = env[n] + nl + 1; + vl = strlen(v); + if (out) + memcpy(out + len, v, vl); + len += vl; + } + + if (out) + out[len] = '\0'; + + return len; +} + +/* + * Prepare the environment for a child of platform sp (which may be NULL if + * we don't know the platform, then it's just the base set) in ac. Returns a + * NULL-terminated array of "NAME=value" for lws_spawn_piped_info.env_array, + * or NULL on OOM. It only needs to live until lws_spawn_piped() returns. + */ + +const char ** +saib_env_build(const sai_plat_t *sp, struct lwsac **ac) +{ + int count = 0, max; + const char **env; +#if defined(WIN32) + LPCH blk = GetEnvironmentStringsA(); + const char *b; + + if (!blk) + return NULL; + + for (b = blk; *b; b += strlen(b) + 1) + count++; + max = count; +#else + max = (int)LWS_ARRAY_SIZE(env_base); +#endif + + max += (int)(sp ? sp->env_head.count : 0) + 1; + + env = lwsac_use(ac, sizeof(*env) * (size_t)max, 512); + if (!env) + goto bail; + +#if defined(WIN32) + count = 0; + for (b = blk; *b; b += strlen(b) + 1) { + size_t l = strlen(b) + 1; + char *p = lwsac_use(ac, l, 4096); + + if (!p) + goto bail; + memcpy(p, b, l); + env[count++] = p; + } + FreeEnvironmentStringsA(blk); + blk = NULL; +#else + for (count = 0; count < (int)LWS_ARRAY_SIZE(env_base); count++) + env[count] = env_base[count]; +#endif + + if (sp) { + lws_start_foreach_dll(struct lws_dll2 *, d, sp->env_head.head) { + const saib_env_t *e = lws_container_of(d, saib_env_t, + list); + size_t nl = strlen(e->name), vl; + const char *v = e->value; + char *p; + int n; + + if (!v) { +#if defined(WIN32) + /* we started from our own env already */ + continue; +#else + v = getenv(e->name); + if (!v) + continue; + /* the builder's own value is used literally */ + vl = strlen(v); +#endif + } else + vl = env_expand(env, count, v, NULL); + + p = lwsac_use(ac, nl + 1 + vl + 1, 512); + if (!p) + goto bail; + + memcpy(p, e->name, nl); + p[nl] = '='; + if (e->value) + env_expand(env, count, v, p + nl + 1); + else + memcpy(p + nl + 1, v, vl + 1); + + n = env_find(env, count, e->name, nl); + if (n < 0) + n = count++; + env[n] = p; + + } lws_end_foreach_dll(d); + } + + env[count] = NULL; + + return env; + +bail: +#if defined(WIN32) + if (blk) + FreeEnvironmentStringsA(blk); +#endif + lwsac_free(ac); + + return NULL; +} diff --git a/src/builder/b-nspawn.c b/src/builder/b-nspawn.c index 842aa82..f623e40 100644 --- a/src/builder/b-nspawn.c +++ b/src/builder/b-nspawn.c @@ -641,11 +641,7 @@ static const char * const runscript_win_next = static const char * const runscript_first = "#!/usr/bin/env bash\n" /* use -x to see what it does for these */ -#if defined(__APPLE__) - "export PATH=/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/sbin:/usr/sbin\n" -#else - "export PATH=/usr/local/bin:$PATH\n" -#endif + /* PATH comes from saib_env_build(), with any conf "env" applied */ "export HOME=%s\n" "export SAI_OVN=%s\n" "export SAI_VN=%s\n" @@ -670,11 +666,7 @@ static const char * const runscript_first = static const char * const runscript_next = "#!/usr/bin/env bash\n" /* use -x to see what it does for these */ -#if defined(__APPLE__) - "export PATH=/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/sbin:/usr/sbin\n" -#else - "export PATH=/usr/local/bin:$PATH\n" -#endif + /* PATH comes from saib_env_build(), with any conf "env" applied */ "export HOME=%s\n" "export SAI_OVN=%s\n" "export SAI_VN=%s\n" @@ -698,11 +690,7 @@ static const char * const runscript_next = static const char * const runscript_build = "#!/usr/bin/env bash\n" /* use -x to see what it does for these */ -#if defined(__APPLE__) - "export PATH=/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/sbin:/usr/sbin\n" -#else - "export PATH=/usr/local/bin:$PATH\n" -#endif + /* PATH comes from saib_env_build(), with any conf "env" applied */ "export HOME=%s\n" "export SAI_OVN=%s\n" "export SAI_VN=%s\n" @@ -731,6 +719,7 @@ saib_spawn_script(struct sai_nspawn *ns) { struct lws_spawn_piped_info info; struct saib_opaque_spawn *op; + struct lwsac *ac_env = NULL; #if !defined(WIN32) const char *script_template; #endif @@ -739,14 +728,6 @@ saib_spawn_script(struct sai_nspawn *ns) "/bin/ps", NULL }; -#if !defined(WIN32) - const char *env[] = { - "PATH=/usr/local/bin:/usr/bin:/bin", - "LANG=en_US.UTF-8", - "TERM=xterm-256color", - NULL - }; -#endif char one_step[4096], idle_env[64], pool_env[1024]; char st[8192]; unsigned int timeout_secs; @@ -878,18 +859,6 @@ saib_spawn_script(struct sai_nspawn *ns) memset(&info, 0, sizeof(info)); info.vh = builder.vhost; -#if !defined(WIN32) - info.env_array = (const char **)env; -#else - /* - * Since lws C-328 (f92e831dd) the Windows spawn honours env_array as - * the child's entire environment, as execve does; before it was - * ignored and the child inherited ours. The sanitizing set above is - * a unix PATH with no SystemRoot or Visual Studio variables, so a - * child given it cannot even find nmake or cl. Inherit instead. - */ - info.env_array = NULL; -#endif info.exec_array = cmd; info.protocol_name = "sai-stdxxx"; info.max_log_lines = 10000; @@ -908,9 +877,19 @@ saib_spawn_script(struct sai_nspawn *ns) info.p_cgroup_ret = &in_cgroup; #endif + /* the base set plus the platform's conf "env", see b-env.c */ + info.env_array = saib_env_build(ns->sp, &ac_env); + if (!info.env_array) { + saib_task_logf(ns->spm, ns, NULL, + "Unable to prepare the step environment: OOM"); + return 1; + } + op = malloc(sizeof(*op)); - if (!op) + if (!op) { + lwsac_free(&ac_env); return 1; + } memset(op, 0, sizeof(*op)); op->ns = ns; @@ -929,6 +908,7 @@ saib_spawn_script(struct sai_nspawn *ns) lwsl_user("%s: calling lws_spawn_piped for task uuid %s\n", __func__, ns->task->uuid); lws_spawn_piped(&info); + lwsac_free(&ac_env); /* the child has its copy by now */ if (!op->lsp) { saib_task_logf(ns->spm, ns, NULL, "Unable to spawn the step process (errno %d (%s)): " @@ -1070,16 +1050,25 @@ int saib_shell_spawn(struct sai_plat_server *spm, const char *task_uuid) { struct lws_spawn_piped_info info; + const sai_plat_t *tsp = NULL; + struct lwsac *ac_env = NULL; struct sai_shell *sh; const char *cmd[] = { "/bin/bash", "-i", NULL }; -#if !defined(WIN32) - const char *env[] = { - "PATH=/usr/local/bin:/usr/bin:/bin", - "LANG=en_US.UTF-8", - "TERM=xterm-256color", - NULL - }; -#endif + + /* the shell sees the same env as the task's platform gives its steps */ + + lws_start_foreach_dll(struct lws_dll2 *, d, builder.sai_plat_owner.head) { + const sai_plat_t *sp = lws_container_of(d, sai_plat_t, + sai_plat_list); + + lws_start_foreach_dll(struct lws_dll2 *, d1, sp->nspawn_owner.head) { + const struct sai_nspawn *ns = lws_container_of(d1, + struct sai_nspawn, list); + + if (ns->task && !strcmp(ns->task->uuid, task_uuid)) + tsp = sp; + } lws_end_foreach_dll(d1); + } lws_end_foreach_dll(d); sh = malloc(sizeof(*sh)); if (!sh) @@ -1091,18 +1080,6 @@ saib_shell_spawn(struct sai_plat_server *spm, const char *task_uuid) memset(&info, 0, sizeof(info)); info.vh = builder.vhost; -#if !defined(WIN32) - info.env_array = (const char **)env; -#else - /* - * Since lws C-328 (f92e831dd) the Windows spawn honours env_array as - * the child's entire environment, as execve does; before it was - * ignored and the child inherited ours. The sanitizing set above is - * a unix PATH with no SystemRoot or Visual Studio variables, so a - * child given it cannot even find nmake or cl. Inherit instead. - */ - info.env_array = NULL; -#endif info.exec_array = cmd; info.protocol_name = "sai-saishell"; info.max_log_lines = 10000; @@ -1113,10 +1090,16 @@ saib_shell_spawn(struct sai_plat_server *spm, const char *task_uuid) info.opaque = sh; info.owner = &builder.lsp_owner; info.plsp = &sh->lsp; + info.env_array = saib_env_build(tsp, &ac_env); + if (!info.env_array) { + free(sh); + return 1; + } lws_dll2_add_tail(&sh->list, &builder.shell_owner); lws_spawn_piped(&info); + lwsac_free(&ac_env); if (!sh->lsp) { lwsl_err("%s: Failed to spawn shell for %s\n", __func__, task_uuid); lws_dll2_remove(&sh->list); diff --git a/src/builder/b-private.h b/src/builder/b-private.h index 4f827cb..d6d8cac 100644 --- a/src/builder/b-private.h +++ b/src/builder/b-private.h @@ -69,6 +69,17 @@ extern char suspender_exists; struct lws_spawn_piped; struct lws_stub_manager; +/* + * One "env" item from a platform in the builder conf, in the conf lwsac and + * listed on the sai_plat .env_head, in conf order + */ + +typedef struct saib_env { + lws_dll2_t list; + const char *name; + const char *value; /* NULL = the builder's own value */ +} saib_env_t; + struct saib_opaque_spawn { struct sai_nspawn *ns; struct lws_spawn_piped *lsp; @@ -488,6 +499,13 @@ saib_pool_env(struct sai_nspawn *ns, char *buf, size_t len); int saib_pool_busy(void); +int +saib_env_add(sai_plat_t *sp, struct lwsac **ac, const char *name, + size_t nlen, const char *value); + +const char ** +saib_env_build(const sai_plat_t *sp, struct lwsac **ac); + void saib_pool_destroy_all(void);