Skip to content

Adds support for Fossil SCM source tree archives as zig build dependencies #18168

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

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

cipharius
Copy link
Contributor

Fossil SCM can generate tar.gz compressed source tree archives at any given artifiact, which aligns well with zig build dependency fetching system.

Problem is that fossil serves these archives with mime type application/x-compressed. While it's too generic, zig should accept it nonetheless, but fall back to filename parsing to determine source archive type.

Other problem is that fossil generates tar archives where directory headers don't have trailing slash which causes issues with the way how std/tar.zig parses archive paths.

@cipharius
Copy link
Contributor Author

cipharius commented Nov 30, 2023

This also potentially resolves #17779 and #17620.

Seems pretty likely, that the archives in these issues also have directory paths without trailing slashes. Otherwise, if regular file path will get stripped too much, the BadFileName error will be raised instead.

Edit:
Seems like in #17620 case it would still be an error, because zig expects the archive to have root directory, which is a good style for source archives. The case of missing root directory in tar archive would have to be handled separately

@nektro
Copy link
Contributor

nektro commented Nov 30, 2023

has it been reported to fossil how wrong application/x-compressed is? I think there should be a tracking issue linked if this change makes it in

@cipharius
Copy link
Contributor Author

Seems like it wasn't given much thought, perhaps back when this was implemented application/x-compressed seemed fine.

https://fossil-scm.org/home/info/d5d676f0c7f37d6b

@andrewrk
Copy link
Member

Thanks for the contribution. I'm happy to accept this patch. Would you mind rebasing it on the latest master branch code? The changes to std.tar might not be necessary anymore since d55d1e3.

@cipharius cipharius force-pushed the feature/zig-build-fossil-support branch from 64c8197 to f25c499 Compare January 15, 2024 18:01
@cipharius
Copy link
Contributor Author

Tested the std.tar change, still relevant. It makes it a non-error if too much is stripped from tar path, it returns empty string. Since the stripComponents is a local function, such solution seems alright.

@andrewrk
Copy link
Member

Thanks for the rebase and re-testing. Nice patch 👍

@andrewrk andrewrk merged commit d1ce8b8 into ziglang:master Jan 16, 2024
@cipharius cipharius deleted the feature/zig-build-fossil-support branch January 16, 2024 07:46
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.

3 participants