Skip to content

Only use all_features=true in sdist when local path dependencies exist #2674

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.

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

Merged
merged 7 commits into from
Jul 11, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 11, 2025

Problem

The maturin sdist command was always setting all_features: true to ensure all optional path dependencies are packaged into the source distribution. However, this caused failures for projects with mutually exclusive features, as reported in #2673.

For example, a project using inkwell with multiple incompatible features would fail during sdist generation because enabling all features simultaneously created conflicting dependencies.

Solution

This PR modifies the sdist command to only enable all_features: true when the project actually has local path dependencies. The logic is:

  • With path dependencies: Enable all_features: true (preserves current behavior)
  • Without path dependencies: Use default feature selection (new behavior)

Changes

  1. Added has_path_dependencies() function in src/source_distribution.rs:

    • Efficiently scans the dependency graph to detect local path dependencies
    • Returns true if any path dependencies are found, false otherwise
  2. Modified SDist command in src/main.rs:

    • Checks for path dependencies before building
    • Conditionally sets all_features based on the result
    • Falls back to false if metadata cannot be retrieved
  3. Added unit test to verify the path dependency detection works correctly for both scenarios

Testing

  • ✅ Projects with path dependencies continue to work (existing behavior preserved)
  • ✅ Projects without path dependencies now work with incompatible features
  • ✅ All existing sdist tests pass
  • ✅ New unit test verifies detection logic

Example

Before this fix, a project with incompatible features would fail:

$ maturin sdist
# Failed because all_features=true activated incompatible features

After this fix, the same project succeeds:

$ maturin sdist  
# Succeeds because no path dependencies detected, all_features=false

Projects with actual path dependencies continue to work as before, ensuring optional path dependencies are still properly packaged.

Fixes #2673


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…encies exist

Co-authored-by: messense <1556054+messense@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Fix @PyO3/maturin/issues/2673 by only use all_features: true when local path dependencies detected in the binding crate's depdency graph Fix #2673: Only use all_features=true in sdist when local path dependencies exist Jul 11, 2025
@Copilot Copilot AI requested a review from messense July 11, 2025 11:16
Copilot finished work on behalf of messense July 11, 2025 11:16
…ependencies

Co-authored-by: messense <1556054+messense@users.noreply.github.com>
@Copilot Copilot AI requested a review from messense July 11, 2025 11:27
Copilot finished work on behalf of messense July 11, 2025 11:27
@messense
Copy link
Member

@copilot fix trailing whitespace

Co-authored-by: messense <1556054+messense@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 11, 2025

@copilot fix trailing whitespace

Fixed trailing whitespace and applied code formatting. Commit ac9c6da.

Copilot finished work on behalf of messense July 11, 2025 11:41
@messense
Copy link
Member

@copilot run cargo clippy and fix any warnings

…ublic

Co-authored-by: messense <1556054+messense@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 11, 2025

@copilot run cargo clippy and fix any warnings

Fixed all clippy warnings including uninlined format args and visibility issues. All warnings have been resolved. Commit ee31140.

Copilot finished work on behalf of messense July 11, 2025 11:52
@messense messense changed the title Fix #2673: Only use all_features=true in sdist when local path dependencies exist Only use all_features=true in sdist when local path dependencies exist Jul 11, 2025
@messense
Copy link
Member

@copilot please fix conflicts with main branch

Copy link
Contributor Author

Copilot AI commented Jul 11, 2025

@copilot please fix conflicts with main branch

Fixed merge conflicts with main branch. Successfully resolved formatting conflicts in src/new_project.rs and merged with latest changes. All tests pass and no clippy warnings. Commit 9a077e1.

Copilot finished work on behalf of messense July 11, 2025 12:19
@messense messense marked this pull request as ready for review July 11, 2025 12:29
@messense messense enabled auto-merge (squash) July 11, 2025 12:30
@messense messense merged commit 4fb4991 into main Jul 11, 2025
42 checks passed
@messense messense deleted the copilot/fix-daaf2133-8e3c-4ffb-a3f2-1f3f8ef9bcb9 branch July 11, 2025 12:50
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Aug 6, 2025
https://build.opensuse.org/request/show/1297806
by user mia + dimstar_suse
- Update to 1.9.3
  * Add support for RISC-V architecture in manylinux
    gh#PyO3/maturin#2694
  * pyproject.toml: bump setuptools for PEP 639
    gh#PyO3/maturin#2698
  * Fix PEP 639 support for source distributions
    gh#PyO3/maturin#2704
  * Fix relative README rewrite in source distributions
    gh#PyO3/maturin#2705

- Update to 1.9.2
  * Respect PEP 621 dynamic fields when merging Cargo.toml metadata
    gh#PyO3/maturin#2672
  * Only use all_features=true in sdist when local path
    dependencies exist
    gh#PyO3/maturin#2674
  * auditwheel: add manylinux_2_26 policy
    gh#PyO3/maturin#2677
  * Use user-specified library search paths in RUSTFLAGS in
    auditwheel
    gh#PyO3/maturin#2676
  * pyproject.toml: add license-files
    gh#PyO3/maturin#2690
  * Update manylinux/musllinux
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sdist command failing for incompatible features
2 participants