Skip to content

Conversation

@e-nomem
Copy link
Contributor

@e-nomem e-nomem commented Nov 26, 2025

This implements the proposal in #2867

@e-nomem e-nomem force-pushed the exclude-only-sources branch from fa60681 to c59bc46 Compare November 26, 2025 19:53
@messense messense requested a review from Copilot November 28, 2025 04:32
Copilot finished reviewing on behalf of messense November 28, 2025 04:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a deprecation warning for files excluded from archives by matching the target path instead of the source path on the filesystem, as proposed in issue #2867. The implementation helps users identify when their exclude patterns are matching archive paths rather than source paths, preparing them for a future breaking change.

Key Changes:

  • Added early exclusion check for source paths before processing (wheel_writer.rs and sdist_writer.rs)
  • Emits deprecation warnings when files are excluded by target path matching
  • Ensures files excluded by source path return silently without warnings

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
src/module_writer/wheel_writer.rs Added source path exclusion check and deprecation warning for target path exclusion in wheel archives
src/module_writer/sdist_writer.rs Added source path exclusion check and deprecation warning for target path exclusion in source distributions

Comment on lines 51 to 57
eprintln!(
"⚠️ Warning: {} was excluded from the archive by the target path in the archive instead of the source path on the filesystem",
target.display(),
);
eprintln!(
" This behavior is deprecated and will be removed in future versions of maturin"
);
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

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

This warning message is duplicated in sdist_writer.rs (lines 55-61). Consider extracting it to a shared function or constant to maintain consistency and reduce duplication. For example:

// In mod.rs or a util module
fn warn_target_path_exclusion(target: &Path) {
    eprintln!(
        "⚠️ Warning: {} was excluded from the archive by the target path in the archive instead of the source path on the filesystem",
        target.display(),
    );
    eprintln!(
        "           This behavior is deprecated and will be removed in future versions of maturin"
    );
}

Then call it from both WheelWriter::add_bytes and SDistWriter::add_bytes.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not a huge fan of the duplicate code but, if you don't mind it for the moment, I do have a plan to extract the file tracking and exclusion logic out of the wheel and sdist writers into a 'virtual' writer layer (the reason for this is unrelated to file exclusions but it happens to be a useful side-effect). That code isn't ready for review yet but if you're curious you can see it here: https://github.com/e-nomem/maturin/tree/virtual-writer

@e-nomem e-nomem force-pushed the exclude-only-sources branch from c59bc46 to ebef346 Compare November 28, 2025 17:48
@e-nomem e-nomem force-pushed the exclude-only-sources branch from ebef346 to b9ed7d8 Compare November 30, 2025 14:53
@messense messense merged commit e77c292 into PyO3:main Dec 1, 2025
45 checks passed
@e-nomem e-nomem deleted the exclude-only-sources branch December 1, 2025 02:37
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.

2 participants