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

switch extensions of static libs on windows #131

Merged
merged 3 commits into from
Sep 7, 2022

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Sep 7, 2022

Another fallout from #128, which changed the static libs to depend on the shared ones according to conda-forge/conda-forge.github.io#1809.

This needed an adaptation of the import library names (since they share the .lib suffix with the static ones); so far so good, all that is saved in the CMake metadata. However, CMake prefers its own custom FindProtobuf despite having package metadata created by itself, and then ends up not finding the adapted import libs. 🤦

Undo this to stop the bleeding. Switch the extension of the import library to use the default ".lib", see this comment

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@traversaro
Copy link
Contributor

Thanks a lot @h-vetinari ! Just a curiosity, I guess that you considered renaming the static libraries to protobuf-static.lib? If you did not, I guess probably this is not possible, but I do not know why.

@h-vetinari
Copy link
Member Author

I guess that you considered renaming the static libraries to protobuf-static.lib? If you did not, I guess probably this is not possible, but I do not know why.

That would just create the same problem for any user of the libprotobuf-static output. 🤷

Together with:

  • windows using the same extensions for different things
  • CMake not respecting the package metadata

I don't see how else to fix this TBH...

@traversaro
Copy link
Contributor

I guess that you considered renaming the static libraries to protobuf-static.lib? If you did not, I guess probably this is not possible, but I do not know why.

That would just create the same problem for any user of the libprotobuf-static output. 🤷

In which way? If I understood correctly, after #128 libprotobuf-static would depend on libprotobuf, so find_package(Protobuf REQUIRED) would be happy and find the shared version. Probably what would be broken is the use of Protobuf_USE_STATIC_LIBS, that however by default is OFF, right? Anyhow, this PR is definetely the solution with less problems.

@h-vetinari h-vetinari added automerge Merge the PR when CI passes and removed automerge Merge the PR when CI passes labels Sep 7, 2022
@h-vetinari
Copy link
Member Author

In which way? If I understood correctly, after #128 libprotobuf-static would depend on libprotobuf, so find_package(Protobuf REQUIRED) would be happy and find the shared version. Probably what would be broken is the use of Protobuf_USE_STATIC_LIBS, that however by default is OFF, right? Anyhow, this PR is definetely the solution with less problems.

Fair enough, thanks for walking me through that...

@h-vetinari h-vetinari force-pushed the split branch 2 times, most recently from dea6e3b to 9cc2d65 Compare September 7, 2022 14:54
@h-vetinari h-vetinari changed the title separate static window builds from depending on shared ones again switch extensions of static libs on windows Sep 7, 2022
@h-vetinari h-vetinari closed this Sep 7, 2022
@h-vetinari h-vetinari reopened this Sep 7, 2022
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.

3 participants