search

tweag / topiary

https://topiary.tweag.io/
MIT License
549 stars 24 forks source link

Version drift of tree-sitter-bash transitive dependency #702

Open Xophmeister opened 5 months ago

Xophmeister commented 5 months ago

The pinned version of tree-sitter-bash which Topiary depends upon appears to reference a rev of Bash that can no longer be pulled from Savannah. e.g.:

  1. The nix develop command appears to be in a broken state right now. I spent some time debugging it and it appears that the revision of tree-sitter-bash that you are pinned to references a revision of a submodule for https://git.savannah.gnu.org/git/bash.git that does not exist. The specific error I get is Cannot find Git revision 'a99d905216cc0aac5de0c3050f4afc54e21c6bc5' in ref 'refs/heads/master' of repository 'https://git.savannah.gnu.org/git/bash.git'!

Originally posted by @lavigneer in https://github.com/tweag/topiary/pull/699

This has happened before, but it's not clear why. The rev mentioned above is certainly on Savannah, for example.

In the past, we've fixed this by bumping the version of tree-sitter-bash. We can only do this so often, until the tree-sitter-bash grammar changes and our rules become invalid.

yannham commented 5 months ago

Ah, we have the same error on the update-flake-lock action of Nickel. At least there's an explanation now.

nbraud commented 3 weeks ago

FYI, I found out in nixpkgs#340964 that this is a known issue in fetchgit.

yannham commented 4 days ago

We're hitting that again on the Nickel CI, and it's getting tiring. I wonder if we shouldn't just fork or mirror the bash grammar, adding proper refs for commits that we want to target given the insights from @nbraud.

Xophmeister commented 4 days ago

@yannham This shouldn't be happening any more for Nickel, as the Bash grammar was feature-gated in #711 -- this PR was specifically to workaround this problem -- and since, (almost) all grammars were removed as dependencies in #716 (dynamic loading). Neither of these PRs are in a release yet: Is that the issue?

yannham commented 3 days ago

Neither of these PRs are in a release yet: Is that the issue?

It's not, but the issue is that we can't use a revision posterior to dynamic loading because it still doesn't work on main in a Nix build - we would need to get https://github.com/tweag/topiary/pull/747 for that. However I can try to pin Topiary to the merge commit of #711 and see if the error goes away. Once #747 is merged, I guess we'll be fine forever :crossed_fingers: