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

Luarocks: use cmd mkdir #12002

Closed
wants to merge 1 commit into from
Closed

Luarocks: use cmd mkdir #12002

wants to merge 1 commit into from

Conversation

osch
Copy link
Contributor

@osch osch commented Jul 5, 2022

Here comes a fix for the following situation: Luarocks cannot create missing parent directories if the Lua package "lfs" is not installed.

Steps to reproduce:

1.) Assure that Lua package "lfs" (luafilesystem) is not installed (P.S. uninstallation may by tricky, see below #12002 (comment)):

$ luarocks remove luafilesystem
$ lua -e 'print(require"lfs")'
C:\msys64\mingw64\bin\lua.exe: (command line):1: module 'lfs' not found:
...

2.) Install some package (e.g. "lpeg")

$ luarocks remove lpeg
$ luarocks install lpeg
Error: failed making directory C:/msys64/mingw64/lib/luarocks/rocks-5.4/lpeg/1.0.2-1/bin

The reason for this is, that Luarocks has a fallback for mkdir if "lfs" is not installed, see the code for Windows:
https://github.com/luarocks/luarocks/blob/ecb63347bf7d44c68210befc9d03c013350c7831/src/luarocks/fs/win32/tools.lua#L36-L44

function tools.make_dir(directory)
   assert(directory)
   directory = dir.normalize(directory)
   fs.execute_quiet(vars.MKDIR, directory)
   if not fs.is_dir(directory) then
      return false, "failed making directory "..directory
   end
   return true
end

However it is assumed here for Windows, that mkdir creates missing parent directories. Compare the above code with this code for Linux:
https://github.com/luarocks/luarocks/blob/ecb63347bf7d44c68210befc9d03c013350c7831/src/luarocks/fs/unix/tools.lua#L24-L31

function tools.make_dir(directory)
   assert(directory)
   local ok, err = fs.execute(vars.MKDIR.." -p", directory)
   if not ok then
      err = "failed making directory "..directory
   end
   return ok, err
end

Note the "-p" here.

The problem is caused by:

+ defaults.variables.PWD = "cd"
+ local pipe2 = io.popen("cygpath --windows /usr/bin/mkdir")
+ defaults.variables.MKDIR = pipe2:read("*l")
+ pipe2:close()

IMHO this seems to be wrong in several ways, e.g. cygpath is invoked here. This was also discussed in #9037, see #9037 (comment)

One solution could be to append a " -p" to MKDIR here. The solution in this pull request is to use the cmd mkdir as it is done without the invocation of "cygpath --windows /usr/bin/mkdir".

So IMHO this pull request is only a temporary fix to keep this package working as it is if lfs is not installed. A better solution is needed in the long term. Best would be to make the Luarocks package a "real" mingw package that does also work if not invoked under MSYS environment.

@lazka
Copy link
Member

lazka commented Jul 17, 2022

@ImperatorS79 any thoughts on this?

@ImperatorS79
Copy link
Contributor

I am not an expert about luarocks, the patch was adapted from a PR that was never merged into luarocks. Last time I worked on luarocks on msys2, there were not any problem and I could install packages, but luarocks was updated in the mean time.

I tested installing list with this PR, and it worked, but if OP could test a few more packages it would be great.

I agree with the last comment from OP, but I think it should be an issue on the luarocks side. They could certainly pinpoint what needs to be done to use luarocks both from cmd and msys shell (does it fails at the moment ?).

Is it possible to add a check() which try to install a luarocks package in the PKGBUILD ? That way one could notice directly problems.

@osch
Copy link
Contributor Author

osch commented Jul 19, 2022

I would like to separate the following two issues:

  1. how to fix the above desribed error when creating directories when "luafilesystem" is not installed
  2. what has to be done to make this Luarocks package a "real" mingw package

For 2. I would like to create a new issue for further discussion.

@osch
Copy link
Contributor Author

osch commented Jul 21, 2022

BTW: uninstallation of lfs may be tricky, see also luarocks/luarocks#1428

To be shure that lfs is not around it must be removed manually:

$  luarocks remove luafilesystem
...
$ lua -e 'print(require"lfs")'
table: 000001ade2579ef0 C:\msys64\mingw64\bin\..\lib\lua\5.4\lfs.dll
$ rm /mingw64/lib/lua/5.4/lfs.dll
$ lua -e 'print(require"lfs")'
C:\msys64\mingw64\bin\lua.exe: (command line):1: module 'lfs' not found:
...

