fix(std): use gtar if available #1433
Conversation
These changes are needed because the relevant code currently assumes GNU tar features like automatic decompression and the `--no-same-owner` flag, but doesn't detect and use GNU tar on platforms where it's named gtar. This causes a number of failures. With these changes, the health check passes and tarballs are correctly unpacked on platforms other than Linux. This fix affects at least OpenBSD, and probably other systems like FreeBSD, NetBSD, DragonFly BSD, etc. tests(std_spec.lua): add a local function named get_tar_cmd() that returns the correct version of tar to call. Adjust the tests so that they call gtar when appropriate. docs(README.md): update requirements so that they explicitly state that GNU tar is used. Clean up requirements by placing them in separate bullet points, rather than listing them in one line.
| local tar_cmd = ctx.spawn.tar | ||
| if platform.is.unix and not platform.is.linux then | ||
| tar_cmd = ctx.spawn.gtar | ||
| end |
There was a problem hiding this comment.
Sections like this with the platform check would be nice to refactor into a function somewhere else (but I don't know how you would want to organize this so I've left them as is for now). I see this as being useful because if the code needed to change for some reason, it could be changed in one place.
lua/mason-core/managers/std/init.lua
Outdated
| if platform.is.unix and not platform.is.linux then | ||
| tar_cmd = ctx.spawn.gtar | ||
| end | ||
| tar_cmd { |
There was a problem hiding this comment.
This module and all others inside mason-core.managers are legacy and not used anywhere in mason.nvim. I think we can leave these as-is seeing as they're no longer supported anyways.
| if platform.is.unix and not platform.is.linux then | ||
| tar_cmd = ctx.spawn.gtar | ||
| end | ||
| return tar_cmd({ "--no-same-owner", "-xvf", rel_path }):on_success(function() |
There was a problem hiding this comment.
I think we can simplify this to simply prefer gtar if it exists and if not fall back to tar. That way we can avoid relying on broader platform detection and all the quirks it may incur. Something like the following ought to do it.
local a = require("mason-core.async")
-- …
function M.untar()
-- …
a.scheduler()
local tar = vim.fn.executable("gtar") == 1 and "gtar" or "tar"
return ctx.spawn[tar] { ... }
endThe a.scheduler() (also notice the additional import) is required because this function may execute outside Neovim's main loop, in which case vim.fn functions cannot be called. a.scheduler() synchronizes execution back on the main loop.
| end | ||
| return ctx.spawn.tar | ||
| end | ||
|
|
There was a problem hiding this comment.
By leveraging vim.fn.executable("gtar") we can keep these tests the same they were before without having to incorporate any logic.
describe("...", function ()
before_each(function ()
stub(vim.fn, "executable")
vim.fn.executable.on_call_with("gtar").returns(0)
end)
end)Then we can also add a dedicated test for gtar detection:
it("should use gtar if available", function ()
-- … boilerplate
stub(vim.fn, "executable")
vim.fn.executable.on_call_with("gtar").returns(1)
-- … execute & assert
end)The platform checks are fragile and this is a better solution.
Remove "mason-core.platform" import. Revert changes to assert.spy in tar tests (they use ctx.spawn.tar once again).
|
Hi @williamboman, I made some more commits. I don't know that much Lua, so I was unsure how to do certain things. I did my best, I hope my updates are close to what you had in mind. |
* origin/main: feat(cargo): support fetching versions for git crates hosted on github (#1459) chore(async): add Channel (#1456) fix(ui): properly reset new package version state (#1454) chore(main): release 1.6.2 (#1446) fix(ui): don't disable search mode if empty pattern and last-pattern is set (#1445) chore(logging): fix log string (#1444)
williamboman
changed the title
|
Is there a way to use tar by default. I'm facing "gtar: This does not look like a tar archive" with many of lsp servers(latest lua-language-server for example). |
Addresses #1415.
These changes are needed because the relevant code currently assumes GNU tar features like automatic decompression and the
--no-same-ownerflag, but doesn't detect and use GNU tar on platforms where it's named gtar. This causes a number of failures.With these changes, the health check passes and tarballs are correctly unpacked on platforms other than Linux. This fix affects at least OpenBSD, and probably other systems like FreeBSD, NetBSD, DragonFly BSD, etc.
tests(std_spec.lua): add a local function named get_tar_cmd() that returns the correct version of tar to call. Adjust the tests so that they call gtar when appropriate.
docs(README.md): update requirements so that they explicitly state that GNU tar is used. Clean up requirements by placing them in separate bullet points, rather than listing them in one line.