Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
* Add comment about check_inits.py
* Fix the precommit hook
* Fix setup_py.py to not override those 6 values, and explain why we do that

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano committed Sep 3, 2020
1 parent 9a9492b commit 2793b4d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 8 deletions.
12 changes: 9 additions & 3 deletions build-support/bin/check_inits.py
Expand Up @@ -21,11 +21,17 @@ def main() -> None:
files = itertools.chain.from_iterable(
[Path().glob(f"{d}/**/__init__.py") for d in DIRS_TO_CHECK]
)
bad_inits = [f for f in files if bool(f.read_text())]
if bad_inits:
root_init = Path("src/python/pants/__init__.py")
if '__import__("pkg_resources").declare_namespace(__name__)' not in root_init.read_text():
die(
f"{root_init} must have the line "
'`__import__("pkg_resources").declare_namespace(__name__)` in it.'
)
non_empty_inits = [f for f in files if bool(f.read_text()) and f != root_init]
if non_empty_inits:
die(
"All `__init__.py` file should be empty, but the following had content: "
f"{', '.join(str(f) for f in bad_inits)}"
f"{', '.join(str(f) for f in non_empty_inits)}"
)


Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/__init__.py
@@ -1 +1,3 @@
# NB: We need to declare a namespace package because we have the dists `pantsbuild.pants` and
# `pantsbuild.pants.testutil`, which both have the module `pants`.
__import__("pkg_resources").declare_namespace(__name__)
16 changes: 11 additions & 5 deletions src/python/pants/backend/python/goals/setup_py.py
Expand Up @@ -531,13 +531,19 @@ async def generate_chroot(request: SetupPyChrootRequest) -> SetupPyChroot:
target = exported_target.target
resolved_setup_kwargs = await Get(SetupKwargs, ExportedTarget, exported_target)
setup_kwargs = resolved_setup_kwargs.kwargs.copy()
# NB: We are careful to not overwrite these values, but we also don't expect them to have been
# set. The user must have have gone out of their way to use a `SetupKwargs` plugin, and to have
# specified `SetupKwargs(_allow_banned_keys=True)`.
setup_kwargs.update(
{
"package_dir": {"": CHROOT_SOURCE_ROOT},
"packages": sources.packages,
"namespace_packages": sources.namespace_packages,
"package_data": dict(sources.package_data),
"install_requires": tuple(requirements),
"package_dir": {"": CHROOT_SOURCE_ROOT, **setup_kwargs.get("package_dir", {})},
"packages": (*sources.packages, *(setup_kwargs.get("packages", []))),
"namespace_packages": (
*sources.namespace_packages,
*setup_kwargs.get("namespace_packages", []),
),
"package_data": {**dict(sources.package_data), **setup_kwargs.get("package_data", {})},
"install_requires": (*requirements, *setup_kwargs.get("install_requires", [])),
}
)
key_to_binary_spec = exported_target.provides.binaries
Expand Down

0 comments on commit 2793b4d

Please sign in to comment.