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.

Sign up for GitHub

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

Jump to bottom

Add support for generating non-abi3 python import libraries for Windows targets #2364

Merged
merged 5 commits into from
May 12, 2022
Merged

Add support for generating non-abi3 python import libraries for Windows targets #2364

merged 5 commits into from
May 12, 2022

Conversation

messense
Copy link
Member

Continuing from PyO3/python3-dll-a#12

@messense messense added the CI-no-fail-fast If one job fails, allow the rest to keep testing label May 10, 2022
ravenexp
ravenexp approved these changes May 10, 2022
View reviewed changes
pyo3-build-config/src/import_lib.rs Outdated Show resolved Hide resolved
guide/src/building_and_distribution.md Outdated Show resolved Hide resolved
Architecture.md Outdated Show resolved Hide resolved
@messense
Copy link
Member Author 

error[E0432]: unresolved import `python3_dll_a::ImportLibraryGenerator`
 --> /home/runner/work/pyo3/pyo3/pyo3-build-config/src/import_lib.rs:6:5
  |
6 | use python3_dll_a::ImportLibraryGenerator;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `ImportLibraryGenerator` in the root

This is odd, I've already patched it to git dependency.

@ravenexp Could you release a new version of python3-dll-a so we can just test with crates.io version? Thanks!

@ravenexp
Copy link
Contributor

ravenexp commented May 10, 2022 •
edited
Loading

@ravenexp Could you release a new version of python3-dll-a so we can just test with crates.io version? Thanks!

Created PyO3/python3-dll-a#16

Edit: v0.2.2 is on crates.io now.

ravenexp
ravenexp reviewed May 10, 2022
View reviewed changes
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@messense
Copy link
Member Author

messense commented May 10, 2022 •
edited
Loading

Needs PyO3/python3-dll-a#17

@ravenexp
Copy link
Contributor

Needs PyO3/python3-dll-a#17

Mirroring my concerns here because they also affect PyO3 PyO3/python3-dll-a#17 (comment)

@messense messense mentioned this pull request May 11, 2022
Merged
@messense messense force-pushed the import-lib branch 2 times, most recently from 5fe78f2 to ea37acb Compare May 11, 2022 07:34
ravenexp
ravenexp approved these changes May 11, 2022
View reviewed changes
Copy link
Contributor

@ravenexp ravenexp left a comment

Choose a reason for hiding this comment

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

LGTM

pyo3-build-config/src/impl_.rs
Comment on lines -1400 to +1398
mingw,
false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is correct, because the other call site gets the mingw flag value from the Python interpreter itself, not from the current Rust compile target, like I did above:

pyo3/pyo3-build-config/src/impl_.rs

Lines 258 to 265 in ea37acb

let lib_name = if cfg!(windows) {
default_lib_name_windows(
version,
implementation,
abi3,
map["mingw"].as_str() == "True",
)
} else {

@messense messense removed the CI-no-fail-fast If one job fails, allow the rest to keep testing label May 11, 2022
messense added a commit to messense/pyo3 that referenced this pull request May 11, 2022
@messense
Update changelog for PyO3#2364
53cfb60
davidhewitt
davidhewitt approved these changes May 12, 2022
View reviewed changes
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

You folks have been doing some really cool stuff here, thanks so much for this!

@messense messense merged commit 87bd10c into PyO3:main May 12, 2022
@messense messense deleted the import-lib branch May 12, 2022 07:30
This was referenced May 14, 2022
Merged
Merged
davidhewitt pushed a commit that referenced this pull request May 15, 2022
@messense @davidhewitt
Update changelog for #2364
d3b9a48
davidhewitt pushed a commit that referenced this pull request May 15, 2022
@messense @davidhewitt
Update changelog for #2364
ea2467a
davidhewitt pushed a commit that referenced this pull request May 15, 2022
@messense @davidhewitt
Update changelog for #2364
f60d24b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhewitt davidhewitt davidhewitt approved these changes

@ravenexp ravenexp ravenexp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@messense @ravenexp @davidhewitt