@osch
Copy link
Contributor Author

osch commented Aug 4, 2022

To be sure I just re-checked the problem with a fresh install of MSYS2 under c:\tmp and this illustrates that the above bug is severe: you cannot use luarocks on a fresh MSYS2 without the fix for mkdir. I (and perhaps also @ImperatorS79 ) just didn't notice it from the beginning because luafilesystem (lfs) was just around from a prior install.

Steps to reproduce

  • Install fresh MSYS2 into C:\tmp\msys64

  • Launch "MSYS2 MinGW x64" (MSYSTEM=MINGW64)

$ type lua
bash: type: lua: not found

$ pacman -S mingw-w64-x86_64-lua
....

$ type lua
lua is /mingw64/bin/lua

$ lua -e 'print(require"lfs")'
C:\tmp\msys64\mingw64\bin\lua.exe: (command line):1: module 'lfs' not found:

$ lua -e 'print(require"lpeg")'
C:\tmp\msys64\mingw64\bin\lua.exe: (command line):1: module 'lpeg' not found:

$ pacman -S mingw-w64-x86_64-lua-luarocks
...

$ pacman -Q mingw-w64-x86_64-lua
mingw-w64-x86_64-lua 5.4.4-1

$ pacman -Q mingw-w64-x86_64-lua-luarocks
mingw-w64-x86_64-lua-luarocks 3.9.0-1

$ lua -e 'print(require"lfs")'
C:\tmp\msys64\mingw64\bin\lua.exe: (command line):1: module 'lfs' not found:

$ lua -e 'print(require"lpeg")'
C:\tmp\msys64\mingw64\bin\lua.exe: (command line):1: module 'lpeg' not found:

$ luarocks install lpeg
Installing https://luarocks.org/lpeg-1.0.2-1.src.rock

lpeg 1.0.2-1 depends on lua >= 5.1 (5.4-1 provided by VM)

Error: failed making directory C:/tmp/msys64/mingw64/lib/luarocks/rocks-5.4/lpeg/1.0.2-1/conf

$ luarocks install luafilesystem
Installing https://luarocks.org/luafilesystem-1.8.0-1.src.rock

luafilesystem 1.8.0-1 depends on lua >= 5.1 (5.4-1 provided by VM)

Error: failed making directory C:/tmp/msys64/mingw64/lib/luarocks/rocks-5.4/luafilesystem/1.8.0-1/lib

@solemnwarning
Copy link

Is there any workaround for this? I've not been able to deploy/update a machine image that needs to install modules using Luarocks for a few months. Haven't managed to get it to work using mkdir/cygpath wrapper scripts or similar hacks.

Thanks

@osch
Copy link
Contributor Author

osch commented Oct 20, 2022

@solemnwarning

