Wine - #3602
Closed
PeterNeiss wants to merge 86 commits into
Closed
Wine#3602PeterNeiss wants to merge 86 commits into
PeterNeiss wants to merge 86 commits into
Conversation
Scopes a Windows port with C++ builds as the driving use case, developed and tested on Linux via cross-compilation. Ten milestones across two repos (please + please-build/cc-rules), with five recorded decisions. M0 was measured rather than predicted, and the results reshaped the plan: - Compile blockers are 5 sites in 4 packages, not the ~11 the source survey suggested. They are layered, not parallel: src/process is a dependency of nearly everything, so a single `go build ./...` leaves 30 of 51 packages unchecked. - syscall.Exec, syscall.Chdir and the signal constants all compile on Windows (Go ships stubs returning EWINDOWS). They fail at runtime instead, which is harder to catch, not easier. - go-flags uses '/' as its option delimiter on Windows, which breaks Please's entire label syntax: //pkg:target parses as option /pkg with argument target. Fixed by -tags forceposix (D5). - src/output/shell_output.go reaches into cmd.SysProcAttr from outside src/process -- an abstraction leak the survey missed. - busybox-w64 ships a bash applet but rejects --noprofile/--norc, unlike Linux busybox. Configurable ShellArgs is a requirement, not a hedge. With those addressed, please.exe parses BUILD files and runs builds under Wine, including the find|sort|tr pipeline the cc rules depend on. M1 is re-estimated from 2-3 weeks to 1-2. probe/m1-skeleton.patch records the minimal changes used to get there. It is not an implementation -- its lock_windows.go is a no-op that would corrupt concurrent builds. No source files are touched by this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Two changes needed before anything else can be tested on Windows. src/cli/logging.go used path.Dir on logFile, which is a real filesystem path, not a build label. On Windows that returns "." for any backslash-separated path, so MkdirAll creates the wrong directory and plz dies at startup with "Error opening log file: ... Path not found". filepath.Dir is correct on every platform; this happened to be harmless on POSIX. .plzconfig_windows_amd64 sets BuildTags = forceposix for the go plugin. go-flags uses '/' as its option delimiter and ':' as its name/argument delimiter on Windows, so //pkg:target parses as option /pkg with argument target and //... is rejected outright -- every command taking a build label is broken without this. Note go_binary has no tags parameter; the go plugin reads CONFIG.GO.BUILD_TAGS, so this has to be config rather than a per-target edit, and scoping it to the arch config keeps it off other platforms. The file also carries the MinGW toolchain settings, empty defaultldflags (-lpthread and -ldl are both wrong there), and disables xattrs and the sandbox. Docs updated: D5 previously described forceposix as a BUILD-file change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
These are the four things that stopped plz compiling for GOOS=windows, plus the shell flags needed to make a build actually run there. Process control. Windows has no process group that descendants inherit, so killing a tree needs a job object: every process assigned to one dies together on TerminateJobObject, and JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE covers the case where we exit abnormally without cleaning up, which is what Pdeathsig gives us on Linux. SIGTERM maps to a Ctrl-Break on the console process group, keeping the existing graceful-then-forceful sequence in killProcess intact. There is an unavoidable race assigning the job after Start(); closing it would need CREATE_SUSPENDED, which os/exec gives us no way to do. Noted in the code. File locking. LockFileEx replaces flock. It locks a byte range rather than a file, so we take a single byte far past any content and leave the PID written in the lock file readable by other processes -- that is what produces the "process N has already acquired the lock" message. Unlike flock it cannot convert between shared and exclusive atomically, so we drop and re-take; acquireRepoLock only changes mode at startup so this isn't contended. src/output/shell_output.go was reaching into cmd.SysProcAttr.Setpgid from outside src/process. Replaced with process.ShareParentProcessGroup, so the platform detail stays in one package. clean's ForkExec becomes a detached exec.Command, with DETACHED_PROCESS on Windows so the async delete isn't killed with our console. Shell flags are now platform-specific: busybox, which is what we will ship as the Windows shell, rejects --noprofile and --norc outright. It reads no profile or rc files anyway, so nothing is lost. Note this is about the shell being invoked, not the host -- remote execution always talks to a real bash on the worker, so it keeps the full flag set via a new RemoteBashCommand. lock_test.go now uses the portable constants, which lets the core tests cross-compile. All twelve lock tests pass under Wine, including both mode transitions and the non-blocking contention case, so LockFileEx is genuinely excluding rather than silently succeeding. Linux behaviour is unchanged throughout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Tracker updated for the process, locking, clean and shell-args work. Also documents a trap that produced a false pass during M0: rm -rf plz-out is not enough to force a cold build under Wine, because Please's directory cache lives in the Wine prefix under AppData/Local/please. A build can look like it succeeded while replaying artifacts from an earlier, differently-built binary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
None of these stopped the Windows build: Go's syscall package ships Windows stubs for Exec and Chdir, and defines the signal constants, so every one of them compiled and would have failed only at runtime. That makes them easy to miss, so they are covered by running the binary rather than by the compiler. process.ExecReplace replaces the five syscall.Exec calls (plz op, plz tool, plz run, plz update, please_shim). On Unix it is still syscall.Exec. On Windows there is no way to replace a process image, so it runs the command as a child and exits with its status; stdout passthrough and exit codes 0 and 3 were verified under Wine. That difference has a consequence at the update call site. On Unix the exec drops the repo lock for us, because Go opens files O_CLOEXEC. On Windows we stay alive as the new process's parent, so the exclusive lock update holds would deadlock the binary it just launched. The lock is now released explicitly before handing over, which is what already happened implicitly elsewhere. Signal handling is now per-platform. Windows only ever delivers Ctrl-C as os.Interrupt and a synthesised SIGTERM; SIGHUP, SIGQUIT and SIGABRT are defined but never sent. The 128+signum exit convention is a shell idiom with no meaning there, so it reports a plain failure instead. core.LookPath was searching PATH entries split on a literal ":" and matching exact filenames, so on Windows it would neither split C:\foo correctly nor find bash.exe when asked for bash. It now uses the platform list separator via fs.SplitPathList -- promoted from the private helper that already existed for the FreeBSD fallback -- and tries the PATHEXT candidates from fs.ExecutableNames. Note the other ":"-splitting sites are untouched; those are a separate pass. Xattrs now default off on Windows, which has no equivalent; the fallback that writes separate files already existed. github.com/pkg/xattr needs no build tag since it ships xattr_unsupported.go. toExitError uses ExitError.ExitCode() rather than casting Sys() to a syscall.WaitStatus. Equivalent on Unix, including the -1 for a signalled process, and portable. The comment it replaces conceded there wasn't a good way to do this. Linux behaviour is unchanged throughout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Also corrects two things the design docs got wrong. isExecutable's 0111 check is only reachable on the FreeBSD code path, so it needed no Windows work at all; the real gap was core.LookPath, which neither split PATH correctly nor knew about PATHEXT. And fs.SplitPathList was promoted during M1 rather than M2, because LookPath needed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
The previous two commits were only ever checked with go build directly, which does not exercise the BUILD files at all. Going through plz found three problems. golang.org/x/sys/windows cannot be depended on unconditionally. go_repo generates the subrepo's BUILD files using the host build context, so on Linux no target is produced for that package at all -- the sources are extracted, but there is no BUILD file and the dependency fails to resolve. The deps are now guarded with is_platform(os = "windows"), which is already the idiom used in src/BUILD.plz. go_library filters srcs by build constraint, so the _windows.go files are dropped on other platforms and the dependency genuinely isn't needed there. src/tool and tools/please_shim import src/process now but never declared it. src/core's go_test had filter_srcs = False, with a comment referring to something that no longer exists. That is harmless while a package has no platform-specific sources and fatal once it does: the internal test compiles the package sources alongside the test sources, so lock_other.go and lock_windows.go were both fed to the compiler and every symbol collided. Removing it fixes the build and all 281 tests in the package still pass. Verified: plz build //src:please, plz build --arch windows_amd64 //src:please, and plz test //src/... --exclude=e2e (837 tests, 835 passed, 2 skipped). The cross-built binary parses labels and runs cold-cache builds under Wine, so forceposix is reaching the compiler through the real BUILD path. Note the windows_amd64 Go toolchain hash turns out not to be needed: Go cross-compiles from the host toolchain, so there is nothing to download. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
The windows_amd64 Go toolchain hash is struck off: Go cross-compiles from the host toolchain, so there is nothing to fetch. M1's exit criterion is met, and the M0 CI job's command already passes -- only the wiring is outstanding. Adds two risks found by actually running plz. go_repo generates third-party BUILD files with the host build context, so a Windows-only package like x/sys/windows has no target on Linux and cannot be depended on unconditionally. And verifying with go build rather than plz hides real breakage -- one pass through plz found three bugs. Also softens the arcat "hard gate": it did not block parsing or genrules under Wine, so it bites later than the plan implied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
PATH-style lists were split and joined on a literal ":" in six places, which on Windows would neither parse C:\foo nor produce a list anything can read. They now go through fs.SplitPathList and os.PathListSeparator. This is a no-op on Unix, where the separator is ":" anyway. src/remote/action.go is deliberately asymmetric: it splits with the local separator, because the value was built locally, but still joins with ":", because the worker on the other end is a POSIX machine. Same reasoning as RemoteBashCommand. Remote execution from a Windows host is not otherwise addressed here. fs.ExpandHomePath read $HOME directly and its regex assumed ":" separated PATH entries and "/" separated paths. It now uses os.UserHomeDir and builds the pattern from the platform separators, accepting either slash on Windows. The Unix pattern is byte-for-byte what it was. MachineConfigFileName was /etc/please/plzconfig; on Windows it resolves under ProgramData. DefaultPath is empty there rather than /usr/local/bin and friends: Windows has no equivalent directory holding build tools, so there is nothing honest to point at and users configure [build] path instead. Both became vars rather than consts, which is why SandboxDir did too. Build actions get USERPROFILE, TEMP and TMP alongside HOME and TMPDIR, but only on Windows -- native tools read those, and adding them unconditionally would change every target hash on Unix for no benefit. Verified under Wine that all five point at the action's tmp dir. Hash check: within a single working directory, the only targets whose hash changes are the dependency cone of the files edited here. cmap, cli, metrics and assets are untouched. Note that comparing hashes across two working directories is not a valid check -- they differ for unrelated reasons. 837 tests pass; the windows_amd64 cross-build still runs builds under Wine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Comparing plz hash output between a git worktree and the main repo is not a valid regression check: the same commit hashes differently in two working directories, producing dozens of false positives. Stash and unstash in place instead. Also confirms the forward-slash normalisation is still needed -- a genrule under Wine sees HOME with backslashes, which survives echo but will break any command that treats backslash as an escape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Build commands are shell strings, and a backslash is an escape character to
much of what runs in them. Expanding a variable is safe -- echo and printf
'%s' both round-trip a Windows path unharmed -- but passing one to anything
that interprets its arguments is not. Under Wine:
cmd = "echo placeholder | sed -e \"s#placeholder#$TMP_DIR#\" > $OUT"
turned Z:\...\plz-out\tmp\sedtest._build into Z:<TAB>mp...sedtest._build:
\t became a literal tab and every other backslash was eaten. That is not a
hypothetical shape -- the C/C++ rules build their link line with sed, which is
the primary use case for this port.
Win32, MinGW and busybox all accept forward slashes, so the environment now
uses them throughout. Normalising here rather than at each of the twenty-odd
places a path is written means new ones cannot quietly regress it.
It runs before withUserProvidedEnv deliberately: values the user wrote
themselves are left exactly as written, since they may not be paths at all.
No-op on platforms whose separator is already a forward slash, so Linux
behaviour and Linux build environments are unchanged. The hashes that move are
exactly the dependency cone of build_env.go; cmap, cli, metrics, assets and
version do not.
The test asserts the invariant on every platform. It is trivially true on
Unix, so it was checked by neutering the normalisation and confirming it fails
under Wine.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
The original note was right that this was the highest-risk detail but wrong about the mechanism. Shell variable expansion does not reprocess escapes, so echo and printf round-trip a Windows path fine; it is commands that interpret their own arguments, like sed, that destroy it. Replaces the speculation with the measurement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
…lege glob() returned nothing at all on Windows, which is fatal for a build system whose BUILD files are full of it -- including this repo's own. The cause is a separator mismatch. The walk goes through io/fs, whose paths are always slash-separated whatever the host, but patternToMatcher built the pattern with filepath.Join, so on Windows it produced globdir\*.txt and matched nothing against globdir/a.txt. toRegexString has the same assumption baked in, hardcoding / in its character classes. builtInGlob.Match had a subtler version of the same problem: filepath.Match treats the separator as a backslash on Windows, so * would have matched across / once the pattern was fixed, and a single-star glob would have wrongly recursed into subdirectories. Both now use path rather than filepath, which is what matching io/fs paths calls for. This is a no-op on Unix, where the two are the same. Verified under Wine: globdir/*.txt matches two files and correctly excludes the one in a subdirectory, and globdir/**/*.txt matches all three. Note the raw "/" handling elsewhere in glob.go and in fs/sort.go is correct for exactly the same reason, and was left alone. Separately: creating a symlink on Windows needs Developer Mode or SeCreateSymbolicLinkPrivilege, which an ordinary user does not have. CopyOrLinkFile now falls back to copying what the link points at, warning once, since for populating plz-out the content is what matters. And RemoveAll clears the read-only attribute on files as well as directories there, because that is what actually stops a delete on Windows. 838 tests pass. Hash changes are confined to the fs and core dependency cone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
The interesting finding was not on the list: glob() matched nothing at all on Windows, because the pattern was built with filepath while the walk yields io/fs paths, which are always slash-separated. That also inverts two entries the design doc got wrong. The raw "/" handling in fs/sort.go and parts of glob.go is correct precisely because io/fs paths are always "/", so those needed no change. Whether filepath or path is right depends on whether the value is an OS path or an io/fs path -- the opposite call from the logging.go fix in M1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
A cc_library + cc_binary + cc_shared_object triple now cross-builds from Linux to lib.a, prog.exe and libshared.dll, and prog.exe runs under Wine and links the static library correctly. The same targets still produce prog and libshared.so on Linux, and all 12 of cc-rules' own tests pass there. D1 is confirmed rather than assumed. Two MinGW toolchains -- WinLibs 16.2.0 under Wine and Ubuntu's 13 cross-compiler -- both match please_cc's existing GCC and GNU ld matchers, and the Clang matcher correctly does not, so no new matchers are needed. Ubuntu reports "13-win32", giving a bare "13", which MustParseVersion and Compare both handle. Three things the experiments taught that the design did not anticipate: - Module-level CONFIG does not see the target architecture. Defining the suffix as a constant silently had no effect, because these build defs are subincluded and CONFIG.OS there reflects the host. It has to be a function. - A repeatable config key cannot be cleared by assigning empty: "defaultldflags =" yields [""], which becomes a bare -Wl, and the linker fails with "cannot find : Invalid argument". - -lpthread is fine on MinGW; only -ldl had to go. The changes live in another repo, so they are recorded as a patch under probe/ with instructions to reproduce, until they can be upstreamed. Still open: please_cc has no windows_amd64 release (it is fetched prebuilt per platform), and UnitTest++ needs its Win32 sources, which blocks cc_test but not cc_library or cc_binary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Upgrades the severity of the arcat item and explains what the tool actually does, having tripped over it trying to run the M6 end-to-end test. arcat is Please's built-in archive toolkit -- extract, tar, and ar -- so rules never depend on the host having tar, zip, ar or unzip. The reason it matters more than "a hash to add" is that plugin_repo extracts the plugin zip with it, and every language plugin is delivered that way. Without it Windows cannot load the cc rules at all, and cc_library then needs it again for .a archives. Parsing and simple genrules work without it, which is why the first assessment understated it. The port itself is easy. arcat is six Go files with no syscall, x/sys/unix or cgo usage; it cross-compiles to PE32+, and under Wine both critical paths work -- arcat x extracts a zip, and arcat ar -r produces an archive that MinGW links into a working exe. The work is publishing a windows_amd64 release and recording its hash, not porting code. One unrelated snag found on the way: arcat's go.mod says go 1.17 while the code uses generics, so it fails to build on any platform with a modern toolchain. One-line upstream fix. M6's headline test is blocked on that release plus Wine having no working DNS here, so the plugin cannot be fetched from inside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Two bugs found by driving a real C++ build with plz.exe under Wine. Both are
the same mistake in different places: treating an io/fs path, or a path that
may still hold backslashes, as though it were already in the host's form.
buildFileName joined the package name and BUILD file with filepath.Join and
then handed the result to iofs.Stat. io/fs paths are always slash-separated,
so on Windows it looked for a single file whose name contained a backslash and
found nothing. The effect is that no package below the top level parses at
all: //sub:target and //sub/nested:deep both fail, and so does every plugin,
since the plugin subrepo's build_defs live in a subdirectory. Only the root
package worked, because filepath.Join("", "BUILD") has no separator to get
wrong -- which is why earlier testing missed it.
toolPath prepends ./ to a bare filename so the shell runs it rather than
searching PATH, deciding via strings.Contains(path, "/"). On Windows the path
may still be backslash-separated at that point, so an absolute path looked
bare and became "./Z:/tmp/.../please_cc.exe". It now checks both separators.
Note this is the inverse of the fix in src/cli/logging.go, where filepath was
the right answer. Which one is correct depends on whether the value is an OS
path or an io/fs path, and that distinction is worth checking rather than
assuming.
With these, the full chain works under Wine: plz.exe extracts the cc plugin
with arcat.exe, runs build actions through busybox, identifies the toolchain
with please_cc.exe, compiles and links with MinGW g++.exe, and the resulting
hello.exe runs and prints correctly.
838 tests pass and the windows_amd64 cross-build is unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
wine plz.exe now builds a C++ binary through an entirely Windows toolchain -- arcat.exe extracts the plugin, busybox runs the build actions, please_cc.exe identifies the compiler, MinGW g++.exe compiles and links -- and the result runs. Records the two bugs that had to be fixed to get there, both of which were invisible to every earlier test. The package-lookup one is the more alarming: no package below the top level parsed on Windows at all, and it went unnoticed because the root package is the one case where the buggy join cannot produce a wrong separator. Also notes two things a Windows user will hit immediately: .plzconfig rejects unquoted backslashes, and with DefaultPath empty on Windows nothing builds until [build] path is configured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Build actions are shell strings, and every local call site hardcoded "bash". Windows ships nothing that can run one, so [build] shell and [build] shellargs now select the shell, defaulting per-platform: bash with --noprofile --norc on Unix, and the bundled busybox's bash applet on Windows, which rejects those two flags and has no startup files to suppress anyway. BashCommand becomes a method on Executor so it carries the shell with it. RemoteBashCommand is untouched: the remote worker is a real bash whatever we happen to be running on, so it keeps the full flag set. The cmd cache's two "sh -c" sites and the shell that plz build --shell opens follow the same config; the latter was a third hardcoded shell that earlier passes missed. Resolution matters more than it looks. A shell that is on Please's own PATH is still left for the OS to find, exactly as before, but a name that isn't there falls back to the build path, which has Please's install directory prepended. Without that the bundling would be pointless, because nothing puts that directory on a Windows user's PATH. busybox-w64 is vendored as a remote_file with a pinned hash and installed alongside please.exe. It is GPL-2.0, which .plzconfig rejected outright, so that is accepted now with a note: Please execs busybox rather than linking it, so the two are separately distributed works and the release carries the licence. Also gated off Windows: tarball(xzip = True), since busybox's xz decompresses only, and please_sandbox, which is built on Linux namespaces. Verified under Wine with no configuration and nothing on the PATH: a genrule running "cat $SRCS | sort > $OUT" builds through the bundled shell, and fails correctly when it is moved away. Target hashes on Linux are byte-identical before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Records the five things this milestone turned up. The one worth remembering is that resolving the shell on $PATH alone would have made bundling it pointless: nothing puts Please's install directory on a Windows user's PATH, so the default would never have been found. Also notes that the busybox bash applet form behaves identically to the renamed bash.exe that M0 tested, which is why ShellArgs selects the applet rather than the packaging renaming the binary and shadowing a user's real bash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Makes `plz build --arch windows_amd64 //package:release_files` produce something a Windows user can actually unzip and run, and wires the CI job that will do it. The release is a .zip, since Windows has no guaranteed tar, holding please.exe, busybox.exe, build_langserver.exe and a plz.cmd shim. The shim replaces the `ln -sf please plz` that install.sh does on Unix, because symlinks on Windows need Developer Mode. The xz tarballs are gated off Windows and the zip stands in for them; please_sandbox is gated off too, being Linux namespaces throughout. The .exe suffix had to be asked for per target. go_binary names its output after the rule, so //src:please produced a file called `please`, which cmd will not run and LookPath will not find. The general fix belongs in the go plugin. Self-update needed two changes beyond that. Symlinking is replaced by a per-platform linkFile: Windows hard-links, which needs no privilege on NTFS. And because a running executable can be neither deleted nor written over, and the file being replaced is usually the Please doing the replacing, a file that cannot be removed is renamed aside to .stale, which the next run sweeps up. pleasew.ps1 is the PowerShell counterpart of pleasew, written to a repo by plz init alongside it - a repo is often worked on from more than one platform, so picking by host would leave a Linux developer no way to set one up for their Windows colleagues. There is no PowerShell on the Linux host, so it has been reviewed but not executed anywhere yet. Verified by extracting the zip as a user would and running plz.cmd under Wine: it builds a genrule with no configuration at all, the shim finding please.exe and please.exe finding busybox.exe beside it. `query alltargets //...` works too, which is the forceposix smoke test. Still outstanding for the milestone: arcat has no windows_amd64 release, so no plugin can load there yet, and the builder image needs pushing before the CI job can run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Everything in the release pipeline is built and wired; what is left is a dependency on another repo. arcat has no windows_amd64 release, and since every language plugin is delivered as a zip that arcat extracts, no plugin can load on Windows until there is one. That is now the single thing between here and the exit criterion. Records that the .exe suffix had to be asked for per target rather than coming from the go plugin, and that plz init now writes both wrapper scripts on every platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Running the unit tests under Wine for the first time turned up four bugs that share a cause: code that operates on repo-relative or label-derived paths was using filepath, whose separator on Windows is a backslash. Every one is a correctness bug, and none of them shows up on Linux, where the two are the same. Globs crossed package boundaries. isBuildFile called filepath.Base on a path that came from io/fs, which is always slash-separated, so on Windows it compared the whole path against "BUILD" and never matched. No subpackage was ever detected, and a glob in one package would happily swallow files belonging to another. The initial package was wrong whenever plz ran from a subdirectory. getRepoRoot returned it with backslashes, which are illegal in a package name, so the label failed validation and Please walked up until something parsed - usually the repo root. Relative labels like path/to:thingy failed outright for the same reason. $(location), $(exe), $(worker) and tool paths expanded with backslashes. These go straight into a shell command, where a backslash is an escape character; the design notes measured sed silently turning \t into a tab. The environment was already normalised, but these are not environment values. The last one changes a decision rather than fixing a slip. plz-out paths are now built with path rather than filepath, so they are slash-separated everywhere. The design doc argued for normalising only at the environment boundary on the grounds it was the smaller change; that turned out to leave the replacements above broken, and the tests already assumed slashes throughout. Win32 accepts either separator, so nothing is given up. This is a no-op on Unix. Also makes the tests that were asserting Unix semantics say what they mean: home paths through os.UserHomeDir rather than $HOME, path lists split on os.PathListSeparator rather than a colon, and LookPath looking for a tool the test wrote itself rather than the bash the host is assumed to have. TestSymlink skips on Windows, where creating one needs Developer Mode and Wine reports success while producing a link it can't stat. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Until now everything about the port's runtime behaviour was checked by hand. This makes it a test target: wine_go_test runs a Go test binary cross-built for Windows, and wine_plz_test runs the cross-built please.exe against a small repo laid out the way the release is, with busybox.exe beside it and nothing on the PATH. Four targets to start: the src/core and src/fs unit tests, the shell smoke test from the design notes - a build action with a pipe and a redirect, so the bundled shell has to work - and a query, because a dropped forceposix tag breaks every build label and is invisible in Please's own source. Finding them at all needed one thing recorded here rather than fixed: Go's exec package on Windows will not run a file whose name has no extension in PATHEXT, even when handed its full path. The go plugin names test binaries after the rule, so the macro copies each one to a .exe before running it. Without that, any test whose subject re-execs itself fails obscurely. They are labelled wine and excluded from the other passes, because building them means cross-compiling the Go standard library for another platform - too much to impose on someone who wanted the unit tests. test.sh runs them as a third pass where wine is installed and says so where it isn't, and the CI job is blocking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Running the unit tests under Wine properly for the first time found four correctness bugs, all the same shape: filepath used on paths that are slash-separated by definition. M2 recorded the inverse of that lesson and fixed the producers; these were the consumers it missed. The decision that changed: normalising path separators only at the environment boundary, which 02-shell-and-build-actions.md chose on the grounds it was the smaller change. It isn't, because $(location) and friends are not environment values. plz-out paths are slash-separated throughout now. Also corrects the testing strategy's prediction about symlinks under Wine. It assumed Wine grants the privilege unconditionally so the copy fallback goes untested. Wine actually reports success and produces a link it cannot stat, so Wine tells us nothing either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Setting [sandbox] build or test on Windows used to produce "Can't find sandbox tool please_sandbox on the path", which invites you to install something that does not exist and cannot. The defaults were already false, so the milestone's real work was this message. Please now says sandboxing is not implemented on the platform and that actions will run without isolation, and builds an executor that does not claim to sandbox rather than one that silently doesn't. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
The event loop compares the paths it recorded against the ones fsnotify reports back. Ours are slash-separated, coming from build labels; fsnotify on Windows reports backslashes. Nothing ever matched, so every event was discarded as belonging to a file we weren't watching and the watch simply never fired. It fails silently, because a discarded event looks exactly like an unrelated file changing, and it is logged at a level nobody runs at. Both sides go through watchKey now. The design notes had this down as documenting fsnotify's Windows limits. It isn't a limit, it's a bug on our side, and the test that guards it only means anything when run on Windows, so it goes in the Wine job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
translateOS needed nothing - windows already passes through the default branch, so that is recorded rather than changed. The src/watch item was down as documenting fsnotify's Windows limits and was a silent bug on our side instead. Also notes that the go plugin's .exe naming now blocks more than it looked like: plz run fails on any go_binary until it lands, because Go's exec on Windows will not run a file with no PATHEXT extension even given its full path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
Four more of the same shape as the last batch, all found by pointing the Wine job at more packages. output_dirs produced doubled paths. copyOutDir strips the temp directory off a path to get an output name, comparing a filepath.Join result against a slash-separated TmpDir. Neither prefix matched on Windows, so the whole path survived as the output name and moveOutputs then joined the temp directory onto a path that already contained it. JS coverage file names were never sanitised, because the paths from the coverage file were compared against filepath.Dir of a plz-out directory. Coverage came out attributed to absolute build paths rather than source files. Coverage by directory had backslashed keys for the same reason, which neither read correctly nor matched anything configured. file:// URLs could not name a Windows path at all. RFC 8089 puts a slash before the drive letter, so file:///C:/foo arrives as /C:/foo, which filepath.IsAbs rejects. No remote_file with a local URL could work. Two tests also had to stop writing to the real home directory. They left a read-only file at ~/secret, which on Windows the next run cannot replace; they now point the home directory at somewhere they own. The tests that need working symlinks or Unix permission bits skip on Windows and say why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
//src/build, //src/test, //src/cli and //src/watch join //src/core and //src/fs: 428 tests, 424 passing and 4 skipped. //src/build is the valuable one, since it runs real build actions and so covers the process layer and the bundled shell as well as whatever it is nominally about. wine_go_test grows a needs_shell option that puts busybox next to the test binary, the way an install has it. Two things about the harness itself, both found by tests failing for reasons that had nothing to do with Please. Wine ships a hosts file with the localhost line commented out, so anything resolving it hangs until it gives up - three tests were each burning fifteen seconds. And ~ resolves inside the shared prefix, so anything a test writes there leaks into the next run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MkSQhgMeCVmuWqbGAP7oVM
The only test that covers what a Windows user actually gets. It extracts the real artifact rather than assembling an install out of its parts, because what is under test is what the artifact carries. One sh_binary exercises the whole chain: the file:// plugin template, the bundled arcat unpacking the archive, the plugin's build defs parsing, and busybox running the action. The repo it builds in asks for its plugin the ordinary way, with the revision the plugin is released as rather than the one that is bundled, so the thing being tested is only where the plugin came from. The negative control is what makes this rigorous rather than suggestive. With the network gone, taking the archive away has to break the build; if it does not, the archive was never read and the passing test meant nothing. The network is denied with a proxy pointed at a closed port, which proves there was no HTTP egress rather than no egress at all. That is the right scope, since fetching a plugin is an HTTP fetch, and it is what there is: Wine aborts outright inside a user namespace, so unshare is not available to make it airtight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
08-offline-release.md is the design and the record of building it: why the plugin archives are generated rather than committed, why they cannot come from a build rule, why the payload has to be flat files, and what the filenames carrying no revision costs. Marked implemented, with the two things that are not done - a python tier for the offline test, and an airtight network denial, which Wine cannot give us. Two traps join the standing list. plz-out/pkg is never refreshed once it exists, because the hlink label goes through LinkIfNotExists and the destination is named after the version, so rebuilding a release at the same version silently leaves the previous bytes there. And running an sh_binary in place complains on Windows, because its payload unpacks over the read-only build outputs it was made from. M4 no longer waits on a published arcat, since arcat is built from source now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
Windows has no installer, no package manager and no post-install script, so the instructions travel in the archive. Extract, put the directory on PATH, run plz. It covers the things that are specific to this platform rather than repeating the manual: that plz.cmd stands in for a symlink because those need Developer Mode, that the files have to stay together because Please finds its shell, its archive tool and any bundled plugins beside its own binary, and that language toolchains are not bundled and never are. The limitations are the ones a user hits first, all measured here: plz update fetches only the binary and leaves everything else at the version first installed; a repo pinning an exact [please] version tries to download a Windows release that does not exist yet, which is a hang and then a hard failure; sandboxing is off; antivirus locks freshly written files; and output paths nest deeply enough to want long-path support. release_shape_test asserts the zip holds exactly the expected members and that each plugin archive really is the single top-level directory with a .plzconfig that plugin_repo expects. It runs on Linux without Wine, so it catches a tool that lost its .exe, or a file quietly dropped from the release, without waiting for a machine to run them on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
//test/windows already cross-builds twenty-odd Go test binaries and runs them under Wine. Those same binaries can run natively, which turns "passes under Wine" into evidence from Windows for almost no new test code. This packages them. The list of tests now has one definition, because it is consumed twice: by the wine_go_test comprehension and by the bundle. Target names are unchanged, so CircleCI and --include=wine are untouched. Each test gets its own directory rather than sharing one tree. The data is a few hundred megabytes, so copying it per test on the runner to isolate them would be most of a gigabyte of IO; the tests have to be isolated, because running these binaries in a shared tree once deleted the whole of test/; and several of them want the harness directory itself rather than only their data. The data lands where the test expects it for free. An architecture subrepo has an empty root and package root, so ///windows_amd64//src/fs:test_data stages at src/fs/test_data, which is the literal path the test opens. bundle_smoke_test runs one entry out of the assembled bundle, under Wine, from the bundle's own layout. It is the only check on the packaging that does not need a Windows machine, and packaging is the part most likely to be quietly wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
run_native_tests.ps1 is wine_go_test without Wine: the test's own directory as the working directory, $DATA from the file the bundle records beside each binary, and the bundled busybox on the PATH where the marker says the test runs build actions. The environment it sets mirrors core.TestEnvironment, with forward slashes, because Please rewrites every backslash in every environment value on Windows. Getting that wrong produces failures that look like port bugs and are not. It also redirects LOCALAPPDATA, which Please does not: without it a test that runs a build shares the machine's directory cache, and a cached artifact from another run is exactly how this port has produced a false pass before. It parses -test.v itself and writes a table to the step summary rather than pulling in a third-party action for cosmetics. src/test/go_results.go is the right parser and has no standalone entry point; that is where a plz test-shaped native runner would eventually go. known_failures.txt is empty on purpose. The first native run is what fills it in, and a test listed there that starts passing fails the job too, so the list only ever shrinks. run_native_probes.ps1 covers the release as an artifact and the two Wine-invisible classes that are reachable from a headless runner: files held open on teardown, probed by building and cleaning repeatedly under a live virus scanner, and path length. The interesting long-path failure is not in Please, since Go prefixes absolute paths with \\?\ by itself, but in what Please hands busybox as a command line - which is exactly the pipe-and-redirect action the fixture builds. Case-insensitivity and the symlink copy fallback are deliberately not here. They belong in Go tests in src/fs, where they ride the bundle and are written in the language the fix will be. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
The first thing anywhere that runs this port on a real Windows machine. Two jobs: a Linux one that cross-builds the test bundle and the release, and a windows-latest one that runs them. It cross-builds its own artifacts rather than taking CircleCI's, because a CircleCI workspace is scoped to one CircleCI run and cannot be read from here. That duplication is the price of CircleCI having no Windows runner in this config. In exchange every pull request now produces a downloadable Windows build. It builds this repo's Please first and uses that for everything after. The released one predates the parse-deadlock fix and hangs parsing //test/windows, silently and for ever - the same two-step test.sh insists on. The Windows job is advisory to begin with. The first run's job is to produce a failure list, and a check that goes red before anyone has read it teaches people to ignore it. Once that list is in known_failures.txt the flag comes off and the job blocks, because an advisory Windows job is worse than none: the port's whole problem is that nobody is thinking about Windows. It prints LongPathsEnabled and SeCreateSymbolicLinkPrivilege before running anything. Both change what is reachable, both differ between runner images, and guessing either has already cost time here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
Any plz command run on Windows outside a repo hung at 100% CPU instead of saying it could not find a root. The walk up towards the filesystem root stopped when the directory became an empty string, and on Windows it never does: trimming the separator off "C:\" leaves "C:", and splitting that returns it unchanged, because the volume name is the whole path. One stat per iteration, for ever. It now stops when the walk stops going anywhere, which is also correct for UNC roots and needs no build tag. This is what hung exec_test on the first native run - the only test in that package that calls MustFindRepoRoot - and it is the one finding from that run that nobody had predicted. Wine never showed it because the harness happens to run those binaries inside this repo, where the walk finds a .plzconfig after a few levels. The walk is split out so it can be tested from a given directory rather than only from the working one. The termination test fails rather than hanging, since hanging is exactly what it is guarding against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
plz clean failed every time on Windows, in both of its modes: the background rename of plz-out got Access denied, and the synchronous fallback got "the process cannot access the file because it is being used by another process" on plz-out/log/build.log. That file is Please's own. --log_file defaults to a path under plz-out, and InitFileLogging keeps the handle in a package global for the life of the process; its only close was behind AtExit, which runs on a terminating signal and never on an ordinary exit. So clean asked Windows to delete a directory containing a file it was holding open. The detached child that does the deletion already avoids opening a log at all, for this reason; the parent now closes its own first. RemoveAll also handles the case properly rather than reporting it as a permissions problem. It used to run a chmod walk over the whole tree - leaving every build output writable - and then fail again for the same reason, since Go maps a sharing violation to ErrPermission. It now recognises one, retries briefly because virus scanners hold freshly written files open for a moment, and says what the problem actually is when the handle is not going away. The risk register said to design RemoveAll defensively for this and it had not been done. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
The guard that stops a remote_file reaching inside the repo never fired on Windows. It compared the path out of a file:// URL, which is slash-separated, against core.RepoRoot, which is in the OS's own separator and so backslashed there. A plain HasPrefix between the two never matches. The test did not catch it under Wine either. TMP_DIR is a Linux path there, so the earlier absolute-path check failed first and the guard was never reached. It has never been exercised until a real Windows machine ran it. The same comparison appeared twice more, both on paths that arrive from outside and so may be spelled either way: the coverage file list, and the filenames in a coverage report written by another tool. All three now go through one helper that normalises both sides, and that only matches at a path boundary, so /repo is not a prefix of /repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
plz run appended a second PATH rather than replacing the one it found. Windows stores the variable as Path, and addOneEnv compared names exactly, so "PATH=" matched nothing. os/exec papers over it by deduplicating case-insensitively and keeping the last entry, so the child process was fine, but everything else that reads that slice saw both - ExecReplace and the audit log among them. Names are now compared the way the platform does, and the entry keeps the OS's own spelling rather than ours. The test was wrong in the same place and in a way that hid this: it asserted the literal string "PATH=<value>" against os.Environ(). It now looks the variable up by name and asserts there is exactly one of it, which is the part that actually guards against appending a duplicate. TestCheckSecrets used /bin/sh as a file that definitely exists. Windows does not have one, and Wine passed it only because its Z: drive maps the host's root, so the assertion proved nothing there. It makes a file now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
Several of the bundled tests call MustFindRepoRoot. Under Wine they get a root by accident, because the harness runs them inside this repo. Natively the bundle sits wherever the runner unpacked it with nothing above it, so they would fail on a missing root rather than on whatever they are about. The bundle now carries a .plzconfig, which is the same thing Wine was giving them for free. The Windows job was advisory for exactly one run, to produce a failure list without a red check nobody had read yet. Everything that run found is fixed, so the flag comes off. Anything genuinely left over belongs in known_failures.txt with a reason, not behind a flag that makes the whole job ignorable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
upload-artifact drops hidden files unless told not to, so the .plzconfig that marks the bundle as a repo never reached the Windows runner. The tests that call MustFindRepoRoot found no root and died. Nothing on the Linux side could have caught this: the Wine test runs the bundle straight out of plz-out, so it never passes through an artifact at all. It only shows up in the round trip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
05-testing-strategy.md's "What Wine does not cover" was a list of predictions. It now carries a table of measured results with a date against each one. The predictions were mostly right: the sharing violation materialised and broke plz clean outright, on Please's own log file. Long paths did not reproduce, because the runner has them enabled. The symlink privilege is disabled there, so the copy fallback has been exercised on every run since. The list also missed one, and it was the worst of the five: every plz run outside a repo hung at 100% CPU for ever. Nothing predicted it because nothing had ever run plz outside a repo on Windows. Three new standing traps, all of which cost time here: filepath.Split does not terminate a walk at a drive root; a handle this process holds is still a handle, and the log file lives inside the directory clean deletes; and upload-artifact drops hidden files, which no Linux-side test can catch because the Wine tests never pass through an artifact. M9's first two items tick. Console and Ctrl-C stay open and are now explicitly out of reach from a CI step rather than merely undone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
M5's last open item. The cc plugin can now build its own targets with the configured toolchain, so this is the proof: a cc_test linked against the same cc_shared_object the DLL test uses, cross-built and run under Wine. It passes. The MinGW C++ runtime travels beside it. Those DLLs cannot simply be linked in: -static-libstdc++ and -static-libgcc are driver flags, the plugin wraps linker_flags in -Wl, so ld gets them and rejects them, and a target's compiler_flags reach the compile step but not the link. They come from the compiler itself rather than being pinned, because they have to match the compiler that built the binary. A cc_binary doing less C++ never needed them, which is why the DLL test did not find this. The assertion is the exit code. UnitTest++ writes XML to test.results and prints nothing when everything passes, returning the number of failures; a binary that cannot start at all exits 53. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
Both were recorded as fact and both were wrong, which is worse than being unknown because they stopped anyone looking. cc_test was blocked on UnitTest++ needing its Win32 sources, said three files. The plugin has selected them since before this port began. The real cause was that the plugin's own targets compiled with the host toolchain, and finding that took ten minutes once someone actually ran the build instead of reading the note. M5 is done. exec_test was listed as unreached past TestCommandMountNotSandboxed. That was true before the repo-root fix and stopped being true when it landed; all ten of its tests pass natively. The pick-up list is re-ordered around what the native run actually shows: three symlink tests that skip on the one machine where the answer is interesting, two plz run tests that skip because their fixtures are shebang scripts, and a Ctrl-Break path that is exercised but never distinguished from the job object killing everything anyway. Also corrects two stale facts: the branch is merged, and the plugin branches now live in forks rather than only on this machine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
Every link: label a build declares was quietly turning into a warning on Windows. Creating a symlink there needs Developer Mode or SeCreateSymbolicLinkPrivilege, an ordinary user has neither, and buildLinks passed os.Symlink straight through to a helper that logs the failure and carries on. So plz-out/please and plz-out/go/src simply did not appear, with nothing louder than a warning to say why. They now fall back to a copy, which is the same trade CopyOrLinkFile already makes: for populating plz-out the content is what matters, not that the link is reproduced. The three tests that cover this skipped on all of Windows, so they skipped on the one machine where the answer is interesting - the CI runner has the privilege disabled, which is the case most users are in and the case the fallback exists for. They now skip only under Wine, where os.Symlink reports success and produces a link os.Lstat cannot find, so asserting against it proves nothing either way. fs.IsWine is how they tell, via a function only Wine exports, which is the way Wine documents for this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
TestSequential and TestParallel skipped on Windows because their fixtures are two shell scripts relying on a #! line, and Windows has no such mechanism. That was the whole of the plz run coverage gap: the code was never in question, only the fixtures. They now have .cmd siblings, which is the same answer the shell plugin reaches for an sh_binary on Windows - a batch file is the only script the platform runs by name. Both tests pass under Wine instead of skipping, and will run natively. .gitattributes stops any of this being translated on checkout. cmd.exe is not reliable on LF-only input, and the fixtures under test/windows are compared byte for byte against output the bundled busybox wrote, which does no translation either. The Windows CI job sets core.autocrlf input for the same reason; this makes it hold for anyone who clones without that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
plz run on an sh_binary failed on Windows with
'plz-out' is not recognized as an internal or external command
An sh_binary is a .cmd there, a .cmd runs through cmd.exe, and cmd.exe reads a
forward slash as the start of a switch - so plz-out/bin/x.cmd is the command
"plz-out" with two switches after it. Please builds that path slash-separated, as
it should everywhere else.
Only the executable's own path is converted, at the point it is handed to the OS.
Wine's cmd parses it happily, which is why this survived every Wine run and
appeared the moment the two tests behind it stopped being skipped. It is the
second time this port has found a bug by deleting a skip rather than by adding a
test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
809 tests now run on a real Windows machine, up from 800, and the two that still skip there are skipped on every platform and always were. Getting from seven skips to two found two real failures that Wine had been passing for months. plz run handed cmd.exe a forward-slashed path, which it reads as a switch, so it could not launch an sh_binary at all. And every link: label silently became a warning, because buildLinks passed os.Symlink straight through while CopyOrLinkFile had had a fallback all along. That is now a standing note in its own right: a skip hides a bug better than a missing test does. Both of these sat behind runtime.GOOS == "windows" skips that looked perfectly reasonable when they were written. Ctrl-Break stays open, with the reason it is harder than it looks: KillProcess gives the graceful path 30ms before terminating the job regardless, so a test asserting graceful shutdown races that timer on a CI machine, and a flaky test in a blocking job is worse than no test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
The Windows fixes for all four plugins are on GitHub now, so the machinery that existed only because they were not can go. That was more than a pin: a local-checkout switch in plugins/BUILD, a vendoring script, a gitignored archive directory, a bundled payload in the release, a file:// plugin repo default in the binary, and eight tests that only existed when a checkout was configured. Pinned to commit SHAs rather than to the windows branch. A branch archive changes whenever it is pushed to, which would silently move every build hash that reaches it and leave the cache serving something else. The gain is larger than the deletion. Those eight tests are now unconditional - both pex tests, the DLL test, the cc_test, the sh_binary test - so they run in CI on every change instead of only on the one machine that had the checkouts. The two //test/export failures go too: they only ever failed because .plzconfig.local was present, which it no longer needs to be. Four workarounds this repo carried because the old pins lacked the fixes are gone with it: the out = "please.exe" overrides in src/BUILD.plz and //tools/build_langserver, and the cc toolchain and defaultldflags lines in .plzconfig_windows_amd64. The plugins work all of that out themselves now, so keeping them would be second-guessing a plugin that is right. PexTool stays. It builds please_pex from the plugin's source because no published release carries the Windows preamble, and the fork publishes no releases at all. arcat stays in the release too, and has to: it is built in-tree from the module proxy, and without it a Windows plz cannot extract a downloaded plugin. What goes with the bundling is please_go.exe, please_cc.exe and please_pex.exe, which were built inside the checkouts and cannot be built from here - cross-compiling a plugin's own tool collides on subrepo names. So a native Windows plz can fetch and parse plugins but not build a Go, C++ or Python target until those three have releases to download. Cross-building from Linux is unaffected. That is now the third item in what to pick up next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
A native Windows plz could not build a Go, C++ or Python target, because please_go, please_cc and please_pex have no windows_amd64 release to fetch and cannot be built from the repo using the plugin - cross-compiling a plugin's own tool collides on subrepo names. Please's release carried copies for a while, but that only helped people who had that release. All three are now published from the forks and downloaded like any other platform. Only windows_amd64 is redirected there; everything else still comes from please-build, so when upstream publishes its own the redirect goes away and nothing else changes. Verified by fetching each asset and checking the hash the BUILD file pins. Each download needs an explicit out on Windows: the asset name carries the version and platform, which leaves it with no extension in PATHEXT, and Go's exec will not run such a file even when handed its full path. PexTool stays in .plzconfig_windows_amd64 and now has a narrower reason. The Windows please_pex has the preamble, but a Linux host cross-building uses the Linux one, and that is still the upstream build without it. Publishing a Linux please_pex from the fork is what would retire the override; it is now the third thing to pick up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
The last of M9. Everything technical was done; what was left was the part that decides whether anyone outside this repo can use it. docs/faq.html said Windows was not supported natively, which had been the honest answer for years and stopped being true. It now says what is supported and names the two things that genuinely behave differently: there is no build sandbox, because Please's is built on Linux namespaces and Windows has no equivalent, and real-time virus scanning holds freshly written files open, which Windows treats as a reason to refuse deleting them. get_plz.ps1 is the counterpart to get_plz.sh, served and signed from the same bucket by the same release script, and run with irm ... | iex since Windows has no shell to curl into. It is deliberately parallel to the sh version; the differences are all forced by the platform - a .zip rather than a tarball, a plz.cmd shim rather than a symlink, and hard-links rather than symlinks to link the install up a level. The changelog entry leads with the feature and then lists the bugs a real Windows machine found, because those are what someone upgrading will recognise: plz clean failing every time, plz hanging outside a repo, link: labels silently doing nothing. Also corrects two statements in the state of play that had gone stale within the hour: plz run on an sh_binary is fixed and tested rather than open, and nothing is blocked on push access any more. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
.plzconfig_windows_amd64 pointed PexTool at the plugin's own source, because the released Linux please_pex has no Windows preamble and a Linux host cross-compiling uses the Linux tool. A .pex is a zip with an executable stub in front, so what came out was an ELF-prefixed file Windows would not run. The fork now publishes a Linux please_pex that carries the preamble, so the override is gone and the pex tests pass without it. That leaves .plzconfig_windows_amd64 with nothing but genuine platform facts: the forceposix build tag, because go-flags reads / as an option delimiter and would break every build label; no extended attributes; and no sandbox. Every entry that was compensating for a plugin has now been fixed in the plugin instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
The Windows support was real and unreachable at the same time. Upstream publishes to a GCS bucket from a CircleCI job that only runs on thought-machine/please, so nothing this branch produced could be downloaded by pleasew.ps1, get_plz.ps1 or plz update. A working port nobody can install is not a working port. The release workflow builds the same artifacts and publishes them as a GitHub Release on this fork. The asset names carry the platform, which is what gen_release.py already does for the GitHub half of an upstream release, so the two agree on names even though they disagree on paths. Both installers now understand either layout and pick by looking at the base URL: a release keeps everything under one tag with the platform in the filename, the bucket keeps a directory per platform and version. Setting [please] downloadlocation, or PLZ_DOWNLOAD_BASE for get_plz.ps1, switches back to the bucket. That is what makes this revertible in one line if upstream ever starts publishing Windows builds. Numbered 18.0.0 rather than 17.34.0. Native support for a new operating system is not a patch, and this fork's numbering is its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EnBWtZhT4ReP23CYYbfkRa
Nothing anywhere had ever executed a line of the codelabs at https://please.build/codelabs.html, on any platform. This adds a blocking codelabs job to the Windows workflow that replays all eight on windows-latest with the release zip, the way a reader would. The steps are extracted from docs/codelabs/*.md rather than transcribed, so the check cannot drift from the published pages. A block no rule can classify is an error, and //test/windows/codelab_script/script:script_test checks that against the real codelabs on Linux. What the Markdown cannot say lives in test/windows/codelab_steps.conf, each stanza with a reason and pinned to the text it was decided about. No codelab is edited. Four failures are listed ahead of the first run, from facts checked directly: plz init plugin writes upstream plugin_repo targets whose please_go and please_pex have no windows_amd64 release, Puku has none either, and a bash environment prefix is not PowerShell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
The first native run showed the harness manufacturing a failure. plz init plugin go already writes GoTool, the Go codelab's fragment sets it again, and appending the fragment verbatim left a plugin section with a repeated key, which Please refuses. A reader edits the key instead, so the runner now does too: a key the section has is replaced, a new key joins its section, and a new section is appended. Subsection names stay case-sensitive. Also from that run: a transcript's output is attached to the command it follows rather than to the block's last command, and a failing command's errors are plain text rather than CLIXML. Two genuine Windows failures join the known list. genrule's plz run of a #!/bin/bash tool fails because Windows runs nothing by shebang, and plz_query's cloned repo has no please_go on Windows and asks golang.org for a Go 1.20 .tar.gz that does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
With fragments merged key by key, the second run left one failure nobody had listed, and it is not a Windows failure. plz init plugin go generates a go_stdlib in third_party/go/BUILD and points STDLib at it; the Go codelabs' own third_party/go/BUILD holds only a go_toolchain, so following them drops the stdlib and every Go build fails to find //third_party/go:std. The Kubernetes codelab stops there, and go_intro hits it ahead of its Windows failures. The state of play now says what the runs found: only using_plugins can be followed to its end on Windows, and every entry in the known-failures list carries the log line behind it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
Replay the codelabs on Windows, and record what they cannot do
Author
|
My AI is doing something wrong, sorry. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.