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

eval cache doesn't extend to shell scripts #149

Open
zimbatm opened this issue Apr 1, 2022 · 2 comments
Open

eval cache doesn't extend to shell scripts #149

zimbatm opened this issue Apr 1, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@zimbatm
Copy link
Member

zimbatm commented Apr 1, 2022

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

@zimbatm zimbatm added the bug Something isn't working label Apr 1, 2022
@basile-henry
Copy link
Contributor

Maybe a bit too hacky of an idea 😅

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
Copy link
Member Author

zimbatm commented Apr 1, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants