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

feat(fs): add vim.fs module #18583

Merged
merged 6 commits into from
May 31, 2022
Merged

feat(fs): add vim.fs module #18583

merged 6 commits into from
May 31, 2022

Conversation

gpanders
Copy link
Member

@gpanders gpanders commented May 15, 2022

This PR adds a minimal vim.fs module. For now, only these six functions are included:

  • vim.fs.parents: an iterator that iterates over all of the parent/ancestor directories of the given file or directory

    Usage example:

    local gitdir
    for parent in vim.fs.parents(vim.api.nvim_buf_get_name(0)) do
      if parent == vim.env.HOME then
        break
      end
    
      if vim.fn.isdirectory(parent .. "/.git") then
        gitdir = parent .. "/.git"
        break
      end
    end
    if gitdir then
      print("Found git dir:", gitdir)
    end
  • vim.fs.dirname: Return the directory name of the given file or directory

    Usage example:

    assert("/home" == vim.fs.dirname("/home/greg"))
  • vim.fs.basename: Return the base name of the given file or directory

    Usage example:

    assert("greg" == vim.fs.basename("/home/greg"))
    
  • vim.fs.dir: Iterator over all files and directories within a given directory. Adapted from Penlight's path.dir() (which is itself adapted from luafilesystem.dir()).

    Usage example:

    local hidden = {}
    for name, type in vim.fs.dir(vim.env.HOME) do
      if name:find("^%.") then
        hidden[#hidden + 1] = name
      end
    end
  • vim.fs.find: Find files or directories upwards or downwards from a given path

    Usage example 1:

    local editorconfig = vim.fs.find(".editorconfig", { upward = true, limit = math.huge })
    if #editorconfig > 0 then
      print("Found .editorconfig files:", vim.pretty_print(editorconfig))
    end

    Usage example 2:

    local markers = vim.fs.find({"compile_commands.json", "compile_flags.txt", ".git"}, { upward = true })
    if #markers > 0 then
      print("Found root directory:", vim.fs.dirname(markers[1]))
    end
  • vim.fs.normalize: Normalize a path to a standard format, expanding tildes and environment variables

    Usage example:

    print(vim.fs.normalize("$XDG_CONFIG_HOME/nvim/init.vim"))
    => "/Users/greg/.config/nvim/init.vim"
    
    print(vim.fs.normalize("C:\\Users\\greg"))
    => "C:/Users/greg"

@gpanders gpanders requested a review from justinmk May 15, 2022 20:42
@github-actions github-actions bot added the lua stdlib label May 15, 2022
runtime/lua/vim/path.lua Outdated Show resolved Hide resolved
@gpanders gpanders marked this pull request as draft May 15, 2022 23:06
@github-actions github-actions bot removed the request for review from justinmk May 15, 2022 23:07
@gpanders gpanders changed the title feat(path): add vim.path.ascend() feat(path): add vim.path module May 16, 2022
@gpanders gpanders marked this pull request as ready for review May 16, 2022 02:57
@gpanders gpanders force-pushed the path-root branch 2 times, most recently from 893377a to 9cf27ea Compare May 16, 2022 03:23
runtime/lua/vim/path.lua Outdated Show resolved Hide resolved
runtime/lua/vim/path.lua Outdated Show resolved Hide resolved
@mjlbach
Copy link
Contributor

mjlbach commented May 16, 2022

related: #16800

@gpanders gpanders force-pushed the path-root branch 4 times, most recently from c3cdd94 to 31e54d8 Compare May 16, 2022 15:39
Copy link
Member

@mfussenegger mfussenegger left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I wonder if we should start indicating in the docs if a function can be used in a luv callback? Currently one would have to try or read the code.

@gpanders gpanders force-pushed the path-root branch 2 times, most recently from 05ffd2b to e1a0f7e Compare May 16, 2022 18:14
@justinmk
Copy link
Member

justinmk commented May 17, 2022

related: #16800

if we start with plain strings, that should keep the door open on a Uri concept: the functions in this PR can later be overloaded. (I don't see a real need for having actual methods on the Uri though--the main purpose is to have an unambiguous representation.)

runtime/doc/lua.txt Outdated Show resolved Hide resolved
Copy link
Member

@muniter muniter left a comment

Choose a reason for hiding this comment

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

I really like this functions. Have a few comments/questions, hope is not too spammy :)

I also see no mention of supporting ~ for user home dir, which is fine but perhaps it's worth mentioning.

What about windows paths is the separator normalized (I imagine not), maybe the examples should have a disclaimer.


Return: ~
Iterator over files and directories in {path}. Each
iteration yields two values: name and type.
Copy link
Member

Choose a reason for hiding this comment

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

Which are the types dir, directory, file ?

Copy link
Member Author

Choose a reason for hiding this comment

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

"file" and "directory", I added this to the docs

{file} (string) File or directory

Return: ~
(string) Parent directory of {file}
Copy link
Member

Choose a reason for hiding this comment

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

Relative or absolute path?

Copy link
Member Author

Choose a reason for hiding this comment

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

Either, it depends on what it is passed. vim.fs.dirname("foo/bar") returns "foo", vim.fs.dirname("/foo/bar") returns "/foo".

(string) Basename of {file}

dir({path}) *vim.path.dir()*
Return an iterator over the files and directories located in
Copy link
Member

Choose a reason for hiding this comment

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

The iterator provides the file or directory basename or full path?

Copy link
Member Author

Choose a reason for hiding this comment

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

Each file/directory name returned from the iterator is relative to the provided {path} argument. I added a note about this to the docs.

@gpanders gpanders changed the title feat(path): add vim.path module feat(fs): add vim.fs module May 17, 2022
@gpanders
Copy link
Member Author

I also see no mention of supporting ~ for user home dir, which is fine but perhaps it's worth mentioning.

What about windows paths is the separator normalized (I imagine not), maybe the examples should have a disclaimer.

I've added fs.normalize() for these purposes as well.

For now this just uses / for all separators, as Neovim internally uses that for all platforms. There may be some use cases where we still need to support \ too, but I think we could tackle that in a separate PR (chasing down the peculiarities of Windows can be time-consuming, and I'd rather not block this PR on that).

@gpanders
Copy link
Member Author

This is currently blocked by a memory leak in fs_scandir caught by ASAN:

ASAN output
==================== File /home/runner/work/neovim/neovim/build/log/ubsan.18828 ====================
= 
= =================================================================
= ==18828==ERROR: LeakSanitizer: detected memory leaks
= 
= Direct leak of 80 byte(s) in 1 object(s) allocated from:
=     #0 0x4deaa3 in realloc ??:?
=     #1 0x7f433618f9b2 in __scandir64_tail /build/glibc-SzIz7B/glibc-2.31/dirent/./scandir-tail-common.c:62:31
= 
= Direct leak of 32 byte(s) in 1 object(s) allocated from:
=     #0 0x4de7ad in malloc ??:?
=     #1 0x1e9474d in luv_fs_scandir luv.c
= 
= Indirect leak of 200 byte(s) in 6 object(s) allocated from:
=     #0 0x4de7ad in malloc ??:?
=     #1 0x7f433618f8cc in __scandir64_tail /build/glibc-SzIz7B/glibc-2.31/dirent/./scandir-tail-common.c:69:27
= 
= SUMMARY: AddressSanitizer: 312 byte(s) leaked in 8 allocation(s).
====================================================================================================
nan ms test/helpers.lua:257: Found runtime errors in logfile(s): /home/runner/work/neovim/neovim/build/log/ubsan.18828

stack traceback:
	test/helpers.lua:257: in function 'check_logs'
	test/functional/helpers.lua:874: in function <test/functional/helpers.lua:873>

[ RUN      ] vim.fs normalize() works with backward slashes: 43.94 ms OK
==================== File /home/runner/work/neovim/neovim/build/log/ubsan.18831 ====================
= 
= =================================================================
= ==18831==ERROR: LeakSanitizer: detected memory leaks
= 
= Direct leak of 80 byte(s) in 1 object(s) allocated from:
=     #0 0x4deaa3 in realloc ??:?
=     #1 0x7f4f150219b2 in __scandir64_tail /build/glibc-SzIz7B/glibc-2.31/dirent/./scandir-tail-common.c:62:31
= 
= Direct leak of 32 byte(s) in 1 object(s) allocated from:
=     #0 0x4de7ad in malloc ??:?
=     #1 0x1e9474d in luv_fs_scandir luv.c
= 
= Indirect leak of 200 byte(s) in 6 object(s) allocated from:
=     #0 0x4de7ad in malloc ??:?
=     #1 0x7f4f150218cc in __scandir64_tail /build/glibc-SzIz7B/glibc-2.31/dirent/./scandir-tail-common.c:69:27
= 
= SUMMARY: AddressSanitizer: 312 byte(s) leaked in 8 allocation(s).
====================================================================================================

We are using vim.loop.fs_scandir in the vim.fs.dir() iterator. I might need to poke the luv folks about this, but if anyone is keen on moving this PR forward this is something they could look into.

@gpanders gpanders force-pushed the path-root branch 2 times, most recently from 8926661 to 8db917b Compare May 23, 2022 15:57
@gpanders
Copy link
Member Author

More detailed backtrace:


=================================================================
==33096==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0xaaaac43b6f98 in __interceptor_realloc (/home/greg/src/neovim/build/bin/nvim+0xc66f98) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)
    #1 0xffffa54c2e68  (/lib/aarch64-linux-gnu/libc.so.6+0xb2e68) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #2 0xaaaac43720ec in __interceptor_scandir (/home/greg/src/neovim/build/bin/nvim+0xc220ec) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)
    #3 0xaaaac5d9708c in uv__fs_scandir /home/greg/src/neovim/.deps/build/src/libuv/src/unix/fs.c:552:7
    #4 0xaaaac5d9708c in uv__fs_work /home/greg/src/neovim/.deps/build/src/libuv/src/unix/fs.c:1736:5
    #5 0xaaaac5d98b7c in uv_fs_scandir /home/greg/src/neovim/.deps/build/src/libuv/src/unix/fs.c:2033:3
    #6 0xaaaac5d82710 in luv_fs_scandir luv.c
    #7 0xaaaac5de0044 in luaD_precall /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:320:9
    #8 0xaaaac5def4b4 in luaV_execute /home/greg/src/neovim/.deps/build/src/lua/src/lvm.c:591:17
    #9 0xaaaac5de08a8 in luaD_call /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:378:5
    #10 0xaaaac5ddc760 in f_call /home/greg/src/neovim/.deps/build/src/lua/src/lapi.c:800:3
    #11 0xaaaac5ddf6e8 in luaD_rawrunprotected /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:116:3
    #12 0xaaaac5de0d18 in luaD_pcall /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:464:12
    #13 0xaaaac5ddc6d0 in lua_pcall /home/greg/src/neovim/.deps/build/src/lua/src/lapi.c:821:12
    #14 0xaaaac4efa128 in nlua_pcall /home/greg/src/neovim/src/nvim/lua/executor.c:129:16
    #15 0xaaaac4efc04c in nlua_exec /home/greg/src/neovim/src/nvim/lua/executor.c:1319:7
    #16 0xaaaac45e447c in nvim_exec_lua /home/greg/src/neovim/src/nvim/api/vim.c:461:10
    #17 0xaaaac451397c in handle_nvim_exec_lua /home/greg/src/neovim/build/src/nvim/auto/api/private/dispatch_wrappers.generated.h:4366:15
    #18 0xaaaac5182fe0 in request_event /home/greg/src/neovim/src/nvim/msgpack_rpc/channel.c:376:19
    #19 0xaaaac58ce654 in state_handle_k_event /home/greg/src/neovim/src/nvim/state.c:107:7
    #20 0xaaaac521bea8 in nv_event /home/greg/src/neovim/src/nvim/normal.c:7237:3
    #21 0xaaaac51d8294 in normal_execute /home/greg/src/neovim/src/nvim/normal.c:1161:3
    #22 0xaaaac58ce3b4 in state_enter /home/greg/src/neovim/src/nvim/state.c:89:26
    #23 0xaaaac51b07fc in normal_enter /home/greg/src/neovim/src/nvim/normal.c:450:3
    #24 0xaaaac4f2443c in main /home/greg/src/neovim/src/nvim/main.c:575:3
    #25 0xffffa54373f8  (/lib/aarch64-linux-gnu/libc.so.6+0x273f8) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #26 0xffffa54374c8 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x274c8) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #27 0xaaaac433f9ec in _start (/home/greg/src/neovim/build/bin/nvim+0xbef9ec) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0xaaaac43b6ba0 in malloc (/home/greg/src/neovim/build/bin/nvim+0xc66ba0) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)
    #1 0xaaaac5d8c08c in luv_setup_req luv.c
    #2 0xaaaac5d826dc in luv_fs_scandir luv.c
    #3 0xaaaac5de0044 in luaD_precall /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:320:9
    #4 0xaaaac5def4b4 in luaV_execute /home/greg/src/neovim/.deps/build/src/lua/src/lvm.c:591:17
    #5 0xaaaac5de08a8 in luaD_call /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:378:5
    #6 0xaaaac5ddc760 in f_call /home/greg/src/neovim/.deps/build/src/lua/src/lapi.c:800:3
    #7 0xaaaac5ddf6e8 in luaD_rawrunprotected /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:116:3
    #8 0xaaaac5de0d18 in luaD_pcall /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:464:12
    #9 0xaaaac5ddc6d0 in lua_pcall /home/greg/src/neovim/.deps/build/src/lua/src/lapi.c:821:12
    #10 0xaaaac4efa128 in nlua_pcall /home/greg/src/neovim/src/nvim/lua/executor.c:129:16
    #11 0xaaaac4efc04c in nlua_exec /home/greg/src/neovim/src/nvim/lua/executor.c:1319:7
    #12 0xaaaac45e447c in nvim_exec_lua /home/greg/src/neovim/src/nvim/api/vim.c:461:10
    #13 0xaaaac451397c in handle_nvim_exec_lua /home/greg/src/neovim/build/src/nvim/auto/api/private/dispatch_wrappers.generated.h:4366:15
    #14 0xaaaac5182fe0 in request_event /home/greg/src/neovim/src/nvim/msgpack_rpc/channel.c:376:19
    #15 0xaaaac58ce654 in state_handle_k_event /home/greg/src/neovim/src/nvim/state.c:107:7
    #16 0xaaaac521bea8 in nv_event /home/greg/src/neovim/src/nvim/normal.c:7237:3
    #17 0xaaaac51d8294 in normal_execute /home/greg/src/neovim/src/nvim/normal.c:1161:3
    #18 0xaaaac58ce3b4 in state_enter /home/greg/src/neovim/src/nvim/state.c:89:26
    #19 0xaaaac51b07fc in normal_enter /home/greg/src/neovim/src/nvim/normal.c:450:3
    #20 0xaaaac4f2443c in main /home/greg/src/neovim/src/nvim/main.c:575:3
    #21 0xffffa54373f8  (/lib/aarch64-linux-gnu/libc.so.6+0x273f8) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #22 0xffffa54374c8 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x274c8) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #23 0xaaaac433f9ec in _start (/home/greg/src/neovim/build/bin/nvim+0xbef9ec) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)

Indirect leak of 200 byte(s) in 6 object(s) allocated from:
    #0 0xaaaac43b6ba0 in malloc (/home/greg/src/neovim/build/bin/nvim+0xc66ba0) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)
    #1 0xffffa54c2d9c  (/lib/aarch64-linux-gnu/libc.so.6+0xb2d9c) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #2 0xaaaac43720ec in __interceptor_scandir (/home/greg/src/neovim/build/bin/nvim+0xc220ec) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)
    #3 0xaaaac5d9708c in uv__fs_scandir /home/greg/src/neovim/.deps/build/src/libuv/src/unix/fs.c:552:7
    #4 0xaaaac5d9708c in uv__fs_work /home/greg/src/neovim/.deps/build/src/libuv/src/unix/fs.c:1736:5
    #5 0xaaaac5d98b7c in uv_fs_scandir /home/greg/src/neovim/.deps/build/src/libuv/src/unix/fs.c:2033:3
    #6 0xaaaac5d82710 in luv_fs_scandir luv.c
    #7 0xaaaac5de0044 in luaD_precall /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:320:9
    #8 0xaaaac5def4b4 in luaV_execute /home/greg/src/neovim/.deps/build/src/lua/src/lvm.c:591:17
    #9 0xaaaac5de08a8 in luaD_call /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:378:5
    #10 0xaaaac5ddc760 in f_call /home/greg/src/neovim/.deps/build/src/lua/src/lapi.c:800:3
    #11 0xaaaac5ddf6e8 in luaD_rawrunprotected /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:116:3
    #12 0xaaaac5de0d18 in luaD_pcall /home/greg/src/neovim/.deps/build/src/lua/src/ldo.c:464:12
    #13 0xaaaac5ddc6d0 in lua_pcall /home/greg/src/neovim/.deps/build/src/lua/src/lapi.c:821:12
    #14 0xaaaac4efa128 in nlua_pcall /home/greg/src/neovim/src/nvim/lua/executor.c:129:16
    #15 0xaaaac4efc04c in nlua_exec /home/greg/src/neovim/src/nvim/lua/executor.c:1319:7
    #16 0xaaaac45e447c in nvim_exec_lua /home/greg/src/neovim/src/nvim/api/vim.c:461:10
    #17 0xaaaac451397c in handle_nvim_exec_lua /home/greg/src/neovim/build/src/nvim/auto/api/private/dispatch_wrappers.generated.h:4366:15
    #18 0xaaaac5182fe0 in request_event /home/greg/src/neovim/src/nvim/msgpack_rpc/channel.c:376:19
    #19 0xaaaac58ce654 in state_handle_k_event /home/greg/src/neovim/src/nvim/state.c:107:7
    #20 0xaaaac521bea8 in nv_event /home/greg/src/neovim/src/nvim/normal.c:7237:3
    #21 0xaaaac51d8294 in normal_execute /home/greg/src/neovim/src/nvim/normal.c:1161:3
    #22 0xaaaac58ce3b4 in state_enter /home/greg/src/neovim/src/nvim/state.c:89:26
    #23 0xaaaac51b07fc in normal_enter /home/greg/src/neovim/src/nvim/normal.c:450:3
    #24 0xaaaac4f2443c in main /home/greg/src/neovim/src/nvim/main.c:575:3
    #25 0xffffa54373f8  (/lib/aarch64-linux-gnu/libc.so.6+0x273f8) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #26 0xffffa54374c8 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x274c8) (BuildId: 4c414febb48863e7ed248ade5a2ebbfc8fe10dca)
    #27 0xaaaac433f9ec in _start (/home/greg/src/neovim/build/bin/nvim+0xbef9ec) (BuildId: af67e37e6ad44eeb4394f094627798732a10c2aa)

SUMMARY: AddressSanitizer: 312 byte(s) leaked in 8 allocation(s).

I can replicate this locally but only when running make functionaltest. Running nvim "manually" and calling vim.fs.dir() does not produce any output from ASAN. So the test runner is doing "something" differently, though I'm not sure what.

@gpanders
Copy link
Member Author

I was finally able to reproduce this fully and I think I've identified the root cause. I've opened an issue with luv.

@clason clason added the status:blocked-external Needs a third-party / external change or fix label May 29, 2022
@clason clason removed the status:blocked-external Needs a third-party / external change or fix label May 31, 2022
@clason
Copy link
Member

clason commented May 31, 2022

Memory leak is gone, but vim.fs.normalize() test fails on Windows:

[  ERROR   ] test/functional\lua\fs_spec.lua @ 87: vim.fs normalize() works with ~
  test/functional\lua\fs_spec.lua:88: attempt to concatenate a nil value
  stack traceback:
  	test/functional\lua\fs_spec.lua:88: in function <test/functional\lua\fs_spec.lua:87>

[  ERROR   ] test/functional\lua\fs_spec.lua @ 90: vim.fs normalize() works with environment variables
 test/functional\lua\fs_spec.lua:91: attempt to concatenate a nil value
  stack traceback:
  	test/functional\lua\fs_spec.lua:91: in function <test/functional\lua\fs_spec.lua:90>

Comment on lines 200 to 203
function M.normalize(path)
vim.validate({ path = { path, 's' } })
return (path:gsub('\\', '/'):gsub('^~/', vim.env.HOME .. '/'):gsub('%$([%w_]+)', vim.env))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will work on Windows, unless you use PowerShell, or at least not reliably.

The environment variable expansion will give backslashes that you'd also need to sanitize before concatenating

$HOME
C:\Users\USERNAME

runtime/lua/vim/fs.lua Show resolved Hide resolved
vim.fs.parents() is a Lua iterator that returns the next parent
directory of the given file or directory on each iteration.
This function is modeled after the path.dir() function from Penlight and
the luafilesystem module.
This is a pure Lua implementation of the Vim findfile() and finddir()
functions without the special syntax.
@gpanders gpanders force-pushed the path-root branch 2 times, most recently from 11d6a05 to 99b3985 Compare May 31, 2022 19:28
@gpanders gpanders merged commit c632f64 into neovim:master May 31, 2022
@gpanders gpanders deleted the path-root branch May 31, 2022 20:00
@lewis6991
Copy link
Member

@gpanders: you seemed to have forgotten to address comments made by me and @mfussenegger about documenting that some functions implemented here cannot be run in api-fast.

Fortunately, since #18306 has been merged, we should be able to mark fnamemodify() as allowed to run in api-fast.

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

Successfully merging this pull request may close these issues.

None yet

9 participants