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.

Sign up for GitHub

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

nfpm(bugfix): handle package targets w/ output_path #21806

Merged
merged 3 commits into from
Jan 2, 2025
Merged

nfpm(bugfix): handle package targets w/ output_path #21806

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
59d013c
nfpm(bugfix): handle package targets w/ output_path
cognifloyd Jan 2, 2025
755cde1
nfpm: Improve sandbox error messages
cognifloyd Jan 2, 2025
381ceb8
nfpm: fix util_rule tests
cognifloyd Jan 2, 2025
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
7 changes: 7 additions & 0 deletions src/python/pants/backend/nfpm/field_sets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,19 @@ def nfpm_config(
) -> NfpmContent:
source: str | None = self.source.file_path
src: str | None = self.src.file_path
src_value: str | None = self.src.value
dst: str = self.dst.value
if source is not None and not src:
# If defined, 'source' provides the default value for 'src'.
src = source
src_value = self.source.value
if src is None: # src is NOT required; prepare to raise an error.
raise self.InvalidTarget()
if src not in content_sandbox_files and src_value in content_sandbox_files:
# A field's file_path assumes the field's value is relative to the BUILD file.
# But, for packages the field's value can be relative to the build_root instead,
# because a package's output_path can be any arbitrary build_root relative path.
src = src_value
sandbox_file: FileEntry | None = content_sandbox_files.get(src)
if sandbox_file is None:
raise self.SrcMissingFomSandbox()
Expand Down
10 changes: 7 additions & 3 deletions src/python/pants/backend/nfpm/util_rules/generate_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ async def generate_nfpm_yaml(
defaults to the file referenced in the '{NfpmContentFileSourceField.alias}' field.
Please fix the {'targets at these addresses' if plural else 'target at this address'}:
"""
+ ",\n".join(str(address) for address in invalid_content_file_addresses)
)
+ "\n".join(str(address) for address in invalid_content_file_addresses)
)
if src_missing_from_sandbox_addresses:
plural = len(src_missing_from_sandbox_addresses) > 1
Expand All @@ -129,10 +129,12 @@ async def generate_nfpm_yaml(
from the nfpm sandbox. This sandbox contains packages, generated code, and sources
from the '{NfpmDependencies.alias}' field. It also contains any file from the
'{NfpmContentFileSourceField.alias}' field. Please fix the '{NfpmContentFile.alias}'
{'targets at these addresses' if plural else 'target at this address'}.:
{'targets at these addresses' if plural else 'target at this address'}:
"""
+ ",\n".join(str(address) for address in src_missing_from_sandbox_addresses)
)
+ "\n".join(str(address) for address in src_missing_from_sandbox_addresses)
+ "\n\nThe sandbox contains these files:\n"
+ "\n".join(str(path) for path in content_sandbox_files)
)