It would be easier, if my provided patch would by applied. There seems only little interest to have working luarocks in msys2, so for now luarocks remains unusable in msys2, at least in a fresh install, as my above example shows :-(

As a workaroud you could try to create all needed directories for luafilesystem, e.g. C:/tmp/msys64/mingw64/lib/luarocks/rocks-5.4/luafilesystem/1.8.0-1/lib in the above example (you could try-and-error to figure out any other missing directories) and then try to install luafilesystem first. After this, luarocks is using lfs for creating directories.

solemnwarning added a commit to solemnwarning/build-server-deployment that referenced this pull request Oct 21, 2022
@SlySven
Copy link
Contributor

SlySven commented Apr 8, 2023

For a crude workaround I use this - actually it is a fragment that I use in a for loop (with luafilesystem first) to install a series of luarocks - which is inserted here as ${rock} - as the luarocks install just doesn't seem capable of creating the needed directories.

 neededPath=$(luarocks $@ install ${rock} 2>&1 | grep "failed making directory" | cut -c 32-)
 until [ -z ${neededPath} ]; do
   echo "Inserting a needed directory: ${neededPath} ..."
   mkdir -p ${neededPath}
   neededPath=$(luarocks $@ install ${rock} 2>&1 | grep "failed making directory" | cut -c 32-)
 done

It might be because I am working with Lua 5.1 rocks (so I am passing --lua-version 5.1 as an argument to the script) so do not have a luafilesystem module for the Lua 5.4 luarocks to work with to create those directories itself...?

@SlySven
Copy link
Contributor

SlySven commented Apr 8, 2023

Yeah, that is confirmed - with the Lua 5.4 luafilesystem rock installed all my 5.1 ones install cleanly without further problems. As is it the 5.4 one only needs a single mkdir -p usage - for, in my case in a Git-for-Windows SDK environment of C:/git-sdk-64/mingw64/lib/luarocks/rocks-5.4/luafilesystem/1.8.0-1/bin ...

@hishamhm
Copy link

However it is assumed here for Windows, that mkdir creates missing parent directories.

AFAIK, this is true for CMD.EXE: https://stackoverflow.com/a/905237

Is the msys shell picking up a different MKDIR that happens to be a Unix clone? Would it help to avoid confusion if LuaRocks instead used MD?

@osch
Copy link
Contributor Author

osch commented Feb 20, 2024

However it is assumed here for Windows, that mkdir creates missing parent directories.

AFAIK, this is true for CMD.EXE

Yes of course.

Is the msys shell picking up a different MKDIR that happens to be a Unix clone?

Yes, see my first post in this pull request where everything is explained: The problem is caused by the msys2 patch:
MINGW-packages/mingw-w64-lua-luarocks/0001-luarocks_msys2_mingw_w64.patch

This patch is executed by the msys2 build/installsystem when luarocks is installed with the command pacman -S mingw-w64-x86_64-lua-luarocks. So the error is caused by msys2 patching luarocks during install and not by luarocks code itself.

My pull request is a quick fix to correct the msys2 patch such that luarocks is using again CMD.EXE instead of using the unix like /usr/bin/mkdir from the MSYS environment.

This is a very simple solution for a trivial problem to just get it working and it's sad that I had written so much text about it, filed a pull request und also a problem report #15458 and no one seems to be interested to merge this pull requst since 7/2022 :-(

As I also wrote above this is only a quick fix to just get luarocks working again. In the future there should be more work done, to make luarocks a real MINGW package. In msys2 there are different environments (see also https://www.msys2.org/docs/environments/): MSYS packages (applications that are running under unix like environment in windows) and MINGW*/CLANG* packages (application that are running without any unix environment and without any hardcoded location). For this there must no unix like environment be used and no hardcoded path where packages etc. are located. For Example the msys2 lua package (https://github.com/msys2/MINGW-packages/tree/e09173589270815a80e287dfe7c98e5f8880b7e4/mingw-w64-lua) is a real MINGW package. You can copy the binary lua.exe and needed dlls or lua packages to any location under any windows system and it works as long as the relative directory structure regarding to the lua binary lua.exe is preserved. I would have started to work on this problem and to provide patches for the luarocks msys2 package but I'm not willing to invest further work here if there is no chance that the msys2 community has interest on a working luarocks :-( The main point for this work would be to have a possibility for lua script code to get the location of the lua.exe. The lua.exe knows this location since it can replace the ! character: In Windows, any exclamation mark ('!') in the path is replaced by the path of the directory of the executable file of the current process. So a great approach would be to patch msys2 lua pacakge to provide a mechanism for lua script code to get the location of the lua.exe binary. With this, luarocks could determine where its rock packages tree is located. Otherwise a little bit more complicated solution could be to have a C module in luarocks that provides the executable location by calling the windows function GetModuleFileNameA to lua script code. With this one could build easily a lightwight lua distribution (optionally including a complete luarocks build system) by using the msys2 build system and copy all needed DLLs and EXEs into a ZIP file that can be used on any plain windows system by just extracting the ZIP archive into any location. (Update: It's also possible to retrieve the lua.exe location in lua script by using io.popen("cd") for obtaining the current directory, together with the global arg table for obtaining the full invocation including the path to the invoked binary).

@osch
Copy link
Contributor Author

osch commented Feb 24, 2024

fixed in #20166

@osch osch closed this Feb 24, 2024
@osch osch deleted the luarocks branch February 24, 2024 16:29
@Biswa96 Biswa96 added bug and removed bug labels Feb 24, 2024
vadi2 added a commit to Mudlet/Mudlet that referenced this pull request Dec 11, 2024
<!-- Keep the title short & concise so anyone non-technical can
understand it,
     the title appears in PTB changelogs -->
#### Brief overview of PR changes/additions
Simplify Luarocks setup on Windows
#### Motivation for adding to Mudlet
Less is more and msys2/MINGW-packages#12002 is
fixed
#### Other info (issues closed, discussion etc)
To be tested if Windows setup still works after this
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.

7 participants