Skip to content
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

fix!: exclude installed files if listed in exclude #652

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

henryiii
Copy link
Collaborator

As mentioned in our recent Community Meeting.

Copy link

codecov bot commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 86.30%. Comparing base (e6ae46d) to head (22d839e).

Files Patch % Lines
src/scikit_build_core/build/_wheelfile.py 80.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #652      +/-   ##
==========================================
- Coverage   86.32%   86.30%   -0.02%     
==========================================
  Files          64       64              
  Lines        3306     3316      +10     
==========================================
+ Hits         2854     2862       +8     
- Misses        452      454       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@henryiii henryiii marked this pull request as draft February 27, 2024 02:36
@henryiii henryiii force-pushed the henryiii/fix/exinst branch from 25ff81d to 52b0276 Compare February 28, 2024 21:21
@henryiii henryiii marked this pull request as ready for review February 28, 2024 22:16
@henryiii henryiii force-pushed the henryiii/fix/exinst branch 2 times, most recently from 817ed39 to f5fc9d8 Compare February 29, 2024 02:54
Comment on lines -113 to +117
"wheel.exclude": "src/simplest/sdist_only.txt",
"wheel.exclude": [
"simplest/sdist_only.txt",
"simplest/generated_no_wheel.txt",
],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a behavior change, though I'd be willing to consider the old behavior a bug. Previously, wheel exclude matched on the SDist/source path. Now it matches on the final wheel path.

Not sure to just call this a bug fix, or to add back-compat for it via minimum-version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No packages (by either PyPI search or GitHub search) were found that would be affected by this change, so the plan is to make it as a bug fix, document this in the changelog, and be open to adding a compat setting for it if someone does end up needing it.

@henryiii henryiii changed the title fix: exclude installed files if listed in exclude fix!: exclude installed files if listed in exclude Feb 29, 2024
henryiii added 2 commits March 4, 2024 11:16
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii force-pushed the henryiii/fix/exinst branch from e3f7b4e to 2c1c008 Compare March 4, 2024 16:16
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii merged commit 50a9386 into main Mar 4, 2024
47 of 53 checks passed
@henryiii henryiii deleted the henryiii/fix/exinst branch March 4, 2024 17:06
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.

1 participant