refactor: platform abstraction layer + fix macOS stdin hang

.agents/docs/platform-abstraction-plan.md

@@ -0,0 +1,102 @@
+# Platform Abstraction Layer — Architecture & Implementation Plan
+
+## 1. Problem Statement
+
+mcpp has ~45+ `#if defined(...)` blocks scattered across 10+ source files for platform-specific logic. This causes:
+- **stdin leakage**: subprocess calls on macOS don't close stdin → first-run hangs
+- **Code duplication**: `self_exe_path()` duplicated in `config.cppm` and `ninja_backend.cppm`
+- **Inconsistent abstractions**: `process.cppm` wraps some calls, but many files still use raw `std::system()`/`popen()`
+- **Maintenance burden**: adding platform support requires editing 10+ files
+
+## 2. Target Architecture
+
+```
+src/platform/
+├── platform.cppm          # Unified facade (re-exports all platform capabilities)
+├── common.cppm            # Cross-platform constants + compile-time detection
+├── process.cppm           # Unified process execution (auto-closes stdin on POSIX)
+├── fs.cppm                # Platform filesystem ops (exe path, file lock, which)
+├── env.cppm               # Environment variable operations
+├── shell.cppm             # Shell quoting + command building
+├── macos.cppm             # macOS: xcrun, SDK discovery, Xcode CLT detection
+├── linux.cppm             # Linux: /proc, LD_LIBRARY_PATH, patchelf
+└── windows.cppm           # Windows: vswhere, MSVC, _putenv_s, registry
+```
+
+## 3. Module Responsibilities
+
+### common.cppm — Compile-time constants & platform detection
+- Platform booleans: `is_windows`, `is_macos`, `is_linux`
+- Binary naming: `exe_suffix`, `static_lib_ext`, `shared_lib_ext`, `lib_prefix`
+- Platform name string
+- Null redirect string
+
+### process.cppm — Unified process execution (CORE FIX)
+- `capture()` — run command, capture stdout (auto `</dev/null` on POSIX)
+- `run_silent()` — run without caring about output
+- `run_streaming()` — stream stdout line-by-line via callback
+- All subprocess calls go through here → stdin leak impossible
+
+### fs.cppm — Platform filesystem operations
+- `self_exe_path()` — current executable path (Win: GetModuleFileName, macOS: _NSGetExecutablePath, Linux: /proc/self/exe)
+- `FileLock` — RAII exclusive file lock (Win: LockFileEx, POSIX: flock)
+- `which()` — find executable (Win: `where`, POSIX: `command -v`)
+
+### env.cppm — Environment variable operations
+- `set()` / `get()` — platform-aware env var manipulation
+- `build_env_prefix()` — construct env prefix for commands
+
+### shell.cppm — Shell quoting
+- `quote()` — platform-aware shell argument quoting
+- `silent_redirect` — full silent redirect string
+
+### macos.cppm — macOS-specific
+- `has_xcode_clt()` — check Xcode Command Line Tools
+- `sdk_path()` — xcrun SDK discovery
+- `runtime_lib_dirs()` — macOS library search paths
+
+### linux.cppm — Linux-specific
+- `build_ld_library_path()` — LD_LIBRARY_PATH construction
+- `runtime_lib_dirs()` — Linux library search paths
+
+### windows.cppm — Windows-specific
+- `find_visual_studio()` — VS discovery via vswhere/env/paths
+- `find_std_module_source()` — MSVC STL module source
+- `prepend_path()` — Windows PATH manipulation
+
+### platform.cppm — Unified facade
+- Re-exports all common modules
+- Conditionally exports platform-specific modules
+
+## 4. Migration Impact
+
+| Source File | Changes | Effort |
+|-------------|---------|--------|
+| `config.cppm` | Use `fs::self_exe_path()`, `fs::which()`, `process::run_silent()` | Medium |
+| `xlings.cppm` | Use `shell::quote()`, `process::run_streaming()`, `env::*` | Large |
+| `process.cppm` | DELETE — absorbed by `platform/process.cppm` + `platform/shell.cppm` | Delete |
+| `probe.cppm` | Use `fs::which()`, `macos::sdk_path()` | Medium |
+| `bmi_cache.cppm` | Use `fs::FileLock` | Small |
+| `ninja_backend.cppm` | Use `fs::self_exe_path()` | Small |
+| `flags.cppm` | Use `common` constants | Small |
+| `clang.cppm` | Use `windows::find_std_module_source()` | Small |
+| `msvc.cppm` | Discovery logic moves to `platform/windows.cppm` | Medium |
+| `cli.cppm` | Use `common::name` | Small |
+
+## 5. Implementation Phases
+
+### Phase 1: Skeleton (parallel-safe)
+Create directory + `common.cppm` (move from existing `platform.cppm`)
+
+### Phase 2: Core modules (parallelizable)
+- 2a: `process.cppm` — includes stdin fix
+- 2b: `fs.cppm` — self_exe_path + FileLock + which
+- 2c: `env.cppm` + `shell.cppm`
+- 2d: `macos.cppm` + `linux.cppm` + `windows.cppm`
+- 2e: `platform.cppm` facade
+
+### Phase 3: Consumer migration
+Migrate all files to use new platform modules, remove old `process.cppm`
+
+### Phase 4: Build config + CI verification
+Update `mcpp.toml`, verify on all platforms

