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

std.tar: handle tar files with symbolic links #16678

Closed
Tracked by #16672
mitchellh opened this issue Aug 4, 2023 · 7 comments · Fixed by #17363
Closed
Tracked by #16672

std.tar: handle tar files with symbolic links #16678

mitchellh opened this issue Aug 4, 2023 · 7 comments · Fixed by #17363
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@mitchellh
Copy link
Sponsor Contributor

mitchellh commented Aug 4, 2023

Zig Version

0.11.0-dev.4404+4f6013bf5

Steps to Reproduce and Observed Behavior

Use any data.tar.gz below that contains a symbolic link.

You'll get a TarUnsupportedFileType which is explicitly returned from std.tar if it seems a symlink.

const std = @import("std");

/// USE BAD TAR.GZ HERE
const data = @embedFile("data.tar.gz");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer _ = gpa.deinit();
    const alloc = gpa.allocator();

    var out_dir = try std.fs.cwd().openDir("test-out", .{});

    var stream = std.io.fixedBufferStream(data);
    var decompress = try std.compress.gzip.decompress(alloc, stream.reader());
    defer decompress.deinit();

    try std.tar.pipeToFileSystem(out_dir, decompress.reader(), .{
        .strip_components = 1,
        .mode_mode = .ignore,
    });
}

Expected Behavior

This should work. A lot of source archives for the Zig package manager contain symbolic links. Major source archives that I use that have symbolic links:

  • libxml2
  • harfbuzz

This will also impair #16643 (not block) because downloading source archives for me will be a big use case.

@mitchellh mitchellh added the bug Observed behavior contradicts documented or intended behavior label Aug 4, 2023
@mitchellh
Copy link
Sponsor Contributor Author

I'll note so far this has been relatively easy to work around because the symlinks in the source packages I use are inconsequential to the actual build so I've just been stripping them. This will just block using clean source packages.

@andrewrk
Copy link
Member

Zig will strip symbolic links from any directories fetched, since that feature is not portable. All symbolic links will be implicitly in the exclusion set (#14311). Otherwise, packages would be broken for Windows users using the OS defaults.

@mitchellh
Copy link
Sponsor Contributor Author

That’s understandable, but what are we supposed to do about non-Windows packages that rely [heavily] on symlinks? For example, the macOS SDK uses hundreds of symlinks, and they’re recursive so they can’t be flattened. This is the main problem I’m running up against. I’d love to deliver the SDK using the package manager but right now we’re doing a manual download in a custom step (in my zig pkg branch).

@andrewrk
Copy link
Member

I thought about this some more.

New plan: package manager will recognize symlinks and model them in the hash and file system. If a package cannot be fetched due to the file system or operating system not supporting symlinks, then it will be an error downloading/unpacking. According to zig it will be handled basically the same as an error of running out of disk space while downloading/unpacking. It will be important to communicate this error clearly to the user, and then it will be up to package authors to decide if they want to risk using packages that contain symlinks inside them. Note that symlinks could be excluded via #14311 and this would never trigger the error because zig would not try to create a symlink if it were excluded, and also not include it in the hash.

@mitchellh
Copy link
Sponsor Contributor Author

I like this plan.

I believe there is some other issue tracking this but something that will be critical to make this work well is being able to exclude certain package downloads from certain platforms (or if they're not used for a platform).

For example, if I'm building on Windows, I won't rely on the macOS SDK, so we should avoid downloading that and avoid the symlink error issues on that platform.

@andrewrk
Copy link
Member

@andrewrk andrewrk added the standard library This issue involves writing Zig code for the standard library. label Sep 26, 2023
@nfisher1226
Copy link

Do we only care whether the OS supports symlinks, or do we want to be rigorous and care whether the filesystem supports them? The former is fairly easy and I think I could tackle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants