Upgrade hs.fs with lfs 1.8.0

[asmagill]

As hs.fs is at lest partially based on lfs, it needs to be updated as version 1.8.0 of lfs has been released and specifies compatibility with 5.4, which we expect to be moving to soon.

I've not tested the breaking code he gives below, but in any case, we should update for 5.4 if for no other reason.

From the lua-l mailing list:

Hi list,

I am happy to announce LuaFileSystem 1.8.0, its first release in 3 years.

LuaFileSystem is a Lua library developed to complement the set of
functions related to file systems offered by the standard Lua
distribution. LuaFileSystem offers a portable way to access the
underlying directory structure and file attributes.

https://keplerproject.github.io/luafilesystem/

What's new:

• lfs.link and lfs.symlinkattribute now work on Windows!
• Fixed memory leak in case of realloc failure
• MACOSX_DEPLOYMENT_TARGET is configurable in the Makefile
• Fallback to _POSIX_PATH_MAX when MAXPATHLEN is not avaliable
• lfs.lock uses CREATE_ALWAYS instead of CREATE_NEW on Windows

and last but not least:

• Lua 5.4 support, including __close handling in the lfs.dir()
  iterator. This requires no changes from the user, and ensures that the
  directory will be closed correctly even if you break from the loop.
  For example, with uname -n set below 10000, this script crashes on
  Lua < 5.4, and works in Lua 5.4:

    local lfs = require("lfs")
    for t = 1, 10000 do
      local i = 0
      for e in lfs.dir("/usr/lib") do
         i = i + 1
         if i == 100 then
            break
         end
      end
    end

(This is not a forced example: I have hit this problem before when
recursing over large directory trees and collecting files.)

The easiest way to install LuaFileSystem is to use LuaRocks:

luarocks install luafilesystem

This release contains contributions by Dan Church, Stephen E. Baker,
Kıvanç Çakmak, TsT, Peter Melnichenko, James McCoy, Joshua Root,
Eroica, 云风 and myself.

Cheers,

-- Hisham

---

[latenitefilms]

Looks like it'll be a fairly manual effort.

@asmagill and @cmsj - Let me know if this is something you want me to have a play with at some point?

[cmsj]

Yeah this is going to be a painful merge, to the point that I almost wonder if we'd be better off looking at ways to split internal.m into Hammerspoon specific things and a minimally-modified lfs.c from upstream, so future merges are easier.
We took out almost all of the platform specific #ifdefs which makes the code much easier to work with, but.... not fun for these events!

[asmagill]

Well, the module hadn't been previously updated for 2.5 years... not sure how often this will be an issue.

(The following are my thoughts after spending about 5 minutes looking at the code, so it may be partial or incomplete. If you want me to take a whack at it, I can in a couple of days; but in case someone else wants to give it a go sooner...)

To make future updates as close to drop in as possible, and assuming that you didn't previously make any changes to the lfs code you copied in originally (changes to error reporting or arguments, etc.) I think how I'd approach this is as follows:

• copy lfs.c and lfs.h into the fs/ directory
• remove lfs specific functions, references, etc. from internal.m, but leave comments for documentation; update if there were any changes or omissions from the previous version.
• add extern int luaopen_lfs(lua_State * L) ; to internal.m probably after the includes at the top.
• replace contents of luaopen_hs_fs_internal in internal.m with something like the following:
  1. if global lfs already exists, store reference to it in registry (the lfs initializer creates it)
  2. invoke luaopen_lfs(L) ;
  3. iterate through fslib in internal.m and add the remaining entries (i.e. the Hammerspoon specific ones) to the table on the top of the stack (with lua_pushcfunction and lua_setfield)
  4. remove the global lfs created by luaopen_lfs
  5. if global lfs previously stored in registry, restore it
  6. in Hammerspoon Xcode project, make sure lfs.c is compiled with internal.m so both are used to create internal.so

There may be some flags we need to set to make sure the OSX specific code is used... glancing at Makefile and config for lfs, I don't think so -- I think the flags already set for internal.m will be sufficient -- but I'm not positive, it will take testing.

[latenitefilms]

It looks like the only changes are (crossed out the irrelevant ones):

• Lua 5.4 support
• lfs.link and lfs.symlinkattributes now work on Windows
• MACOSX_DEPLOYMENT_TARGET is configurable in the Makefile
• Fallback to _POSIX_PATH_MAX when MAXPATHLEN is not avaliable
• Fixed memory leak in case of realloc failure
• Use CREATE_ALWAYS instead of CREATE_NEW
• Lua 5.4: use __close to close dir if you break the iterator

...so maybe it's just easier to just manually make these changes?

[cmsj]

@asmagill hmm, your plan is interesting, although the one issue I can see is that if someone has installed lfs from luarocks/whatever, and is using it in Hammerspoon, we have a potential problem with the global lfs.

I'm wondering if we could start to shift towards having hs.fs for our code and hs.fs.lfs for unmodified upstream lfs, and we just don't call their luaopen_lfs, but set it up ourselves? We could maintain the existing hs.fs functions as deprecated aliases for the hs.fs.lfs ones, and remove them post-1.0 (in the imaginary far off timeline where 1.0 actually happens ;)

Edit: Or maybe we don't have that namespace differentiation at all, and just keep the calls in hs.fs. That's probably better for users, they don't need to care that lfs is providing simple file/directory functionality.

[latenitefilms]

@cmsj & @asmagill - I believe this is the only thing we now need to fix in regards to hs.fs working with Lua 5.4:

• Lua 5.4: use __close to close dir if you break the iterator

[latenitefilms]

@asmagill - This might be another good thing to test/fix.

[asmagill]

Are you saying that this (the __close for dir objects) is the only difference between the two versions of lfs that we need to adjust our code for?

Or are you saying that we need to update hs.fs?

[latenitefilms]

Correct - as far as I can tell, this is the only difference.

I went through all the commits in the new release in this comment to manually compare.

[asmagill]

While I agree we should do this, I'm kind of burnt out atm and need a break. If I understand correctly, this isn't actually adding "necessary" code, just "better" code that 5.4 can take advantage of, especially if __gc is delayed longer than it used to be because of the new collection methodology.

If you want to take a crack at it, go for it, but unless one of you disagrees, I don't think this should hold up 0.9.81 if @cmsj does decide to land #2509 and release tonight/tomorrow.

[asmagill]

@latenitefilms There is a slight difference between the 1.8.0 lfs version of symlinkattributes and ours and I want to know if you think we should mirror theirs or not...

Hammerspoon's just acts as an equivalent to attributes, but for the link itself. The version in 1.8.0 is a little different -- they add a field target which contains the path the link points to (you can also specify "target" as the second argument if you just want the value of that particular field).

We can get the same info with hs.fs.pathToAbsolute, so it's not like we can't get at the value -- it's just not part of the functions we "inherit" from lfs.