Skip to content

Make LuaRocks installation the default for supported Unix installs#15

Merged
yeshan333 merged 4 commits into
mainfrom
copilot/install-luarocks-default-behavior
May 24, 2026
Merged

Make LuaRocks installation the default for supported Unix installs#15
yeshan333 merged 4 commits into
mainfrom
copilot/install-luarocks-default-behavior

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 24, 2026

LuaRocks was only installed when VFOX_LUA_LUAROCKS=1 was set, which made the default install path inconsistent with expected plugin behavior. This change flips the default on Linux/macOS for Lua 5.x+ and keeps an explicit opt-out for users who do not want LuaRocks.

  • Runtime behavior

    • Enable LuaRocks installation by default in hooks/post_install.lua for non-Windows Lua 5.x+ installs.
    • Preserve an opt-out path via VFOX_LUA_LUAROCKS=0 or VFOX_LUA_LUAROCKS=false.
  • Documentation

    • Update the README to describe LuaRocks as the default behavior on supported platforms.
    • Replace opt-in examples with default install examples and document the opt-out flag.
  • Workflow coverage

    • Remove the explicit VFOX_LUA_LUAROCKS=1 requirement from the LuaRocks E2E workflow paths so CI exercises the new default behavior directly.
# default behavior on Linux/macOS
vfox install lua@5.4.7

# opt out when needed
VFOX_LUA_LUAROCKS=0 vfox install lua@5.4.7

Agent-Logs-Url: https://github.com/yeshan333/vfox-lua/sessions/e5ae6e5b-f652-45b7-881e-81b8a4b1149b

Co-authored-by: yeshan333 <39296814+yeshan333@users.noreply.github.com>
Copilot AI changed the title [WIP] Install luarocks as the default behavior Make LuaRocks installation the default for supported Unix installs May 24, 2026
Copilot AI requested a review from yeshan333 May 24, 2026 07:17
yeshan333 and others added 2 commits May 24, 2026 15:56
The mise_luarocks_tests job invoked `mise use -g vfox:yeshan333/vfox-lua@$LUA_VERSION`,
which clones the plugin from the default branch — not the branch under review. As
a result, this PR's "luarocks by default" change was never exercised and the
verification step failed with `luarocks: No such file or directory`.

Pre-install the plugin from `${{ github.head_ref || github.sha }}` so mise tests
the code in this PR (mirrors what the vfox-based jobs already do via
`vfox add --source .../archive/${GITHUB_REF}.zip`).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add luarocks_optout_tests job (Linux + macOS, matrix over "0" and "false")
that installs Lua with VFOX_LUA_LUAROCKS set and asserts luarocks is NOT on
PATH and $LUA_PREFIX/luarocks does not exist. Guards against regressions in
the opt-out branch in post_install.lua (e.g. dropped string.lower or
inverted truthiness check) that current tests would not catch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@yeshan333 yeshan333 marked this pull request as ready for review May 24, 2026 12:03
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4dbbe8e46

ℹ️ 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".

# Pin the plugin to this PR's ref so mise tests the branch under review,
# not whatever is on the default branch.
PLUGIN_NAME="vfox-$(echo '${{ github.repository }}' | tr '/' '-')"
$HOME/.local/bin/mise plugins install --force "$PLUGIN_NAME" "https://github.com/${{ github.repository }}.git#${PLUGIN_REF}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Fetch plugin ref from PR head repository

This install command can fail on pull_request workflows from forks: PLUGIN_REF is set from github.head_ref (the fork’s branch name), but the git URL is hardcoded to ${{ github.repository }} (the base repo). For forked PRs that branch does not exist in the base repo, so mise plugins install ...#${PLUGIN_REF} cannot resolve the ref and the mise_luarocks_tests job fails before testing anything. Use the PR head repo/SHA (github.event.pull_request.head.repo.full_name + head.sha) when github.event_name == 'pull_request'.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner

@yeshan333 yeshan333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yeshan333 yeshan333 merged commit a44dec7 into main May 24, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Installing luarocks should be the default behavior.

2 participants