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

pkg_tar should not strip top level dirname from tree artifacts #404

Closed
aiuto opened this issue Aug 10, 2021 · 2 comments · Fixed by #555
Closed

pkg_tar should not strip top level dirname from tree artifacts #404

aiuto opened this issue Aug 10, 2021 · 2 comments · Fixed by #555
Assignees
Labels
bug P2 An issue that should be worked on when time is available

Comments

@aiuto
Copy link
Collaborator

aiuto commented Aug 10, 2021

When we have a tree artifact as input, we strip the top level name.
That is not always appropriate.

@aiuto aiuto added the P2 An issue that should be worked on when time is available label Oct 4, 2021
@aiuto aiuto changed the title pkg_tar should have option to not strip top level dirname from tree artifacts pkg_files should have option to not strip top level dirname from tree artifacts Nov 1, 2021
@nacl
Copy link
Collaborator

nacl commented Nov 19, 2021

I think I resolved this with renames and REMOVE_BASE_DIRECTORY in 7605368, but it does not yet allow for squashing multiple outputs in a single directory. It might not be the best solution, but it should work.

Please confirm.

@aiuto
Copy link
Collaborator Author

aiuto commented Mar 3, 2022

Maybe. But we still have a problem with pkg_tar directly consuming tree artifacts.

So, I think the pkg_tar needs to change to not strip the tree, and if you want it gone, you can wrap with pkg_files to remove a level.

@aiuto aiuto changed the title pkg_files should have option to not strip top level dirname from tree artifacts pkg_tar should not strip top level dirname from tree artifacts Mar 3, 2022
@aiuto aiuto added the bug label Mar 3, 2022
aiuto added a commit to aiuto/rules_pkg that referenced this issue Mar 4, 2022
Closes bazelbuild#404

While adding tests to make sure we actually could strip it with
pkg_files, I discovered a deficiency in pkg_tar_test. It was not doing
a check on the file names, only content and type.  Fixing that exposed
a pre-existing bug.

Fortunately, the problem uncovered is fixed by bazelbuild#554, so the tests
should pass once that, followed by this PR are submitted.

Allow pkg_files.strip_prefix to work on tree artifact without having to
use `renames`.

Update to 0.7.0 to reflect that this is sort of a big behavioral change.
aiuto added a commit to aiuto/rules_pkg that referenced this issue Mar 4, 2022
Closes bazelbuild#404

While adding tests to make sure we actually could strip it with
pkg_files, I discovered a deficiency in pkg_tar_test. It was not doing
a check on the file names, only content and type.  Fixing that exposed
a pre-existing bug.

Fortunately, the problem uncovered is fixed by bazelbuild#554, so the tests
should pass once that, followed by this PR are submitted.

Allow pkg_files.strip_prefix to work on tree artifact without having to
use `renames`.

Update to 0.7.0 to reflect that this is sort of a big behavioral change.
@aiuto aiuto closed this as completed in #555 Mar 6, 2022
aiuto added a commit that referenced this issue Mar 6, 2022
Closes #404

While adding tests to make sure we actually could strip it with
pkg_files, I discovered a deficiency in pkg_tar_test. It was not doing
a check on the file names, only content and type.  Fixing that exposed
a pre-existing bug.

Fortunately, the problem uncovered is fixed by #554, so the tests
should pass once that, followed by this PR are submitted.

Allow pkg_files.strip_prefix to work on tree artifact without having to
use `renames`.

Update to 0.7.0 to reflect that this is sort of a big behavioral change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug P2 An issue that should be worked on when time is available
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants