Skip to content

exclude_vcs on default_build_sdist() breaks plugin signature checks #1444

Open

Description

@LalatenduMohanty

What changed

#1417 added a new parameter to the public default_build_sdist() hook:

def default_build_sdist(
    ctx, extra_environ, req, version, sdist_root_dir, build_env, build_dir,
    *,
    exclude_vcs: bool = False,  # new
) -> pathlib.Path:

Fromager sets exclude_vcs=True only for packages with source: and build_sdist: tarball, so .git stays out of the sdist. This change is on new-resolver-config only, not on main. It would ship with #1414.

Builds still work

Fromager never passes exclude_vcs to plugin build_sdist hooks. A plugin hook without the new parameter still works, and so does a plugin that calls default_build_sdist() directly.

Plugin tests break

Plugin projects often test that each hook has the same parameter names as fromager's default, to catch hooks that drift from the API:

default = inspect.signature(fromager.sources.default_build_sdist)
plugin = inspect.signature(my_plugin.build_sdist)
assert set(default.parameters) == set(plugin.parameters)

After upgrading, this test fails for every build_sdist hook, because the default now has exclude_vcs and the plugin doesn't. So upgrading fromager means changing either every hook or the test, even though nothing is wrong.

Why it matters

#1414 should not break existing plugins. This doesn't break builds, but it does break plugin test suites that follow this pattern.

Also: VCS directories removed from non-Git archives

The configured tarball path passes exclude_vcs=True for every package, not only Git checkouts. So pypi-sdist, pypi-download, *-tag-download and hook-sdist archives also lose any .git, .hg, .svn or .bzr directories they contain (for example in test fixtures). The same version then gives a different sdist with and without source:.

Only Git checkouts should drop VCS directories. Reported in #1414 (comment).

Options

  1. Keep the public signature unchanged. Move exclude_vcs into a private helper used by both default_build_sdist() and the configured tarball path, and set it only when the source is a Git checkout. Plugins and their tests need no changes.
  2. Keep the new parameter. Document it in the 0.103.0 notes, tell plugin authors to update their hooks or tests, and still set it only for Git checkouts.

Option 1 is a small change, avoids changing a public hook, and fixes both problems.

Activity

  1. smoparth commented on Nov 4, 2026

    @smoparth
    Contributor

    IMO, adding the optional exclude_vcs parameter is backward compatible for plugin hooks. Downstream tests that require an exact match with default_build_sdist() can be updated to check that hooks accept its required parameters.

  2. LalatenduMohanty commented on Nov 5, 2026

    @LalatenduMohanty
    MemberAuthor

    Agreed, the optional exclude_vcs parameter does not break existing build_sdist hooks at runtime. The failure comes from downstream tests requiring identical parameter names.

    We can update the shared compatibility test alongside the downstream Fromager version bump. It should check that hooks remain callable with the supported arguments, including detecting required arguments that Fromager cannot supply. Existing plugin implementations can stay unchanged.

    The VCS-exclusion concern is separate: configured tarball creation currently removes VCS directories from non-Git archives too. That behavior still needs a Fromager fix.

    I suggest treating the signature-test adjustment as downstream upgrade work and tracking the archive-content issue separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions