When pkg_tar.prefix_dir == base of symlink path, don't double-dip.#749
Merged
aiuto merged 21 commits intobazelbuild:mainfrom Oct 16, 2023
Merged
When pkg_tar.prefix_dir == base of symlink path, don't double-dip.#749aiuto merged 21 commits intobazelbuild:mainfrom
aiuto merged 21 commits intobazelbuild:mainfrom
Conversation
user has already mapped the prefix_dir into those paths. Under the old behavior, prefix_dir was (incorrectly) not added to symlinks, so some users made the desired prefix part of the link. Protecting them against the double inclusion of the prefix is probably the least surprising behavior. Add tests for this, which required improving verify_archive_test.
aiuto
commented
Sep 3, 2023
sdtwigg
approved these changes
Oct 16, 2023
|
Why is |
Collaborator
Author
|
It's obsolete because we have the equivalent in the pkg_files rule. Rather
than maintain features
in tar only, but putting it in pkg_files we can reuse that in zip, deb, and
rpm.
The files and symlinks features in pkg_tar were never quite designed. They
were hacked in over time
at Google to solve some team's immediate needs.
…On Mon, Oct 23, 2023 at 2:10 PM Alexander Coffin ***@***.***> wrote:
Why is pkg_tar-files now labeled as "obsolete"? I use this feature very
heavily, and would be curious what the replacement for it is (or what I
should be doing instead of using pkg_tar-files)?
—
Reply to this email directly, view it on GitHub
<#749 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXHHHDQGBKWYEWBFAC37GLYA2XJ5AVCNFSM6AAAAAA4IBNLTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZVG42DOMJZGQ>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
|
That makes sense, thank you for helping me out. I'll try converting our internal usage to |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Correct for case where tar has prefix_dir and files/symlinks, but the user has already mapped the prefix_dir into those paths. Under the old behavior, prefix_dir was (incorrectly) not added to symlinks, so some users made the desired prefix part of the link. Protecting them against the double inclusion of the prefix is not strictly pedantic, but is the least surprising behavior, as their intent was most likely to get what the old behavior would have achived.
Add tests for this, which required improving verify_archive_test.
Along the way, it seems that tree artifacts with internal symlinks are a problem. This might be a bazel bug.
#750 is a reminder to look in to this some day.