Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does not install correctly on Windows #177

Closed
ColinKennedy opened this issue Jan 14, 2025 · 8 comments
Closed

Does not install correctly on Windows #177

ColinKennedy opened this issue Jan 14, 2025 · 8 comments

Comments

@ColinKennedy
Copy link

ColinKennedy commented Jan 14, 2025

I'm installing luafilesystem via a GitHub action and noticing that even after installing it via luarocks, I still cannot require("lfs").

See: https://github.com/ColinKennedy/luarocks_window_test/actions/runs/12762606264/job/35571383842

Specifically these lines:

Run luarocks install luafilesystem
lfs.c
Microsoft (R) Incremental Linker Version 14.42.34435.0
Copyright (C) Microsoft Corporation.  All rights reserved.

   Creating library C:\Users\RUNNER~1\AppData\Local\Temp\luarocks_build-LuaFileSystem-1.8.0-1-1885464\lfs.lib and object C:\Users\RUNNER~1\AppData\Local\Temp\luarocks_build-LuaFileSystem-1.8.0-1-1885464\lfs.exp
No existing manifest. Attempting to rebuild...
Installing https://luarocks.org/luafilesystem-1.8.0-1.src.rock

luafilesystem 1.8.0-1 depends on lua >= 5.1 (5.1-1 provided by VM: success)
cl /nologo /MD /O2 -c -Fosrc/lfs.obj -ID:/a/luarocks_window_test/luarocks_window_test/.lua/include src/lfs.c
link -dll -def:lfs.def -out:C:\Users\RUNNER~1\AppData\Local\Temp\luarocks_build-LuaFileSystem-1.8.0-1-1885464\lfs.dll D:\a\luarocks_window_test\luarocks_window_test\.lua\lib\lua51.lib src/lfs.obj
luafilesystem 1.8.0-1 is now installed in C:\Users\runneradmin\AppData\Roaming\luarocks (license: MIT/X11)

D:/a/luarocks_window_test/luarocks_window_test/.lua/bin\lua.exe: error loading module 'lfs' from file 'C:\Users\runneradmin\AppData\Roaming\luarocks\lib\lua\5.1\lfs.dll':
	The specified module could not be found.


stack traceback:
	[C]: ?
	[C]: in function 'require'
	(command line):1: in main chunk
	[C]: ?

According to the output, it looks like luarocks completed the install and lua can even see the lfs.dll file. Lua just cannot import it. It's hard to tell exactly if this is a luafilesystem issue or an environment/luarocks issue but I think given the output that it's luafilesystem.

This is a minimal GitHub workflow used to generate the above error: https://github.com/ColinKennedy/luarocks_window_test/actions/runs/12762606264/workflow

name: Test

on:
  pull_request:
    types: [opened, synchronize, reopened, ready_for_review]
    branches:
    - main
  push:
    branches:
      - main
      - first_pass

jobs:
  test:
    strategy:
      matrix:
        os: ["windows-latest"]
        luaVersion:
        - "5.1"
        include:
          - os: "windows-latest"
            toolchain: "msvc"
            luaVersion: "5.1"

    runs-on: ${{ matrix.os }}
    name: "OS: ${{ matrix.os }} - Neovim: ${{ matrix.neovim }}"

    steps:

    - uses: actions/checkout@v4
      with:
        repository: "lunarmodules/luasystem"

    - name: Setup MSVC
      # the 'hishamhm/gh-actions-lua' step requires msvc to build PuC Rio Lua
      # versions on Windows (LuaJIT will be build using MinGW/gcc).
      if: ${{ matrix.toolchain == 'msvc' }}
      uses: ilammy/msvc-dev-cmd@v1

    - uses: leafo/gh-actions-lua@master
      with:
        luaVersion: "5.1"

    - uses: hishamhm/gh-actions-luarocks@v5
      with:
        luarocksVersion: "3.11.0"

    - name: Install LuaFileSystem
      run: |
        luarocks install luafilesystem
        lua -e "require 'lfs'"

The workflow file is all you need to try this yourself. If you'd like to trigger new GitHub workflow runs that can be done by cloning the test repository and pushing to it. e.g.

