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

mpdecimal: Fix lib name on MSVC/shared, fix Mac cross-builds #25889

Merged
merged 13 commits into from
Nov 11, 2024

Conversation

Ahajha
Copy link
Contributor

@Ahajha Ahajha commented Nov 9, 2024

When using the MSBuildDeps generator with mpdecimal(msvc/shared), it generates the wrong lib name and thus fails to build. I assume CMake does some more advanced logic and so this was never caught.

The correct name is (for example) libmpdec-2.5.1.dll.lib, previously this was just libmpdec-2.5.1.lib.

This change is necessary for supporting shared mpdecimal on Windows in CPython.

Also fixes #23994 (this is fixed by both of my other open mpdecimal PRs, since it's necessary to build. First one to be merged will fix it)

I also have two other mpdecimal PRs out: #25896 #25903 (superset of this one)

@Ahajha Ahajha changed the title mpdecimal: Fix lib name on MSVC/shared mpdecimal: Fix lib name on MSVC/shared, fix Mac cross-builds Nov 9, 2024
@jcar87
Copy link
Contributor

jcar87 commented Nov 11, 2024

I also added some extra cleanup commits, none of these are necessary and I'm happy to revert any of them if it makes it easier to review the PR

yes please - this makes it a lot easier to review, especially with the size of the backlog - thanks!

@Ahajha
Copy link
Contributor Author

Ahajha commented Nov 11, 2024

@jcar87 I reverted the extra changes, diff is much smaller now :) I'll make a followup PR after the important 3 are merged to just do all the cleanup.

This PR is now a subset of #25903, so feel free to either skip this and just review that one, or merge this first and then the other one.

Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jcar87 jcar87 merged commit b57c446 into conan-io:master Nov 11, 2024
10 checks passed
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.

[package] mpdecimal/2.5.1: CI fail when compiling on MacOS
3 participants