From 50430fa2ad2e93165efcd0f4339e592d197e78c5 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Tue, 29 Sep 2026 23:08:08 +0800 Subject: [PATCH 1/2] feat(executor): send a hook's child-process output to a log file, and read build_dep by the name xlings exports (0.0.60) hook_log (ExecutionContext) A hook's own print / io.write / io.stderr:write have always been captured into HookResult::output. The child processes it starts with os.execute (system.exec, os.exec, system.run_in_script and a bare os.execute all end there) inherit fd 1 and 2, so their output went straight to the caller's terminal -- or into a protocol the caller keeps on stdout. Measured in xlings: 7-Zip's banner and an ERROR line in the middle of the [n/88] install report. ExecutionContext::hook_log is a path. Empty is byte-for-byte the behaviour of 0.0.59. Non-empty, run_hook() truncates the file (creating its directory), writes "# hook of @", appends the hook's own output to it, and runs every os.execute child with fd 1 and 2 on it, so the file reads in the order things happened. HookResult::output becomes the last kMaxHookOutputBytes of that file, through the same UTF-8 cleanup and truncation marker as before. The redirect is made at process level, not by rewriting the command: appending ">> log 2>&1" only redirects the last command of "a && b", and putting anything in front of a Windows command changes cmd's /c quote rule, which pkgs/m/msvc.lua relies on. POSIX: posix_spawn("/bin/sh", "-c", cmd) with adddup2. Windows: CreateProcessA(COMSPEC, " /c ") with STARTF_USESTDHANDLES -- the command line UCRT system() builds, narrow API included -- and an inheritable FILE_APPEND_DATA handle. The override returns what Lua 5.4's os.execute returns (true|nil, "exit"|"signal", code), so `ret == 0 or ret == true` keeps working. It exists only for the length of run_hook(); run_script() and apply_elfpatch_auto() are untouched. A log that cannot be created or opened never fails the hook: children get /dev/null (NUL), one warning goes through the log module, and the ring buffer stands in for the file. system.exec(cmd, { tty = true }) and system.run_in_script(script, true) keep inheriting the terminal, through the original os.execute exposed only as _LIBXPKG_EXEC_INHERIT. A libxpkg that predates the option ignores `tty` and inherits anyway, so recipes can pass it unconditionally. Known difference from system(): the wait does not ignore SIGINT/SIGQUIT. pkginfo.build_dep xlings exports XLINGS_BUILDDEP__PATH with the namespace and @version dropped first. build_dep("xim:7zip") upper-cased the whole string, looked up XLINGS_BUILDDEP_XIM_7ZIP_PATH, missed, fell back to dep_install_dir (which has no record of build deps) and returned nil -- which is why dpcpp.lua retries with the bare name. It now spells the name the way xlings does, so "xim:7zip", "7zip" and "xim:7zip@26.02" read the same variable. Tests HookLog_* in test_executor.cpp assert, with CaptureStdout/CaptureStderr (the children inherit exactly those descriptors): child stdout+stderr and print land in the log in order and in HookResult::output and not on the process's streams; an empty hook_log leaves the children on the inherited streams, before and after a logged run on the same executor; the os.execute triples equal the native ones for exit 0, exit 3, no argument, and (POSIX) a signal and a wrapped status; tty bypasses the log; run_in_script is logged; an unopenable log warns once and the hook still succeeds; every run starts a fresh file; the output is the tail of a file that keeps everything; run_script is not intercepted. PkgInfo_BuildDepReadsTheVariableXlingsExports covers the three spellings plus a hyphenated name. The Windows branch was also extracted and run under Wine (mingw-w64) against system() for the cmd /c quoting cases, the null device, stdin inheritance, an unopenable path and a non-ASCII directory. --- docs/architecture.md | 30 ++ mcpp.toml | 2 +- src/lua-stdlib/xim/libxpkg/pkginfo.lua | 18 +- src/lua-stdlib/xim/libxpkg/system.lua | 19 +- src/xpkg-executor.cppm | 453 ++++++++++++++++++++++++- tests/test_executor.cpp | 436 ++++++++++++++++++++++++ 6 files changed, 948 insertions(+), 10 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index a0ee2ff..d650f20 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -110,6 +110,36 @@ graph TD `elfpatch.set{ scan = {...} }` 把扫描范围收窄到列出的路径（同样是相对安装目录的路径，不支持通配符）。 +#### hook 子进程的输出：`ExecutionContext::hook_log`（0.0.60） + +hook 里的 `print` / `io.write` / `io.stderr:write` 一直被 `run_hook` 捕获进 `HookResult::output`（末尾 16 KB）。但 hook 用 `os.execute`（`system.exec`、`os.exec`、`system.run_in_script` 最终都走它）启动的**子进程**继承本进程的 fd 1/2，输出会直接写到调用方的终端，或者调用方放在 stdout 上的协议流里。`hook_log` 让调用方决定这些输出去哪。 + +| `hook_log` | 行为 | +|------------|------| +| 空（默认） | 与 0.0.59 完全一致：子进程继承 fd 1/2，`print` 等只进环形缓冲区。其他使用 libxpkg 的程序不受影响 | +| 非空 | `run_hook` 开始时清空并创建这个文件（目录不存在则创建），写第一行 `# hook of @`；hook 运行期间，`print` / `io.write` / `io.stderr:write` 与 `os.execute` 的子进程输出**按发生顺序**写进同一个文件；结束时 `HookResult::output` = 文件末尾 16 KB（沿用 UTF-8 清洗与截断标记） | + +实现约束： + +- **只在 `run_hook` 里接管 `os.execute`**，返回时恢复。`run_script`、`apply_elfpatch_auto` 不受影响；`io.popen` / `os.iorun` 本来就捕获输出，也不改。 +- **命令字符串原样交给 shell，不拼接重定向。** 在末尾拼 `>> log 2>&1` 对 `a && b` 只重定向最后一条；在 Windows 上，在命令前加任何字符都会改变 cmd `/c` 的去引号规则，而 `pkgs/m/msvc.lua` 依赖这条规则。所以重定向做在进程层面：POSIX 用 `posix_spawn("/bin/sh", "-c", cmd)` + `adddup2`，Windows 用 `CreateProcessA(COMSPEC, " /c ")` + `STARTF_USESTDHANDLES`（命令行与 UCRT `system()` 构造的一致，同样用窄字符 API）。stdin 保持继承。 +- **返回值与 Lua 5.4 的 `os.execute` 一致**：退出码 0 → `true, "exit", 0`；退出码 n → `nil, "exit", n`；被信号杀死 → `nil, "signal", s`；`os.execute()` 无参仍问“有没有 shell”。`ret == 0 or ret == true` 之类的现有判断不受影响。 +- **日志打不开不让 hook 失败**：子进程的输出去 `/dev/null`（Windows 是 `NUL`），经 `log` 模块警告一次（因此出现在 `HookResult::output` 里），`HookResult::output` 退回环形缓冲区。 +- 与 `system()` 的一个已知差异：等待子进程期间不像 `system()` 那样忽略 SIGINT/SIGQUIT。 + +需要终端的命令（提示、密码、要给用户看的输出）显式退出重定向： + +```lua +system.exec("make menuconfig", { tty = true }) -- 输出直通终端，不进日志 +system.run_in_script(script, true) -- admin（sudo）同理 +``` + +旧版本的 `system.exec` 忽略 `tty`，而它本来就是直通终端，所以 recipe 可以无条件采用。直通用的原始 `os.execute` 只以内部名 `_LIBXPKG_EXEC_INHERIT` 暴露，不是公开的 `os.*` 函数。 + +#### `pkginfo.build_dep` 的名字规范化（0.0.60） + +xlings 导出 `XLINGS_BUILDDEP__PATH` 时先去掉 `@版本`，再去掉命名空间前缀，再把字母数字转大写、其余转 `_`。`build_dep` 现在按同一规则拼变量名，`build_dep("xim:7zip")`、`build_dep("7zip")`、`build_dep("xim:7zip@26.02")` 读的是同一个 `XLINGS_BUILDDEP_7ZIP_PATH`。之前带命名空间的写法查的是 `XLINGS_BUILDDEP_XIM_7ZIP_PATH`，永远查不到，退回 `dep_install_dir`（它不记录 build 依赖），最后返回 nil。查不到变量时仍退回 `dep_install_dir`。 + #### Lua 运行时兼容层 executor 通过 `prelude.lua`（编译时嵌入 `xpkg-lua-stdlib.cppm`）为包脚本提供运行环境。 diff --git a/mcpp.toml b/mcpp.toml index 2209868..c53c6fc 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] namespace = "mcpplibs" name = "xpkg" -version = "0.0.59" +version = "0.0.60" description = "C++23 reference implementation of the xpkg V2 spec (multi-arch)" license = "Apache-2.0" repo = "https://github.com/openxlings/libxpkg" diff --git a/src/lua-stdlib/xim/libxpkg/pkginfo.lua b/src/lua-stdlib/xim/libxpkg/pkginfo.lua index 2bab65f..9aeddd9 100644 --- a/src/lua-stdlib/xim/libxpkg/pkginfo.lua +++ b/src/lua-stdlib/xim/libxpkg/pkginfo.lua @@ -504,7 +504,10 @@ end -- Resolution order: -- 1. Env var XLINGS_BUILDDEP__PATH (injected by the -- xlings installer when the consumer's `build` deps were resolved --- to a concrete version). +-- to a concrete version). is the dep's bare name -- +-- namespace and @version dropped, then upper-cased with every +-- non-alphanumeric mapped to `_` -- exactly as xlings derives it, so +-- "xim:7zip", "7zip" and "xim:7zip@26.02" all read XLINGS_BUILDDEP_7ZIP_PATH. -- 2. Fallback: scan xpkgs the same way `dep_install_dir` does. -- Returns highest available version when version is omitted. -- @@ -513,8 +516,17 @@ function M.build_dep(dep_name, dep_version) local log = _get_log() if not dep_name or dep_name == "" then return nil end - local function _upper(s) return (s:gsub("[^%w]", "_")):upper() end - local env_key = "XLINGS_BUILDDEP_" .. _upper(dep_name) .. "_PATH" + -- Must spell the name the way xlings does when it exports the variable + -- (installer.cpp): drop from the first '@', then up to and including the + -- first ':', then upper-case alphanumerics and map the rest to '_'. + local function _env_name(s) + local at = s:find("@", 1, true) + if at then s = s:sub(1, at - 1) end + local colon = s:find(":", 1, true) + if colon then s = s:sub(colon + 1) end + return (s:gsub("[^%w]", "_")):upper() + end + local env_key = "XLINGS_BUILDDEP_" .. _env_name(dep_name) .. "_PATH" local env_path = os.getenv(env_key) local install_dir if env_path and env_path ~= "" and os.isdir(env_path) then diff --git a/src/lua-stdlib/xim/libxpkg/system.lua b/src/lua-stdlib/xim/libxpkg/system.lua index 54ea6e1..b2af645 100644 --- a/src/lua-stdlib/xim/libxpkg/system.lua +++ b/src/lua-stdlib/xim/libxpkg/system.lua @@ -1,12 +1,25 @@ -- xim.libxpkg.system: system operations API local M = {} +-- Run a command the way os.execute does. When the host asked for a hook log +-- (ExecutionContext.hook_log) os.execute sends the child's stdout/stderr to +-- that file; `tty` opts one command out, for the few that must reach the +-- terminal (a prompt, a password, output the user has to read). The executor +-- keeps the original os.execute under an internal name; where that name is +-- absent nothing redirects os.execute either, so falling back is the same call. +local function _execute(cmd, tty) + if tty and _LIBXPKG_EXEC_INHERIT then return _LIBXPKG_EXEC_INHERIT(cmd) end + return os.execute(cmd) +end + +-- opt.retry : extra attempts after the first failure +-- opt.tty : true = the command's output is not redirected into the hook log function M.exec(cmd, opt) opt = opt or {} local retries = opt.retry or 0 local attempts = retries + 1 for i = 1, attempts do - local ret = os.execute(cmd) + local ret = _execute(cmd, opt.tty) if ret == 0 or ret == true then return end if i == attempts then error("exec failed after " .. attempts .. " attempt(s): " .. tostring(cmd)) @@ -20,6 +33,8 @@ function M.bindir() return _RUNTIME and _RUNTIME.bin_dir or nil end function M.xpkg_args() return (_RUNTIME and _RUNTIME.args) or {} end function M.subos_sysrootdir() return _RUNTIME and _RUNTIME.subos_sysrootdir or nil end +-- admin = true runs the script under sudo, which prompts on the terminal, so +-- it is never redirected into the hook log. function M.run_in_script(content, admin) local tmpfile = os.tmpname() -- write content to temp file @@ -29,7 +44,7 @@ function M.run_in_script(content, admin) local ok, err = pcall(function() os.execute("chmod +x " .. tmpfile) local prefix = (admin == true) and "sudo " or "" - local ret = os.execute(prefix .. tmpfile) + local ret = _execute(prefix .. tmpfile, admin == true) if ret ~= 0 and ret ~= true then error("script failed with code: " .. tostring(ret)) end diff --git a/src/xpkg-executor.cppm b/src/xpkg-executor.cppm index 696fb8e..be87bbb 100644 --- a/src/xpkg-executor.cppm +++ b/src/xpkg-executor.cppm @@ -1,5 +1,34 @@ module; +// Platform headers for the process layer below (hook output redirection). +// `import std;` does not provide them, and the named-module purview forbids +// including them there. +#if defined(_WIN32) +// windows.h's min/max macros break std::min({...}) in every module that sees +// them, and only Windows ever reports it. Both defines come before the include. +# ifndef NOMINMAX +# define NOMINMAX +# endif +# ifndef WIN32_LEAN_AND_MEAN +# define WIN32_LEAN_AND_MEAN +# endif +# include +#else +# include +# include +# include +# include +# include +# include +# if defined(__APPLE__) +# include +# else +// POSIX leaves this declaration to the application; glibc/musl only declare +// it under _GNU_SOURCE. +extern "C" char** environ; +# endif +#endif + export module mcpplibs.xpkg.executor; import mcpplibs.xpkg; import mcpplibs.xpkg.lua_stdlib; @@ -69,6 +98,30 @@ struct ExecutionContext { DepExport self_exports; std::string subos_sysrootdir; std::string pkgindex_dir; // package index repo root (for custom module loading) + + // Where run_hook() sends what a hook's child processes write (0.0.60). + // + // Empty (the default) is the behaviour of every release before it: children + // started by os.execute / system.exec inherit this process's fd 1 and 2, + // and only the hook's own print / io.write / io.stderr:write are captured + // into HookResult::output. A caller that owns the terminal or a protocol on + // stdout has no way to keep a child's output out of it, which is what this + // field is for. + // + // Non-empty: run_hook() truncates the file (creating its directory), writes + // the line "# hook of @", and for the length of + // the hook + // * appends the hook's own print / io.write / io.stderr:write to it, and + // * starts every os.execute child with fd 1 and 2 pointing at it, + // so the file reads in the order things happened. HookResult::output is then + // the last kMaxHookOutputBytes of that file. system.exec(cmd, { tty = true }) + // and system.run_in_script(script, true) still inherit the terminal. + // + // A file that cannot be created or opened never fails the hook: children + // get the null device, one warning goes through the log module (and so into + // HookResult::output), and the ring buffer stands in for the file. + // run_script() and apply_elfpatch_auto() ignore this field. + fs::path hook_log; }; inline constexpr std::size_t kMaxHookOutputBytes = 16 * 1024; @@ -177,6 +230,14 @@ void register_os_funcs(lua::State* L) { lua::getglobal(L, "os"); } + // The native os.execute -- system(), the child inherits fd 1 and 2 -- under + // an internal name. While a hook runs with ExecutionContext::hook_log, + // os.execute is replaced by a version that redirects the child's output; + // a command that needs the terminal (system.exec's `tty`) reaches this one. + // Deliberately not an os.* function: recipes should not depend on it. + lua::getfield(L, -1, "execute"); + lua::setglobal(L, "_LIBXPKG_EXEC_INHERIT"); + // os.isdir(path) -> bool lua::pushcfunction(L, [](lua::State* L) -> int { const char* p = lua::tostring(L, 1); @@ -714,9 +775,272 @@ void inject_context(lua::State* L, const mcpplibs::xpkg::ExecutionContext& ctx) constexpr std::string_view HOOK_OUTPUT_TRUNCATED_MARKER = "\n[libxpkg: hook output truncated]\n"; +// ---- Native process layer: a child's fd 1/2 in a file ---------------------- +// +// Standalone on purpose -- no lua::, nothing else from this file. The Windows +// half cannot be compiled by this project's Linux CI, so the block between the +// markers is kept small enough to be extracted and built (and run, under Wine) +// on its own. Do not add a dependency on the rest of the file to it. +// +// Why a process-level redirect and not "cmd >> log 2>&1" appended to the string: +// appended, it only redirects the LAST command of "a && b"; and anything put in +// front of a Windows command changes cmd's /c quote-stripping rule, which +// pkgs/m/msvc.lua depends on. The command string is passed through untouched. + +// BEGIN native-exec +#if defined(_WIN32) +using NativeFile = HANDLE; +inline const NativeFile kNoFile = INVALID_HANDLE_VALUE; +#else +using NativeFile = int; +inline constexpr NativeFile kNoFile = -1; +#endif + +// How a command ended, in Lua 5.4's os.execute vocabulary. +struct ExecStatus { + enum class Kind { Exit, Signal, Failed }; + Kind kind = Kind::Failed; + int code = 0; // exit status, signal number, or the OS error + std::string message; // Failed only +}; + +// An append-only handle to `path`, which must already exist. On Windows it is +// inheritable: it becomes the child's stdout and stderr. +NativeFile open_append(const fs::path& path) { +#if defined(_WIN32) + SECURITY_ATTRIBUTES sa{}; + sa.nLength = sizeof(sa); + sa.bInheritHandle = TRUE; + // FILE_APPEND_DATA alone: every write lands at the end of the file, whoever + // holds the handle, so the parent's and the child's writes stay in order. + return ::CreateFileW(path.c_str(), FILE_APPEND_DATA, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); +#else + return ::open(path.c_str(), O_WRONLY | O_APPEND | O_CLOEXEC); +#endif +} + +void close_file(NativeFile file) { + if (file == kNoFile) return; +#if defined(_WIN32) + ::CloseHandle(file); +#else + ::close(file); +#endif +} + +bool write_all(NativeFile file, std::string_view bytes) { + while (!bytes.empty()) { +#if defined(_WIN32) + const std::size_t chunk = bytes.size() < (1u << 20) ? bytes.size() : (1u << 20); + DWORD written = 0; + if (!::WriteFile(file, bytes.data(), static_cast(chunk), &written, nullptr) || + written == 0) { + return false; + } +#else + const ssize_t written = ::write(file, bytes.data(), bytes.size()); + if (written < 0) { + if (errno == EINTR) continue; + return false; + } +#endif + bytes.remove_prefix(static_cast(written)); + } + return true; +} + +// Run `cmd` through the system shell -- what system() does -- with the child's +// stdout and stderr on `out` (kNoFile: the null device), stdin inherited, and +// wait for it. +ExecStatus run_shell(const char* cmd, NativeFile out) { + ExecStatus status; +#if defined(_WIN32) + // The command line UCRT system() builds: " /c ", the pieces + // joined by single spaces with no quoting, so cmd's /c quote-stripping sees + // exactly what it sees under os.execute. The narrow (ACP) API is deliberate + // for the same reason: system() is narrow too. + const char* env = std::getenv("COMSPEC"); + const std::string comspec = (env && *env) ? env : "cmd.exe"; + std::string commandLine = comspec + " /c " + cmd; + + SECURITY_ATTRIBUTES sa{}; + sa.nLength = sizeof(sa); + sa.bInheritHandle = TRUE; + HANDLE sink = out; + HANDLE nul = INVALID_HANDLE_VALUE; + if (sink == INVALID_HANDLE_VALUE) { + nul = ::CreateFileW(L"NUL", GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, + &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + sink = nul; + } + + STARTUPINFOA startup{}; + startup.cb = sizeof(startup); + startup.dwFlags = STARTF_USESTDHANDLES; + startup.hStdInput = ::GetStdHandle(STD_INPUT_HANDLE); + startup.hStdOutput = sink; + startup.hStdError = sink; + PROCESS_INFORMATION process{}; + const BOOL started = ::CreateProcessA(comspec.c_str(), commandLine.data(), + nullptr, nullptr, TRUE, 0, nullptr, nullptr, + &startup, &process); + if (started) { + ::CloseHandle(process.hThread); + ::WaitForSingleObject(process.hProcess, INFINITE); + DWORD exitCode = 1; + ::GetExitCodeProcess(process.hProcess, &exitCode); + ::CloseHandle(process.hProcess); + status.kind = ExecStatus::Kind::Exit; + status.code = static_cast(exitCode); + } else { + status.code = static_cast(::GetLastError()); + status.message = "cannot start " + comspec + " (error " + + std::to_string(status.code) + ")"; + } + if (nul != INVALID_HANDLE_VALUE) ::CloseHandle(nul); +#else + posix_spawn_file_actions_t actions; + int error = ::posix_spawn_file_actions_init(&actions); + if (error == 0) { + if (out != kNoFile) { + error = ::posix_spawn_file_actions_adddup2(&actions, out, 1); + } else { + error = ::posix_spawn_file_actions_addopen(&actions, 1, "/dev/null", + O_WRONLY, 0); + } + if (error == 0) error = ::posix_spawn_file_actions_adddup2(&actions, 1, 2); + if (error == 0) { + char* const argv[] = { const_cast("sh"), const_cast("-c"), + const_cast(cmd), nullptr }; +# if defined(__APPLE__) + char** const envp = *_NSGetEnviron(); +# else + char** const envp = environ; +# endif + pid_t pid = 0; + error = ::posix_spawn(&pid, "/bin/sh", &actions, nullptr, argv, envp); + if (error == 0) { + int raw = 0; + pid_t reaped = 0; + do { + reaped = ::waitpid(pid, &raw, 0); + } while (reaped < 0 && errno == EINTR); + if (reaped < 0) { + error = errno; + } else if (WIFEXITED(raw)) { + status.kind = ExecStatus::Kind::Exit; + status.code = WEXITSTATUS(raw); + } else if (WIFSIGNALED(raw)) { + status.kind = ExecStatus::Kind::Signal; + status.code = WTERMSIG(raw); + } else { + status.kind = ExecStatus::Kind::Exit; + status.code = raw; + } + } + } + ::posix_spawn_file_actions_destroy(&actions); + } + if (status.kind == ExecStatus::Kind::Failed) { + status.code = error; + status.message = std::strerror(error); + } +#endif + return status; +} +// END native-exec + +std::string printable(const fs::path& path) { + // path::string() can throw on Windows for a name the ACP cannot spell, and + // this is only ever used to word a warning. + try { + return path.string(); + } catch (...) { + return ""; + } +} + +// The file one run_hook() writes when ExecutionContext::hook_log is set. +class HookLog { + fs::path path_; + NativeFile file_ = kNoFile; + bool active_ = false; // a hook is running with a log requested + bool readable_ = false; // the file exists and no write to it has failed + +public: + HookLog() = default; + ~HookLog() { end(); } + HookLog(const HookLog&) = delete; + HookLog& operator=(const HookLog&) = delete; + + // Start a log. `active()` is true afterwards even when the file could not + // be made: the caller asked for the children's output to stay off the + // terminal, and the null device does that. Returns why the file is + // unavailable, or an empty string. + std::string begin(const fs::path& path, std::string_view header) { + end(); + // Absolute now: a hook may os.cd() away, and the file is read back by + // this name after it returns. + std::error_code ec; + path_ = fs::absolute(path, ec); + if (ec) path_ = path; + active_ = true; + readable_ = false; + + if (path_.has_parent_path()) fs::create_directories(path_.parent_path(), ec); + { + std::ofstream out(path_, std::ios::binary | std::ios::trunc); + out.write(header.data(), static_cast(header.size())); + out.close(); + if (out.fail()) return "cannot create " + printable(path_); + } + file_ = open_append(path_); + if (file_ == kNoFile) return "cannot open " + printable(path_) + " for append"; + readable_ = true; + return {}; + } + + bool active() const { return active_; } + // The handle a child's stdout and stderr should be; kNoFile is the null device. + NativeFile file() const { return file_; } + + // The hook's own output, in the order it happened relative to the children's. + void append(std::string_view bytes) { + if (file_ == kNoFile || bytes.empty()) return; + if (!write_all(file_, bytes)) readable_ = false; + } + + void end() { + active_ = false; + close_file(std::exchange(file_, kNoFile)); + } + + // The last `limit` bytes of the file, or nothing when it cannot stand in for + // the ring buffer (never created, or a write to it failed). Valid after end(). + std::optional tail(std::size_t limit) const { + if (!readable_) return std::nullopt; + std::ifstream in(path_, std::ios::binary); + if (!in) return std::nullopt; + in.seekg(0, std::ios::end); + const auto size = static_cast(in.tellg()); + if (size < 0) return std::nullopt; + const auto skip = size > static_cast(limit) + ? size - static_cast(limit) + : std::streamoff{0}; + in.seekg(skip); + std::string bytes(static_cast(size - skip), '\0'); + in.read(bytes.data(), static_cast(bytes.size())); + bytes.resize(static_cast(in.gcount())); + return bytes; + } +}; + class HookOutput { std::string bytes_; bool truncated_ = false; + HookLog* log_ = nullptr; // also receives everything appended, while set static bool is_continuation_byte_(unsigned char byte) { return (byte & 0xc0) == 0x80; @@ -782,8 +1106,11 @@ public: truncated_ = false; } + void attach(HookLog* log) { log_ = log; } + void append(std::string_view bytes) { if (bytes.empty()) return; + if (log_) log_->append(bytes); if (bytes.size() >= kMaxHookOutputBytes) { truncated_ = truncated_ || !bytes_.empty() || bytes.size() > kMaxHookOutputBytes; @@ -870,9 +1197,71 @@ int capture_stderr_write(lua::State* L) { return lua::gettop(L); } +// os.execute while a hook runs with a log: the same call with the same results +// as Lua 5.4's, but the child's fd 1 and 2 are the log. Upvalue 1 is the +// HookLog, upvalue 2 the original os.execute. +int hooked_os_execute(lua::State* L) { + auto* log = static_cast(lua::touserdata(L, lua::upvalueindex(1))); + unsigned long long length = 0; + // Nothing with a destructor may be live across this call: a bad argument + // raises through it, exactly as it does in the original. + const char* cmd = lua::L_optlstring(L, 1, nullptr, &length); + if (!cmd || !log || !log->active()) { + // os.execute() with no command asks whether a shell exists, and a + // reference kept past the hook must behave like the original again. + const int argumentCount = lua::gettop(L); + lua::pushvalue(L, lua::upvalueindex(2)); + lua::insert(L, 1); + lua::call(L, argumentCount, lua::MULTRET); + return lua::gettop(L); + } + + const ExecStatus status = run_shell(cmd, log->file()); + switch (status.kind) { + case ExecStatus::Kind::Exit: + // luaL_execresult: success is true, any other status is fail. + if (status.code == 0) lua::pushboolean(L, 1); + else lua::pushnil(L); + lua::pushstring(L, "exit"); + break; + case ExecStatus::Kind::Signal: + lua::pushnil(L); + lua::pushstring(L, "signal"); + break; + case ExecStatus::Kind::Failed: + // luaL_fileresult: fail, message, errno. + lua::pushnil(L); + lua::pushstring(L, status.message.c_str()); + break; + } + lua::pushinteger(L, status.code); + return 3; +} + +// One warning through the log module. Called with capture already installed, +// so it lands in HookResult::output and not on the terminal. +void warn_through_log_module(lua::State* L, const std::string& message) { + const int top = lua::gettop(L); + lua::getglobal(L, "_LIBXPKG_MODULES"); + if (lua::type(L, -1) == lua::TTABLE) { + lua::getfield(L, -1, "log"); + if (lua::type(L, -1) == lua::TTABLE) { + lua::getfield(L, -1, "warn"); + if (lua::type(L, -1) == lua::TFUNCTION) { + lua::pushstring(L, "%s"); + lua::pushstring(L, message.c_str()); + lua::pcall(L, 2, 0, 0); + } + } + } + lua::settop(L, top); +} + class HookCapture { lua::State* L_ = nullptr; HookOutput& output_; + HookLog* log_ = nullptr; + int osExecuteRef_ = 0; int printRef_ = 0; int ioRef_ = 0; int ioWriteRef_ = 0; @@ -883,6 +1272,21 @@ class HookCapture { void restore_() { if (!L_) return; + if (osExecuteRef_ != 0) { + lua::getglobal(L_, "os"); + if (lua::type(L_, -1) == lua::TTABLE) { + lua::rawgeti(L_, lua::REGISTRYINDEX, osExecuteRef_); + lua::setfield(L_, -2, "execute"); + } + lua::pop(L_, 1); + lua::L_unref(L_, lua::REGISTRYINDEX, osExecuteRef_); + osExecuteRef_ = 0; + } + if (log_) { + output_.attach(nullptr); + log_->end(); + } + lua::rawgeti(L_, lua::REGISTRYINDEX, ioRef_); lua::rawgeti(L_, lua::REGISTRYINDEX, ioWriteRef_); lua::setfield(L_, -2, "write"); @@ -907,9 +1311,11 @@ class HookCapture { } public: - HookCapture(lua::State* L, HookOutput& output) - : L_(L), output_(output) { + // `log` is null for a hook that asked for none: nothing below changes then. + HookCapture(lua::State* L, HookOutput& output, HookLog* log = nullptr) + : L_(L), output_(output), log_(log) { output_.reset(); + output_.attach(log_); lua::getglobal(L_, "print"); printRef_ = lua::L_ref(L_, lua::REGISTRYINDEX); @@ -948,6 +1354,19 @@ public: lua::pushcclosure(L_, capture_stderr_write, 3); lua::setfield(L_, -2, "write"); lua::pop(L_, 2); + + if (log_) { + lua::getglobal(L_, "os"); + if (lua::type(L_, -1) == lua::TTABLE) { + lua::getfield(L_, -1, "execute"); + osExecuteRef_ = lua::L_ref(L_, lua::REGISTRYINDEX); + lua::pushlightuserdata(L_, log_); + lua::rawgeti(L_, lua::REGISTRYINDEX, osExecuteRef_); + lua::pushcclosure(L_, hooked_os_execute, 2); + lua::setfield(L_, -2, "execute"); + } + lua::pop(L_, 1); + } } ~HookCapture() { restore_(); } @@ -957,6 +1376,14 @@ public: std::string finish() { restore_(); + if (log_) { + // The file holds the children's output as well as the hook's own, + // in order; the ring buffer only ever saw the latter. + if (auto tail = log_->tail(kMaxHookOutputBytes + 1)) { + output_.reset(); + output_.append(*tail); + } + } return output_.finish(); } }; @@ -972,6 +1399,8 @@ class PackageExecutor { fs::path pkg_ ; std::unique_ptr hookOutput_ = std::make_unique(); + std::unique_ptr hookLog_ = + std::make_unique(); public: explicit PackageExecutor(lua::State* L, fs::path pkg) @@ -987,7 +1416,8 @@ public: PackageExecutor(PackageExecutor&& o) noexcept : L_(std::exchange(o.L_, nullptr)), pkg_(std::move(o.pkg_)), - hookOutput_(std::move(o.hookOutput_)) {} + hookOutput_(std::move(o.hookOutput_)), + hookLog_(std::move(o.hookLog_)) {} PackageExecutor& operator=(PackageExecutor&& o) noexcept { if (this != &o) { @@ -995,6 +1425,7 @@ public: L_ = std::exchange(o.L_, nullptr); pkg_ = std::move(o.pkg_); hookOutput_ = std::move(o.hookOutput_); + hookLog_ = std::move(o.hookLog_); } return *this; } @@ -1020,7 +1451,21 @@ public: .error = "hook not found: " + std::string(name) }; } - detail::HookCapture capture(L_, *hookOutput_); + detail::HookLog* log = nullptr; + std::string logProblem; + if (!ctx.hook_log.empty()) { + log = hookLog_.get(); + logProblem = log->begin( + ctx.hook_log, + "# " + std::string(name) + " hook of " + ctx.pkg_name + "@" + + ctx.version + "\n"); + } + detail::HookCapture capture(L_, *hookOutput_, log); + if (!logProblem.empty()) { + detail::warn_through_log_module( + L_, "hook output log unavailable (" + logProblem + + "); output of child processes is discarded"); + } HookResult result; if (lua::pcall(L_, 0, 1, 0) == lua::OK) { int t = lua::type(L_, -1); diff --git a/tests/test_executor.cpp b/tests/test_executor.cpp index ef8cfa3..c438da8 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -3242,3 +3242,439 @@ TEST(ExecutorTest, PkgInfo_UniqueRecordWithNoPayloadOnDiskIsStillAMiss) { EXPECT_NE(result.output.find("missing payload"), std::string::npos) << "an absent payload must say so:\n" << result.output; } + +// ---- ExecutionContext::hook_log: child-process output of a hook (0.0.60) ---- +// +// Children started by os.execute inherit this process's fd 1 and 2, which is +// exactly what testing::internal::CaptureStdout/CaptureStderr swap out -- so a +// marker that shows up in the captured text reached "the terminal", and one that +// does not was kept off it. + +namespace { + +std::string slurp(const fs::path& path) { + std::ifstream in(path, std::ios::binary); + return std::string(std::istreambuf_iterator(in), {}); +} + +std::vector split_lines(std::string_view text) { + std::vector lines; + std::size_t begin = 0; + while (begin < text.size()) { + auto end = text.find('\n', begin); + if (end == std::string_view::npos) end = text.size(); + std::string line(text.substr(begin, end - begin)); + if (!line.empty() && line.back() == '\r') line.pop_back(); + lines.push_back(std::move(line)); + begin = end + 1; + } + return lines; +} + +fs::path write_hook_package(const fs::path& dir, std::string_view name, + std::string_view body) { + const fs::path pkg = dir / (std::string(name) + ".lua"); + write_text(pkg, + "package = { name = \"" + std::string(name) + + "\", xpm = { linux = { [\"0.0.1\"] = {} } } }\n" + "local system = import(\"xim.libxpkg.system\")\n" + std::string(body)); + return pkg; +} + +struct Escaped { + std::string out, err; +}; + +template +Escaped run_captured(Fn&& fn) { + testing::internal::CaptureStdout(); + testing::internal::CaptureStderr(); + fn(); + Escaped escaped; + escaped.out = testing::internal::GetCapturedStdout(); + escaped.err = testing::internal::GetCapturedStderr(); + return escaped; +} + +} // namespace + +TEST(ExecutorTest, HookLog_ChildOutputAndPrintLandInOrderNotOnTheTerminal) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-order-"); + const fs::path pkg = write_hook_package(temp, "hook-log-order", + "function install()\n" + " print(\"MARK-1-print\")\n" + " os.execute(\"echo MARK-2-child-out\")\n" + " os.execute(\"echo MARK-3-child-err 1>&2\")\n" + " io.write(\"MARK-4-io-write\\n\")\n" + " io.stderr:write(\"MARK-5-io-stderr\\n\")\n" + " os.execute(\"echo MARK-6-child-last\")\n" + " return true\n" + "end\n"); + const fs::path logPath = temp / "logs" / "hooks" / "order.log"; + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + auto ctx = make_context(temp / "install", "linux"); + ctx.pkg_name = "hook-log-order"; + ctx.version = "0.0.1"; + ctx.hook_log = logPath; + + HookResult result; + const auto escaped = run_captured([&] { + result = exec->run_hook(HookType::Install, ctx); + }); + + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + EXPECT_EQ(escaped.out.find("MARK-"), std::string::npos) + << "a child reached the process's stdout:\n" << escaped.out; + EXPECT_EQ(escaped.err.find("MARK-"), std::string::npos) + << "a child reached the process's stderr:\n" << escaped.err; + + const std::string logged = slurp(logPath); + EXPECT_EQ(logged.rfind("# install hook of hook-log-order@0.0.1\n", 0), 0u) << logged; + for (const std::string& text : {logged, result.output}) { + std::size_t previous = 0; + for (const char* marker : {"MARK-1-print", "MARK-2-child-out", "MARK-3-child-err", + "MARK-4-io-write", "MARK-5-io-stderr", + "MARK-6-child-last"}) { + const auto at = text.find(marker); + ASSERT_NE(at, std::string::npos) << marker << " missing from:\n" << text; + EXPECT_GE(at, previous) << marker << " is out of order in:\n" << text; + previous = at; + } + } + EXPECT_EQ(result.output.rfind("# install hook of hook-log-order@0.0.1\n", 0), 0u) + << result.output; + + fs::remove_all(temp); +} + +TEST(ExecutorTest, HookLog_EmptyKeepsTheChildrenOnTheInheritedStreams) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-empty-"); + const fs::path pkg = write_hook_package(temp, "hook-log-empty", + "function install()\n" + " print(\"MARK-print\")\n" + " os.execute(\"echo MARK-child-out\")\n" + " os.execute(\"echo MARK-child-err 1>&2\")\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + auto ctx = make_context(temp / "install", "linux"); + + HookResult legacy; + const auto before = run_captured([&] { + legacy = exec->run_hook(HookType::Install, ctx); + }); + EXPECT_TRUE(legacy.success) << legacy.error; + EXPECT_NE(before.out.find("MARK-child-out"), std::string::npos) << before.out; + EXPECT_NE(before.err.find("MARK-child-err"), std::string::npos) << before.err; + EXPECT_EQ(before.out.find("MARK-print"), std::string::npos); + EXPECT_NE(legacy.output.find("MARK-print"), std::string::npos); + EXPECT_EQ(legacy.output.find("MARK-child"), std::string::npos) << legacy.output; + EXPECT_EQ(legacy.output.find("# install hook"), std::string::npos) << legacy.output; + + // The same executor with a log, then without one again: nothing the + // logged run installed may outlive it. + ctx.hook_log = temp / "empty.log"; + HookResult logged; + const auto during = run_captured([&] { + logged = exec->run_hook(HookType::Install, ctx); + }); + EXPECT_EQ(during.out.find("MARK-"), std::string::npos) << during.out; + EXPECT_EQ(during.err.find("MARK-"), std::string::npos) << during.err; + EXPECT_NE(logged.output.find("MARK-child-out"), std::string::npos) << logged.output; + + ctx.hook_log.clear(); + const auto after = run_captured([&] { + legacy = exec->run_hook(HookType::Install, ctx); + }); + EXPECT_NE(after.out.find("MARK-child-out"), std::string::npos) + << "os.execute stayed redirected after the hook returned:\n" << after.out; + EXPECT_NE(after.err.find("MARK-child-err"), std::string::npos) << after.err; + EXPECT_EQ(legacy.output.find("MARK-child"), std::string::npos) << legacy.output; + + fs::remove_all(temp); +} + +TEST(ExecutorTest, HookLog_ExecuteReturnsWhatLuaOsExecuteReturns) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-triples-"); + std::string body = + "local function show(...)\n" + " local parts = {}\n" + " for i = 1, select('#', ...) do parts[i] = tostring((select(i, ...))) end\n" + " print(\"RESULT \" .. table.concat(parts, \" \"))\n" + "end\n" + "function install()\n" + " show(os.execute(\"exit 0\"))\n" + " show(os.execute(\"exit 3\"))\n" + " show(os.execute())\n" +#ifndef _WIN32 + " show(os.execute(\"kill -TERM $$\"))\n" + " show(os.execute(\"exit 300\"))\n" +#endif + " return true\n" + "end\n"; + const fs::path pkg = write_hook_package(temp, "hook-log-triples", body); + + auto results = [&](const fs::path& hookLog) { + auto exec = create_executor(pkg); + EXPECT_TRUE(exec.has_value()); + auto ctx = make_context(temp / "install", "linux"); + ctx.hook_log = hookLog; + const auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + std::vector lines; + for (auto& line : split_lines(result.output)) { + if (line.rfind("RESULT ", 0) == 0) lines.push_back(std::move(line)); + } + return lines; + }; + + const auto native = results({}); + const auto redirected = results(temp / "triples.log"); + + EXPECT_EQ(redirected, native) + << "the redirecting os.execute must answer exactly like Lua's"; + ASSERT_GE(redirected.size(), 3u); + EXPECT_EQ(redirected[0], "RESULT true exit 0"); + EXPECT_EQ(redirected[1], "RESULT nil exit 3"); + EXPECT_EQ(redirected[2], "RESULT true"); +#ifndef _WIN32 + ASSERT_EQ(redirected.size(), 5u); + EXPECT_EQ(redirected[3], "RESULT nil signal 15"); + EXPECT_EQ(redirected[4], "RESULT nil exit 44"); // 300 & 0xff +#endif + + fs::remove_all(temp); +} + +TEST(ExecutorTest, HookLog_TtyOptOutReachesTheTerminalAndNotTheLog) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-tty-"); + const fs::path pkg = write_hook_package(temp, "hook-log-tty", + "function install()\n" + " system.exec(\"echo MARK-tty\", { tty = true })\n" + " system.exec(\"echo MARK-logged\")\n" + " system.exec(\"echo MARK-logged-too\", { tty = false })\n" + " return true\n" + "end\n"); + const fs::path logPath = temp / "tty.log"; + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + auto ctx = make_context(temp / "install", "linux"); + ctx.hook_log = logPath; + + HookResult result; + const auto escaped = run_captured([&] { + result = exec->run_hook(HookType::Install, ctx); + }); + + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + EXPECT_NE(escaped.out.find("MARK-tty"), std::string::npos) << escaped.out; + EXPECT_EQ(escaped.out.find("MARK-logged"), std::string::npos) << escaped.out; + const std::string logged = slurp(logPath); + const auto loggedLines = split_lines(logged); + EXPECT_EQ(logged.find("MARK-tty"), std::string::npos) << logged; + EXPECT_NE(std::ranges::find(loggedLines, "MARK-logged"), loggedLines.end()) << logged; + EXPECT_NE(std::ranges::find(loggedLines, "MARK-logged-too"), loggedLines.end()) << logged; + EXPECT_EQ(result.output.find("MARK-tty"), std::string::npos) << result.output; + + fs::remove_all(temp); +} + +#ifndef _WIN32 +TEST(ExecutorTest, HookLog_RunInScriptIsLoggedUnlessAdmin) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-script-"); + const fs::path pkg = write_hook_package(temp, "hook-log-script", + "function install()\n" + " system.run_in_script(\"#!/bin/sh\\necho MARK-script-out\\necho MARK-script-err >&2\\n\")\n" + " return true\n" + "end\n"); + const fs::path logPath = temp / "script.log"; + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + auto ctx = make_context(temp / "install", "linux"); + ctx.hook_log = logPath; + + HookResult result; + const auto escaped = run_captured([&] { + result = exec->run_hook(HookType::Install, ctx); + }); + + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + EXPECT_EQ(escaped.out.find("MARK-"), std::string::npos) << escaped.out; + EXPECT_EQ(escaped.err.find("MARK-"), std::string::npos) << escaped.err; + const std::string logged = slurp(logPath); + EXPECT_NE(logged.find("MARK-script-out"), std::string::npos) << logged; + EXPECT_NE(logged.find("MARK-script-err"), std::string::npos) << logged; + + fs::remove_all(temp); +} +#endif + +TEST(ExecutorTest, HookLog_UnopenableLogNeverFailsTheHook) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-unopenable-"); + const fs::path pkg = write_hook_package(temp, "hook-log-unopenable", + "function install()\n" + " print(\"MARK-kept\")\n" + " local ok = os.execute(\"echo MARK-lost-child\")\n" + " assert(ok == true, \"the command must still run\")\n" + " return true\n" + "end\n"); + // A path below a regular file can be neither created nor opened. + const fs::path blocker = temp / "blocker"; + write_text(blocker, "not a directory"); + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + auto ctx = make_context(temp / "install", "linux"); + ctx.hook_log = blocker / "hooks" / "x.log"; + + HookResult result; + const auto escaped = run_captured([&] { + result = exec->run_hook(HookType::Install, ctx); + }); + + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + EXPECT_EQ(escaped.out.find("MARK-lost-child"), std::string::npos) + << "with no log the child's output goes to the null device, not the terminal:\n" + << escaped.out; + EXPECT_NE(result.output.find("MARK-kept"), std::string::npos) << result.output; + EXPECT_NE(result.output.find("hook output log unavailable"), std::string::npos) + << result.output; + const auto first = result.output.find("hook output log unavailable"); + EXPECT_EQ(result.output.find("hook output log unavailable", first + 1), + std::string::npos) << "the warning must be given once:\n" << result.output; + EXPECT_EQ(escaped.out.find("hook output log unavailable"), std::string::npos); + + fs::remove_all(temp); +} + +TEST(ExecutorTest, HookLog_CreatesItsDirectoryAndStartsEveryRunFresh) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-fresh-"); + const fs::path pkg = write_hook_package(temp, "hook-log-fresh", + "function install()\n" + " print(\"MARK-\" .. tostring(_RUNTIME.args[1]))\n" + " return true\n" + "end\n"); + const fs::path logPath = temp / "a" / "b" / "fresh.log"; + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + auto ctx = make_context(temp / "install", "linux"); + ctx.hook_log = logPath; + + ctx.args = {"first"}; + EXPECT_TRUE(exec->run_hook(HookType::Install, ctx).success); + ctx.args = {"second"}; + EXPECT_TRUE(exec->run_hook(HookType::Install, ctx).success); + + const std::string logged = slurp(logPath); + EXPECT_EQ(logged.find("MARK-first"), std::string::npos) << logged; + EXPECT_NE(logged.find("MARK-second"), std::string::npos) << logged; + EXPECT_EQ(logged.find("# install hook"), 0u) << logged; + EXPECT_EQ(logged.find("# install hook", 1), std::string::npos) << logged; + + fs::remove_all(temp); +} + +TEST(ExecutorTest, HookLog_OutputIsTheTailOfAFileThatKeepsEverything) { + constexpr std::size_t outputCap = 16 * 1024; + constexpr std::string_view truncatedMarker = + "\n[libxpkg: hook output truncated]\n"; + const fs::path temp = make_temp_dir("libxpkg-hook-log-tail-"); + const fs::path pkg = write_hook_package(temp, "hook-log-tail", + "function install()\n" + " io.write(\"HEAD-MARK\")\n" + " io.write(string.rep(\"x\", 40000))\n" + " os.execute(\"echo TAIL-MARK\")\n" + " return true\n" + "end\n"); + const fs::path logPath = temp / "tail.log"; + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + auto ctx = make_context(temp / "install", "linux"); + ctx.hook_log = logPath; + const auto result = exec->run_hook(HookType::Install, ctx); + + EXPECT_TRUE(result.success) << result.error; + EXPECT_LE(result.output.size(), outputCap + truncatedMarker.size()); + EXPECT_NE(result.output.find("TAIL-MARK"), std::string::npos); + EXPECT_EQ(result.output.find("HEAD-MARK"), std::string::npos); + EXPECT_EQ(result.output.find(truncatedMarker), 0u) << "the cut must be announced"; + EXPECT_TRUE(is_valid_utf8(result.output)); + + const std::string logged = slurp(logPath); + EXPECT_GT(logged.size(), 40000u); + EXPECT_NE(logged.find("HEAD-MARK"), std::string::npos) + << "the file is the full record; only HookResult::output is a tail"; + + fs::remove_all(temp); +} + +TEST(ExecutorTest, HookLog_ScriptsAreNotIntercepted) { + const fs::path temp = make_temp_dir("libxpkg-hook-log-script-main-"); + const fs::path pkg = temp / "script-main.lua"; + write_text(pkg, + "package = { name = \"script-main\", xpm = { linux = { [\"0.0.1\"] = {} } } }\n" + "function xpkg_main()\n" + " os.execute(\"echo MARK-script-child\")\n" + "end\n"); + const fs::path logPath = temp / "script-main.log"; + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + ExecutionContext ctx; + ctx.platform = "linux"; + ctx.hook_log = logPath; + + HookResult result; + const auto escaped = run_captured([&] { result = exec->run_script(ctx); }); + + EXPECT_TRUE(result.success) << result.error; + EXPECT_NE(escaped.out.find("MARK-script-child"), std::string::npos) << escaped.out; + EXPECT_FALSE(fs::exists(logPath)) << "run_script has no hook to log"; + + fs::remove_all(temp); +} + +TEST(ExecutorTest, PkgInfo_BuildDepReadsTheVariableXlingsExports) { + const fs::path temp = make_temp_dir("libxpkg-build-dep-name-"); + const fs::path sevenZip = temp / "payloads" / "7zip"; + const fs::path myTool = temp / "payloads" / "my-tool"; + fs::create_directories(sevenZip); + fs::create_directories(myTool); + // xlings exports the bare name: no namespace, no @version, upper-cased, + // everything that is not alphanumeric mapped to '_'. + ScopedEnvVar sevenZipVar("XLINGS_BUILDDEP_7ZIP_PATH", sevenZip.string()); + ScopedEnvVar myToolVar("XLINGS_BUILDDEP_MY_TOOL_PATH", myTool.string()); + + const fs::path pkg = write_hook_package(temp, "build-dep-name", + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "local function expect(spelling, dir)\n" + " local dep = pkginfo.build_dep(spelling)\n" + " assert(dep ~= nil, spelling .. \" resolved to nothing\")\n" + " assert(dep.path == dir, spelling .. \" -> \" .. tostring(dep.path))\n" + "end\n" + "function install()\n" + " expect(\"xim:7zip\", [[" + sevenZip.string() + "]])\n" + " expect(\"7zip\", [[" + sevenZip.string() + "]])\n" + " expect(\"xim:7zip@26.02\", [[" + sevenZip.string() + "]])\n" + " expect(\"7zip@26.02\", [[" + sevenZip.string() + "]])\n" + " expect(\"xim:my-tool@1.0\", [[" + myTool.string() + "]])\n" + " expect(\"my-tool\", [[" + myTool.string() + "]])\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg); + ASSERT_TRUE(exec.has_value()) << exec.error(); + const auto result = exec->run_hook(HookType::Install, + make_context(temp / "install", "linux")); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + + fs::remove_all(temp); +} From b268dad56b17cfce6140a3a8ca84d274a83067aa Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Tue, 29 Sep 2026 23:13:58 +0800 Subject: [PATCH 2/2] fix(executor): find cmd.exe on the search path when COMSPEC is unset system() looks cmd.exe up on the search path when COMSPEC is unset. Passing the bare name as CreateProcessA's lpApplicationName would only look in the current directory, so the application name is left to CreateProcess, which then parses it from the command line. With COMSPEC set (the normal case) the call is unchanged. Cross-compiled with x86_64-w64-mingw32-g++ (-Wall -Wextra) on the extracted native-exec block. --- src/xpkg-executor.cppm | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/xpkg-executor.cppm b/src/xpkg-executor.cppm index be87bbb..a111d18 100644 --- a/src/xpkg-executor.cppm +++ b/src/xpkg-executor.cppm @@ -883,7 +883,12 @@ ExecStatus run_shell(const char* cmd, NativeFile out) { startup.hStdOutput = sink; startup.hStdError = sink; PROCESS_INFORMATION process{}; - const BOOL started = ::CreateProcessA(comspec.c_str(), commandLine.data(), + // With COMSPEC unset, system() finds cmd.exe on the search path; a bare + // name as lpApplicationName would only be looked for in the current + // directory, so let CreateProcess parse the command line instead. + const bool fromEnv = env && *env; + const BOOL started = ::CreateProcessA(fromEnv ? comspec.c_str() : nullptr, + commandLine.data(), nullptr, nullptr, TRUE, 0, nullptr, nullptr, &startup, &process); if (started) {