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

Add zeromq/4.3.2 recipe #489

Merged
merged 7 commits into from
Jan 28, 2020
Merged

Add zeromq/4.3.2 recipe #489

merged 7 commits into from
Jan 28, 2020

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Dec 14, 2019

Specify library name and version: zeromq/4.3.2

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot
Copy link
Collaborator

Some configurations of 'zeromq/4.3.2' have failed:

@conan-center-bot
Copy link
Collaborator

Some configurations of 'zeromq/4.3.2' have failed:

@conan-center-bot
Copy link
Collaborator

Some configurations of 'zeromq/4.3.2' have failed:

@conan-center-bot
Copy link
Collaborator

All green! 😊

@uilianries
Copy link
Member

@madebr please take a look: madebr#6

@SSE4
Copy link
Contributor

SSE4 commented Jan 6, 2020

@madebr please merge madebr#6

@conan-center-bot
Copy link
Collaborator

All green in build 5 (3fed4124467e03e5debbfcde12b9495ac00758e1)! 😊

danimtb
danimtb previously approved these changes Jan 13, 2020
@danimtb danimtb self-assigned this Jan 13, 2020


class LibZMQConan(ConanFile):
name = "libzmq"
Copy link
Member

Choose a reason for hiding this comment

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

why libzmq and not zeromq? Is there any idea for zeromq as installer package? The folder name is zeromq btw.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reasoning is that the name of the project is libzma: https://github.com/zeromq/libzmq/

Copy link
Member

Choose a reason for hiding this comment

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

fair enough!

uilianries
uilianries previously approved these changes Jan 14, 2020
@danimtb
Copy link
Member

danimtb commented Jan 14, 2020

I see vcpkg names it zeromq... we will need some consensus here @Croydon @Minimonium

@Minimonium
Copy link
Contributor

There are inconsistencies in how they name their own releases on various platforms, both names are used: https://zeromq.org/download/

It's better to have the name parity in package managers - better discoverability for users as per #52

On the other hand, their target's name is libzmq so we need then to specify it as cpp_info.name[...].

@madebr madebr dismissed stale reviews from uilianries and danimtb via a77696d January 14, 2020 14:38
@madebr
Copy link
Contributor Author

madebr commented Jan 14, 2020

I've renamed the package to zeromq.
The cmake script create a ZeroMQConfig.cmake file, so I use self.cpp_info.names["cmake_find_package"] = ZeroMQ.

@conan-center-bot
Copy link
Collaborator

All green in build 6 (a77696d89a2472f9d6fe6fbeb0f890beaa309561)! 😊

@danimtb
Copy link
Member

danimtb commented Jan 14, 2020

So this means that there would be no "transparent integration" as the target name will be different, right?

@Minimonium
Copy link
Contributor

@danimtb The upstream project doesn't seem to support proper targets with alias support.
It suggests using find_package(ZeroMQ) but linking to the libzmq. I think we can't model it like that right now.

@danimtb
Copy link
Member

danimtb commented Jan 14, 2020

Ok, so I think the name is ok as is right now

@danimtb danimtb requested review from jgsogo and SSE4 January 14, 2020 15:54
recipes/zeromq/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot
Copy link
Collaborator

All green in build 7 (a77696d89a2472f9d6fe6fbeb0f890beaa309561)! 😊

@madebr
Copy link
Contributor Author

madebr commented Jan 20, 2020

@uilianries Can I have a rebuild?

@uilianries uilianries closed this Jan 21, 2020
@uilianries uilianries reopened this Jan 21, 2020
@uilianries
Copy link
Member

@madebr I just restarted the build now. Let's wait.

@conan-center-bot
Copy link
Collaborator

All green in build 12 (98c968e078eb25850468f10160720286d9f368eb)! 😊

@conan-center-bot
Copy link
Collaborator

All green in build 13 (e0d2de30ea7275f064e30d37db887b46c936e1f2)! 😊

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

I'm requesting several changes related to the test_package:

  • Consume it using cmake_find_package so we realize what the name of the target is. (We will also need cmake generator and conan_basic_setup() call in order to adjust compiler flags... the toolchain should arrive sooner than later, but meanwhile, it is needed)
  • The WITH_LIBSODIUM definition should be added to the cpp_info, Conan will add it to the target and apply it to the test executable

recipes/zeromq/all/test_package/conanfile.py Outdated Show resolved Hide resolved
@Minimonium
Copy link
Contributor

We will also need cmake generator and conan_basic_setup() call in order to adjust compiler flags

That's a bit of news for me. Could you elaborate?

@jgsogo
Copy link
Contributor

jgsogo commented Jan 27, 2020

We will also need cmake generator and conan_basic_setup() call in order to adjust compiler flags

That's a bit of news for me. Could you elaborate?

Nothing new, in the conan_basic_setup() call there is a lot of magic, a lot of functions are being called that use the CONAN_XXXX variables to set all kinds of stuff. If we just use the cmake_find_package generator we can take care of everything related to the dependencies, but not to things that should affect the binaries we are generating (runtime, cppstd,...) even if those are just the test_executable.exe

@Minimonium
Copy link
Contributor

It's just that I have a few test packages here without it (only with the cmake_find_package). 🙁

Thanks for the information!

@jgsogo
Copy link
Contributor

jgsogo commented Jan 27, 2020

Most of the time the test_package is so simple that it compiles without it, but I prefer to have it there to avoid some copy/paste issues 😄

@conan-center-bot
Copy link
Collaborator

All green in build 14 (c63349d31160ebc21948f8532481803d817af1cb)! 😊

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

🎉

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.

7 participants