.agents/docs/platform-remaining-ifdefs-report.md

@@ -0,0 +1,49 @@
+# Remaining Platform Macros Outside src/platform/ — Analysis Report
+
+## Summary
+
+14 files, 39 occurrences of platform-specific macros/conditionals outside `src/platform/`.
+
+## P0: Eliminate `#define popen _popen` (6 files, ~16 occurrences)
+
+Migrate all raw popen/pclose calls to `platform::process::capture()` / `run_streaming()` / `run_silent()`.
+
+Files: cli.cppm, ninja_backend.cppm, msvc.cppm, stdmod.cppm, pack/pack.cppm, pm/publisher.cppm, modgraph/p1689.cppm
+
+## P1: Shell quoting in cli.cppm (5 occurrences)
+
+Replace manual `'...'` vs `"..."` with `platform::shell::quote()`.
+
+Lines: 1954, 1965, 2527, 2620, 3178
+
+## P2: Terminal abstraction — ui.cppm (3 occurrences)
+
+Create `platform/terminal.cppm` for isatty, terminal width detection.
+
+Lines: 8-9, 149, 329
+
+## P3: kXpkgPlatform in resolver.cppm (1 occurrence)
+
+Move to `platform::common` as a constant.
+
+Line: 30
+
+## P4: Link strategy constants in flags.cppm (3 occurrences)
+
+Add `supports_full_static`, `supports_rpath`, `needs_explicit_libcxx` to `platform::common`.
+
+Lines: 162, 175, 183
+
+## P5: xlings.cppm build_command_prefix (3 occurrences)
+
+Abstract Windows env-setting vs POSIX shell-prefix into platform layer.
+
+Lines: 401, 612, 739
+
+## P6: ninja_backend.cppm rule generation (10 occurrences)
+
+Abstract `$toolenv` prefix and shell syntax differences.
+
+Lines: 178, 207, 231, 243, 254, 287, 297, 548, 560
+
+## Priority: P0+P1 = 54% elimination with minimal risk.

src/bmi_cache.cppm

@@ -15,18 +15,11 @@
 // dep cache without trashing manifest.txt (docs/26 §5.4 V2).
 
 module;
-#if defined(_WIN32)
-#include <io.h>
-#include <windows.h>
-#else
-#include <fcntl.h>
-#include <sys/file.h>
-#include <unistd.h>
-#endif
 
 export module mcpp.bmi_cache;
 
 import std;
