Remove dependencies on Tree-sitter grammars

[Xophmeister]

Remove dependencies on Tree-sitter grammars

Resolves #1022
Resolves #1023

Description

Remove the development dependencies on tree-sitter-json and tree-sitter-nickel. Instead, use the dynamic loading from topiary-config in the relevant tests.

Checklist

Checklist before merging, wherever relevant:

• CHANGELOG.md updated
• Documentation (The Topiary Book, README.md, etc.) up-to-date

• Updated regression tests

[Xophmeister]

These changes work outside of Nix, but the CI complains inside (e.g., see here). My best guess is: Now that topiary-core depends on topiary-config and topiary-queries, for testing, I think that the derivation under test is simply missing the query files. That is, the "permission denied" error is misleading and should be, rather, "file not found".

Does that sound plausible, @Niols?

Adding the JSON and Nickel query files to the topiary-core package doesn't sound like the correct solution, as they're only needed for testing. What would be the right approach?

[Niols]

Sorry for the late reply.

These changes work outside of Nix, but the CI complains inside (e.g., see here). My best guess is: Now that topiary-core depends on topiary-config and topiary-queries, for testing, I think that the derivation under test is simply missing the query files. That is, the "permission denied" error is misleading and should be, rather, "file not found".

Does that sound plausible, @Niols?

That sounds plausible indeed. Especially, if the path to the query files involves .., this might be attempting to go into the store our to escape the sandbox, leading to the permission denied error.

Adding the JSON and Nickel query files to the topiary-core package doesn't sound like the correct solution, as they're only needed for testing. What would be the right approach?

We could disable tests in the Nix package, but that's a bit sad. We could also make topiary-core 's Nix package depend on topiary-queries (doesn't it sort of already, if there is such a dependency in Rust as well?) and point (don't know how yet) the tests at the right path in the store?

[Xophmeister]

Adding the JSON and Nickel query files to the topiary-core package doesn't sound like the correct solution, as they're only needed for testing. What would be the right approach?

We could disable tests in the Nix package, but that's a bit sad. We could also make topiary-core 's Nix package depend on topiary-queries (doesn't it sort of already, if there is such a dependency in Rust as well?) and point (don't know how yet) the tests at the right path in the store?

We currently rely on the tests to run in the CI, so disabling them is a bit of a non-starter. topiary-queries is a dev-dependency of topiary-core, so perhaps there's scope for two Nix packages: one with tests -- which we invoke in the CI -- and one without, which downstream users can benefit from... I don't know how "idiomatic" that would be, or how convincing it would seem that the non-test version passes like its sibling 🤷

@yannham and @jneem: Have you any experience in this, from Nickel or elsewhere?

[Niols]

Nix already has this sort of distinction between build dependencies and runtime dependencies. Downstream users would presumably just download the pre-built binaries from a Nix cache, which will also require runtime dependencies, but not build dependencies.

[yannham]

@Xophmeister what does the Nix derivation precisely need from the removed dependencies? Just the text file, that is the query file and the sample sources? If yes, the simplest is to make a small trivial derivation making those available (no cargo, no Rust source, etc.), and depends on that. There's a second problem though, in that the paths to those files seem to be hardcoded in the test, but they will be different in the normal setup and in the Nix setup. A simple albeit ad-hoc solution could be to look into a specific environment variable, like TOPIARY_TEST_NICKEL_SAMPLE or whatever first, and default to the relative path otherwise; then Nix could just set this variable? Maybe there are cleaner ways, I'm just thinking out loud here.

[Xophmeister]

Thanks, @yannham 🙏 Right, AFAIK, the only thing it's looking for are the JSON and Nickel query files.

[yannham]

For instance, we have dedicated derivation for the manual markdown files or generating the stdlib documentation in Nickel, which are custom but lightweight. I guess something similar (simply copying some files to $out so that they are in the store) would work:

https://github.com/tweag/nickel/blob/cd8513e73a7ef2651b29a7e8786269d9862e63a2/flake.nix#L607-L637

Here is an example of nickel-lang.org devshell consuming the manual derivation:

https://github.com/tweag/nickel-lang.org/blob/a00806fe12419e2626f0e701fde83ec788ef97b5/flake.nix#L42

[Niols]

Very nice, @yannham! I do think this is the way to go; I'm not sure we need to re-inject it in the devShell the way you do it (but I might be missing for what use this is intended). The Nix part of this is not very complicated (maybe @AlexTereshenkov wants to have a go?), but it does require a way to tell the tests where to find the files that it wants.

[mkatychev]

@Xophmeister does this need another review?

[Xophmeister]

@mkatychev No, not yet: It needs some more thought. The code changes I've made are all fine and work correctly, but it breaks the Nix build and I haven't had much opportunity to address that.