Remove absolute paths from build command lines
The GN convention is to specify paths in command lines relative to the
build directory. Unfortunately and contrary to pigweed's expectations
this is not what rebase_path(path) does; that outputs an absolute path.
Absolute paths are not desirable in most circumstances as they contain
sources of nondeterminism such as the developer's home directory. Using
them can for example reduce hit rate in build caches.
Replace rebase_path(path) with rebase_path(path, root_build_dir) which
is the correct idiom and matches GN's builtin behavior (e.g. for
sources, include_dirs, etc).
This also removes the --directory argument to python_action(). Changing
the directory during the build while using relative paths is likely to
result in confusion and should be discouraged.
There's a couple more things to do on top of this for identical
binaries between build directories / machines / developers:
- pass options to avoid embedding the working directory
- pass options to use relative paths for the vendored clang & libc++
See [1]-[2] for how to do that.
[1] https://source.chromium.org/chromium/chromium/src/+/main:build/config/compiler/BUILD.gn;l=1170-1239;drc=ab531c265c533cba1c2f6d8240cc0bf7679f605a
[2] https://cs.opensource.google/fuchsia/fuchsia/+/main:build/config/BUILD.gn;l=145-216;drc=f6d705f0937c778d5d5f807a4580113612b02f5a
Change-Id: I17708102c03d6488d68c8571b6e9343191fd47de
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/47461
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Michael Spang <spang@google.com>
diff --git a/pw_build/python.gni b/pw_build/python.gni
index b7bf71a..217a7f8 100644
--- a/pw_build/python.gni
+++ b/pw_build/python.gni
@@ -64,17 +64,17 @@
]
if (defined(invoker.mypy_ini)) {
- args += [ "--config-file=" + rebase_path(invoker.mypy_ini) ]
+ args +=
+ [ "--config-file=" + rebase_path(invoker.mypy_ini, root_build_dir) ]
inputs = [ invoker.mypy_ini ]
}
- args += rebase_path(invoker.sources)
+ args += rebase_path(invoker.sources, root_build_dir)
# Use this environment variable to force mypy to colorize output.
# See https://github.com/python/mypy/issues/7771
environment = [ "MYPY_FORCE_COLOR=1" ]
- directory = invoker.directory
stamp = true
deps = invoker.deps
@@ -92,13 +92,13 @@
pw_python_action_foreach(target_name) {
module = "pylint"
args = [
- rebase_path(".") + "/{{source_target_relative}}",
+ rebase_path(".", root_build_dir) + "/{{source_target_relative}}",
"--jobs=1",
"--output-format=colorized",
]
if (defined(invoker.pylintrc)) {
- args += [ "--rcfile=" + rebase_path(invoker.pylintrc) ]
+ args += [ "--rcfile=" + rebase_path(invoker.pylintrc, root_build_dir) ]
inputs = [ invoker.pylintrc ]
}
@@ -108,7 +108,6 @@
}
sources = invoker.sources
- directory = invoker.directory
stamp = "$target_gen_dir/{{source_target_relative}}.pylint.passed"
@@ -151,10 +150,10 @@
# generate_setup is required in place of setup if proto_library is used.
# static_analysis: List of static analysis tools to run; "*" (default) runs
# all tools. The supported tools are "mypy" and "pylint".
-# pylintrc: Optional path to a pylintrc configuration file to use. If not
-# provided, Pylint's default rcfile search is used. Pylint is executed
-# from the package's setup directory, so pylintrc files in that directory
-# will take precedence over others.
+# pylintrc: Path to a pylintrc configuration file to use. If not
+# provided, Pylint's default rcfile search is used. As this may
+# use the the local user's configuration file, it is highly
+# recommended to pass this option to specify the rcfile explicitly.
# mypy_ini: Optional path to a mypy configuration file to use. If not
# provided, mypy's default configuration file search is used. mypy is
# executed from the package's setup directory, so mypy.ini files in that
@@ -346,10 +345,10 @@
"--label",
get_label_info(":$target_name", "label_no_toolchain"),
"--generated-root",
- rebase_path(_setup_dir),
+ rebase_path(_setup_dir, root_build_dir),
"--setup-json",
- rebase_path("$_setup_dir/setup.json"),
- ] + rebase_path(_sources)
+ rebase_path("$_setup_dir/setup.json", root_build_dir),
+ ] + rebase_path(_sources, root_build_dir)
# Pass in the .json information files for the imported proto libraries.
foreach(proto, _import_protos) {
@@ -359,7 +358,7 @@
get_label_info(_label, "name") + ".json"
args += [
"--proto-library",
- rebase_path(_file),
+ rebase_path(_file, root_build_dir),
]
}
@@ -406,7 +405,7 @@
args += [ "--editable" ]
}
- args += [ rebase_path(_setup_dir) ]
+ args += [ rebase_path(_setup_dir, root_build_dir) ]
stamp = true
@@ -431,12 +430,13 @@
module = "build"
- args = [
- rebase_path(_setup_dir),
- "--wheel",
- "--no-isolation",
- "--outdir",
- ] + rebase_path(metadata.pw_python_package_wheels)
+ args =
+ [
+ rebase_path(_setup_dir, root_build_dir),
+ "--wheel",
+ "--no-isolation",
+ "--outdir",
+ ] + rebase_path(metadata.pw_python_package_wheels, root_build_dir)
deps = [ ":${invoker.target_name}" ]
foreach(dep, _python_deps) {
@@ -509,12 +509,6 @@
deps = _test_install_deps
python_deps = _python_deps
- if (defined(_setup_dir)) {
- directory = rebase_path(_setup_dir)
- } else {
- directory = rebase_path(".")
- }
-
_optional_variables = [
"mypy_ini",
"pylintrc",
@@ -729,7 +723,7 @@
foreach(_requirements_file, inputs) {
args += [
"--requirement",
- rebase_path(_requirements_file),
+ rebase_path(_requirements_file, root_build_dir),
]
}