-
-
Notifications
You must be signed in to change notification settings - Fork 5
fix: Add parent dirs to mtree #96
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
base: main
Are you sure you want to change the base?
Conversation
results.append(_mtree_line(parts, type = "dir", mode = "0755")) | ||
|
||
return results | ||
|
||
# Copied from aspect-bazel-lib/lib/private/tar.bzl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this comment and your the code you are adding, is this something we could simplify by using some rules from https://github.com/bazel-contrib/tar.bzl/blob/main/docs/mtree.md?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What part are you looking to simplify exactly?
The mtree_spec
rule simply creates an mtree file for the files passed in srcs
, it won't create entries for parent dirs of these files. mtree_mutate
has many properties but none that would help either except having a whole different awk_script
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't look too much into the details. I just saw this comment and would have guessed that this could be implemented already somewhere else. Nevermind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should try to de-duplicate these if we can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't realize this is related to the comment.
What about making _mtree_line
public in @tar.bzl//tar:mtree.bzl, similarly to what is done for s3_sync
in rules_aws ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's the obvious answer. However I suspect there's a somewhat more substantial code sharing possible here, since https://github.com/aspect-build/rules_py/blob/main/py/private/py_image_layer.bzl was created after this rule is sketched out. @thesayyn can confirm - or if you have a few minutes to compare the implementations, maybe it's obvious how to do it.
Nested files/folders in image layers result in
InvalidImage(MissingParentDirectory: Parent directory does not exist for file
when deploying on AWS if the parent folders aren't listed in the layer archive mtree.Changes are visible to end-users: no
Test plan
aws_py_lambda
with internal and external dependencies, pushed to ECR and deployed on AWS Lambda. Successfully ran code using internal and external dependencies.//examples/python_lambda:tarball
bazel run //examples/python_lambda:tarball # Then push, deploy and run the image on AWS Lambda