luarocks / hererocks

Python script for installing Lua/LuaJIT and LuaRocks into a local directory
MIT License
71 stars 12 forks source link

Adopt LuaJIT rolling release changes #26

Closed un-def closed 9 months ago

un-def commented 9 months ago

LuaJIT recently switched to the rolling release scheme:

See LuaJIT commits:

Fixes: https://github.com/luarocks/hererocks/issues/25


In addition to the changing of the versioning scheme, there has been introduced some changes in the MinGW build.

See the commit and the PR:

The problem is that if sh is in PATH, mingw32-make will use sh instead of cmd.exe, but some recipes in Makefile use cmd.exe syntax if MinGW is detected. I have no solid proof of it, only this post on Stack Overflow, but I did a simple experiment:

  1. I have a machine with Windows and sh.exe/bash.exe provided by Git for Windows.

  2. I've tried to install LuaJIT@v2.1 from the official repository (not using hererocks, only official instructions) with no success:

    D:\LuaJIT\src>mingw32-make
    VERSION   luajit.h
    /usr/bin/bash: -c: line 1: syntax error near unexpected token `('
    /usr/bin/bash: -c: line 1: `if exist ..\.git ( git show -s --format=%%ct >luajit_relver.txt ) else ( type ..\.relver >luajit_relver.txt )'
    mingw32-make: *** [Makefile:658: luajit.h] Error 2

    As you can see, bash tries to execute Windows batch script and fails (syntax error near unexpected token)

    The same with clean recipe:

    D:\LuaJIT\src>mingw32-make clean
    del luajit.exe libluajit.a lua51.dll host\minilua.exe host\buildvm.exe lj_vm.S lj_bcdef.h lj_ffdef.h lj_libdef.h lj_recdef.h lj_folddef.h host\buildvm_arch.h luajit.h luajit_relver.txt jit\vmdef.lua *.o host\*.o *.obj *.lib *.exp *.dll *.exe *.manifest *.pdb *.ilk
    /usr/bin/bash: line 1: del: command not found
    mingw32-make: *** [Makefile:626: clean] Error 127

    Yep, del instead of rm.

  3. But when I either removed sh.exe from PATH or run mingw32-make with SHELL=cmd, all problem disappeared.

With this patch, we force mingw32-make to use Windows shell (SHELL=cmd).

codecov-commenter commented 9 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (7f74fc8) 79.78% compared to head (7151f5e) 68.71%.

Files Patch % Lines
hererocks.py 80.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #26 +/- ## =========================================== - Coverage 79.78% 68.71% -11.08% =========================================== Files 1 1 Lines 1123 1125 +2 =========================================== - Hits 896 773 -123 - Misses 227 352 +125 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

un-def commented 9 months ago

@hishamhm Hi, you review is not mandatory, just trying to keep up to date. I am by no means an expert in C/build tools/enviroments, I am mostly a web developer, so an extra pair of eyes would be great!

hishamhm commented 9 months ago

@un-def yes, the change itself and the description of your experiment in the PR description LGTM!