Repository navigation
Windows fixes and CI coverage (depends on #535) - #536
Merged
medvednikov merged 24 commits intoOct 5, 2026
Merged
Conversation
PR vlang#521 changed index_max_file_bytes from int to u64 so it compares directly against os.file_size. string.repeat still takes an int, so the three oversized-file tests stopped compiling and the whole suite failed to build on every platform. Convert at the call site and keep the constant u64, which is what index.v compares against.
test_integration_sublime_text_lsp_handshake hand-writes the initialize payload to mimic a real client, then interpolates the native project path straight into the rootPath JSON string. On Windows that path contains backslashes, and \U is an unknown JSON escape, so json2.decode of the params fails and on_initialize returns InvalidParams. received_initialize never becomes true and the test fails on every Windows run. Real clients escape those separators, so escape them here too. The workspace_roots assertion now compares against the URI's own path form, because roots are resolved from the folder URI and carry '/' separators rather than the native backslashes.
…iler
Hover, completion, signature help, and go to definition all go through
`v -vls-mode -line-info`, which the V launcher routes to the V1
compatibility compiler. When that compiler is missing, the launcher
refuses with a single line and exits:
`-vls-mode` requires the compatibility compiler, but no usable V
0.5.2 fallback was found and make is unavailable. Install make,
then run `make v1` in `C:\Users\me\v`.
That refusal is not an "unknown option" line, so neither
`compiler_rejects_line_info` nor `compiler_refused_and_stopped`
recognized it. `line_info_mode` stayed `.direct`, so VLS kept spawning
the compiler once per request for an answer that can never arrive, and
every one of those lookups resolved to empty with nothing said about why.
Recognize the refusal, retire the lookups as `.missing` so they are
answered from VLS's own index instead of paying a process launch each,
and tell the user what to install. A launcher that can build the
fallback itself announces "running `make v1` now" and then answers, so
that form is explicitly not a dead end.
The notice needs no "already warned" flag: the caller sets
`line_info_mode` to `.missing` first, and from then on `run_v_line_info`
returns from its early `.missing` check without reaching this point, so
it is sent at most once per session.
`uri_to_path` already resolves a `file://host/...` URI to a `//host/share/...`
UNC path, but `path_to_uri` then treated that as an ordinary absolute path and
emitted four slashes:
client sends file://server/share/proj/main.v
uri_to_path //server/share/proj/main.v
path_to_uri file:////server/share/proj/main.v
The share ends up in the path instead of the authority. That is not a valid
file URI (RFC 8089 puts the host in the authority), and it breaks the round
trip in a way that matters: VLS keys open buffers, the index, and every
published diagnostic and code lens by URI, so the URI it derives for a file on
a network share never matches the one the client sent for the same file.
Encode the host as the authority instead:
path_to_uri('//server/share/proj/main.v') == 'file://server/share/proj/main.v'
Percent-encoding of the path component is unchanged, so a share with a space
still encodes correctly and still round-trips. Single-slash absolute paths and
Windows drive paths are untouched, so POSIX and local Windows behaviour is
identical.
`v fmt` always writes LF, on every platform including Windows. VLS formats by
writing the buffer to a temp file, running `v fmt -inprocess -w` on it, and
reading the result back, so a CRLF document came back with every CR stripped:
```
module main\r\n\r\nfn main() {\r\n\tprintln('hi')\r\n}\r\n
|
v fmt -inprocess -w
v
module main\n\nfn main() {\n\tprintln('hi')\n}\n
```
That output was then returned verbatim as a whole-document `TextEdit`. Two
problems follow, and the second is the worse one:
1. Format Document silently converts the file's line endings, so every line
shows as changed and the diff is the entire file.
2. `format_content` returns no edits when the formatted text equals the input.
With the CRs gone that comparison could never hold for a CRLF document, so
VLS reported a change even for code that was already correctly formatted.
Restore the terminator the document already uses:
```v
formatted = restore_line_endings(content, formatted)
```
The terminator is taken from the document's first line break. An LF document is
untouched, output that already contains CRLF is not given a second CR, and a
document with no line break has no convention to preserve. A file with mixed
endings is normalized to its first ending, which is what a formatter that
respects the dominant convention does.
This is the same bug reported independently by users on Windows, where CRLF is
the default for `core.autocrlf` and for editors that preserve the file's
endings. The fix is platform-neutral: a CRLF document keeps CRLF on any
platform.
vlang/v#29369 (merged as 76d88b9) rewrites the tail of the launcher's refusal: the sentence after "make is unavailable" is now a platform-specific hint, and the comma became a period. On Windows the clause reads On Windows, install GNU make in MSYS2 (`make` or `mingw32-make`) and put its tools, including `sh`, on PATH. and elsewhere it is still "Install make.". `compiler_lacks_compatibility_compiler` keys on "requires the compatibility compiler", which both spellings contain, so detection is unaffected. This test pinned the old wording verbatim, so it now asserts both: green before the compiler change and after it, and the coupling is written down instead of being rediscovered the next time the wording shifts. Validated against V 0.5.2 76d88b9, the merged compiler: the Windows sample above is its output verbatim. `interop_test.v` passes; the module suite reports the same 3 failures with and without this change - `index_test.v`, `handlers_test.v` and `integration_test.v`, none of them in this file. `integration_test.v` fails on an empty completion list, which is the symptom of the missing V1 fallback this refusal describes, and is what vlang/v#29369 and vlang/vscode-vlang#543 are about.
`-prod` removes assert statements whole, as documented in doc/docs.md and
implemented in `vlib/v/gen/c/stmt.v`:
```v
.assert_stmt {
if g.is_prod {
return
}
}
```
Two tests here used a side-effecting call *as* the assertion's condition:
```v
assert os.fd_dup2(transport.read_fd, 0) >= 0
```
That dup2 is the operation the test depends on, not a check on it. Under `-prod`
the statement disappears, fd 0 keeps pointing at the real stdin, and the read
that follows blocks on a descriptor nobody is going to write to. So:
- `v -prod test lsp_test.v` never terminated (>40 min for a file that takes 25s)
- `v -prod test integration_test.v` likewise
Perform the call outside the assert, matching how the neighbouring code already
does it (`os.fd_close(transport.write_fd)` a few lines above has no check either).
V's behaviour here is correct and is not changed by this commit; the defect was
in the tests. Verified on Windows with V 0137eb5:
- `v -prod test lsp_test.v` -> OK, 53s (was hanging)
- `v -prod test integration_test.v` -> OK, 62s (was hanging)
- `v -prod test .` -> completes in 76s, 4/6; the two failures are the unrelated
`index_max_file_bytes` compile error that vlang#526 fixes. Applying that fix on top
gives `6 passed, 6 total` under `-prod`.
- Normal builds unchanged: `v test lsp_test.v` OK, and `integration_test.v`
fails only on the pre-existing Sublime Text handshake bug that vlang#527 fixes.
Reported upstream as vlang/v#29426, where the `-prod` codegen evidence is
included. That issue is closed as not-a-bug: the behaviour is documented and
matches C's `assert` under `NDEBUG`.
V3 is the default backend on Linux and macOS, so the V3 compile step was the
only place its output was ever checked, and it was gated to `runner.os !=
'Windows'`. Windows keeps V1 as its default, so a V3-only codegen breakage could
sit on master until somebody built with `-new-compiler` by hand and reported it.
Dropping the condition covers all three platforms. The extra step is one compile
of a program this size, so it costs a fraction of the build time already spent.
Verified on Windows with V 0137eb5, using the same environment CI sets:
$env:V_MACOS_V3_NO_FALLBACK = '1'
v -no-memory-limit -nocache -new-compiler . # exit 0
The separate Windows-only `Compile project` step stays, since that is the
backend Windows users actually get by default.
`-prod` removes assert statements, and that silently changes the behaviour of any
test that performs work inside one. In this repository that reached Windows as a
hang rather than a failure: `test_stdio_reader_processes_frame_before_eof` and
`test_integration_stdio_initialize_completion_and_hover` both did
assert os.fd_dup2(transport.read_fd, 0) >= 0
so under `-prod` the descriptor was never redirected and the following read
blocked forever. Nothing caught it because the `-prod` step was gated to Linux:
- name: Run tests with production optimizations
if: runner.os == 'Linux'
Two changes:
1. Add the same step for Windows. `V_MACOS_V3_NO_FALLBACK` is "0" to match the
existing Windows test step, since Windows defaults to V1 and 0 permits
fallback.
2. Put `timeout-minutes: 30` on both. A hang in a test step otherwise consumes the
job's entire budget and reports nothing useful; a bounded step turns it into
a red build that says which step stopped. The Linux step gets the same guard
because it has the same failure mode. Drop that one line if you would rather
keep the diff to Windows only.
The Windows `-prod` step deliberately does not set `VLS_VLANG_V_REPO`, so it does
not repeat the slow vlang/v workspace tests. Those are already covered on Windows
by the non-prod step above, which does set it. That also keeps the added CI time
to the ~1-2 minutes the suite takes locally under `-prod`, instead of the
considerably longer run the workspace tests add on top of `-prod`.
Verified on Windows with V 0137eb5:
v -no-memory-limit -nocache -prod test vls/ -> completes in 76s
**Ordering: this must land after vlang#532.** Before that fix the two stdio tests
above hang under `-prod` on Windows, so this step goes red. With the timeout in
place it fails in a bounded, legible way instead of hanging, but it is still red.
The Windows job also needs vlang#526 (the suite must compile) and vlang#528 (the job
currently dies at `Check formatting`) to be green at all.
Practical merge order: vlang#526, vlang#527, vlang#528, vlang#532, then this.
find_v_dir trusted the resolved compiler executable directory and returned it unconditionally. V's own Windows launcher is a .bat wrapper kept in .bin/ that forwards to the real v.exe one level up, so that directory contains no vlib at all. Every caller builds <find_v_dir()>/vlib, so all of them silently resolved to nothing: import completions returned an empty list, and hover and go-to-definition stopped resolving vlib symbols. find_v_dir_from_exe now walks up from the executable until it finds a directory that actually contains vlib, bounded to eight levels. A directory with no vlib anywhere on the way up now returns an empty string instead of a path whose vlib does not exist. Measured on Windows with v resolved to the wrapper: 7 of 454 handlers_test cases failed before this change and all 454 pass after it, under identical conditions. The same run also fixes 2 of the 3 failing integration cases that a stdio test would otherwise have masked.
This was referenced Oct 5, 2026
medvednikov
approved these changes
Oct 5, 2026
medvednikov
left a comment
medvednikov
left a comment
Member
There was a problem hiding this comment.
Independent review completed by an agent separate from the fix agent.
Reviewed commit f901296; no remaining blockers.
Successful compatibility handoffs retain compiler-backed responses; automatic-build
progress no longer masks subsequent failures. Repair guidance names VLS_V_COMMAND.
Document and range formatting preserve CRLF, and UNC paths preserve the authority.
Validation: all seven test files pass, plus four affected production-optimized suites.
The reviewer independently exercised successful and failed compatibility launches over
live stdio LSP. Formatting and a V3-only build also pass with clean V a4811a3 on macOS.
Hosted CI is still pending and is not being represented as passed.
medvednikov
approved these changes
Oct 5, 2026
medvednikov
left a comment
medvednikov
left a comment
Member
There was a problem hiding this comment.
Independent review approves final commit 3eb9491.
The reviewer is separate from the fix agent and verified the final detached head.
The additional Windows CI finding is fixed: rooted parent traversal cannot fall into
cwd when walking above a drive or UNC root. Clean V a4811a3 build, formatter, and all
six vlib-discovery tests pass. Previous validation passed all seven normal test files,
four affected optimized suites, and independent live-LSP compatibility regressions.
Hosted CI for the final head remains pending.
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
2 participants
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.
Windows behaviour fixes and CI coverage. Depends on #535, which must merge first.
vlsonmasterdoes not build a green suite, so this branch carries #535's commits aswell. Review #535 on its own first; once it lands, everything left in this branch is the
three Windows fixes and the two CI changes below.
1. Recognise a launcher that cannot reach the V1 compatibility compiler
Hover, completion, signature help and go-to-definition go through
-line-info, which onlythe compatibility compiler implements now that V3 is the default backend. When the V on
PATHcannot reach that checker and has no V1 fallback, vls answered from its own indexwith nothing logged: highlighting worked, popups did not. That is the silent failure in
#495.
This makes the case announce itself and says what to do, naming MSYS2's
mingw32-makerather than plain
make, sincemakev.batis the supported Windows build path.2. Keep the authority when encoding a UNC path as a file URI
\\server\share\file.vlost its authority when converted to afile://URI, so the resultpointed somewhere else entirely. Windows only, so CI elsewhere would not catch it.
3. Preserve CRLF when formatting a document
textDocument/formattingrewrote CRLF files to LF, producing a whole-file diff afterformatting a document that was not otherwise changed.
4. CI: compile with V3 on Windows
The Windows job built VLS with the default backend only, so the V3 build path was never
exercised there.
makev.batalready builds V3 on Windows.5. CI: run the
-prodsuite on Windows-proddrops assert statements, and with their side effects. That reached Windows as a hangin
test_stdio_reader_processes_frame_before_eof, invisible because-prodonly ran onLinux. The step is bounded on both platforms so a future hang is a red build rather than a
job timeout.
Verification
All on Windows, V 0.5.2 (
bb0d229),VLS_VLANG_V_REPOpointed at a checkout of vlang/v.v fmt -verify .7 passed, 7 total-prod7 passed, 7 totalConfirmed end to end in VS Code 1.140.0 with extension 0.2.1.
didOpenproduces a compilerdiagnostic, and a member completion returns the compiler's own members. For that file the
compiler prints
1 possibility: norm2, the same entry the popup shows, so the completioncomes from the compiler rather than from vls's index. All 21 advertised capabilities were
exercised over real LSP and 21 answered.
Building pristine
upstream/masterbehaves identically to this branch, which is how I ruledmy own changes out while investigating #495.
Not verified: Linux and macOS. The UNC fix is Windows-only by construction and has no
coverage there. Nothing has run in CI yet.