Repository navigation
cmd/v: let the V1 fallback find MSYS2's make on Windows - #29369
Merged
Merged
Conversation
`-vls-mode` is answered by the V 0.5.2 compatibility compiler, so `v -check -vls-mode ...` first has to have that fallback. When it is missing, `ensure_v1_fallback` builds it with `make v1`, and `find_make` only ever looked for `make` and `gmake`. A stock Windows install of V has neither on PATH: the supported Windows build goes through `makev.bat`, and a machine that installs MSYS2 gets GNU make under its Windows name, `mingw32-make`. Both `v1:` targets are POSIX shell recipes, so MSYS2 is the toolchain that can actually run them. The result on such a host was a hard refusal with empty stdout, which the VLS reports as `failed to parse json`: no completion, no hover, no diagnostics. That is what vlang/vscode-vlang#543 reports from Windows. `find_make` now also looks for `mingw32-make`, on Windows only. On a Unix host that name is a Windows cross-make, and using it here would cross-compile the compatibility compiler instead of running it. The "make is unavailable" diagnostic now says where to get make on this platform, because "install make" is not actionable advice for a Windows user. vlang/vls#529 asserts the previous wording and needs the matching update. Verified: `cmd/v/find_make_test.v` (4 tests, including that `mingw32-make` is found on Windows and deliberately not consulted elsewhere) and `cmd/v/v3_fallback_diagnostics_test.v` (8 tests). The fallback build itself was not run here, so the fix is covered by unit tests and not by an end-to-end `mingw32-make v1` build.
medvednikov
approved these changes
Oct 3, 2026
medvednikov
left a comment
Member
There was a problem hiding this comment.
Reviewed GNU make search precedence and platform gating. Corrected the MSYS2 diagnostic/documentation in 7ec7599 to include the shell required by make v1 and avoid assigning mingw32-make to the wrong MSYS2 directory. Rebuilt; find_make_test.v and v3_fallback_diagnostics_test.v pass, check-md compiler-fallback.md passes, and output fixtures pass 101 cases with one skip. Compiler errors retain the six clean-baseline mismatches. Broad testing hits the baseline memory cap and existing C fixture failures; all seven additional failing C fixtures were reproduced separately at clean baseline. No remaining finding in this PR.
metif12
added a commit
to metif12/vls
that referenced
this pull request
Oct 3, 2026
vlang/v#29369 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 whole clause names MSYS2's `mingw32-make`; elsewhere it is still "Install make.". `compiler_lacks_compatibility_compiler` keys on "requires the compatibility compiler", which both spellings contain, so detection is unaffected. The test pinned the old wording verbatim, so it now asserts both: green before the compiler change lands and after it, and the coupling is written down instead of being rediscovered when the wording shifts again. Validated with the pre-change compiler (V 0.5.2 863ae78): `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` - and none of them is 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 tracked by vlang/v#29369 and vlang/vscode-vlang#543.
metif12
added a commit
to metif12/vls
that referenced
this pull request
Oct 3, 2026
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.
medvednikov
added a commit
to vlang/vls
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.
On a stock Windows install of V,
v -check -vls-mode ...cannot work, and the reason is incmd/v, not in the editor extension.The chain
-vls-modeis answered by the V 0.5.2 compatibility compiler's AST parser, sov.vroutes the request tolaunch_v1(cmd/v/v.v:136). That needs thev1_fallbackbinary, which is built on demand byensure_v1_fallbackwithmake v1.find_makelooked formakeandgmakeonly.A stock Windows install of V has neither on PATH: the supported Windows build path is
makev.bat, and a machine that installs MSYS2 gets GNU make under its Windows name,mingw32-make. So on such a host:Empty stdout is what the language server reports as
failed to parse json: no completion, no hover, no diagnostics. That is the symptom in vlang/vscode-vlang#543 ("VLS not working on Windows").The change
find_makealso looks formingw32-make, on Windows only. On a Unix host that name is a Windows cross-make, and using it here would cross-compile the compatibility compiler instead of running it.mingw32-makeappears nowhere in the tree today, and bothv1:targets (GNUmakefile:262,Makefile:147) are POSIX shell recipes, so MSYS2 is the toolchain that can actually run them.The "make is unavailable" diagnostic now says where to get make on this platform. "Install make" is not actionable advice for a Windows user, which is part of why this went unnoticed: the error named a tool that the documented Windows install does not use.
Validation
cmd/v/find_make_test.v(new, 4 tests):makewins overgmakeandmingw32-make;gmakealone is used;mingw32-makeis found on Windows and deliberately not consulted elsewhere; nothing is reported when PATH has none. The Windows case was checked negatively as well: with the one-line fix removed,test_find_make_uses_mingw32_make_on_windows_onlyfails.cmd/v/v3_fallback_diagnostics_test.v: 8 passed, including a new case asserting the hint namesmingw32-makeon Windows.v fmt -verifyclean on all three files.vlib/v/compiler_errors_test.v: 24 failed / 1725 passed / 7 skipped, identical toorigin/masteron this host (those failures predate this change), so no inout fixture regressed. No.outfixture contains the reworded message.Limits, stated plainly: the fallback build itself was not run here, so this is verified by unit tests and not by an end-to-end
mingw32-make v1.cmd/v/launcher_test.vandcmd/v/toolcache_test.vfail onorigin/mastertoo (the first does not compile:launcher_test.v:336propagates aResultfrom a test function that does not return one), which is whytest cmd/v/cannot be all green on this branch.Related
interop_test.vand needs the matching one-line update.