Use git submodules for assets directory?

[tomaskallup]

I'm currently trying to make this server build & work under Nix package manager (my progress so far is here - https://github.com/tomaskallup/c3-lsp/blob/main/flake.nix).

One thing I have to "hack" around is git operations inside Makefile. From the looks of it, you just clone two repos and checkout one on specific commit, which sounds exactly like a job for submodules, since it could then be removed from Makefile and the checkout would be done simply when git cloneing to repo.

Would you be fine with me making a PR for this?

Also some tests seem to be failing when building the package, I have disabled them for now, but I have no idea if they are supposed to work or not. Specificaly the "Should find local enumerator definition associated value" test failed.

[pherrymason]

The only thing I wonder is if with submodules I will be able to switch branches easily. This is used for the command make index-c3-std VERSION=0.6.2 where VERSIOn allows to checkout a specific tag and build the stdlib symbols.

About the test, they definitely should all pass

[tomaskallup]

This might be possible to do, using some condition in Makefile, so it would checkout to specific version if a parameter is provided and checkout back once the build is finished.

I will try to build the package outside the Nix build env and run the tests to see if they also fail.

[tomaskallup]

Ah you already have conditions in there, so then it should be possible. For submodules, simply cding into the directory and running git checkout is a valid use. I believe unless git submodule sync is ran, it will not update the references in repo, so your checkouts won't be synced to the repo and will remain local.

[tomaskallup]

What is the intended way to run the tests? They seem to fail even locally, I'm simply running make && make index-c3-std VERSION=0.6.2 && make build-dev && make test and I endup with two fails:

--- FAIL: TestLanguage_findClosestSymbolDeclaration_enums (0.02s)
    --- FAIL: TestLanguage_findClosestSymbolDeclaration_enums/Should_find_local_enumerator_definition_associated_value (0.00s)
        search_closest_declaration_test.go:352:
                Error Trace:    /home/armeeh/Pkg/c3-lsp/server/internal/lsp/search/search_closest_declaration_test.go:352
                Error:          Should be false
                Test:           TestLanguage_findClosestSymbolDeclaration_enums/Should_find_local_enumerator_definition_associated_value
                Messages:       Element not found
panic: cannot get from None type [recovered]
    panic: cannot get from None type

and

--- FAIL: TestParses_TypedEnums (0.02s)
    --- FAIL: TestParses_TypedEnums/associate_values_<_v6.0 (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x5d80ce]

I'm runnig it with go version go1.22.5 linux/amd64.

[pherrymason]

I just run make test Let me check and see what can be failing.

[tomaskallup]

Nevermind, seems like my fork was outdated, I rebased it on your main and it seems to be passing now.

[tomaskallup]

For some reason the tests fail in Nix build env, but it's the same exact messages from above, so it's probably something being cached. Do you have any idea why those errors might have been happening? Was it perhaps some dependency not being the correct version?

[pherrymason]

The only thing I can think is you downloaded a different treesitter version and rebuilt the parser with make build-parser.

If you did not run make build-parser you should be using the validated one and should not fail. I'm on go 1.22.3 darwin/arm64

[tomaskallup]

By treesitter version you mean actual tree-sitter, or the tree-sitter-c3? I've got the tree-sitter-c3 locked on the hash from Makefile, but the make test is run AFTER the build of the lsp (which includes a tree-sitter generate).

[pherrymason]

I mean the treesitter that was being cloned through the makefile.
Anyway make build does not rebuild the parser, it just does a go build.

If you did not touch that, then I have no clue. You can see here tests are passing: https://github.com/pherrymason/c3-lsp/actions/runs/10678290604

[tomaskallup]

So if I understand correctly, make build-parser is only for development and if I want to build the "release" version, it's not needed, since you copy the built parser into the soruce code and version control it?

[pherrymason]

Exactly

[tomaskallup]

Ah gotcha, then that makes the packaging way simpler, since I can just build the go module the builtin way.

Also it makes this issue irrelevant, since for local building I can just use the Makefile, so it can be closed, if you don't want to bother with submodules.

[pherrymason]

Yes, that's what I was about going to say, those external repositories are only required to update the parser and stdlib symbols table. I will close as suggested, feel free to reopen if you find anything else.