numtide / treefmt

one CLI to format your repo [maintainers=@zimbatm,@brianmcgee]
https://treefmt.com
MIT License
592 stars 37 forks source link

eval cache doesn't extend to shell scripts #149

Open zimbatm opened 2 years ago

zimbatm commented 2 years ago

Describe the bug

Recently I encountered an issue where shellcheck got upgraded (through a nixpkgs bump), but treefmt's eval cache didn't get invalidated.

To Reproduce

Treefmt tracks the mtime of the program being invoked. But if the program is a shell wrapper then it doesn't work anymore. treefmt is not able to find out that shellcheck is being used in something like this:

[formatter.shell]
command = "sh"
options = [
    "-eucx",
    """
shellcheck "$@"
shfmt -i 2 -s -w "$@"
    """
]
includes = ["*.sh"]
excludes = []

Expected behavior

The cache needs to be precise, otherwise, it introduces uncertainty and wastes the user's time. This is a general problem as any code formatter can invoke arbitrary programs and files.

On possible solution would be to allow the user to manually list the dependencies and collect them.

Eg:

[formatter.shell]
command = "sh"
options = [
    "-eucx",
    """
shellcheck "$@"
shfmt -i 2 -s -w "$@"
    """
]
includes = ["*.sh"]
excludes = []
dependent_programs = ["shellcheck", "shfmt"]

System information

Additional context

basile-henry commented 2 years ago

Maybe a bit too hacky of an idea :sweat_smile:

But how about we strace the formatter's process (we can get its id), and then look for what binaries it execve (on Linux) to find its dependencies.

That is definitely not very portable, so having an explicit list of dependencies is likely preferable, despite being manual.

zimbatm commented 2 years ago

I thought about it as well :) I think strace actually slows down the execution of the program (but I don't know of how much) and is not very portable as you mentioned.

Another idea was to write a devshell module. In that case, we lean on Nix to capture all the runtime dependencies. The module system generates the config file and then generates a treefmt wrapper that invokes treefmt --config-file /nix/store/... "$@". But it's only for nix users.

Fundamentally the treefmt cache is leaky.