Windows port, rebased onto upstream #3569 (CI only, do not merge) - #8
Open
PeterNeiss wants to merge 106 commits into
Open
PeterNeiss wants to merge 106 commits into
PeterNeiss wants to merge 106 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
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
… the forks Windows has no #! mechanism, so plz run could not start a script built the Unix way, which is how the genrule codelab builds its word-count tool: "%1 is not a valid Win32 application". plz run now hands an extensionless #!/bin/sh or #!/bin/bash script to the shell build actions already use. Build actions needed nothing: busybox reads the #! line itself, which a new smoke-repo target checks under Wine and on windows-latest. plz init plugin wrote please-build plugin_repos, whose tools have no Windows release. go, cc, shell and python now come from the PeterNeiss forks at the revisions plugins/BUILD pins, with no call to GitHub's API; a test keeps the two lists in step. An explicit --owner or --version still looks up tags, now only vX.Y.Z tags, highest first, across every page, with GITHUB_TOKEN when set and a message that names the rate limit. Previously the first tag listed won, which for python-rules is wheel_resolver-v2.1.0. plz init plugin go also overwrote GoTool with the stdlib's label when a go_stdlib already existed, leaving STDLib pointing at nothing. The codelab check accepts PowerShell blocks, for the Windows forms to come, and skips tree -a, which is tree.com on Windows and exits 0 on a switch it rejects. The genrule entry leaves the known failures with this fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
go-rules and python-rules move to fork commits that build on a Windows host, here and in plz init plugin's pins. go_toolchain downloads Go's .zip there and names its tools with .exe (52d9e0f); the build interpreter defaults to python, since a Windows install has no python3 (eecfd15). The Go codelab's third_party/go/BUILD now declares the go_stdlib its .plzconfig points at; it replaced the one plz init plugin go generates, on every platform. The Go and Python codelabs give a Windows form of [build] path, PassEnv = PATH, since the default path there is empty, and config.html says so too. The Python codelab's numpy moves to 1.26.4, the first with wheels for Python 3.12. plz_query clones the PeterNeiss fork of please-codelabs, which pins the same go-rules fork. Their three entries leave the known failures with these fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
Puku published no Windows build, so plz puku failed with a 404. The PeterNeiss fork now publishes puku-1.17.1-windows_amd64 from the upstream v1.17.1 source, and the codelab's remote_file downloads from the fork on Windows, as puku.exe, and from please-build everywhere else. puku-version moves to 1.17.1 to match. Its bash-only lines, GODEBUG=... and GOTOOLCHAIN=local ..., gain PowerShell forms, and [build] path gains its Windows form, PassEnv = PATH. The replay runs those forms and skips the bash ones, which is what a Windows reader does too. Both Puku entries leave the known failures; only the out-of-scope k8s one remains. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
…hout go.mod The fourth native run of the codelabs reached further and found three more things, none of them in the codelabs' text. plz test //src/... panicked on Windows with "Invalid package name: src\". The walk for ... returns paths with backslashes, and only "/" was trimmed from the directory before it became a package name. Go on Windows finds its build cache under LOCALAPPDATA, which build actions did not have, so every go command in one failed with "GOCACHE is not defined and %LocalAppData% is not defined". LOCALAPPDATA and APPDATA now point into the build's directory, as HOME, USERPROFILE and TEMP already did. go-rules moves to 9c26bfd, which installs the standard library with GO111MODULE=off. In module mode it read the repo's go.mod from inside the build directory, and a go.mod from a newer host Go stopped the Go 1.20 toolchain cold. The genrule codelab's default tool, plain wc, has no Windows counterpart on the build path; that is recorded in the known failures with the log line. A test helper also gains the t.Helper() lint asked for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
go-rules 2906dda names cc as a tool on a Windows host only where cgo or an external link needs it, since tools resolve before anything runs and no cc exists there by default. It also moves to please_go 1.24.0-windows.1, which trims go_repo package paths on Windows instead of looking under a doubled source root. python-rules fe3f124 moves to please_pex 3.0.2-windows.1, which reads its embedded test runners by slash-separated path; every python_test on Windows failed to build without it. Both Windows tool builds are released under their own versions rather than replacing assets that earlier commits pin by hash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
7166e39 passes -extld "$TOOLS_LD" to the Go linker only when it names cc as a tool. Naming cc only where Windows needs it had left the flag behind, and the build shell stopped on the unset variable: "TOOLS_LD: parameter not set". It also moves to please_go 1.24.0-windows.2, which slash-joins go_repo subrepo names and label packages on Windows. With backslashes they named no subrepo, and the plz_query and Puku codelabs failed with "Subrepo third_party\go\... is not defined". The Puku fork makes the same fix to the labels puku itself writes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
The codelabs job never started on 0671e3c: docker info exits non-zero when the daemon is not running, and the pwsh wrapper fails a step on the last native exit code. The step only reports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
… Windows arcat's AddInitPyFiles looked for each directory's __init__.py under a backslash-separated name, never found it, and wrote an empty one beside it. zipimport normalises separators and loaded the empty one, so every python_test built on Windows failed with "module 'xmlrunner' has no attribute 'XMLTestRunner'" - the Python codelab's plz test stopped there. The Wine pex tests could not see it, since their pexes are assembled on Linux. A second patch makes it, and shouldInclude, use path rather than filepath on member names. //test/windows:arcat_init_py_test runs the Windows arcat under Wine against a nested package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
A rule written for Unix names tools like wc and expects them on the build path. Windows has no such directory, so the genrule codelab's word_count stopped with "wc not found in path". The bundled busybox has wc built in, and picks its applet from the name it is started under, so a bare tool name found nowhere else now resolves to a copy of busybox named for it, under plz-out/busybox. Its content is busybox's, so the tool hash follows a busybox upgrade. That path is the first system tool inside the repo, which found a hasher bug: it trimmed only a / after the repo root, leaving \plz-out\... - absolute again, to the root of the drive. Checked under Wine by //test/windows:applet_tool_test, and natively by the probes. The genrule codelab's known failure goes with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
…ted name The same fault as arcat's, in please_pex's own copy of the zip writer. python-rules 9cd3f0d downloads please_pex 3.0.2-windows.2 on Windows, which carries the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
The codelab told readers to run please_go get, which go-rules removed (thought-machine#306); it fails with "Unknown command get" on every platform. go get and go list -m all give the same list of modules for the go_repo rules that follow, and Puku is pointed to for keeping them in sync. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
go-rules 883d615 downloads please_go 1.24.0-windows.3, whose package_info slash-joins import paths: every go_repo package failed on Windows with "Cannot determine export file path", which stopped the plz query and Puku codelabs. It also finds the stdlib archives for a toolchain's importconfig from inside GOROOT, whose backslashes were escapes in the sed pattern that trimmed it, so a compile against it could not import math/rand. python-rules c10d10a downloads please_pex 3.0.2-windows.3, which locks the cache a non-zip-safe pex explodes into with msvcrt. portalocker needs pywin32 on Windows, and the Python codelab's numpy binary stopped with "No module named 'win32con'". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
The block that adds the go_repo rules is the whole of third_party/go/BUILD, and it held only those, so following it deleted the toolchain and stdlib the codelab set up earlier. plz test then failed with "//third_party/go:std doesn't exist". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
please_go 1.24.0-windows.4 normalises both paths before trimming a go_repo's source root in package_info; 1.24.0-windows.3 slash-joined the import path but still left the root on it. please_pex 3.0.2-windows.4 deletes the read-only files an uncached, non-zip-safe pex extracted. The Python codelab's numpy binary ran and then failed on f2py.exe with "Access is denied". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
The codelab's labelled BUILD file named //third_party/go:assert, but the repo it clones names that go_repo testify, so plz test failed with "doesn't contain target assert" on every platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
please_pex 3.0.2-windows.5 leaves the extension modules a running pex still has loaded when it cleans up on Windows, which will not delete them; the Python codelab's numpy binary ran and then failed on a .pyd with "Access is denied". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
The codelabs job on windows-latest passes with one known failure left, k8s, which is outside the Windows work. The ChangeLog records what that took; the design docs mark M10 done, record the decision it left open, and describe the busybox applet fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djqj7h7hQ1Y9Gj99s4DLm9
…with slashes --shell had never been exercised on Windows. Under Wine it works, with and without =run, for builds and tests: busybox opens in the target's directory with its environment set. The one defect was the directory printed above it, which for a test came out backslash-separated and so could not be pasted into that shell. Adds Wine tests for both forms and native probes for the Windows job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AaLmkJrkt7eRJC9uPXvbKe
Upstream thought-machine#3569 deleted parse_step_test.go along with the queueing code it tested. The arcat tests the Windows port had added to it now live in internal_package_test, so the Wine and native runs take that target instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AaLmkJrkt7eRJC9uPXvbKe
The fork cut 18.0.0 and 18.0.1 itself, because it could not publish through upstream's release pipeline. Upstream owns the version number and writes its ChangeLog at release time, so this series goes back to 17.33.0 and drops the fork's two ChangeLog entries and its 18.0.0 milestone page. The Windows support itself is unchanged, including the README and FAQ, which describe it rather than a release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AaLmkJrkt7eRJC9uPXvbKe
plz update on Windows fetched only the bare please binary. Everything else a Windows install ships - busybox, which runs build actions there, arcat and the plz.cmd shim - stayed at whatever version was first installed, silently getting staler with each update. A Windows release is a zip of all of it, so plz update now downloads that and unpacks it, stripping the top-level please/ directory the way the tarball path does and refusing any entry that would land outside the version directory. The existing checksum and signature checks apply to the zip unchanged. The download takes the OS as a parameter, so the tests fetch the Windows release from any platform; cross-built under Wine and on the native job, the existing download tests take the zip path as well. plz update still only knows the bucket layout. The fork's GitHub releases are shaped differently, which the state-of-play doc now records. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AaLmkJrkt7eRJC9uPXvbKe
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.
This PR only exists to run the Windows CI on the rebased port. Do not merge it.
upstream-masteris a copy of thought-machine/pleasemasteratfa553360(thought-machine#3569), so the diff here is exactly what the Windows port adds on top of upstream.The branch replays the fork's 103 commits onto thought-machine#3569 (the PR merge commits were dropped, flattening the history) and adds two commits on top.
Conflicts resolved during the rebase
Outputs,toolPathand similar now take the graph; the fork's slash-separated path fixes are kept on top of them.parse.InitParser(state, &r)inRun.//pkg/...walk. It moved intoRecursiveParsewith the same backslash bug; it now usespackageNameOf.22a3a3fa: dropped. It patchedSyncParsePackage, which Convert build parallelism to a more synchronous model thought-machine/please#3569 removed, and the only caller ofcmap.Deletewent with it. The commit now contains only its two-subrepo regression test,test/subrepo/nested_subrepo_probe, and has a new message.src/parse/parse_step_test.go, so they moved to a new//src/parse:internal_package_test, andtest/windows/BUILDpoints at that target.New commits
test/windows/BUILDpoints at the new parse test target.VERSIONgoes back to 17.33.0, the fork's 18.0.0 and 18.0.1 ChangeLog entries are dropped, and so is its milestone page. Upstream owns releases; the README and FAQ text about Windows stays.Verified locally
//src:pleasebuilds, and so does thewindows_amd64cross-build.🤖 Generated with Claude Code
https://claude.ai/code/session_01AaLmkJrkt7eRJC9uPXvbKe