Skip to content

Commit 6975f63

Browse files
lukokr-aarch64uhthomas
authored andcommitted
Fixes output_image_name in install_pkgs to be optional
Previously `output_image_name` needed to be unique across the entire build tree. When building many containers in parallel this could result in clashes between the containers without warning or error. This patches changes `output_image_name` to be optional, with the default value being set to the current target full label making it unique across large builds.
1 parent aada190 commit 6975f63

File tree

3 files changed

+33
-6
lines changed

3 files changed

+33
-6
lines changed

docker/package_managers/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,9 +453,9 @@ within a container.
453453
<tr id="install_pkgs-output_image_name">
454454
<td><code>output_image_name</code></td>
455455
<td>
456-
String; required
456+
String; optional
457457
<p>
458-
Name of container_image produced with the packages installed.
458+
Name of container_image produced with the packages installed. By default the label of the target will be used. *Must be unique* across the entire build tree.
459459
</p>
460460
</td>
461461
</tr>

docker/package_managers/install_pkgs.bzl

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,17 @@ def _impl(ctx, image_tar = None, installables_tar = None, installation_cleanup_c
6666
image_tar: File, overrides ctx.file.image_tar
6767
installables_tar: File, overrides ctx.file.installables_tar
6868
installation_cleanup_commands: str, overrides ctx.attr.installation_cleanup_commands
69-
output_image_name: str, overrides ctx.attr.output_image_name
69+
output_image_name: str, overrides the image name
7070
output_tar: File, overrides ctx.outputs.out
7171
"""
72+
73+
# Construct the name of the image if not given based on the full label path with :output tag.
74+
default_output_image_name = (ctx.label.package + "_" + ctx.label.name).replace(":", "_").replace("@", "_").replace("/", "_") + ":output"
75+
7276
image_tar = image_tar or ctx.file.image_tar
7377
installables_tar = installables_tar or ctx.file.installables_tar
7478
installation_cleanup_commands = installation_cleanup_commands or ctx.attr.installation_cleanup_commands
75-
output_image_name = output_image_name or ctx.attr.output_image_name
79+
output_image_name = output_image_name or ctx.attr.output_image_name or default_output_image_name
7680
output_tar = output_tar or ctx.outputs.out
7781

7882
installables_tar_path = installables_tar.path
@@ -167,8 +171,9 @@ _attrs = {
167171
default = "",
168172
),
169173
"output_image_name": attr.string(
170-
doc = ("Name of container_image produced with the packages installed."),
171-
mandatory = True,
174+
doc = ("Name of container_image produced with the packages installed. " +
175+
"If not specified, resolves to the current target target label. " +
176+
"*Must be unique* across the entire build tree."),
172177
),
173178
"_config_stripper": attr.label(
174179
default = "//docker/util:config_stripper",

tests/docker/package_managers/BUILD

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,28 @@ rule_test(
101101
rule = "test_install_pkgs",
102102
)
103103

104+
# Install without specifying output_image_name.
105+
[
106+
install_pkgs(
107+
name = "test_install_pkgs{}".format(n),
108+
image_tar = "@ubuntu1604//:ubuntu1604_vanilla.tar",
109+
installables_tar = ":test_download_pkgs.tar",
110+
)
111+
for n in range(2)
112+
]
113+
114+
[
115+
rule_test(
116+
name = "test_install_pkgs{}_rule".format(n),
117+
generates = [
118+
"test_install_pkgs{}.build".format(n),
119+
"test_install_pkgs{}.tar".format(n),
120+
],
121+
rule = "test_install_pkgs{}".format(n),
122+
)
123+
for n in range(2)
124+
]
125+
104126
add_apt_key(
105127
name = "gpg_image",
106128
image = "@debian9//:builder.tar",

0 commit comments

Comments
 (0)