diff --git a/pkg/install.bzl b/pkg/install.bzl index 846401d7..262554b3 100644 --- a/pkg/install.bzl +++ b/pkg/install.bzl @@ -29,6 +29,7 @@ def _pkg_install_script_impl(ctx): content_map = {} for src in ctx.attr.srcs: process_src( + ctx, content_map, files_to_run, src = src, diff --git a/pkg/private/pkg_files.bzl b/pkg/private/pkg_files.bzl index e5470332..86a438b9 100644 --- a/pkg/private/pkg_files.bzl +++ b/pkg/private/pkg_files.bzl @@ -61,7 +61,7 @@ _DestFile = provider( }, ) -def _check_dest(content_map, dest, src, origin): +def _check_dest(ctx, content_map, dest, src, origin): old_entry = content_map.get(dest) if not old_entry: return @@ -73,16 +73,20 @@ def _check_dest(content_map, dest, src, origin): # people specify the owner in one place, but another overly broad glob # brings in the file with a different owner. if old_entry.src.path != src.path: - # buildifier: disable=print - print( - "Duplicate output path: <%s>, declared in %s and %s" % ( + msg = "Duplicate output path: <%s>, declared in %s and %s\n SRC: %s" % ( dest, origin, content_map[dest].origin, - ), - "\n SRC:", - src, - ) + src, + ) + if ctx.attr.allow_duplicates_with_different_content: + # buildifier: disable=print + print("WARNING:", msg) + else: + # When we default to this behaviour, we should consider telling + # users the attribute to set to deal with this. + # For now though, let's not, since they've explicitly opted in. + fail(msg) def _merge_attributes(info, mode, user, group, uid, gid): if hasattr(info, "attributes"): @@ -90,6 +94,7 @@ def _merge_attributes(info, mode, user, group, uid, gid): mode = attrs.get("mode") or mode user = attrs.get("user") or user group = attrs.get("group") or group + new_uid = attrs.get("uid") if new_uid != None: uid = new_uid @@ -98,11 +103,11 @@ def _merge_attributes(info, mode, user, group, uid, gid): gid = new_gid return (mode, user, group, uid, gid) -def _process_pkg_dirs(content_map, pkg_dirs_info, origin, default_mode, default_user, default_group, default_uid, default_gid): +def _process_pkg_dirs(ctx, content_map, pkg_dirs_info, origin, default_mode, default_user, default_group, default_uid, default_gid): attrs = _merge_attributes(pkg_dirs_info, default_mode, default_user, default_group, default_uid, default_gid) for dir in pkg_dirs_info.dirs: dest = dir.strip("/") - _check_dest(content_map, dest, None, origin) + _check_dest(ctx, content_map, dest, None, origin) content_map[dest] = _DestFile( src = None, entry_type = ENTRY_IS_DIR, @@ -114,11 +119,11 @@ def _process_pkg_dirs(content_map, pkg_dirs_info, origin, default_mode, default_ origin = origin, ) -def _process_pkg_files(content_map, pkg_files_info, origin, default_mode, default_user, default_group, default_uid, default_gid): +def _process_pkg_files(ctx, content_map, pkg_files_info, origin, default_mode, default_user, default_group, default_uid, default_gid): attrs = _merge_attributes(pkg_files_info, default_mode, default_user, default_group, default_uid, default_gid) for filename, src in pkg_files_info.dest_src_map.items(): dest = filename.strip("/") - _check_dest(content_map, dest, src, origin) + _check_dest(ctx, content_map, dest, src, origin) content_map[dest] = _DestFile( src = src, entry_type = ENTRY_IS_TREE if src.is_directory else ENTRY_IS_FILE, @@ -130,10 +135,10 @@ def _process_pkg_files(content_map, pkg_files_info, origin, default_mode, defaul origin = origin, ) -def _process_pkg_symlink(content_map, pkg_symlink_info, origin, default_mode, default_user, default_group, default_uid, default_gid): +def _process_pkg_symlink(ctx, content_map, pkg_symlink_info, origin, default_mode, default_user, default_group, default_uid, default_gid): dest = pkg_symlink_info.destination.strip("/") attrs = _merge_attributes(pkg_symlink_info, default_mode, default_user, default_group, default_uid, default_gid) - _check_dest(content_map, dest, None, origin) + _check_dest(ctx, content_map, dest, None, origin) content_map[dest] = _DestFile( src = None, entry_type = ENTRY_IS_LINK, @@ -146,18 +151,19 @@ def _process_pkg_symlink(content_map, pkg_symlink_info, origin, default_mode, de link_to = pkg_symlink_info.target, ) -def _process_pkg_filegroup(content_map, pkg_filegroup_info, origin, default_mode, default_user, default_group, default_uid, default_gid): +def _process_pkg_filegroup(ctx, content_map, pkg_filegroup_info, origin, default_mode, default_user, default_group, default_uid, default_gid): if hasattr(pkg_filegroup_info, "pkg_dirs"): for d in pkg_filegroup_info.pkg_dirs: - _process_pkg_dirs(content_map, d[0], d[1], default_mode, default_user, default_group, default_uid, default_gid) + _process_pkg_dirs(ctx, content_map, d[0], d[1], default_mode, default_user, default_group, default_uid, default_gid) if hasattr(pkg_filegroup_info, "pkg_files"): for pf in pkg_filegroup_info.pkg_files: - _process_pkg_files(content_map, pf[0], pf[1], default_mode, default_user, default_group, default_uid, default_gid) + _process_pkg_files(ctx, content_map, pf[0], pf[1], default_mode, default_user, default_group, default_uid, default_gid) if hasattr(pkg_filegroup_info, "pkg_symlinks"): for psl in pkg_filegroup_info.pkg_symlinks: - _process_pkg_symlink(content_map, psl[0], psl[1], default_mode, default_user, default_group, default_uid, default_gid) + _process_pkg_symlink(ctx, content_map, psl[0], psl[1], default_mode, default_user, default_group, default_uid, default_gid) def process_src( + ctx, content_map, files, src, @@ -191,6 +197,7 @@ def process_src( found_info = False if PackageFilesInfo in src: _process_pkg_files( + ctx, content_map, src[PackageFilesInfo], origin, @@ -203,6 +210,7 @@ def process_src( found_info = True if PackageFilegroupInfo in src: _process_pkg_filegroup( + ctx, content_map, src[PackageFilegroupInfo], origin, @@ -215,6 +223,7 @@ def process_src( found_info = True if PackageSymlinkInfo in src: _process_pkg_symlink( + ctx, content_map, src[PackageSymlinkInfo], origin, @@ -227,6 +236,7 @@ def process_src( found_info = True if PackageDirsInfo in src: _process_pkg_dirs( + ctx, content_map, src[PackageDirsInfo], origin, @@ -261,10 +271,11 @@ def add_directory(content_map, dir_path, origin, mode = None, user = None, group gid = gid, ) -def add_empty_file(content_map, dest_path, origin, mode = None, user = None, group = None, uid = None, gid = None): +def add_empty_file(ctx, content_map, dest_path, origin, mode = None, user = None, group = None, uid = None, gid = None): """Add a single file to the content map. Args: + ctx: rule context. content_map: The content map dest_path: Where to place the file in the package. origin: The rule instance adding this entry @@ -273,7 +284,7 @@ def add_empty_file(content_map, dest_path, origin, mode = None, user = None, gro group: fallback mode to use for Package*Info elements without group """ dest = dest_path.strip("/") - _check_dest(content_map, dest, None, origin) + _check_dest(ctx, content_map, dest, None, origin) content_map[dest] = _DestFile( src = None, entry_type = ENTRY_IS_EMPTY_FILE, @@ -323,6 +334,7 @@ def add_label_list( for src in srcs: if not process_src( + ctx, content_map, file_deps, src = src, @@ -335,6 +347,7 @@ def add_label_list( ): # Add in the files of srcs which are not pkg_* types add_from_default_info( + ctx, content_map, file_deps, src, @@ -349,6 +362,7 @@ def add_label_list( ) def add_from_default_info( + ctx, content_map, file_deps, src, @@ -363,6 +377,7 @@ def add_from_default_info( """Helper method to add the DefaultInfo of a target to a content_map. Args: + ctx: rule context. content_map: (r/w) The content map to update. file_deps: (r/w) The list of file Depsets that srcs depend on. src: A source object. @@ -394,6 +409,7 @@ def add_from_default_info( else: fmode = "0755" if f == the_executable else default_mode add_single_file( + ctx, content_map, dest_path = d_path, src = f, @@ -409,7 +425,7 @@ def add_from_default_info( for rf in runfiles.files.to_list(): d_path = base_path + "/" + rf.short_path fmode = "0755" if rf == the_executable else default_mode - _check_dest(content_map, d_path, rf, src.label) + _check_dest(ctx, content_map, d_path, rf, src.label) content_map[d_path] = _DestFile( src = rf, entry_type = ENTRY_IS_FILE, @@ -450,10 +466,12 @@ def get_my_executable(src): return ftr.executable return None -def add_single_file(content_map, dest_path, src, origin, mode = None, user = None, group = None, uid = None, gid = None): + +def add_single_file(ctx, content_map, dest_path, src, origin, mode = None, user = None, group = None, uid = None, gid = None): """Add an single file to the content map. Args: + ctx: rule context. content_map: The content map dest_path: Where to place the file in the package. src: Source object. Must have len(src[DefaultInfo].files) == 1 @@ -463,7 +481,7 @@ def add_single_file(content_map, dest_path, src, origin, mode = None, user = Non group: fallback mode to use for Package*Info elements without group """ dest = dest_path.strip("/") - _check_dest(content_map, dest, src, origin) + _check_dest(ctx, content_map, dest, src, origin) content_map[dest] = _DestFile( src = src, entry_type = ENTRY_IS_FILE, @@ -475,10 +493,12 @@ def add_single_file(content_map, dest_path, src, origin, mode = None, user = Non gid = gid, ) -def add_symlink(content_map, dest_path, src, origin, mode = None, user = None, group = None, uid = None, gid = None): +def add_symlink(ctx, content_map, dest_path, src, origin, mode = None, user = None, group = None, uid = None, gid = None): + """Add a symlink to the content map. Args: + ctx: rule context. content_map: The content map dest_path: Where to place the file in the package. src: Path to link to. @@ -488,7 +508,7 @@ def add_symlink(content_map, dest_path, src, origin, mode = None, user = None, g group: fallback mode to use for Package*Info elements without group """ dest = dest_path.strip("/") - _check_dest(content_map, dest, None, origin) + _check_dest(ctx, content_map, dest, None, origin) content_map[dest] = _DestFile( src = None, link_to = src, diff --git a/pkg/private/tar/tar.bzl b/pkg/private/tar/tar.bzl index 013ee1d3..b3619884 100644 --- a/pkg/private/tar/tar.bzl +++ b/pkg/private/tar/tar.bzl @@ -120,6 +120,7 @@ def _pkg_tar_impl(ctx): # Start with all the pkg_* inputs for src in ctx.attr.srcs: if not process_src( + ctx, content_map, file_deps, src = src, @@ -149,7 +150,12 @@ def _pkg_tar_impl(ctx): if f.is_directory: add_tree_artifact(content_map, dest, f, src.label) else: - add_single_file(content_map, dest, f, src.label) + # Note: This extra remap is the bottleneck preventing this + # large block from being a utility method as shown below. + # Should we disallow mixing pkg_files in srcs with remap? + # I am fine with that if it makes the code more readable. + dest = _remap(remap_paths, d_path) + add_single_file(ctx, content_map, dest, f, src.label) # TODO(aiuto): I want the code to look like this, but we don't have lambdas. # transform_path = lambda f: _remap( @@ -164,6 +170,7 @@ def _pkg_tar_impl(ctx): fail("Each input must describe exactly one file.", attr = "files") file_deps.append(depset([target_files[0]])) add_single_file( + ctx, content_map, f_dest_path, target_files[0], @@ -183,13 +190,14 @@ def _pkg_tar_impl(ctx): "%s=%s" % (_quote(key), ctx.attr.ownernames[key]), ) for empty_file in ctx.attr.empty_files: - add_empty_file(content_map, empty_file, ctx.label) + add_empty_file(ctx, content_map, empty_file, ctx.label) for empty_dir in ctx.attr.empty_dirs or []: add_directory(content_map, empty_dir, ctx.label) for f in ctx.files.deps: args.add("--tar", f.path) for link in ctx.attr.symlinks: add_symlink( + ctx, content_map, link, ctx.attr.symlinks[link], @@ -279,6 +287,14 @@ The name may contain variables, same as [package_file_name](#package_file_name)" doc = "See [Common Attributes](#package_variables)", providers = [PackageVariablesInfo], ), + "allow_duplicates_with_different_content": attr.bool( + default=True, + doc="""If true, will allow you to reference multiple pkg_* which conflict +(writing different content or metadata to the same destination). +Such behaviour is always incorrect, but we provide a flag to support it in case old +builds were accidentally doing it. Never explicitly set this to true for new code. +""" + ), "stamp": attr.int( doc = """Enable file time stamping. Possible values:
  • stamp = 1: Use the time of the build as the modification time of each file in the archive. diff --git a/pkg/private/zip/zip.bzl b/pkg/private/zip/zip.bzl index 3cb655f2..e038b488 100644 --- a/pkg/private/zip/zip.bzl +++ b/pkg/private/zip/zip.bzl @@ -142,6 +142,14 @@ The list of compressions is the same as Python's ZipFile: https://docs.python.or default = 0, ), + "allow_duplicates_with_different_content": attr.bool( + default=True, + doc="""If true, will allow you to reference multiple pkg_* which conflict +(writing different content or metadata to the same destination). +Such behaviour is always incorrect, but we provide a flag to support it in case old +builds were accidentally doing it. Never explicitly set this to true for new code. +""" + ), # Is --stamp set on the command line? # TODO(https://github.com/bazelbuild/rules_pkg/issues/340): Remove this. "private_stamp_detect": attr.bool(default = False),