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

(#22661) openssl: fix handling of tc.defines in config_template #22662

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

obnyis
Copy link
Contributor

@obnyis obnyis commented Feb 5, 2024

Specify library name and version: openssl/3.2.1

Fix the handling of tc.defines into the config_template for openssl. Currently, it is generating a single string that is space-separated, but the toolchain is expecting a comma-separated list of strings.

Fixes #22661


Copy link
Contributor

github-actions bot commented Feb 5, 2024

🤖 Beep Boop! This pull request is making changes to 'recipes/openssl//'.

👋 @Hopobcn @Croydon you might be interested. 😉

@ericLemanissierBot
Copy link

ericLemanissierBot commented Feb 5, 2024

I detected other pull requests that are modifying openssl/3.x.x 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.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@obnyis
Copy link
Contributor Author

obnyis commented Feb 19, 2024

Just saw in the openssl/1.x.x conanfile.py that this has a slightly different format for handling the comma separation of defines.

Should I update this PR with the following snippet so there is parity between the recipes for how this is handled?

defines = ", ".join(f'"{d}"' for d in defines)
defines = 'defines => add(%s),' % defines if defines else ""

Copy link
Contributor

@Croydon Croydon left a comment

Choose a reason for hiding this comment

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

Much easier than the code used in the 1.x.x recipe

Bug seems to exist since the 3.x.x recipe was created, nice catch!

@RubenRBS
Copy link
Member

Hi! Sorry for the delay, I've scheduled this PR to be reviewed next monday, thanks for your patience :)

@klausholstjacobsen
Copy link
Contributor

klausholstjacobsen commented Mar 7, 2024

@RubenRBS Any news on this PR?

@obnyis
Copy link
Contributor Author

obnyis commented Mar 11, 2024

Hi! Sorry for the delay, I've scheduled this PR to be reviewed next monday, thanks for your patience :)

Hi @RubenRBS, is there anything that is blocking this from being reviewed?

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 1 (a4548899728b0069bb96ee74f6f53e40b600b43d):

  • openssl/3.2.1:
    All packages built successfully! (All logs)

  • openssl/3.2.0:
    All packages built successfully! (All logs)

  • openssl/3.1.3:
    All packages built successfully! (All logs)

  • openssl/3.1.4:
    All packages built successfully! (All logs)

  • openssl/3.1.5:
    All packages built successfully! (All logs)

  • openssl/3.0.12:
    All packages built successfully! (All logs)

  • openssl/3.0.13:
    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 1 (a4548899728b0069bb96ee74f6f53e40b600b43d):

  • openssl/3.2.1:
    All packages built successfully! (All logs)

  • openssl/3.1.5:
    All packages built successfully! (All logs)

  • openssl/3.1.4:
    All packages built successfully! (All logs)

  • openssl/3.2.0:
    All packages built successfully! (All logs)

  • openssl/3.1.3:
    All packages built successfully! (All logs)

  • openssl/3.0.13:
    All packages built successfully! (All logs)

  • openssl/3.0.12:
    All packages built successfully! (All logs)

Copy link
Member

@RubenRBS RubenRBS left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, after catching up with @uilianries we've agreed to push this forward :)

@conan-center-bot conan-center-bot merged commit 424bbf6 into conan-io:master Apr 5, 2024
43 checks passed
yhsng pushed a commit to yhsng/conan-center-index that referenced this pull request Apr 12, 2024
ericLemanissier pushed a commit to ericLemanissier/conan-center-index that referenced this pull request Apr 16, 2024
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] openssl/3.x.x: can't compile if tools.build:defines set in profile
7 participants