contents.sort(key=lambda d: d["dst"])
Expand All @@ -155,6 +157,8 @@ async def generate_nfpm_yaml(
{repr(script_src_missing_from_sandbox)}
"""
)
+ "\n\nThe sandbox contains these files:\n"
+ "\n".join(str(path) for path in content_sandbox_files)
)

nfpm_yaml = "# Generated by Pantsbuild\n"
Expand Down
69 changes: 69 additions & 0 deletions src/python/pants/backend/nfpm/util_rules/generate_config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,57 @@ def get_digest(rule_runner: RuleRunner, source_files: dict[str, str]) -> Digest:
None,
id="rpm-with-deps-and-ghost",
),
# with dummy_package dependency
pytest.param(
"apk",
NfpmApkPackageFieldSet,
["contents:dummy_package"],
{},
["output_path/package"],
{},
None,
id="apk-with-pkg-dep",
),
pytest.param(
"archlinux",
NfpmArchlinuxPackageFieldSet,
["contents:dummy_package"],
{},
["output_path/package"],
{},
None,
id="archlinux-with-pkg-dep",
),
pytest.param(
"deb",
NfpmDebPackageFieldSet,
["contents:dummy_package"],
{},
["output_path/package"],
{},
None,
id="deb-with-pkg-dep",
),
pytest.param(
"rpm",
NfpmRpmPackageFieldSet,
["contents:dummy_package"],
{},
["output_path/package"],
{},
None,
id="rpm-with-pkg-dep",
),
pytest.param(
"rpm",
NfpmRpmPackageFieldSet,
["contents:dummy_package"],
{},
["output_path/package"],
{"ghost_contents": ["/var/log/pkg.log"]},
None,
id="rpm-with-pkg-dep-and-ghost",
),
# with malformed dependency
pytest.param(
"apk",
Expand Down Expand Up @@ -477,6 +528,24 @@ def test_generate_nfpm_yaml(
dst="/etc/{_PKG_NAME}",
file_mode=0o700,
)
target(
name="dummy",
# For this test, the dummy target should be
# treated as if it were a package that defines:
# output_path="output_path/package"
# Instead of wiring an actual package target,
# the test imitates packaging by injecting the file
# into the digest.
)
nfpm_content_file(
name="dummy_package",
dependencies=[":dummy"],
# NOTE: src pointing to package dep is relative to
# the build_root instead of this BUILD file.
src="ouptut_path/package",
dst="/usr/bin/dummy_package",
file_mode=0o550,
)
nfpm_content_file(
name="malformed", # needs either source or src.
dst="/usr/bin/foo",
Expand Down
44 changes: 40 additions & 4 deletions src/python/pants/backend/nfpm/util_rules/sandbox_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,48 +326,72 @@ def rule_runner() -> RuleRunner:
pytest.param(
"apk",
NfpmApkPackageFieldSet,
["codegen:generated", "contents:files", "package:package"],
[
"codegen:generated",
"contents:files",
"package:package",
"package:output_path_package",
],
{},
{
"codegen/foobar.codegen.generated",
"contents/sandbox-file.txt",
"package/archive.tar",
"relative_to_build_root.tar",
},
id="apk-codegen-and-package",
),
pytest.param(
"archlinux",
NfpmArchlinuxPackageFieldSet,
["codegen:generated", "contents:files", "package:package"],
[
"codegen:generated",
"contents:files",
"package:package",
"package:output_path_package",
],
{},
{
"codegen/foobar.codegen.generated",
"contents/sandbox-file.txt",
"package/archive.tar",
"relative_to_build_root.tar",
},
id="archlinux-codegen-and-package",
),
pytest.param(
"deb",
NfpmDebPackageFieldSet,
["codegen:generated", "contents:files", "package:package"],
[
"codegen:generated",
"contents:files",
"package:package",
"package:output_path_package",
],
{},
{
"codegen/foobar.codegen.generated",
"contents/sandbox-file.txt",
"package/archive.tar",
"relative_to_build_root.tar",
},
id="deb-codegen-and-package",
),
pytest.param(
"rpm",
NfpmRpmPackageFieldSet,
["codegen:generated", "contents:files", "package:package"],
[
"codegen:generated",
"contents:files",
"package:package",
"package:output_path_package",
],
{},
{
"codegen/foobar.codegen.generated",
"contents/sandbox-file.txt",
"package/archive.tar",
"relative_to_build_root.tar",
},
id="rpm-codegen-and-package",
),
Expand Down Expand Up @@ -459,6 +483,18 @@ def test_populate_nfpm_content_sandbox(
dst="/opt/foo/archive.tar",
dependencies=[":archive"],
)
archive(
name="output_path_archive",
format="tar",
output_path="relative_to_build_root.tar",
files=[":file"],
)
nfpm_content_file(
name="output_path_package",
src="relative_to_build_root.tar",
dst="/opt/foo/relative_to_build_root.tar",
dependencies=[":archive"],
)
"""
),
"package/archive-contents.txt": "",
Expand Down
Loading