builder: Note that append_dir_all doesn't hardlink
#368
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I got caught out by this not generating hardlinks. At this point it's probably part of our "ABI", so just add a note in the docs. Signed-off-by: Colin Walters <[email protected]>
schneems
reviewed
Jul 5, 2024
| /// | ||
| /// This function will never create hardlinks; to do that, you will | ||
| /// need to implement a custom recursive walk. | ||
| /// |
There was a problem hiding this comment.
Not the maintainer (thanks for the review of my PRs!). I'm also a newb when it comes to a lot of the details of tar implementations and details. Leaving a comment here so you can dismiss/hide it and it won't take up space on the main thread.
- What is the behavior when it encounters hardlinks? Will it copy them like Add doc hint that default is different than
tar#366? Would it make sense to spell out what "never create[ing] hardlinks" means in practice? I would appreciate that info when viewing the docs. - Do you know if that behavior is intentional or accidental? I.e. Could this perhaps be a bug to be fixed rather than a behavior to be documented? Genuine question.
- If it's intentional, could adding a method to the builder like
preserve_hardlinks(bool)be a way forward?
My goal here is to better understand the library and its existing behavior and to explore and understand the possibility of ways we could improve this library or possibly the PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I got caught out by this not generating hardlinks. At this point it's probably part of our "ABI", so just add a note in the docs.