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/openssl patch/all #19496

Closed
wants to merge 8 commits into from

Conversation

KevDi
Copy link

@KevDi KevDi commented Aug 30, 2023

Specify library name and version: librdkafka/all

Starting from librdkafka Version 1.8.0 the following pragma calls for openssl where added librdkafka/src/rdkafka_ssl.c

#pragma comment(lib, "libcrypto.lib")
#pragma comment(lib, "libssl.lib")

On a Debug Build this causes problems, because Openssl builds the Debug Libraries with an additional d (libcryptod.lib/libssld.lib). Linking the Program then fails because it won't find the correct libraries.

Adding these with the correct ending fixes this.
I also opened a PR on the Librdkafka Project with those changes but i'm not sure if they will be added.


KevDi and others added 2 commits August 25, 2023 13:32
When building as debug linking against the librdkafka library fails because
it is looking for the libcrpto.lib instead of the libcryptod.lib.
Adding those pragmas to the rdkafka_ssl.c file solves this issue
@ghost
Copy link

ghost commented Aug 30, 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.

@KevDi
Copy link
Author

KevDi commented Aug 30, 2023

As @ericLemanissierBot mentioned there is another PR fixing this Problem. The question is what would be the best way? Adding the additional Pragmas or just replacing them like in the PR #17894

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 6 (06e973a2767c0ec485eb80cd334ef4d21d796cf2):

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

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

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

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

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

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

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

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

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

@AbrilRBS
Copy link
Member

I'm retriggering the build in v2 as it failed for a networking reason, thanks for your patience while we processed the rest of the PR backlog :)

@AbrilRBS AbrilRBS self-assigned this Sep 20, 2023
@KevDi
Copy link
Author

KevDi commented Sep 20, 2023

@RubenRBS 👍 and is this fix as it is ok or is the one mentioned above where the replace is done in python a better solution? I also made a pr at the librdkafka project with those changes so maybe if it gets accepted this patches are not needed then anymore. But we will see 😃☺️

@ghost ghost mentioned this pull request Sep 20, 2023
@ghost ghost mentioned this pull request Oct 7, 2023
3 tasks
@AbrilRBS
Copy link
Member

@RubenRBS 👍 and is this fix as it is ok or is the one mentioned above where the replace is done in python a better solution? I also made a pr at the librdkafka project with those changes so maybe if it gets accepted this patches are not needed then anymore. But we will see 😃☺️

Oh my @KevDi sorry about the radio silence, I totally dropped the ball on this one and did not see the mention notification until now, so I didn't circle back to the PR until now

Do you have a link to the PR you mention you submitted upstream? Having backport links usually helps a lot for future maintenance

@KevDi
Copy link
Author

KevDi commented Oct 26, 2023

@RubenRBS this is the other PR which deals with this problem #17894

@AbrilRBS
Copy link
Member

Oh sorry I didnt make myself clear at all, I was asking about the PR you mentioned opening upstream

@KevDi
Copy link
Author

KevDi commented Oct 26, 2023

Okay sorry you mean the PR on the librdkafka Project. Here it is: Added Pragmas for the Debug Libs from Openssl

@KevDi
Copy link
Author

KevDi commented Jan 31, 2024

@RubenRBS is this PR still considered to be implemented? If so i would try to update it to the latest version. Because it seems the problem still exists. And from librdkafka side there is still no feedback on that topic.

@KevDi KevDi closed this Feb 15, 2024
@KevDi
Copy link
Author

KevDi commented Feb 15, 2024

not needed anymore

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