bazel
/
rules_pkg
mirror of https://github.com/bazelbuild/rules_pkg
2
0
Fork 0
Code Issues Releases Activity

pkg_install: Add destdir attr & read rel paths. (#886) 

Implementation notes:

Relative paths are interpreted against BUILD_WORKSPACE_DIRECTORY, not
BUILD_WORKING_DIRECTORY. This is for the following reasons:

The TODO tag explicitly convey the intention of using
BUILD_WORKSPACE_DIRECTORY for relative paths.

If destdir is specified in the attribute of the pkg_install() target,
interpreting it against BUILD_WORKSPACE_DIRECTORY is much stabler.
That is, no matter where your current cwd is, the destdir attribute
always refers to a path relative to the workspace root. For example:
```
pkg_install(name = "my_pkg_install", destdir = "out/dest")
```
```
cd <workspace_root>/<some_subdir>
bazel run //:my_pkg_install
```
This clearly conveys that the default destdir is
<workspace_root>/out/dest regardless of where the user runs the command.

The cost is that the --destdir command line argument becomes trickier
to understand. For example, if one is not familiar with pkg_install,
and below the workspace root they run:

```
cd <workspace_root>/out
bazel run //:my_pkg_install -- --destdir dest
```

They may expect the destdir to be set to <workspace_root>/out/dest; but
it is in fact `<workspace_root>/dest`.

We could also interpret the target attribute & the command line argument
separately (e.g. pkg_install(destdir_against_workspace)), but honestly
I think that's even more confusing when they interpret relative paths
differently. Please let me know if this is preferred by the maintainers.

Co-authored-by: HONG Yifan <elsk@google.com>
This commit is contained in:
elsk 2024-08-28 12:14:02 -07:00 committed by GitHub
parent 412c8fe955
commit 4afd0b3ccd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 61 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
pkg/install.bzl
View File

@ -62,6 +62,7 @@ def _pkg_install_script_impl(ctx):
"{WORKSPACE_NAME}": ctx.workspace_name,
# Used to annotate --help with "bazel run //path/to/your:installer"
"{TARGET_LABEL}": label_str,
"{DEFAULT_DESTDIR}": ctx.attr.destdir,
},
is_executable = True,
)
@ -98,6 +99,7 @@ _pkg_install_script = rule(
],
doc = "Source mapping/grouping targets",
),
"destdir": attr.string(),
# This is private for now -- one could perhaps imagine making this
# public, but that would require more documentation of the underlying
# scripts and expected interfaces.
@ -109,7 +111,7 @@ _pkg_install_script = rule(
executable = True,
)
def pkg_install(name, srcs, **kwargs):
def pkg_install(name, srcs, destdir = None, **kwargs):
"""Create an installer script from pkg_filegroups and friends.
This macro allows users to create `bazel run`nable installation scripts
@ -123,6 +125,7 @@ def pkg_install(name, srcs, **kwargs):
srcs = [
# mapping/grouping targets here
],
destdir = "out/install",
)
```
@ -153,15 +156,28 @@ def pkg_install(name, srcs, **kwargs):
https://github.com/bazelbuild/rules_pkg/issues/388.
Args:
name: rule name
srcs: pkg_filegroup framework mapping or grouping targets
**kwargs: common rule attributes
name: rule name
srcs: pkg_filegroup framework mapping or grouping targets
destdir: The default destination directory.
If it is specified, this is the default destination to install
the files. It is overridable by explicitly specifying `--destdir`
in the command line or specifying the `DESTDIR` environment
variable.
If it is not specified, `--destdir` must be set on the command line,
or the `DESTDIR` environment variable must be set.
If this is an absolute path, it is used as-is. If this is a relative
path, it is interpreted against `BUILD_WORKSPACE_DIRECTORY`.
**kwargs: common rule attributes
"""
_pkg_install_script(
name = name + "_install_script",
srcs = srcs,
destdir = destdir,
**kwargs
)

47
pkg/private/install.py.tpl
View File

@ -20,6 +20,7 @@
import argparse
import logging
import os
import pathlib
import shutil
import sys
@ -182,6 +183,36 @@ class NativeInstaller(object):
raise ValueError("Unrecognized entry type '{}'".format(entry.type))
def _default_destdir():
# If --destdir is not specified, use these values, in this order
# Use env var if specified and non-empty
env = os.getenv("DESTDIR")
if env:
return env
# Checks if DEFAULT_DESTDIR is an empty string
target_attr = "{DEFAULT_DESTDIR}"
if target_attr:
return target_attr
return None
def _resolve_destdir(path_s):
if not path_s:
raise argparse.ArgumentTypeError("destdir is not set!")
path = pathlib.Path(path_s)
if path.is_absolute():
return path_s
build_workspace_directory = os.getenv("BUILD_WORKSPACE_DIRECTORY")
if not build_workspace_directory:
raise argparse.ArgumentTypeError(f"BUILD_WORKSPACE_DIRECTORY is not set"
f" and destdir {path} is relative. "
f"Unable to infer an absolute path.")
ret = str(pathlib.Path(build_workspace_directory) / path)
return ret
def main(args):
parser = argparse.ArgumentParser(
prog="bazel run -- {TARGET_LABEL}",
@ -194,12 +225,16 @@ def main(args):
help="Be silent, except for errors")
# TODO(nacl): consider supporting DESTDIR=/whatever syntax, like "make
# install".
#
# TODO(nacl): consider removing absolute path restriction, perhaps using
# BUILD_WORKING_DIRECTORY.
parser.add_argument('--destdir', action='store', default=os.getenv("DESTDIR"),
help="Installation root directory (defaults to DESTDIR "
"environment variable). Must be an absolute path.")
default_destdir = _default_destdir()
default_destdir_text = f" or {default_destdir}" if default_destdir else ""
parser.add_argument('--destdir', action='store', default=default_destdir,
required=default_destdir is None,
type=_resolve_destdir,
help=f"Installation root directory (defaults to DESTDIR"
f" environment variable{default_destdir_text}). "
f"Relative paths are interpreted against "
f"BUILD_WORKSPACE_DIRECTORY "
f"({os.getenv('BUILD_WORKSPACE_DIRECTORY')})")
args = parser.parse_args()