git clone [email protected]:ColinKennedy/luarocks_window_test.git --branch first_pass
git commit --allow-empty -m "Test push"
git push
@Tieske
Copy link
Member

Tieske commented Jan 19, 2025

this seems to be an installation issue, since it seems to build properly, but isn't found.

so probably more related to the Lua action than to lfs itself.

@ColinKennedy
Copy link
Author

ColinKennedy commented Jan 19, 2025

If I install another package, require works. For example, this other run:

https://github.com/ColinKennedy/luarocks_window_test/actions/runs/12856066784/job/35842325008

name: Test

on:
  pull_request:
    types: [opened, synchronize, reopened, ready_for_review]
    branches:
    - main
  push:
    branches:
      - main
      - testing_other_packages

jobs:
  test:
    strategy:
      matrix:
        os: ["windows-latest"]
        luaVersion:
        - "5.1"
        include:
          - os: "windows-latest"
            toolchain: "msvc"
            luaVersion: "5.1"

    runs-on: ${{ matrix.os }}
    name: "OS: ${{ matrix.os }} - Neovim: ${{ matrix.neovim }}"

    steps:

    - uses: actions/checkout@v4
      with:
        repository: "lunarmodules/luasystem"

    - name: Setup MSVC
      # the 'hishamhm/gh-actions-lua' step requires msvc to build PuC Rio Lua
      # versions on Windows (LuaJIT will be build using MinGW/gcc).
      if: ${{ matrix.toolchain == 'msvc' }}
      uses: ilammy/msvc-dev-cmd@v1

    - uses: leafo/gh-actions-lua@master
      with:
        luaVersion: "5.1"

    - uses: hishamhm/gh-actions-luarocks@v5
      with:
        luarocksVersion: "3.11.0"

    - name: Install mega.logging
      run: |
        luarocks install mega.logging
        lua -e "require 'mega.logging'"

    - name: Install LuaFileSystem
      run: |
        luarocks install luafilesystem
        lua -e "require 'lfs'"

I install a different package and require it, no issues. Do the same for luafilesystem, errors.

The reason this came up was that I was trying to install busted, which depends on penlight which depends on luafilesystem. I didn't test all dependencies but this seems to be lfs specific since I haven't reproduced this issue for another package yet.

@ColinKennedy
Copy link
Author

ColinKennedy commented Jan 19, 2025

Another point,

D:/a/luarocks_window_test/luarocks_window_test/.lua/bin\lua.exe: error loading module 'lfs' from file 'C:\Users\runneradmin\AppData\Roaming\luarocks\lib\lua\5.1\lfs.dll':

It seems to me that the installation process (specifically, the portions that discovers lua modules) succeeded because it does see the .dll. It just is unable to get the data that it needs after finding it. So that makes me think either

A. The build is marked as succeeded but actually failed in some way (false-positive)
B. A portion of the install process fails to make / register something that lfs needs, which leads to require not working
C. An environment issue (LUA_PATH, LUA_CPATH, etc)
D. Any combination of the above.

Which do you think this is?

@Tieske
Copy link
Member

Tieske commented Jan 21, 2025

that thing again... lunarmodules/luasystem#17 (comment)

@ColinKennedy
Copy link
Author

ColinKennedy commented Jan 22, 2025

I tried this again, this time changing leafo/gh-actions-lua@master to hishamhm/gh-actions-lua@master, no dice.

(replace leafo run)

then I tried changing hishamhm/gh-actions-luarocks@v5 to hishamhm/gh-actions-luarocks@master just in case, so I match .github/workflows/build.yml from https://github.com/lunarmodules/luasystem/pull/17/files

(same error)

It's worth noting that the hishamhm/* redirect to luarocks/* so I think I'm using the most up to date thing. Still, it's not working.

@hishamhm is there any chance you can check this out? It looks like the error from before is back again, some way. For quick reference, this is the latest minimal reproduction #177 (comment). Or also looking at the workfile section of the links above

@luau-project
Copy link

Hello all,

