Skip to content

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: Add parent dirs to mtree #96

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f144cb8
chore(py_lambda): add init_file parent dirs to mtree
rparme May 20, 2025
72bd770
fix(py_lambda): add parent dirs to mtree
rparme May 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions aws/private/py_lambda.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"Rule to produce tar files with py_binary deps and app"

load("@aspect_bazel_lib//lib:tar.bzl", "tar")
load("@bazel_skylib//lib:new_sets.bzl", "sets")

# Write these two separate layers, so application changes are a small delta when pushing to a registry
_LAYERS = ["app", "deps"]
Expand All @@ -15,6 +16,23 @@ def _short_path(file_):
short_path = short_path[second_slash + 1:]
return short_path


def _get_mtree_dirs_list_to_file(path):
results = []
if "/" not in path:
return results

parts = ""
for dir_part in path.split("/")[:-1]:
if len(parts) == 0:
parts += dir_part
else:
parts += "/" + dir_part

results.append(_mtree_line(parts, type = "dir", mode = "0755"))

return results

# Copied from aspect-bazel-lib/lib/private/tar.bzl
Copy link
Contributor

@hofbi hofbi May 21, 2025

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?

Copy link
Author

@rparme rparme May 21, 2025

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.

Copy link
Contributor

@hofbi hofbi May 21, 2025

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.

Copy link
Member

@alexeagle alexeagle May 22, 2025

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.

Copy link
Author

@rparme rparme May 23, 2025

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 ?

Copy link
Member

@alexeagle alexeagle May 23, 2025

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.

def _mtree_line(file, type, content = None, uid = "0", gid = "0", time = "1672560000", mode = "0644"):
spec = [
Expand All @@ -32,22 +50,27 @@ def _mtree_line(file, type, content = None, uid = "0", gid = "0", time = "167256
def _py_lambda_tar_impl(ctx):
deps = ctx.attr.target[DefaultInfo].default_runfiles.files

# NB: this creates one of the parent directories, but others are implicit
mtree = [_mtree_line(ctx.attr.prefix, type = "dir", mode = "0755")]

mtree = []
for dep in deps.to_list():
short_path = _short_path(dep)
mtree_path = None
if dep.owner.workspace_name == "" and ctx.attr.kind == "app":
mtree.append(_mtree_line(ctx.attr.prefix + "/" + dep.short_path, type = "file", content = dep.path))
mtree_path = ctx.attr.prefix + "/" + dep.short_path
elif short_path.startswith("site-packages") and ctx.attr.kind == "deps":
mtree.append(_mtree_line(ctx.attr.prefix + short_path[len("site-packages"):], type = "file", content = dep.path))
mtree_path = ctx.attr.prefix + short_path[len("site-packages"):]
if mtree_path:
mtree.extend(_get_mtree_dirs_list_to_file(mtree_path))
mtree.append(_mtree_line(mtree_path, type = "file", content = dep.path))

if ctx.attr.kind == "app" and ctx.attr.init_files:
path = ""
for dir in ctx.attr.init_files.split("/"):
path = path + "/" + dir
mtree.append(_mtree_line(ctx.attr.prefix + path, type = "dir", mode = "0755"))
mtree.append(_mtree_line(ctx.attr.prefix + path + "/__init__.py", type = "file"))

mtree = sets.to_list(sets.make(mtree))
mtree = sorted(mtree, key = lambda x: len(x.split("/")))
mtree.append("")
ctx.actions.write(ctx.outputs.output, "\n".join(mtree))

Expand Down
Loading