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 support for no parent directory inference (#832) #834

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

eejayes
Copy link
Contributor

@eejayes eejayes commented Mar 15, 2024

Add feature as described in #832.

RELNOTES: Automatic creation of parent directory specifications for paths with depth are prevented in pkg_tar archives.

Copy link

google-cla bot commented Mar 15, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@eejayes
Copy link
Contributor Author

eejayes commented Mar 15, 2024

This is a start, intended to solicit early feedback. Existing tests have been merely adapted. New tests have not been added.

Copy link
Collaborator

@aiuto aiuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think create_parents should be an attribute of TarWriter. Then we set it once on creation instead of having to plumb the constant through on each call.

On the plus side, this makes the code cleaner.
A potential minus is that we can't have add_parents toggle per src to the tarball.
I don't think that is a needed, or even desired, ability. It would only marginally facilitate a sparse tar with a few added directories. You could simulate that easily by composing
tars or (perhaps) by having an option on pkg_files to auto-insert needed directories.

@eejayes
Copy link
Contributor Author

eejayes commented Mar 20, 2024

I agree and would have done what you suggested to start with had it been taken into account that the change subject was a Class. And no, I don't see an obvious use case for having fine-grained control of the subject behavior within a target.
Let me know if you think the tests suffice. Otherwise I don't intend to add more to them.

Copy link
Collaborator

@aiuto aiuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will do for tests.
Thanks for the fix.

pkg/private/tar/tar_writer.py Show resolved Hide resolved
Add feature as described in bazelbuild#832.

RELNOTES: Automatic creation of parent directory specifications for
paths with depth are prevented in pkg_tar archives.
@eejayes
Copy link
Contributor Author

eejayes commented Mar 21, 2024

I think this will do for tests. Thanks for the fix.

My pleasure!

@aiuto aiuto merged commit a0eb69a into bazelbuild:main Mar 21, 2024
2 checks passed
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