+import mcpp.platform;
 
 export namespace mcpp::bmi_cache {
 
@@ -188,56 +181,6 @@ stage_into(const CacheKey& key,
 }
 
 namespace {
-
-// Acquire an exclusive non-blocking lock on <dir>/.lock. Returns a handle
-// on success, or -1/INVALID_HANDLE if another mcpp is already populating.
-#if defined(_WIN32)
-// Windows: use LockFileEx on a file handle
-HANDLE try_lock_dir(const std::filesystem::path& dir) {
-    std::error_code ec;
-    std::filesystem::create_directories(dir, ec);
-    auto lockPath = dir / ".lock";
-    HANDLE h = CreateFileW(lockPath.wstring().c_str(),
-        GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE,
-        NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
-    if (h == INVALID_HANDLE_VALUE) return h;
-    OVERLAPPED ov = {};
-    if (!LockFileEx(h, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY,
-                    0, 1, 0, &ov)) {
-        CloseHandle(h);
-        return INVALID_HANDLE_VALUE;
-    }
-    return h;
-}
-
-void release_lock(HANDLE h) {
-    if (h == INVALID_HANDLE_VALUE) return;
-    OVERLAPPED ov = {};
-    UnlockFileEx(h, 0, 1, 0, &ov);
-    CloseHandle(h);
-}
-#else
-// POSIX: use flock(2)
-int try_lock_dir(const std::filesystem::path& dir) {
-    std::error_code ec;
-    std::filesystem::create_directories(dir, ec);
-    auto lockPath = dir / ".lock";
-    int fd = ::open(lockPath.c_str(), O_CREAT | O_RDWR | O_CLOEXEC, 0644);
-    if (fd < 0) return -1;
-    if (::flock(fd, LOCK_EX | LOCK_NB) != 0) {
-        ::close(fd);
-        return -1;
-    }
-    return fd;
-}
-
-void release_lock(int fd) {
-    if (fd < 0) return;
-    ::flock(fd, LOCK_UN);
-    ::close(fd);
-}
-#endif
-
 } // namespace
 
 std::expected<void, std::string>
@@ -246,26 +189,11 @@ populate_from(const CacheKey& key,
               const DepArtifacts& arts)
 {
     auto cacheDir = key.dir();
-#if defined(_WIN32)
-    HANDLE lockHandle = try_lock_dir(cacheDir);
-    if (lockHandle == INVALID_HANDLE_VALUE) {
-        return {};
-    }
-    struct LockGuard {
-        HANDLE h;
-        ~LockGuard() { release_lock(h); }
-    } guard{ lockHandle };
-#else
-    int lockFd = try_lock_dir(cacheDir);
-    if (lockFd < 0) {
+    auto lock = mcpp::platform::fs::FileLock::try_acquire(cacheDir);
+    if (!lock) {
         // Another writer holds the lock; treat as success (they'll do it).
         return {};
     }
-    struct LockGuard {
-        int fd;
-        ~LockGuard() { release_lock(fd); }
-    } guard{ lockFd };
-#endif
 
     auto cacheBmi = key.bmiDir();
     auto cacheObj = key.objDir();

src/build/flags.cppm

@@ -10,6 +10,7 @@ export module mcpp.build.flags;
 
 import std;
 import mcpp.build.plan;
+import mcpp.platform;
 import mcpp.toolchain.clang;
 import mcpp.toolchain.detect;
 import mcpp.toolchain.provider;
@@ -159,46 +160,23 @@ CompileFlags compute_flags(const BuildPlan& plan) {
     // Link flags
     f.staticStdlib = plan.manifest.buildConfig.staticStdlib;
     f.linkage = plan.manifest.buildConfig.linkage;
-#if defined(_WIN32)
-    // Windows: MSVC linker handles static/dynamic linking differently
-    std::string full_static;
-    std::string static_stdlib;
-#elif defined(__APPLE__)
-    // macOS does not support full static linking (libSystem must be dynamic)
-    std::string full_static;
-    std::string static_stdlib = (f.staticStdlib && !isClang) ? " -static-libstdc++" : "";
-#else
-    std::string full_static = (f.linkage == "static") ? " -static" : "";
-    std::string static_stdlib = (f.staticStdlib && !isClang) ? " -static-libstdc++" : "";
-#endif
+    std::string full_static = (mcpp::platform::supports_full_static && f.linkage == "static") ? " -static" : "";
+    std::string static_stdlib = (f.staticStdlib && !isClang && !mcpp::platform::is_windows) ? " -static-libstdc++" : "";
     std::string runtime_dirs;
-#if !defined(_WIN32)
-    // -L and -rpath are ELF/Mach-O linker flags; MSVC linker doesn't use them.
-    for (auto& dir : plan.toolchain.linkRuntimeDirs) {
-        runtime_dirs += " -L" + escape_path(dir);
-        runtime_dirs += " -Wl,-rpath," + escape_path(dir);
+    if constexpr (mcpp::platform::supports_rpath) {
+        for (auto& dir : plan.toolchain.linkRuntimeDirs) {
+            runtime_dirs += " -L" + escape_path(dir);
+            runtime_dirs += " -Wl,-rpath," + escape_path(dir);
+        }
+    }
+
+    if constexpr (mcpp::platform::is_windows) {
+        f.ld = "";
+    } else if constexpr (mcpp::platform::needs_explicit_libcxx) {
+        f.ld = std::format("{}{}{} -lc++", full_static, static_stdlib, b_flag);
+    } else {
+        f.ld = std::format("{}{}{}{}{}", full_static, static_stdlib, sysroot_flag, b_flag, runtime_dirs);
     }
-#endif
-
-#if defined(_WIN32)
-    // Windows: Clang targeting MSVC links against MSVC runtime automatically.
-    // No -L/-rpath/-static flags needed.
-    f.ld = "";
-#elif defined(__APPLE__)
-    // macOS linking strategy:
-    // - No --sysroot: SDK .tbd stubs miss libc++abi exports.
-    // - No -L<llvm>/lib: xlings LLVM's libc++.dylib doesn't pull in
-    //   libc++abi. System /usr/lib/libc++ does (and is ABI-compatible
-    //   with LLVM 20 headers since macOS ships a recent libc++).
-    // - No -rpath for LLVM lib: binary should use system libc++ at runtime.
-    // - Explicit -lc++: clang++.cfg's -nostdinc++ suppresses implicit linkage.
-    // Result: compile with LLVM headers, link with system libc++ + libc++abi.
-    f.ld = std::format("{}{}{} -lc++", full_static, static_stdlib, b_flag);
-#else
-    // Linux: sysroot + runtime dirs needed (glibc/libc++ live in sandbox)
-    f.ld = std::format("{}{}{}{}{}", full_static, static_stdlib, sysroot_flag, b_flag,
-                       runtime_dirs);
-#endif
 
     return f;
 }