Allow specification of location of elm-json package via a flag

[notquiteamonad]

Overview

Similar to the --compiler flag, I'd like to be able to specify the location of the elm-json binary rather than elm-review trying to download it from GitHub.

Motivation

The context for this is that I'm trying to develop a nix-based pre-commit hook to run elm-review as a check. Running in a pure shell, it isn't able to download the CLI from GitHub.

Potential Issues

It'll be necessary to figure out how to name this to differentiate it from the unrelated --elmjson flag.

---

I'm happy to tackle this myself if it's something you're not against including.

[jfmengels]

Hi @notquiteamonad 👋

So we're using elm-tooling to fetch/install that binary. IIRC, if it's already in the node_modules or in ELM_HOME/elm-tooling (@lydell could you confirm?) it won't download the binary. I don't know if it's a reasonable thing to add the binary there (I assume not).

@lydell Have you thought about making elm-tooling work better with Nix? I know nothing about Nix so I don't even know if this question makes sense. It would be nice if elm-tooling played nice with Nix so that other Elm tools could benefit from this work.

For the flag name, I agree that it's a bit annoying. We could name it --elm-json-path to be consistent with --elm-format-path, but that's still a bit ambiguous.

[notquiteamonad]

I wasn't actually using elm-tooling already, but I've just checked and even using elm-tooling, nix won't find it. It's my first time attempting a nix tooling setup on an Elm project after getting used to it with Haskell.

I'm similarly unsure if the change would be possible at the elm-tooling CLI level, although I feel it could be something solveable in nix without relying on an external change. Currently, though, I've specified ELM_HOME and am getting the following message from elm-review:

-- CONFIGURATION COMPILATION ERROR ---------------------------------------------

I encountered a problem when solving dependencies:

I need a tool called elm-json for some of my inner workings,
but there was some trouble installing it. This is what we know:

EACCES: permission denied, mkdir '/home/notquiteamonad/.elm/elm-tooling/elm-json/0.2.10'

This seems odd as /home/notquiteamonad/.elm/elm-tooling/elm-json/0.2.10/elm-json exists and is the elm-json binary. I can't help but think I might be missing something obvious here

[lydell]

I don’t know much about Nix. But I have helped Brian Hicks debug a thing once: BrianHicks/elm-csv#3. Not sure if that helps. It was about installing elm-test, which also uses elm-tooling to install elm-json.

IIRC, if it's already in the node_modules or in ELM_HOME/elm-tooling (@lydell could you confirm?) it won't download the binary. I don't know if it's a reasonable thing to add the binary there (I assume not).

Close. elm-tooling doesn’t look for binaries in node_modules/. Only $ELM_HOME (which defaults to ~/.elm). If ~/.elm/elm-tooling/elm-json/0.2.10/elm-json already exists then elm-tooling won’t attempt to download anything from the Internet. So as long as you have a package-lock.json, making sure that file exists yourself works. But it feels a little brittle. Every time you update package-lock.json, there’s the chance elm-review will end up using a newer version of elm-json, so you’d have to change making sure ~/.elm/elm-tooling/elm-json/0.2.11/elm-json or something exists.

@lydell Have you thought about making elm-tooling work better with Nix? I know nothing about Nix so I don't even know if this question makes sense. It would be nice if elm-tooling played nice with Nix so that other Elm tools could benefit from this work.

I haven’t, because I don’t know much about Nix either. But if there’s a way for elm-tooling to just do the right thing in Nix it sounds like a good thing to implement. So if someone enthusiastic about Nix, Elm and npm steps up it could happen.

EACCES: permission denied, mkdir '/home/notquiteamonad/.elm/elm-tooling/elm-json/0.2.10'

This seems odd as /home/notquiteamonad/.elm/elm-tooling/elm-json/0.2.10/elm-json exists and is the elm-json binary.

Do you mean it exists before running elm-review? If so, it’s strange that you get that mkdir error. elm-tooling doesn’t even run fs.mkdirSync if the tool already exists: https://github.com/elm-tooling/elm-tooling-cli/blob/58746489bf0f155e58ca4d65a1159b2cbd19b0bf/commands/install.ts#L825

Maybe you can make a small .js file that just tries to stat /home/notquiteamonad/.elm/elm-tooling/elm-json/0.2.10? Is it able to? And then work it from there.

[notquiteamonad]

Having played around with the paths a bit more, I think I've figured out why it's not finding $ELM_HOME - it seems to be an entirely sandboxed copy of the project directory (and such sandboxing is, of course, one of the appealing factors of nix if inconvenient at times). I'm going to see if I can figure out a way of using elm-tooling to install into the sandbox and getting elm-review to use that. I'll keep you updated once I have anything working

[jfmengels]

@notquiteamonad Have you been able to figure this out? If so, I'll close this issue :)

[notquiteamonad]

Hey :) I think I've figured it out this morning, so I think the issue can be closed. I'm gonna do a bit more testing over the next couple of days and then find somewhere appropriate to either document it or make something other people can use. Thanks for your help at the beginning :)

[notquiteamonad]

elm-review is now available as a hook "out of the box" with cachix/pre-commit-hooks.nix as of cachix/git-hooks.nix#109

[jfmengels]

Nice, thanks for adding this 👍