Skip to content
Open
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
5 changes: 4 additions & 1 deletion src/fromager/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import resolvelib
from packaging.requirements import Requirement
from packaging.utils import (
canonicalize_name,
parse_sdist_filename,
)
from packaging.version import Version
Expand Down Expand Up @@ -533,7 +534,8 @@ def default_build_sdist(
#
# For cases where the PEP 517 approach works, use
# pep517_build_sdist().
sdist_filename = ctx.sdists_builds / f"{req.name}-{version}.tar.gz"
dist_name = canonicalize_name(req.name).replace("-", "_")
sdist_filename = ctx.sdists_builds / f"{dist_name}-{version}.tar.gz"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conflicts with the code written in #1328, you might want to stagger the PRs on top of each other.

if sdist_filename.exists():
sdist_filename.unlink()
ensure_pkg_info(
Expand All @@ -550,6 +552,7 @@ def default_build_sdist(
tar=sdist,
basedir=build_dir,
prefix=build_dir.parent,
arcname_root=f"{dist_name}-{version}",
)
return sdist_filename

Expand Down
17 changes: 14 additions & 3 deletions src/fromager/tarballs.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,17 @@ def tar_reproducible(
prefix: pathlib.Path | None = None,
*,
exclude_vcs: bool = False,
arcname_root: str | None = None,
) -> None:
"""Create reproducible tar file

Add content from basedir to already opened tar. If prefix is provided, use
it to set relative paths for the content being added.

If arcname_root is provided, prepend it to all archive entry names.
This allows the top-level directory to be explicitly set regardless of
the basedir or prefix.

If ``exclude_vcs`` is True, then Bazaar, git, Mercurial, and subversion
directories and files are excluded.
"""
Expand All @@ -53,7 +58,13 @@ def tar_reproducible(
content.sort()

for fn in content:
# Ensure that the paths in the tarfile are rooted at the prefix
# directory, if we have one.
arcname = fn if prefix is None else os.path.relpath(fn, prefix)
if arcname_root is not None:
# When arcname_root is specified, compute paths relative to basedir
# to avoid including intermediate directory names from build_dir
rel = os.path.relpath(fn, basedir)
arcname = arcname_root if rel == "." else os.path.join(arcname_root, rel)
else:
# Ensure that the paths in the tarfile are rooted at the prefix
# directory, if we have one.
arcname = fn if prefix is None else os.path.relpath(fn, prefix)
tar.add(fn, filter=_tar_reset, recursive=False, arcname=arcname)
68 changes: 68 additions & 0 deletions tests/test_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -742,3 +742,71 @@ def test_build_sdist_raises_wrong_directory(
sdist_root_dir=tmp_path / "src",
build_env=build_env,
)


@patch("fromager.overrides.find_and_invoke")
@patch("fromager.packagesettings.get_extra_environ", return_value={})
def test_default_build_sdist_normalizes_name_and_root(
mock_environ: Mock,
mock_invoke: Mock,
tmp_context: context.WorkContext,
tmp_path: pathlib.Path,
) -> None:
"""Test default_build_sdist with name normalization in monorepo case.

Exercises the full integration: name normalization in filename,
correct archive root via arcname_root, and monorepo build_dir wiring.
Regression test for issues #1315 and #1317.
"""
import tarfile

# Monorepo structure: Foo.Bar-1.0/src/
sdist_root = tmp_path / "Foo.Bar-1.0"
build_dir = sdist_root / "src"
build_dir.mkdir(parents=True)
(build_dir / "setup.py").write_text("from setuptools import setup; setup()\n")
(build_dir / "module.py").write_text("# module\n")

req = Requirement("Foo.Bar==1.0")
version = Version("1.0")
build_env = Mock()

with patch("fromager.sources.ensure_pkg_info"):
with patch("fromager.sources.tarballs.tar_reproducible"):
# Call default_build_sdist directly to test the full flow
sdist_file = sources.default_build_sdist(
ctx=tmp_context,
extra_environ={},
req=req,
version=version,
sdist_root_dir=sdist_root,
build_env=build_env,
build_dir=build_dir,
)

# Verify filename is normalized (foo_bar-1.0.tar.gz, not Foo.Bar-1.0.tar.gz)
assert sdist_file.name == "foo_bar-1.0.tar.gz"
assert sdist_file.parent == tmp_context.sdists_builds

# Now test with actual tar to verify the archive root is correct
sdist_root2 = tmp_path / "Foo.Bar-1.0-v2"
build_dir2 = sdist_root2 / "src"
build_dir2.mkdir(parents=True)
(build_dir2 / "setup.py").write_text("from setuptools import setup; setup()\n")
(build_dir2 / "module.py").write_text("# module\n")

sdist_file2 = tmp_context.sdists_builds / "foo_bar-1.0.tar.gz"
with tarfile.open(sdist_file2, "x:gz") as tar:
from fromager import tarballs

tarballs.tar_reproducible(
tar=tar,
basedir=build_dir2,
prefix=sdist_root2,
arcname_root="foo_bar-1.0",
)

# Verify the archive root is exactly {"foo_bar-1.0"}
with tarfile.open(sdist_file2, "r:gz") as tar:
top_levels = {n.split("/")[0] for n in tar.getnames()}
assert top_levels == {"foo_bar-1.0"}
31 changes: 31 additions & 0 deletions tests/test_tarballs.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,34 @@ def test_vcs_exclude(tmp_path: pathlib.Path) -> None:
with tarfile.open(t1, "r") as tf:
names = tf.getnames()
assert names == [str(p).lstrip(os.sep) for p in [root, root / "a"]]


def test_arcname_root(tmp_path: pathlib.Path) -> None:
"""Test that arcname_root sets the top-level directory name.

This reproduces issue #1315: when basedir is a subdirectory (monorepo case),
arcname_root should ensure the top-level archive entry is {name}-{version},
not the basedir's name.
"""
# Simulate a monorepo structure: mypkg-1.0/python/
sdist_root = tmp_path / "mypkg-1.0"
build_dir = sdist_root / "python"
build_dir.mkdir(parents=True)
(build_dir / "setup.py").write_text("from setuptools import setup; setup()\n")

t1 = tmp_path / "out.tar"
with tarfile.open(t1, "w") as tf:
tarballs.tar_reproducible(
tar=tf,
basedir=build_dir,
prefix=sdist_root,
arcname_root="mypkg-1.0",
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
with tarfile.open(t1, "r") as tf:
names = tf.getnames()

# All entries should be rooted at mypkg-1.0, not python/
# This ensures the sdist unpacks to mypkg-1.0/, not python/
assert "mypkg-1.0" in names[0]
assert "python" not in names[0] # build_dir's name should not appear
assert "mypkg-1.0/setup.py" in names
Loading