-
-
Notifications
You must be signed in to change notification settings - Fork 299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make tracy a lazy dependency #1742
Conversation
7ee55e4
to
4c4822d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1742 +/- ##
=======================================
Coverage 76.62% 76.62%
=======================================
Files 35 35
Lines 10143 10143
=======================================
Hits 7772 7772
Misses 2371 2371 ☔ View full report in Codecov by Sentry. |
4c4822d
to
b0f4e1f
Compare
Hello, any chance this is merged (given that both Zig 0.12.0 and ZLS 0.12.0 are now available)? |
Any motivation behind this comment or is this just a random question? I would rather want to import a "zig tracy" package than doing this in ZLS. I started working on that but I am currently working on other things. I could still merge this and change it later. |
It would make distro packaging easier and be more consistent with other Zig projects. |
I can agree with the "be more consistent with other Zig projects" part but this doesn't help downstream distributions. You don't need tracy to compile ZLS since it is completely optional. |
In my case distro need to download it and unpack separately before compiling, or it will fail if user wants to enable it (simplified for not users of Gentoo): # How it's now
TRACY_COMMIT="bla_bla" # zls-0.12.0.tar.gz from GitHub releases does not contain submodules, as tarball is auto-generated
# Downloading...
zig-build_src_unpack
rm -r src/tracy/
mv "../tracy-${TRACY_COMMIT}" src/tracy/
# ...
zig build ${package_flags} ${user_flags} --system "/bla_bla/" |
Why even package ZLS with tracy support? |
If user wants to enable it, he will set up args and they will be passed like in code above. We don't know before building what args he chose, and at the same time want fully offline compilation, so along with ZLS source code we also have to download both tracy submodule and deps from build.zig.zon . I'm just saying it would be nice to get rid of tracy submodule archive and just reuse second archive withh all deps from build.zig.zon . |
There is no reason for the users to enable tracy. It would be a waste of resources to even bother downloading it just in case. |
Ok. Should I hardcode |
Yes, or just omit it if possible. |
b0f4e1f
to
229a232
Compare
229a232
to
8b2da5c
Compare
This allows us to completely get rid of git submodules.
There is just one unfortunate issue which is why I made this a separate Draft from #1741.
When using an old zig version like 0.11.0 you will get this unhelpful error when compiling ZLS.
This is because 0.11.0 has no support for dependencies that do not have a
build.zig
like the tracy repo.So we either lose our minimum build version check (and gain new issues reports because of it) or we use a tracy wrapper library that provides a
build.zig
. It would also be possible to wait for the Zig 0.12.0 release since it will become more unlikely that someones tries to compile ZLS with 0.11.0 after that.