Skip to content
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

flake: add per data version derivations #1585

Closed
wants to merge 1 commit into from
Closed

flake: add per data version derivations #1585

wants to merge 1 commit into from

Conversation

polykernel
Copy link

This commit refactors the flake packages output to generate derivations serving different data versions. This allows downstream flake consumer to use zls with a versioned release of zig without effectively duplicating the zls derivation as per the status quo. Currently, zls packages serving the latest three tagged releases and the master branch of zig are produced.

I wasn't sure what the support range for the data_version build option is, so I naively opted for the latest three versioned releases + master. Please let me know if this should be changed.

This commit refactors the flake `packages` output to generate
derivations serving different data versions. This allows
downstream flake consumer to use zls with a versioned release
of zig without effectively duplicating the zls derivation as
per the status quo. Currently, zls packages serving the latest
three tagged releases and the master branch of zig are produced.
@Techatrix
Copy link
Member

This change would suggest that ZLS master is supporting tagged releases of Zig which is not planned as discussed in #1020.
If you are using a tagged release of Zig, you should also be using a tagged release of ZLS. This means that the should not be a data_version option to avoid this kind of confusion. Once further breaking changes happen to Zig, it also makes sense to remove ZLS's build runner for tagged releases of Zig (0.10.0 & 0.11.0) as discussed here.

I do have to admit that this is not the most user friendly decision, but Zig before 1.0 is improving/breaking rapidly and it is not always easy to keep up with and ensuring backwards compatibility would increase the required effort even more. Everyone is free to join the discussion in #1020.

NOTE: it is kind of possible to use ZLS master with Zig 0.11.0 right now, but this is not a guarantee and you should not depend on this being the case once further breaking changes to Zig happen.

@polykernel
Copy link
Author

polykernel commented Nov 6, 2023

If you are using a tagged release of Zig, you should also be using a tagged release of ZLS. This means that the should not be a data_version option to avoid this kind of confusion. Once further breaking changes happen to Zig, it also makes sense to remove ZLS's build runner for tagged releases of Zig (0.10.0 & 0.11.0) as discussed #1020 (comment).

Ah I see, I did noticed there are tagged releases of ZLS but seeing data_version build option and multiple build runners, I thought there wasn't a strict requirement for matching ZLS and Zig versions. Thanks for the clarification.

I do have to admit that this is not the most user friendly decision, but Zig before 1.0 is improving/breaking rapidly and it is not always easy to keep up with and ensuring backwards compatibility would increase the required effort even more. Everyone is free to join the discussion in #1020.

I agree with employing the strict approach discussed #1020. The split branch approach seems nice but given the current unstability of Zig, it is probably more trouble than its worth. In this case, I will close the PR in this case as the change doesn't align with the proposed version compatibility approach for ZLS.

@polykernel polykernel closed this Nov 6, 2023
@polykernel polykernel deleted the flake/per-data-version-drvs branch November 8, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants