Restore a green build on master, and resolve vlib through a compiler wrapper - #535
Merged
medvednikov merged 10 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.
`-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`.
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
Member
There was a problem hiding this comment.
Independent review completed by an agent separate from the fix agent.
Reviewed commit 6596485; no remaining blockers.
The wrapper lookup verifies the canonical compiler path and finds an existing vlib.
The macOS test helper now compares canonical temporary paths consistently.
Validation: v fmt -verify . and v . passed. All six existing test files passed;
all six new vlib-discovery tests passed after the test-helper correction.
Hosted CI is still pending and is not being represented as passed.
medvednikov
added a commit
that referenced
this pull request
Oct 5, 2026
* fix: make index_max_file_bytes usable as a string repeat count PR #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. * fix: escape interpolated paths in the Sublime Text handshake test 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. * chore: restore v fmt compliance on master * fix: recognize a launcher that cannot reach the V1 compatibility compiler 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. * fix: keep the authority when encoding a UNC path as a file URI `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. * fix: preserve CRLF line endings when formatting a document `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. * test: recognise the reworded "make is unavailable" refusal too 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. * cgen: keep side effects out of `assert` conditions in stdio tests `-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 #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 #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`. * ci: compile VLS with V3 on Windows too 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. * ci: run the -prod test suite on Windows as well `-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 #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 #526 (the suite must compile) and #528 (the job currently dies at `Check formatting`) to be green at all. Practical merge order: #526, #527, #528, #532, then this. * fix: resolve vlib through a compiler wrapper script 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. * test: canonicalize vlib discovery temporary roots * fix: preserve working compiler fallback and CRLF range edits * fix: stop vlib discovery at absolute filesystem roots --------- Co-authored-by: metif12 <metif12@users.noreply.github.com> Co-authored-by: Alexander Medvednikov <alexander@medvednikov.com>
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.
Restores a green build on master, and fixes a vlib resolution bug that this change
exposes.
On current
masterwith V 0.5.2 the suite does not run at all:handlers_test.vfails to compile, so its 454 cases never execute. Fixing that exposes asecond problem, described below.
What is in this branch
Consolidates four of my open PRs, plus one new fix, in the order they must land:
v fmtcompliance across 17 files (was chore: restore v fmt compliance on master #528)index_max_file_bytes + 1is au64passed tostring.repeat, which takesint(was fix: make index_max_file_bytes usable as a string repeat count #526)
(was fix: escape interpolated paths in the Sublime Text handshake test #527)
os.fd_dup2sits inside anassert, and-prodremoves assert statements along withtheir side effects (was fix: perform the stdin dup2 outside assert so it survives -prod #532)
The new fix: vlib resolution through a wrapper script
With the suite compiling, 7 of 454 cases failed. All seven were vlib lookups.
find_v_dirreturned the resolved compiler executable's directory unconditionally:Every caller builds
<find_v_dir()>/vlib, but the compiler onPATHis not alwaysv.exesitting beside
vlib. V's own Windows launcher is a wrapper script:.bincontains onlyv.bat; the realvlibis one level up. Sofind_v_dirpointed at adirectory whose
vlibdoes not exist, and every vlib lookup silently resolved to nothing.Seven call sites are affected, spanning import completions, builtin type resolution, and
hover and go-to-definition for vlib symbols.
find_v_dir_from_exenow walks up from the executable until it finds a directory thatactually contains
vlib, bounded to eight levels. A tree with novlibanywhere on the wayup returns an empty string instead of a path whose
vlibis missing.This matters beyond the tests: any Windows user whose
vis a launcher script gets emptyimport completions and no vlib hover, with nothing logged.
Verification
All on Windows, V 0.5.2 (
bb0d229),VLS_VLANG_V_REPOpointed at a checkout of vlang/v.masterhandlers_test.vdoes not compile7 passed, 7 total,v fmt -verify .cleanMeasured directly for the vlib fix, with
vresolved to the.batwrapper andC:\Users\xman\vdeliberately absent fromPATH:handlers_test.vNew
main_test.vcovers the wrapper layout, the plain layout, a deeper nesting, a tree withno
vlib, an unresolved name, and a missing file.Not verified: Linux and macOS, and nothing has run in CI yet. Everything above is local
Windows output.