diff --git a/pkg/private/install.py.tpl b/pkg/private/install.py.tpl index 1e82fae5..004049fb 100644 --- a/pkg/private/install.py.tpl +++ b/pkg/private/install.py.tpl @@ -74,11 +74,11 @@ class NativeInstaller(object): logging.info("CHOWN %s:%s %s", user, group, dest) shutil.chown(dest, user, group) - def _do_file_copy(self, src, dest, mode, user, group): + def _do_file_copy(self, src, dest): logging.info("COPY %s <- %s", dest, src) shutil.copyfile(src, dest) - def _do_mkdir(self, dirname, mode, user, group): + def _do_mkdir(self, dirname, mode): logging.info("MKDIR %s %s", mode, dirname) os.makedirs(dirname, int(mode, 8), exist_ok=True) @@ -93,25 +93,56 @@ class NativeInstaller(object): def _install_file(self, entry): self._maybe_make_unowned_dir(os.path.dirname(entry.dest)) - self._do_file_copy(entry.src, entry.dest, entry.mode, entry.user, entry.group) + self._do_file_copy(entry.src, entry.dest) self._chown_chmod(entry.dest, entry.mode, entry.user, entry.group) def _install_directory(self, entry): self._maybe_make_unowned_dir(os.path.dirname(entry.dest)) - self._do_mkdir(entry.dest, entry.mode, entry.user, entry.group) + self._do_mkdir(entry.dest, entry.mode) self._chown_chmod(entry.dest, entry.mode, entry.user, entry.group) + def _install_treeartifact_file(self, entry, src, dst): + self._do_file_copy(src, dst) + self._chown_chmod(dst, entry.mode, entry.user, entry.group) + def _install_treeartifact(self, entry): logging.info("COPYTREE %s <- %s/**", entry.dest, entry.src) - raise NotImplementedError("treeartifact installation not yet supported") - for root, dirs, files in os.walk(entry.src): - relative_installdir = os.path.join(entry.dest, root) + shutil.copytree( + src=entry.src, + dst=entry.dest, + copy_function=lambda s, d: + self._install_treeartifact_file(entry, s, d), + dirs_exist_ok=True, + # Bazel gives us a directory of symlinks, so we dereference it. + # TODO: Handle symlinks within the TreeArtifact. This is not yet + # tested for other rules (e.g. + # https://github.com/bazelbuild/rules_pkg/issues/750) + symlinks=False, + ignore_dangling_symlinks=True, + ) + + # Set mode/user/group for intermediate directories. + # Bazel has no API to specify modes for this, so the least surprising + # thing we can do is make it the canonical rwxr-xr-x + intermediate_dir_mode = "755" + for root, dirs, _ in os.walk(entry.src, topdown=False): + relative_installdir = os.path.join(entry.dest, + os.path.relpath(root, entry.src)) for d in dirs: - self._maybe_make_unowned_dir(os.path.join(relative_installdir, d)) - - logging.info("COPY_FROM_TREE %s <- %s", entry.dest, entry.src) - logging.info("CHMOD %s %s", entry.mode, entry.dest) - logging.info("CHOWN %s:%s %s", entry.user, entry.group, entry.dest) + self._chown_chmod(os.path.join(relative_installdir, d), + intermediate_dir_mode, + entry.user, entry.group) + + # For top-level directory, use entry.mode +r +x if specified, otherwise + # use least-surprising canonical rwxr-xr-x + top_dir_mode = entry.mode + if top_dir_mode: + top_dir_mode = int(top_dir_mode, 8) + top_dir_mode |= 0o555 + top_dir_mode = oct(top_dir_mode).removeprefix("0o") + else: + top_dir_mode = "755" + self._chown_chmod(entry.dest, top_dir_mode, entry.user, entry.group) def _install_symlink(self, entry): raise NotImplementedError("symlinking not yet supported") diff --git a/tests/install/BUILD b/tests/install/BUILD index 5a2c127c..4449a899 100644 --- a/tests/install/BUILD +++ b/tests/install/BUILD @@ -15,7 +15,7 @@ load("@rules_python//python:defs.bzl", "py_test") load("//pkg:install.bzl", "pkg_install") load("//pkg:mappings.bzl", "pkg_attributes", "pkg_files", "pkg_mkdirs") -load("//tests/util:defs.bzl", "fake_artifact") +load("//tests/util:defs.bzl", "directory", "fake_artifact") package(default_applicable_licenses = ["//:license"]) @@ -47,6 +47,7 @@ pkg_install( ":artifact-in-owned-dir", ":artifact-in-unowned-dir", ":dirs", + ":generate_tree_pkg_files", ], ) @@ -75,3 +76,24 @@ pkg_mkdirs( "owned-dir", ], ) + +directory( + name = "generate_tree", + contents = "hello there", + filenames = [ + # buildifier: don't sort + "b/e", + "a/a", + "b/c/d", + "b/d", + "a/b/c", + ], +) + +pkg_files( + name = "generate_tree_pkg_files", + srcs = [":generate_tree"], + attributes = pkg_attributes( + mode = "640", + ), +) diff --git a/tests/install/test.py b/tests/install/test.py index bbb02ab5..3ee2562f 100644 --- a/tests/install/test.py +++ b/tests/install/test.py @@ -61,6 +61,12 @@ def entity_type_at_path(self, path): def assertEntryTypeMatches(self, entry, actual_path): actual_entry_type = self.entity_type_at_path(actual_path) + + # TreeArtifacts looks like directories. + if (entry.type == manifest.ENTRY_IS_TREE and + actual_entry_type == manifest.ENTRY_IS_DIR): + return + self.assertEqual(actual_entry_type, entry.type, "Entity {} should be a {}, but was actually {}".format( entry.dest, @@ -68,7 +74,8 @@ def assertEntryTypeMatches(self, entry, actual_path): manifest.entry_type_to_string(actual_entry_type), )) - def assertEntryModeMatches(self, entry, actual_path): + def assertEntryModeMatches(self, entry, actual_path, + is_tree_artifact_content=False): # TODO: permissions in windows are... tricky. Don't bother # testing for them if we're in it for the time being if os.name == 'nt': @@ -76,14 +83,28 @@ def assertEntryModeMatches(self, entry, actual_path): actual_mode = stat.S_IMODE(os.stat(actual_path).st_mode) expected_mode = int(entry.mode, 8) + + if (not is_tree_artifact_content and + entry.type == manifest.ENTRY_IS_TREE): + expected_mode |= 0o555 + self.assertEqual(actual_mode, expected_mode, - "Entry {} has mode {:04o}, expected {:04o}".format( - entry.dest, actual_mode, expected_mode, - )) + "Entry {}{} has mode {:04o}, expected {:04o}".format( + entry.dest, + f" ({actual_path})" if is_tree_artifact_content else "", + actual_mode, expected_mode, + )) + + def _find_tree_entry(self, path, owned_trees): + for tree_root in owned_trees: + if path == tree_root or path.startswith(tree_root + "/"): + return tree_root + return None def test_manifest_matches(self): unowned_dirs = set() owned_dirs = set() + owned_trees = dict() # Figure out what directories we are supposed to own, and which ones we # aren't. @@ -95,6 +116,8 @@ def test_manifest_matches(self): for dest, data in self.manifest_data.items(): if data.type == manifest.ENTRY_IS_DIR: owned_dirs.add(dest) + elif data.type == manifest.ENTRY_IS_TREE: + owned_trees[dest] = data # TODO(nacl): The initial stage of the accumulation returns an empty string, # which end up in the set representing the root of the manifest. @@ -119,9 +142,6 @@ def test_manifest_matches(self): if rel_root_path == '.': rel_root_path = '' - # TODO(nacl): check for treeartifacts here. If so, prune `dirs`, - # and set the rest aside for future processing. - # Directory ownership tests if len(files) == 0 and len(dirs) == 0: # Empty directories must be explicitly requested by something @@ -145,7 +165,10 @@ def test_manifest_matches(self): else: # If any unowned directories are here, they must be the # prefix of some entity in the manifest. - self.assertIn(rel_root_path, unowned_dirs) + is_unowned = rel_root_path in unowned_dirs + is_tree_intermediate_dir = bool( + self._find_tree_entry(rel_root_path, owned_trees)) + self.assertTrue(is_unowned or is_tree_intermediate_dir) for f in files: # The path on the filesystem in which the file actually exists. @@ -163,16 +186,22 @@ def test_manifest_matches(self): # The path inside the manifest (relative to the install # destdir). rel_fpath = os.path.normpath("/".join([rel_root_path, f])) - if rel_fpath not in self.manifest_data: + entity_in_manifest = rel_fpath in self.manifest_data + entity_tree_root = self._find_tree_entry(rel_fpath, owned_trees) + if not entity_in_manifest and not entity_tree_root: self.fail("Entity {} not in manifest".format(rel_fpath)) - entry = self.manifest_data[rel_fpath] - self.assertEntryTypeMatches(entry, fpath) - self.assertEntryModeMatches(entry, fpath) + if entity_in_manifest: + entry = self.manifest_data[rel_fpath] + self.assertEntryTypeMatches(entry, fpath) + self.assertEntryModeMatches(entry, fpath) - found_entries[rel_fpath] = True + if entity_tree_root: + entry = owned_trees[entity_tree_root] + self.assertEntryModeMatches(entry, fpath, + is_tree_artifact_content=True) - # TODO(nacl): check for TreeArtifacts + found_entries[rel_fpath] = True num_missing = 0 for dest, present in found_entries.items():