-
-
Notifications
You must be signed in to change notification settings - Fork 42
Add strip_prefix
parameter to py_image_layer
#515
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
Jimmy Tanner seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
# Move everything under the specified root | ||
sub(/^/, ".%s") | ||
sub(/^{strip_prefix}/, ".{root}") |
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.
Note: this line is the primary change. Here we append strip_prefix
to the beginning matcher, so that it is taken out of the path.
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.
strip_prefix needs more than this i believe, see an example logic.
CC @thesayyn |
name = "my_app_layers_strip_prefix", | ||
binary = ":my_app_bin", | ||
platform = ":linux_amd64", | ||
strip_prefix = "py\\/tests\\/py_image_layer\\/my_app_bin", |
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.
it should be possible to pass this string to awk -v strip_prefix=
so that we don't have to escape slashes.
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.
basically pass this information to awk using its -v strip_prefix=/py/test/my_app_bin
and use strip_prefix
variable within awk script.
By default, bazel lays out files using the target name as the root for all runfiles. For example, the target //a/b/c:my_bin has its runfiles directory located at $BAZEL_ROOT/a/b/c/my_bin.runfiles/. When placed into an oci_image this becomes /a/b/c/my_bin.runfiles/.
The problem with the above is that it prevents us from sharing layers between separate images. Even if no
layer_groups
are provided, at a minimum the contents of the interpreter layer are identical across images, but can't be shared because the path prefixes of the files are different.The primary change in this PR is to introduce a strip_prefix option to the
py_image_layer
macro that allows us to remove prefixes from the directories laid out in the image. Combined with the existingroot
option, we can align directory paths across images and produce identical layer hashes.Also changes the template strings to use named replacement syntax, for readability.
Changes are visible to end-users: no (default parameter value produces previous behavior)
Test plan