Repository navigation
Windows support (builds on #31) - #36
Closed
AndrewKirkovski wants to merge 4 commits into
Closed
AndrewKirkovski wants to merge 4 commits into
AndrewKirkovski wants to merge 4 commits into
Conversation
Add win32 to the package's os list and adapt three POSIX assumptions so the CLI runs on Windows: - Config/cache paths fall back to %APPDATA%\jevgrep and %LOCALAPPDATA%\jevgrep on Windows (XDG still wins first). - O_NOFOLLOW/O_NONBLOCK, undefined on Windows, fall back to 0 in the filesystem and cache open() calls. - jg skill spawns npx through a shell on Windows, where npx is npx.cmd.
Config and cache locations were resolved in three places (auth, the CLI cache, and the filesystem's protected paths). Move them into one storageDirectory() helper in @repo/core so protected storage cannot drift from where jg actually writes. scripts/package-notices.mjs split resolved paths on "/", which never matches on Windows, so `bun run build` failed there.
- Cover storageDirectory() on every platform branch. - Pass the provider preload to --import as a file URL; Node rejects a bare C:\ path. - Keep POSIX-only fixtures (file modes, chmod 0, FIFOs, pty, newline filenames) on POSIX. Link-escape checks now use junctions so they also run on Windows without symlink privilege.
Contributor
|
@dzhng he has made sure it is tested |
Contributor
|
Please don't squash commit |
Contributor
Author
|
Additionally hand tested skill itself, seems working |
Owner
|
Thank you @AndrewKirkovski for building on #31, fixing the Windows build and storage paths, and testing the actual installed CLI on Windows. Thank you @makiisthenes for the original implementation and collaboration. All four commits from this PR are included in #47 with both authorships preserved, without squashing them. Closing this PR as integrated through #47; the work ships in v0.8.0. |
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
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.
Fixes #5. Builds on #31 by @makiisthenes.
The first commit is #31 unchanged, rebased onto current
main(0.6.0, after the Tree-sitter change). Their authorship is kept. The commits after it fix the gaps I found while testing #31 on a real Windows machine.What this adds on top of #31
auth.ts,index.tsandfilesystem.ts. It now lives instorageDirectory()in@repo/core/storage, so the protected-path list always matches wherejgactually writes.bun run buildworks on Windows.scripts/package-notices.mjssplit resolved paths on/, so the build failed on Windows. This does not affect published packages, which are built on Ubuntu.storageDirectory().provider-protocol.mjspasses its preload to--importas a file URL. Node rejects a bareC:\path.chmod 0,mkfifo,pty, filenames with a newline) are guarded with oneposixflag intest/helpers/docker.ts.%APPDATA%\jevgrepand%LOCALAPPDATA%\jevgreplocations. They also note that Windows relies on profile permissions rather than POSIX modes.Verification
On Windows 11 with Node 22.22 and Bun 1.3.11:
check-typesandlintpass.bun testover thetest-suite.shpaths: 54 pass, 0 fail (the rest are Docker-gated).JEVGREP_TEST_IN_DOCKER=1forced: 119 pass, 0 fail, 4 skipped as POSIX-only.node --test packages/core/test-node/provider-protocol.mjs: 18/18 pass.npm pack, thennpm install -gfrom the tarball.jg --versionandjg fileswork. An end-to-end run against a local fake provider also works:auth --stdinsaves the key to%APPDATA%\jevgrep.%LOCALAPPDATA%\jevgrep, and a repeat search makes 0 provider requests..envor ignored content is uploaded.jg skillpasses the correct arguments tonpx.cmdand returns its exit code.On Linux, run in Docker from the same branch:
bash scripts/test-docker.sh bash scripts/test-suite.shpasses.bash scripts/test-installed.sh: 40/40 pass.Known limits
test/parser/parity.test.tsneedspython3andtest/release.test.mjsneeds atarthat acceptsC:paths. Both are environment requirements, not product issues.jg skillusesshell: trueon Windows because Node cannot spawnnpx.cmdwithout a shell. Agent names are validated before use. Node 24 may print a deprecation warning for this, which I have not checked.