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

librdkafka: fix Visual Studio Debug ssl >= 1.8.0 #17894

Merged
merged 8 commits into from
Nov 3, 2023

Conversation

sourcedelica
Copy link
Contributor

Specify library name and version: librdkafka/1.8.0

Fixes #17879

@CLAassistant
Copy link

CLAassistant commented Jun 10, 2023

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ghost
Copy link

ghost commented Jun 11, 2023

I detected other pull requests that are modifying librdkafka/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

1 similar comment
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ghost ghost mentioned this pull request Aug 30, 2023
3 tasks
@stale
Copy link

stale bot commented Sep 17, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 17, 2023
@sourcedelica
Copy link
Contributor Author

Bumping this so it’s not automatically closed. Let me know if it should be closed.

@stale stale bot removed the stale label Sep 17, 2023
@ghost ghost mentioned this pull request Sep 20, 2023
@ghost ghost mentioned this pull request Oct 7, 2023
3 tasks
@AbrilRBS AbrilRBS self-assigned this Oct 23, 2023
@AbrilRBS
Copy link
Member

AbrilRBS commented Oct 23, 2023

Hello @sourcedelica - First of all, sorry about the delay on getting back to you on this PR. It got lost among the rest of the PR submissions as it got mislabeled internally in our GitHub project, thanks a lot for your patience on this one.

As for the PR changes themselves, they seem to look good! Reading the original issue you submitted, seems like this might in fact be the only way forward to fix the issue, but will check around just in case.

Also, please remember to sign the CLA for the PR to be mergeable once approved, thanks!

@conan-center-bot

This comment has been minimized.

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

The is_msvc is now part of Conan tools, so we no longer need to maintain it in the recipe 😄

recipes/librdkafka/all/conanfile.py Show resolved Hide resolved
recipes/librdkafka/all/conanfile.py Outdated Show resolved Hide resolved
recipes/librdkafka/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

sourcedelica and others added 3 commits October 29, 2023 11:04
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

@sourcedelica
Copy link
Contributor Author

sourcedelica commented Nov 1, 2023

I'll rebase this on the weekend.

I'm not sure what that failed check (c3i/conan-v2/pr-merge) is. When I click details it takes me to a 403 Forbidden nginx page.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 8 (eb9c0a7302605243df87b36ea5944c5100228a5e):

  • librdkafka/2.3.0:
    All packages built successfully! (All logs)

  • librdkafka/2.2.0:
    All packages built successfully! (All logs)

  • librdkafka/2.0.2:
    All packages built successfully! (All logs)

  • librdkafka/1.9.2:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 6 (eb9c0a7302605243df87b36ea5944c5100228a5e):

  • librdkafka/2.3.0:
    All packages built successfully! (All logs)

  • librdkafka/2.2.0:
    All packages built successfully! (All logs)

  • librdkafka/2.0.2:
    All packages built successfully! (All logs)

  • librdkafka/1.9.2:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 3b056b5 into conan-io:master Nov 3, 2023
5 checks passed
@sourcedelica
Copy link
Contributor Author

Awesome, thanks @uilianries!

Comment on lines +116 to +120
if Version(self.version) >= "1.8.0" and is_msvc(self) and \
self.settings.build_type == "Debug" and self.options.get_safe("ssl", False):
rdkafka_ssl_path = os.path.join(self.source_folder, "src", "rdkafka_ssl.c")
replace_in_file(self, rdkafka_ssl_path, "libcrypto.lib", "libcryptod.lib")
replace_in_file(self, rdkafka_ssl_path, "libssl.lib", "libssld.lib")
Copy link
Contributor

@SpaceIm SpaceIm Nov 3, 2023

Choose a reason for hiding this comment

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

It's fragile. What will happen if we decide to rename these lib names (this d postfix shouldn't have been added to openssl libs in openssl recipe, there were many discussions about this issue).

You should refer to cpp_info.libs values of openssl components, so that it can be future proof.

        if Version(self.version) >= "1.8.0" and is_msvc(self) and self.options.get_safe("ssl"):
            rdkafka_ssl_path = os.path.join(self.source_folder, "src", "rdkafka_ssl.c")
            openssl_cpp_info = self.dependencies["openssl"].cpp_info
            crypto_lib = openssl_cpp_info.components["crypto"].libs[0]
            ssl_lib = openssl_cpp_info.components["ssl"].libs[0]
            replace_in_file(self, rdkafka_ssl_path, "libcrypto.lib", f"{crypto_lib}.lib")
            replace_in_file(self, rdkafka_ssl_path, "libssl.lib", f"{ssl_lib}.lib")

seppeon pushed a commit to seppeon/conan-center-index that referenced this pull request Oct 21, 2024
* librdkafka: fix Windows debug ssl >= 1.8.0

* Missing import

* Support Visual Studio or msvc

* Update recipes/librdkafka/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Update recipes/librdkafka/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Update recipes/librdkafka/all/conanfile.py

Co-authored-by: Uilian Ries <uilianries@gmail.com>

* Use source folder instead of build folder

---------

Co-authored-by: Uilian Ries <uilianries@gmail.com>
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] librdkafka/1.8.0+: cannot open file libcrypto.lib
7 participants