Given the description and analyzing the workflow you posted, I'm pretty confident that this issue happens on hishamhm/gh-actions-luarocks@v5 end, probably when setting Lua-related paths as you guys commented.

I have a Lua installer project (MSVC only) that, as part of the testing process, I continously:

  1. install Lua by my tool;
  2. install LuaRocks versions manually from https://luarocks.github.io/luarocks/releases/;
  3. configure LuaRocks for the Lua version just installed;
  4. install LuaFileSystem by LuaRocks;
  5. test LuaFileSystem just installed.

Up to date, the current LFS hosted on LuaRocks is working just fine built with MSVC toolchain.

If you guys want, I can post the manual LuaRocks installation process (+ configuration), so you guys can help diagnose the problem on hishamhm/gh-actions-luarocks@v5.

@ColinKennedy
Copy link
Author

ColinKennedy commented Jan 23, 2025

My understanding is that gh-actions-luarocks just installs luarocks and doesn't touch env vars. I thought the env vars are updated later after a package is installed. Though I haven't tested any of those parts yet so I could be wrong.

If you guys want, I can post the manual LuaRocks installation process (+ configuration), so you guys can help diagnose the problem on hishamhm/gh-actions-luarocks@v5.

That would be excellent yes. Is this it or were you referring to something different?

https://github.com/luau-project/LuaInstaller/blob/43aea5284a18a68bbabde18e7645348d18633ba9/.github/workflows/CI.yaml#L15-L31

https://github.com/luau-project/LuaInstaller/blob/43aea5284a18a68bbabde18e7645348d18633ba9/.github/workflows/lfs.yaml

That lfs.yaml is pretty daunting to read but if it can help give us insight I think that'd be a very good thing. I'm interested in luarocks config lua_dir "${{ env.LUA_DIR }}" is doing and the surrounding lines.

If the issue really is env-var related and nothing build specific then I can try messing around with variables and seeing if I can make any progress over the weekend

@hishamhm
Copy link
Member

@hishamhm is there any chance you can check this out? It looks like the error from before is back again, some way. For quick reference, this is the latest minimal reproduction #177 (comment). Or also looking at the workfile section of the links above

It is a configuration issue in your Action. You are mixing the leafo Lua action with the hishamhm LuaRocks action. In your luarocks_window_test repo you were also forcing luajit-openresty on Windows and mixing it with MSVC leading to a combination that is not supported by the LuaRocks action.

Here's a fork of your test repo showing the luarocks actions working and building LuaFileSystem successfully:

https://github.com/hishamhm/luarocks_window_test/actions/runs/12924615247

name: Test

on:
  pull_request:
    types: [opened, synchronize, reopened, ready_for_review]
    branches:
    - main
  push:
    branches:
      - main

jobs:
  test:
    strategy:
      matrix:
        include:
        - os: "windows-latest"
          toolchain: "msvc"
          luaVersion: "5.1"
          neovim: "v0.10.0"
        - os: "windows-latest"
          toolchain: "mingw"  # unused, other than for display in the UI
          luaVersion: "luajit"
          neovim: "v0.10.0"

    runs-on: ${{ matrix.os }}
    name: "${{ matrix.os }} ${{ matrix.toolchain }}, Lua ${{ matrix.luaVersion }}, Neovim: ${{ matrix.neovim }}"

    steps:
    - name: Setup MSVC
      # the 'luarocks/gh-actions-lua' step requires msvc to build PUC-Rio Lua
      # versions on Windows (LuaJIT will be build using MinGW/gcc).
      if: ${{ matrix.toolchain == 'msvc' }}
      uses: ilammy/msvc-dev-cmd@v1

    - uses: luarocks/gh-actions-lua@master
      with:
        luaVersion: "${{ matrix.luaVersion }}"

    - uses: luarocks/gh-actions-luarocks@v5

    - name: Install mega.logging
      run: |
        luarocks install mega.logging
        lua -e "require 'mega.logging'"

    - name: Install LuaFileSystem
      run: |
        luarocks install luafilesystem
        lua -e "require 'lfs'"

Closing this since this is not a LuaFileSystem bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants