diff --git a/lang/image.bzl b/lang/image.bzl index 2eca59ea5..8c58fc28a 100644 --- a/lang/image.bzl +++ b/lang/image.bzl @@ -126,6 +126,12 @@ def _default_symlinks(dep): else: return dep[DefaultInfo].default_runfiles.symlinks +def _default_root_symlinks(dep): + if FilterLayerInfo in dep: + return dep[FilterLayerInfo].runfiles.root_symlinks + else: + return dep[DefaultInfo].default_runfiles.root_symlinks + def _app_layer_impl(ctx, runfiles = None, emptyfiles = None): """Appends a layer for a single dependency's runfiles. @@ -186,11 +192,24 @@ def _app_layer_impl(ctx, runfiles = None, emptyfiles = None): # app layer, we can already create symlinks to the runfiles path. if ctx.attr.binary: # Include any symlinks from the runfiles of the target for which we are synthesizing the layer. - symlinks.update({ - (_reference_dir(ctx) + "/" + s.path): layer_file_path(ctx, s.target_file) - for s in _default_symlinks(dep).to_list() - if hasattr(s, "path") # "path" and "target_file" are exposed to starlark since bazel 0.21.0. - }) + for s in _default_symlinks(dep).to_list(): + symlink_path = _reference_dir(ctx) + "/" + s.path + if filepath(ctx, s.target_file) in file_map: # If the target is a real file, link to it + symlinks.update({ + symlink_path: _final_file_path(ctx, s.target_file), + }) + else: + file_map[symlink_path] = s.target_file # Otherwise, synthesize the "symlink" as a real file + + # Include root_symlinks + for s in _default_root_symlinks(dep).to_list(): + symlink_path = _runfiles_dir(ctx) + "/" + s.path + if filepath(ctx, s.target_file) in file_map: + symlinks.update({ + symlink_path: _final_file_path(ctx, s.target_file), + }) + else: + file_map[symlink_path] = s.target_file symlinks.update({ _final_file_path(ctx, f): layer_file_path(ctx, f) diff --git a/testdata/utils.bzl b/testdata/utils.bzl index 116584168..ecedf636c 100644 --- a/testdata/utils.bzl +++ b/testdata/utils.bzl @@ -28,9 +28,21 @@ def generate_deb(name, args = [], metadata_compression_type = "none"): ) def _rule_with_symlinks_impl(ctx): - f = ctx.actions.declare_file("foo.txt") - ctx.actions.write(f, "test content") - runfiles = ctx.runfiles(files = [f], symlinks = {"foo-symlink.txt": f}) + foo = ctx.actions.declare_file("foo.txt") + ctx.actions.write(foo, "test content") + bar = ctx.actions.declare_file("bar.txt") + ctx.actions.write(bar, "test content") + baz = ctx.actions.declare_file("baz.txt") + ctx.actions.write(baz, "test content") + runfiles = ctx.runfiles( + # bar and baz are specifically excluded from runfiles.files + # We want to test that we don't create broken symlinks for [root_]symlinks that don't + # have a corresponding file in runfiles.files. + # We expect that for those [root_]symlinks, a real file exists at the symlink path. + files = [foo], + symlinks = {"foo-symlink.txt": foo, "baz/dir/baz-symlink-real.txt": baz}, + root_symlinks = {"foo-root-symlink.txt": foo, "bar-root-symlink-real.txt": bar}, + ) return DefaultInfo(runfiles = runfiles) rule_with_symlinks = rule( diff --git a/tests/container/image_test.py b/tests/container/image_test.py index a3d9fa658..d83cda708 100644 --- a/tests/container/image_test.py +++ b/tests/container/image_test.py @@ -13,6 +13,7 @@ # limitations under the License. from io import BytesIO +import contextlib import datetime import json import os @@ -53,20 +54,33 @@ def assertTarballContains(self, tar, paths): self.maxDiff = None self.assertEqual(paths, tar.getnames()) - def assertLayerNContains(self, img, n, paths): + def assertTarballSymlink(self, tar, path, target): + self.assertEqual(target, tar.getmember(path).linkname) + + @contextlib.contextmanager + def _tarball_layer_n(self, img, n): buf = BytesIO(img.blob(img.fs_layers()[n])) - with tarfile.open(fileobj=buf, mode='r') as layer: + yield tarfile.open(fileobj=buf, mode='r') + + def assertLayerNContains(self, img, n, paths): + with self._tarball_layer_n(img, n) as layer: self.assertTarballContains(layer, paths) + def assertLayerNSymlink(self, img, n, path, target): + with self._tarball_layer_n(img, n) as layer: + self.assertTarballSymlink(layer, path, target) + def assertNonZeroMtimesInTopLayer(self, img): - buf = BytesIO(img.blob(img.fs_layers()[0])) - with tarfile.open(fileobj=buf, mode='r') as layer: + with self._tarball_layer_n(img, 0) as layer: for member in layer.getmembers(): self.assertNotEqual(member.mtime, 0) def assertTopLayerContains(self, img, paths): self.assertLayerNContains(img, 0, paths) + def assertTopLayerSymlink(self, img, path, target): + self.assertLayerNSymlink(img, 0, path, target) + def assertConfigEqual(self, img, key, value): cfg = json.loads(img.config_file()) self.assertEqual(value, cfg.get('config', {}).get(key)) @@ -562,6 +576,12 @@ def test_py_image_with_symlinks_in_data(self): './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/testdata/py_image.py', './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/testdata/py_image_with_symlinks_in_data.binary', './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/testdata/foo.txt', + # baz-symlink-real.txt is a real file, since the File that it points to is not in runfiles.files + './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/baz', + './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/baz/dir', + './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/baz/dir/baz-symlink-real.txt', + # bar-root-symlink-real.txt is a real file, since the File that it points to is not in runfiles.files + './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/bar-root-symlink-real.txt', './app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/testdata/__init__.py', './app/io_bazel_rules_docker', # TODO(mattmoor): The path normalization for symlinks should match @@ -571,10 +591,18 @@ def test_py_image_with_symlinks_in_data(self): '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles', '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker', '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/foo-symlink.txt', + '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/foo-root-symlink.txt', '/app/testdata/py_image_with_symlinks_in_data.binary', '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/external', ]) + # Test that root_symlinks that point to a file that is also in the runfiles tree is a symlink + self.assertTopLayerSymlink( + img, + '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/foo-root-symlink.txt', + '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/testdata/foo.txt', + ) + # Below that, we have a layer that generates symlinks for the library layer. self.assertLayerNContains(img, 1, [ '.', @@ -586,6 +614,14 @@ def test_py_image_with_symlinks_in_data(self): '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/testdata/py_image_library.py', ]) + # Validate that library symlink is actually a symlink to the right path + self.assertLayerNSymlink( + img, + 1, + '/app/testdata/py_image_with_symlinks_in_data.binary.runfiles/io_bazel_rules_docker/testdata/py_image_library.py', + '/app/io_bazel_rules_docker/testdata/py_image_library.py', + ) + # Check the library layer, which is two below our application layer. self.assertLayerNContains(img, 2, [ '.',