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

adding a long hardlink without _fs APIs #192

Closed
the8472 opened this issue Apr 19, 2019 · 3 comments · Fixed by #273
Closed

adding a long hardlink without _fs APIs #192

the8472 opened this issue Apr 19, 2019 · 3 comments · Fixed by #273

Comments

@the8472
Copy link

the8472 commented Apr 19, 2019

Similar to #108, the internal helper method for appending long links is only accessible through APIs that attempt to access the filesystem.

It would be great if it were possible to pass a header (with symlink or hardlink entry_type) and a name + link_name as Path and have the builder insert the necessary long-name headers. I.e. an equivalent to append_data

Maybe some general intermediate-level API is missing for adding headers and data separately to orthogonalize the construction of complicated headers that must be written in several parts (long name, long links, sparse header) and their associated data chunks. A single function taking several enums to cover all those combinations might work too.

@cgwalters
Copy link
Contributor

I'm hitting this in ostreedev/ostree-rs-ext#162

I'm willing to work on a PR here. But reading the code, one thing that strikes me as somewhat odd is that we have this "base interface" in tar::Header and APIs to "cast" it to e.g. a GnuHeader.

But why do we not directly expose all the API methods like set_link_name() on GnuHeader? In my case, I know I am writing a GNU-formatted tar archive. I always want that behavior.

It seems weird to me how many of the Header methods do things like:

    fn _set_path(&mut self, path: &Path) -> io::Result<()> {
        if let Some(ustar) = self.as_ustar_mut() {
            return ustar.set_path(path);
        }
        ...
    // Or base case here

The inverse problem here is that the append_data() behavior of trying the base case but silently "upgrading" to GNU may not be what some people want - they may want to fail instead of writing a GNU entry. I admit this use case is probably obscure, but to rephrase I think we should be consistent in our interface - the format of an archive as a whole is either GNU or PAX or base, etc.

cgwalters added a commit to cgwalters/tar-rs that referenced this issue Nov 18, 2021
We should support appending long symlink targets, because this
occurs in real world filesystems.  In my case, RPM set up
`.build-id` symlinks which can get long.

Add an `append_link()` method following the precendent of
`append_path()` - we're just supporting *two* potentially
long filenames.

As a side benefit, we can just do the `std::io::empty()` dance
internally and not require the caller to specify it.

The addition of special case `append()` methods is unfortunate,
because the header API methods are then really an attractive nuisance.
We should potentially consider deprecating them.

Closes: alexcrichton#192
@cgwalters
Copy link
Contributor

PR in #273

cgwalters added a commit to cgwalters/tar-rs that referenced this issue Nov 18, 2021
We should support appending long symlink targets, because this
occurs in real world filesystems.  In my case, RPM set up
`.build-id` symlinks which can get long.

Add an `append_link()` method following the precendent of
`append_path()` - we're just supporting *two* potentially
long filenames.

As a side benefit, we can just do the `std::io::empty()` dance
internally and not require the caller to specify it.

The addition of special case `append()` methods is unfortunate,
because the header API methods are then really an attractive nuisance.
We should potentially consider deprecating them.

Closes: alexcrichton#192
@cgwalters
Copy link
Contributor

(Regarding my confusion around Header above, the reason we need a special case append_data() is internally it's represented as two tar entries, so we need to handle it via Builder)

cgwalters added a commit to cgwalters/tar-rs that referenced this issue Nov 18, 2021
We should support appending long symlink targets, because this
occurs in real world filesystems.  In my case, RPM set up
`.build-id` symlinks which can get long.

Add an `append_link()` method following the precendent of
`append_path()` - we're just supporting *two* potentially
long filenames.

As a side benefit, we can just do the `std::io::empty()` dance
internally and not require the caller to specify it.

The addition of special case `append()` methods is unfortunate,
because the header API methods are then really an attractive nuisance.
We should potentially consider deprecating them.

Closes: alexcrichton#192
alexcrichton pushed a commit that referenced this issue Nov 19, 2021
We should support appending long symlink targets, because this
occurs in real world filesystems.  In my case, RPM set up
`.build-id` symlinks which can get long.

Add an `append_link()` method following the precendent of
`append_path()` - we're just supporting *two* potentially
long filenames.

As a side benefit, we can just do the `std::io::empty()` dance
internally and not require the caller to specify it.

The addition of special case `append()` methods is unfortunate,
because the header API methods are then really an attractive nuisance.
We should potentially consider deprecating them.

Closes: #192
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 a pull request may close this issue.

2 participants