Add optional Windows LuaBinaries installs#13
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in Windows installation path for the vfox Lua plugin that downloads and installs prebuilt LuaBinaries artifacts (instead of compiling Lua via MSYS2), and validates that flow in CI.
Changes:
- Add
VFOX_LUA_WINDOWS_LUABINARIES=1flag to switch Windows installs to LuaBinaries downloads (with a supported-version map). - Add Windows post-install normalization that renames versioned executables (e.g.,
lua54.exe) intolua.exe/luac.exe. - Document the opt-in flow and add a dedicated GitHub Actions job to exercise it.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Documents the Windows opt-in LuaBinaries flow and supported versions. |
lib/utils.lua |
Adds LuaBinaries version/package metadata and helpers to enable the opt-in path. |
hooks/pre_install.lua |
Switches download URL selection to LuaBinaries when the opt-in flag is set. |
hooks/post_install.lua |
Adds a PowerShell-based post-install step to normalize LuaBinaries layout into expected bin/root executables. |
hooks/env_keys.lua |
Prepends install root to PATH when a root lua.exe exists (LuaBinaries path). |
.github/workflows/e2e_test.yaml |
Adds a Windows CI job to validate LuaBinaries installs for selected versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The previous PowerShell-based copy was being echoed to stdout instead of executed in CI, leaving lua.exe missing from the install dir. Use binary io.open read/write so the copies are independent of any shell quoting or invocation rules. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The mkdir for an extra bin/ subdirectory was failing on the Windows
runner ("The filename, directory name, or volume label syntax is
incorrect"). Since env_keys.lua already prepends the install root to
PATH, the canonical-name aliases (lua.exe, luac.exe, wlua.exe) only
need to live in the install root next to the original lua54.exe etc.
This removes the only os.execute call from the LuaBinaries path.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Why: PR #13 reviewer flagged the supply-chain risk of installing prebuilt LuaBinaries archives without integrity checking. The skip is deliberate (SourceForge mirror-redirect breaks the hash), but that rationale wasn't visible in the code or to end users. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@codex[agent] review please |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e1b3bb1d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Reviewed current PR state at |
|
To use Codex here, create a Codex account and connect to github. |
Why: Codex review on PR #13 flagged that EnvKeys was gating the extra PATH entry on VFOX_LUA_WINDOWS_LUABINARIES. That flag is an install-time opt-in and isn't persisted per SDK, so any later `vfox use` in a shell without the flag would leave the LuaBinaries install unusable. Probing for the root lua.exe is stable across activation shells and cleanly distinguishes the LuaBinaries layout from the MSYS2 source build (which only populates bin/). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Add an opt-in Windows installation path that downloads prebuilt LuaBinaries packages instead of compiling Lua from source.
Changes
VFOX_LUA_WINDOWS_LUABINARIES=1to switch Windows installs from source builds to LuaBinaries packageslua54.exeintolua.exe/luac.exe5.5.0,5.4.8, and5.3.6Testing
git diff --check HEAD~3..HEADruby -e "require 'yaml'; YAML.load_file('.github/workflows/e2e_test.yaml')"Win64_bin.zippackages for5.5.0,5.4.8,5.3.6,5.2.4Notes
5.5.0,5.4.8,5.3.6, and5.2.4