Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor all the input processing code. #756

Merged
merged 10 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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