Picking up root build.zig in monorepo setup

iamcdonald commented 1 week ago

Zig Version

0.14.0-dev.2034+56996a280

ZLS Version

0.14.0-dev.179+5acc066

Client / Code Editor / Extensions

Zed 0.158.2

Steps to Reproduce and Observed Behavior

Hi, not sure if this is intended but I'm just flagging as it feels slightly off.

Have popped up an example here

open the repo in zed
open libs/test/src/test.zig
change the i32 to something else wrong i.e. i24
save file

No syntax highlighting appears where error exists.

I originally thought the root build.zig was overriding the use of the libs/test/build.zig but even with that removed I'm not seeing anything ran or diagnostics returned on the received textDocument/didSave message.

A quick last test and I've found that syntax highlighting works if I open libs/test in zed as the root folder i.e. zed zed-syntax-highlighting-test/libs/test so that appears to play a part.

Expected Behavior

I would expect the closest build.zig to the code in question to be used.

Relevant log output

With a root build.zig file present

debug (message): received: {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///Users/ooooo/Documents/code/zig-audio/zed-syntax-highlighting-test/libs/test/src/test.zig","languageId":"zig","version":0,"text":"const std = @import(\"std\");\n\ntest \"test\" {\n    const x: i16 = std.math.maxInt(i24);\n    _ = x;\n}\n"}}}
info  (store ): Loaded build file 'file:///Users/ooooo/Documents/code/zig-audio/zed-syntax-highlighting-test/libs/test/build.zig'
info  (store ): Loaded build file 'file:///Users/ooooo/Documents/code/zig-audio/zed-syntax-highlighting-test/build.zig'
debug (store ): Opened document 'file:///Users/ooooo/Documents/code/zig-audio/zed-syntax-highlighting-test/libs/test/src/test.zig'

With no root build.zig file present

debug (message): received: {"jsonrpc":"2.0","method":"textDocument/didSave","params":{"textDocument":{"uri":"file:///Users/ooooo/Documents/code/zig-audio/zed-syntax-highlighting-test/libs/test/src/test.zig"},"text":"const std = @import(\"std\");\n\ntest \"test\" {\n    const x: i16 = std.math.maxInt(i32);\n    _ = x;\n}\n"}}
debug (server): Took 0ms to process notification-textDocument/didSave on Thread 1857442

Techatrix commented 1 week ago

Select only the closest `build.zig`

The problem with selecting the closest build.zig is that it can cause the reverse problem to occur. What if you root build.zig has a check step that you expect to be invoked? Why did you even open the root project if you only care about the subfolder?

Select all `build.zig`s

This would be unreasonable because it can cause an unlimited number of compilations to occur. This doesn't scale and is a waste of computational resources.

Select only the root `build.zig`

This is also more in-line with what you would do in the terminal. You run zig build check and not zig build check --build-file libs/test/build.zig. Usually you will have the root build.zig depend on the inner build.zig so that running a check stop at the root would report errors on all files even if the are in some subfolder with their own build.zig.

I would suggest to have a check step at the root of the project and depend on the code of the subfolder. Whether that is a feasible solution is hard to tell since I don't know about your entire project setup, just the simplified version.

iamcdonald commented 1 week ago

Hi, thanks for taking the time to get back to me, real appreciate the work you're putting into zls.

I've tried to address some of the raised questions/solutions below.

Admittedly, I still feel that selecting the closest build.zig when the language server runs build on save makes the most sense to me because it aligns with how monorepo development works within other languages but I can also work around that.

Select only the closest build.zig

What if you root build.zig has a check step that you expect to be invoked?

I don't think a root build.zig should concern itself with code within a subrepo. A subrepo (with it's own build.zig) should be a black box consumed via the zig dependency system.

I can see the angle of wanting to run check across all subrepos now and then. However that feels like something better accomplished with a step which run's all downstream check steps rather than a check step which explicitly loads subrepo code (and therefore needs to know about the makeup of those subrepos).

Additionally, running check at the root of a monorepo is something I would do explicitly and not something I would expect a language server to do.

Why did you even open the root project if you only care about the subfolder?

Because whilst I want to work on individual libs within a monorepo and get fast feedback they exist within a holistic context and I want to be able to jump between them if required.

I could get round this in zed by adding each monorepo folder to the project but it feels like a weird workflow, especially considering working with monorepos within other language ecosystems.

Select only the root build.zig

I would suggest to have a check step at the root of the project and depend on the code of the subfolder.

The benefit of having the check step in each monorepo subfolder is that the time between save and diagnostics showing up should be faster because there's less code to attempt to build. This wouldn't be the case if I had the check step for all code in the root folder.

Afterthought: Maybe this isn't a big deal with build caching. I'm not sure how the zig build system works though and therefore to what level it would be able to avoid re-building downstream dependents.

zigtools / zls