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.

Sign up for GitHub

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

Jump to bottom

pkg_tar should not put ./ at the beginning of all paths #531

Closed
aiuto opened this issue Feb 9, 2022 · 0 comments · Fixed by #554
Closed

pkg_tar should not put ./ at the beginning of all paths #531

aiuto opened this issue Feb 9, 2022 · 0 comments · Fixed by #554
Assignees
@aiuto
Labels
P1 An issue that must be resolved. Must have an assignee
Milestone
1.0

Comments

@aiuto
Copy link
Collaborator

aiuto commented Feb 9, 2022 •
edited
Loading

It does not match the behavior or regular tar. It is also unneeded. Time to take it out.

Consider a directory named only ".", or "./". On untar, should it change the permissions of the current directory? That is probably never what the user would want. So that file simply should not exist.

For any other file it is redundant, since tar extracts "./a" identically to "a".

I had some conversations with people building docker containers with pkg_tar and pkg_deb. They never understood what the leading "./" was for in bazel's rules. Most containers us straight, relative file paths.
@aiuto aiuto self-assigned this Feb 12, 2022
@aiuto aiuto added debrules rules to use debian packages - unsupported P1 An issue that must be resolved. Must have an assignee and removed debrules rules to use debian packages - unsupported labels Feb 12, 2022
@aiuto aiuto added this to the 1.0 milestone Feb 12, 2022
@aiuto aiuto changed the title The ./ at the beginning of all paths in pkg_tar is wrong pkg_tar should not put ./ at the beginning of all paths Feb 12, 2022
aiuto added a commit to aiuto/rules_pkg that referenced this issue Feb 13, 2022
@aiuto
Stop archive writer from always adding './' as a root path.
ef09c5d 
- clean up tests appropriately
- remove achive.add_dir because we do not use it

This is a precursor change to removing the unwanted leading ./ from tar writers.
See bazelbuild#50, bazelbuild#531
@aiuto aiuto mentioned this issue Feb 13, 2022
Merged
aiuto added a commit that referenced this issue Feb 14, 2022
@aiuto
Stop archive writer from always adding './' as a root path. (#540)
d915e26 
* Stop archive writer from always adding './' as a root path.

- clean up tests appropriately
- remove achive.add_dir because we do not use it

This is a precursor change to removing the unwanted leading ./ from tar writers.
See #50, #531
@aiuto aiuto mentioned this issue Feb 28, 2022
Merged
aiuto added a commit to aiuto/rules_pkg that referenced this issue Mar 3, 2022
@aiuto
Fix pkg_tar to not add the ./ to the prefix of every member.
dc44c47 
Fixes: bazelbuild#531

- Remove leading './' from merged tar files. Also applies to pkg_deb.
- Remove package_base from pkg_tar. Closes bazelbuild#549
- Use build_tar --directory as the prefix to add to everything.

Note that the code is intentionaly not testing use cases that try to add "./"
as an explicit root. This might still work, but it is a bad idea, for the
reasons described in bazelbuild#531 . If someone comes up with a real need for
using a leading "./" AND it does not work, they can file a feature
request to support it.
@aiuto aiuto mentioned this issue Mar 3, 2022
Merged
aiuto added a commit to aiuto/rules_pkg that referenced this issue Mar 4, 2022
@aiuto
Fix pkg_tar to not add the ./ to the prefix of every member.
97a4e2d 
Fixes: bazelbuild#531

- Remove leading './' from merged tar files. Also applies to pkg_deb.
- Remove package_base from pkg_tar. Closes bazelbuild#549
- Use build_tar --directory as the prefix to add to everything.

Note that the code is intentionaly not testing use cases that try to add "./"
as an explicit root. This might still work, but it is a bad idea, for the
reasons described in bazelbuild#531 . If someone comes up with a real need for
using a leading "./" AND it does not work, they can file a feature
request to support it.
@aiuto aiuto closed this as completed in #554 Mar 5, 2022
aiuto added a commit that referenced this issue Mar 5, 2022
@aiuto
Fix pkg_tar to not add the ./ to the prefix of every member. (#554)
e68e175 
* Fix pkg_tar to not add the ./ to the prefix of every member.

Fixes: #531

- Remove leading './' from merged tar files. Also applies to pkg_deb.
- Remove package_base from pkg_tar. Closes #549
- Use build_tar --directory as the prefix to add to everything.

Note that the code is intentionaly not testing use cases that try to add "./"
as an explicit root. This might still work, but it is a bad idea, for the
reasons described in #531 . If someone comes up with a real need for
using a leading "./" AND it does not work, they can file a feature
request to support it.
@ljessendk ljessendk mentioned this issue Aug 26, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aiuto aiuto

Labels
P1 An issue that must be resolved. Must have an assignee
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

Fix pkg_tar to not add the ./ to the prefix of every member. aiuto/rules_pkg
1 participant
@aiuto