fix: Windows — shell detection, CRLF catalogs, and slink that actually links - #10
Merged
Merged
Conversation
Reported from a real Windows host running v0.3.0-preview.1, which is what the preview was for. The roadmap said Windows detection was written from the documented behavior of those shells and unverified; it was also wrong. Every PowerShell user was told they were in cmd. Detection read the parent process, and something is usually in between: scoop installs godo as shims\godo.exe, which starts the real godo.exe as a child, so the parent of the process asking the question was *godo*. Not a shell, so it fell through to %ComSpec%. It now walks up to the nearest shell, which also covers npm, bun and make wrappers and an editor's terminal. The walk is a pure function over a process table, so it is tested off Windows too. Also from the same session: - A decorator at the end of a line was dropped in silence. YAML files it as a line comment on the value, and nothing reads it, so the catalog ran with a dialect its author believed they had written and failed somewhere else. Same for a decorator after the last key. Both are errors now, naming where the decorator belongs. - fs.slink on Windows returned "A required privilege is not held by the client" — true, and useless unless you already know Developer Mode grants it. - godo -e runners says how the shell was chosen. A wrong answer is only diagnosable if you can tell a stale GODO_SHELL from a fallback.
Two corrections to the previous commit, both mine. A catalog written on Windows has CRLF, and with it the YAML parser files the comment above a key as the *previous* key's foot comment — so the decorator decorated nothing, unless a blank line happened to sit above it. That is the "no space between the command and the comment" report, and the misplaced- decorator error added in the last commit would have fired on a file where the decorator was placed correctly. Line endings are normalized before parsing, which YAML already calls the same line break; it also keeps a \r out of a block scalar, where it was reaching the shell as part of the command. slink is symlinks on Unix, junctions and links on Windows. It was calling os.Symlink everywhere, and the last commit answered the resulting failure with advice about Developer Mode — which is not the op working, it is the op explaining why it does not. It now creates a junction for a directory and a hard link for a file, neither of which needs a privilege. The reparse buffer is built in portable code so the layout is tested off Windows; the syscall around it is not.
Everything in it came from running preview.1 on a real Windows host. The junction syscall behind slink is the reason this is preview.2 and not 0.3.0: its buffer is tested, the call itself has been run by nobody.
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.
Everything here was found by running
v0.3.0-preview.1on a real Windows host,which is what the preview was for.
Every PowerShell user was told they were in
cmdDetection read the parent process, and something is usually in between: scoop
installs godo as
shims\godo.exe, which starts the realgodo.exeas a child,so the parent of the process asking the question was godo. Not a shell, so
the answer fell through to
%ComSpec%.It now walks up the process tree to the nearest shell — which also covers npm,
bun and make wrappers, and an editor's terminal. The walk is a pure function
over a process table, so the shim case is tested off Windows.
godo -e runnersalso says how it chose, because a wrong answer is onlydiagnosable if you can tell a stale
GODO_SHELLfrom a fallback.A catalog written on Windows lost its decorators
With CRLF, the YAML parser files the comment above a key as the previous
key's foot comment, so
# @dialect matcherdecorated nothing — unless a blankline happened to sit above it, which is why it read as "no space between the
command and the comment breaks it".
Line endings are normalized before parsing, which YAML already calls the same
line break. It also keeps a
\rout of a block scalar, where it was reachingthe shell as part of the command.
A decorator YAML genuinely puts out of reach (trailing on the line, or after
the last key) is now an error naming where it belongs, instead of vanishing.
fs.slinkdid nothing on WindowsIt called
os.Symlink, and a symlink there is a privilege rather than a fileoperation. It now makes the link Windows offers — a junction for a
directory, a hard link for a file — neither of which needs a privilege.
The reparse buffer is built in portable code and tested field by field.
DeviceIoControlitself is unverified; it only runs on Windows, and thatis why the release this cuts is
preview.2rather than0.3.0../godo cigreen,go vetgreen forwindows/amd64.