Skip to content

Commit

Permalink
Refactor all the input processing code. (#756)
Browse files Browse the repository at this point in the history
* Refactor all the input processing code.

- Create a MappingContext to hold behaviors and defaults
- Change calls
  - from `process_x(ctx, content_map, ..., default_x, default_y, ...)`
  - to `process_x(mapping_context, ...)

1. This brings the code more in line with the new preferred rule writing style.
1. A major motivation for the change is adding include_runfiles to pkg_tar, where we want
   to sometimes have add_label_list do the runfiles inclusion for us, and
   other times we want to revert to legacy behavior. If we always passed it
   in `ctx`, that would not be possible.

* Syle nits
* make buildifier happier
  • Loading branch information
aiuto authored Nov 6, 2023
1 parent 8e5570c commit 5664e0e
Show file tree
Hide file tree
Showing 7 changed files with 290 additions and 288 deletions.
24 changes: 8 additions & 16 deletions pkg/install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,20 @@ This module provides an interface (`pkg_install`) for creating a `bazel
run`-able installation script.
"""

load("//pkg:providers.bzl", "PackageDirsInfo", "PackageFilegroupInfo", "PackageFilesInfo", "PackageSymlinkInfo")
load("//pkg/private:pkg_files.bzl", "process_src", "write_manifest")
load("@rules_python//python:defs.bzl", "py_binary")
load("//pkg:providers.bzl", "PackageDirsInfo", "PackageFilegroupInfo", "PackageFilesInfo", "PackageSymlinkInfo")
load("//pkg/private:pkg_files.bzl", "create_mapping_context_from_ctx", "process_src", "write_manifest")

def _pkg_install_script_impl(ctx):
script_file = ctx.actions.declare_file(ctx.attr.name + ".py")

fragments = []
files_to_run = []
content_map = {}
mapping_context = create_mapping_context_from_ctx(ctx, label = ctx.label, default_mode = "0644")
for src in ctx.attr.srcs:
process_src(
ctx,
content_map,
files_to_run,
mapping_context,
src = src,
origin = src.label,
default_mode = "0644",
default_user = None,
default_group = None,
default_uid = None,
default_gid = None,
)

manifest_file = ctx.actions.declare_file(ctx.attr.name + "-install-manifest.json")
Expand All @@ -49,14 +41,14 @@ def _pkg_install_script_impl(ctx):
# Note that these paths are different when used as tools run within a build.
# See also
# https://docs.bazel.build/versions/4.1.0/skylark/rules.html#tools-with-runfiles
write_manifest(ctx, manifest_file, content_map, use_short_path = True)
write_manifest(ctx, manifest_file, mapping_context.content_map, use_short_path = True)

# Get the label of the actual py_binary used to run this script.
#
# This is super brittle, but I don't know how to otherwise get this
# information without creating a circular dependency given the current state
# of rules_python.

# The name of the binary is the name of this target, minus
# "_install_script".
label_str = str(ctx.label)[:-len("_install_script")]
Expand All @@ -77,7 +69,7 @@ def _pkg_install_script_impl(ctx):

my_runfiles = ctx.runfiles(
files = [manifest_file],
transitive_files = depset(transitive = files_to_run),
transitive_files = depset(transitive = mapping_context.file_deps),
)

return [
Expand Down Expand Up @@ -147,7 +139,7 @@ def pkg_install(name, srcs, **kwargs):
```
bazel run -- //path/to:install --help
```
WARNING: While this rule does function when being run from within a bazel
rule, such use is not recommended. If you do, **always** use the
`--destdir` argument to specify the desired location for the installation to
Expand Down
26 changes: 16 additions & 10 deletions pkg/path.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Helper functions that don't depend on Skylark, so can be unit tested."""
"""Helper functions that don't depend on Starlark, so can be unit tested."""

# buildifier: disable=function-docstring-args
# buildifier: disable=function-docstring-return
def safe_short_path(file_):
"""Like `File.short_path` but safe for use with files from external repositories.
"""
Expand All @@ -27,9 +29,10 @@ def safe_short_path(file_):
# Beginning with `file_.path`, remove optional `F.root.path`.
working_path = file_.path
if not file_.is_source:
working_path = working_path[len(file_.root.path)+1:]
working_path = working_path[len(file_.root.path) + 1:]
return working_path

# buildifier: disable=function-docstring-args,function-docstring-return
def _short_path_dirname(path):
"""Returns the directory's name of the short path of an artifact."""
sp = safe_short_path(path)
Expand All @@ -39,6 +42,8 @@ def _short_path_dirname(path):
return ""
return sp[:last_pkg]

# buildifier: disable=function-docstring-args
# buildifier: disable=function-docstring-return
def dest_path(f, strip_prefix, data_path_without_prefix = ""):
"""Returns the short path of f, stripped of strip_prefix."""
f_short_path = safe_short_path(f)
Expand All @@ -56,32 +61,33 @@ def dest_path(f, strip_prefix, data_path_without_prefix = ""):

# Avoid stripping prefix if final directory is incomplete
if prefix_last_dir not in f_short_path.split("/"):
strip_prefix = data_path_without_prefix
strip_prefix = data_path_without_prefix

return f_short_path[len(strip_prefix):]
return f_short_path

def compute_data_path(ctx, data_path):
def compute_data_path(label, data_path):
"""Compute the relative data path prefix from the data_path attribute.
Args:
ctx: rule implementation ctx.
data_path: path to a file, relative to the package of the rule ctx.
label: target label
data_path: path to a file, relative to the package of the label.
Returns:
str
"""
build_dir = ctx.label.package
if data_path:
# Strip ./ from the beginning if specified.
# There is no way to handle .// correctly (no function that would make
# that possible and Skylark is not turing complete) so just consider it
# that possible and Starlark is not turing complete) so just consider it
# as an absolute path.
if len(data_path) >= 2 and data_path[0:2] == "./":
data_path = data_path[2:]
if not data_path or data_path == ".": # Relative to current package
return build_dir
return label.package
elif data_path[0] == "/": # Absolute path
return data_path[1:]
else: # Relative to a sub-directory
tmp_short_path_dirname = build_dir
tmp_short_path_dirname = label.package
if tmp_short_path_dirname:
return tmp_short_path_dirname + "/" + data_path
return data_path
Expand Down
Loading

0 comments on commit 5664e0e

Please sign in to comment.