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

Add an append_link() method to handle long link targets

Merged
merged 1 commit into from Nov 19, 2021

Conversation

cgwalters
Copy link
Contributor

@cgwalters cgwalters commented 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: #192

@cgwalters
Copy link
Contributor Author

cgwalters commented Nov 18, 2021

This passes my unit test, but I'm still doing some more real world testing and comparing this (and my higher level code) with what GNU tar actually does.

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 cgwalters marked this pull request as ready for review Nov 18, 2021
cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Nov 18, 2021
I hit this when exporting Fedora Silverblue, there are some
long symlinks in there.

Depends: alexcrichton/tar-rs#273
Closes: ostreedev#162
@cgwalters
Copy link
Contributor Author

cgwalters commented Nov 18, 2021

OK this works, I tested in concert with ostreedev/ostree-rs-ext#165
What delayed me a bit was debugging ostreedev/ostree-rs-ext#164 because in this code it appears as object (SHA-256) corruption, and it wasn't immediately obvious to me that the link name being truncated was the source.

cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Nov 18, 2021
I hit this when exporting Fedora Silverblue, there are some
long symlinks in there.

Depends: alexcrichton/tar-rs#273
Closes: ostreedev#162
@alexcrichton
Copy link
Owner

alexcrichton commented Nov 19, 2021

Thanks!

@alexcrichton alexcrichton merged commit ec5edf1 into alexcrichton:master Nov 19, 2021
7 checks passed
cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Dec 14, 2021
I hit this when exporting Fedora Silverblue, there are some
long symlinks in there.

Depends: alexcrichton/tar-rs#273
Closes: ostreedev#162
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