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

Compile as C #23894

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

datalogics-staylor
Copy link

@datalogics-staylor datalogics-staylor commented May 6, 2024

Specify library name and version: qr-code-generator/1.8.0

  • Compile as C when the package is built with the new "compile_as_c" option.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@datalogics-staylor datalogics-staylor marked this pull request as ready for review May 8, 2024 18:26
@conan-center-bot

This comment has been minimized.

@jcar87
Copy link
Contributor

jcar87 commented May 9, 2024

Hi @datalogics-staylor , thanks for your contribution.
Is this a case where it might make sense for the package to just build both libraries, C and C++? if the resulting library files have a different name, they can coexist, and then these can be different components? I suspect that's the case with other libraries in the same situation as well.

@datalogics-staylor
Copy link
Author

Hi @jcar87. That's a very good point. I looked around a bit for recipes that do both C and C++ and didn't find many, so I chose to go this route so that it doesn't build packages that aren't required. It would be a pretty minor change to just build both, so I'd be happy to go that route if that seems to make more sense. Conan and CCI are relatively new to me, so I'm still learning common usages. Thanks for the feedback!

@conan-center-bot

This comment has been minimized.

@uilianries
Copy link
Member

@datalogics-staylor Your change looks correct in terms of CMake, but still need to update the Conanfile recipe in order to list the new library too. Could please update the pacakge_info as well?

@conan-center-bot

This comment has been minimized.

@@ -102,7 +102,7 @@ def package(self):

def package_info(self):
self.cpp_info.libs = [
"qrcodegen" if Version(self.version) < "1.7.0" else "qrcodegencpp"
"qrcodegen" if Version(self.version) < "1.7.0" else "qrcodegencpp", "qrcodegenc"
]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
]
]
self.cpp_info.components["qrcodegencpp"].set_property("cmake_target_name", "qr-code-generator::qrcodegencpp")
self.cpp_info.components["qrcodegencpp"].libs = ["qrcodegen" if Version(self.version) < "1.7.0" else "qrcodegencpp"]
self.cpp_info.components["qrcodegenc"].set_property("cmake_target_name", "qr-code-generator::qrcodegenc")
self.cpp_info.components["qrcodegenc"].libs = ["qrcodegenc"]

I would suggest adding separated components too, so you can link to C++ or C only, by using cmake targets.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, that makes sense. I'm pretty new to both Conan and CMake, so I appreciate the feedback!

@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.

Looks good. Both C++ and C libraries are available as separated targets.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 12 (79ba58e4c16e3ca48794c45aef6cc9a02fa69da8):

  • qr-code-generator/1.7.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.8.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.6.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.5.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.4.0:
    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 12 (79ba58e4c16e3ca48794c45aef6cc9a02fa69da8):

  • qr-code-generator/1.6.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.4.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.5.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.8.0:
    All packages built successfully! (All logs)

  • qr-code-generator/1.7.0:
    All packages built successfully! (All logs)

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.

None yet

5 participants