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: 3 additions & 2 deletions src/fromager/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,8 @@ def default_build_sdist(
#
# For cases where the PEP 517 approach works, use
# pep517_build_sdist().
normalized_name = canonicalize_name(req.name).replace("-", "_")
sdist_filename = ctx.sdists_builds / f"{normalized_name}-{version}.tar.gz"
dist_name = canonicalize_name(req.name).replace("-", "_")
sdist_filename = ctx.sdists_builds / f"{dist_name}-{version}.tar.gz"
Comment thread
jlarkin09 marked this conversation as resolved.
if sdist_filename.exists():
sdist_filename.unlink()
ensure_pkg_info(
Expand All @@ -552,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)
35 changes: 35 additions & 0 deletions tests/test_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,3 +786,38 @@ def test_default_build_sdist_normalizes_filename(
expected_filename = f"{expected_filename_part}-1.0.0.tar.gz"
assert sdist_file.name == expected_filename
assert sdist_file.parent == tmp_context.sdists_builds


def test_default_build_sdist_normalizes_name_and_root(
tmp_context: context.WorkContext,
tmp_path: pathlib.Path,
) -> None:
"""Verify the sdist filename and archive root for a monorepo package."""
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()

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,
)

assert sdist_file.name == "foo_bar-1.0.tar.gz"
assert sdist_file.parent == tmp_context.sdists_builds

with tarfile.open(sdist_file, "r:gz") as tar:
names = tar.getnames()
top_levels = {name.split("/")[0] for name in names}
assert top_levels == {"foo_bar-1.0"}
assert "foo_bar-1.0/setup.py" in names
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