thakis added a comment. (sorry for the slow response here)
>> If you need to run an external program, look for it next to the compiler, >> like we do for gas with -fno-integrated-as, linker, etc. People can use the >> existing -B flag to make clang look elsewhere if they need that. > > Unfortunately, wasm-opt isn't typically installed alongside the compiler. > It's also not a typical system tool like an assembler, which is why it didn't > seem right to search the -B paths. > > The first version of my patch used a dedicated environment variable, > WASM_OPT, rather than PATH, but I changed it to PATH after review feedback. > If people feel using -B paths, and COMPILER_PATH, are appropriate, we can use > `GetProgramPath` itself, though note that that still does have a PATH > fallback. Here's the scenario I'm worried about: Imagine someone packaging a hermetic toolchain that bundles clang, lld, and so on, but it doesn't bundle wasm-opt. After this patch, the toolchain won't behave hermetically for wasm compilations, since it now does different things based on if wasm-opt is on PATH (which it is if a dev is locally playing with emscripten, say). It's true that the driver falls back to PATH as a last resort measure for the linker, but, as you say, that's usually next to the compiler binary and the fallback isn't needed, but wasm-opt usually isn't there. What should be the way forward for that? Options: - Add a driver flag that explicitly sets if PATH should be used as final fallback for all tools (including this one) - and maybe change the default to false since this behavior now goes form "basically never used" to "usually used, for wasm compilations" - Add a dedicated flag that controls if wasm-opt should be looked for in PATH - ...? (See also http://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html ctrl-f "environment variables") Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70500/new/ https://reviews.llvm.org/D70500 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
