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

Problem: OSX build not enabled in Travis #230

Merged
merged 13 commits into from
May 13, 2018
Merged

Conversation

kurdybacha
Copy link
Contributor

@kurdybacha kurdybacha commented May 13, 2018

Solution: Enable OSX build for xcode9.1 and clang on Travis.

Besides some more Travis build improvements. Please refer to commits descriptions for more details.

There is no need to install libzmq into the system.
Out of source build can be used later to build cppzmq.

Solution: remove install steps and rely more on cmake to handle
directory creation.
libzmq build with default settings now which build all the tests and
performance tools. This is not required for cppzmq build and slows down
it only.

Solution: disable tests build for libzmq and make it release only.
It seems that there is a bug in libzmq 4.2.0 cmake configs that prevents
linking to it without installing.

Solution: disable libzmq 4.2.0 for cmake builds
Solution: use libzmq 4.2.4, last two releases in use now.
Solution: Enable OSX build with DRAFT and latest libzmq
Default compiler on OSX is clang so it should be default.
Solution: switch from gcc to clang for OSX build.
Solution: remove matrix and include build combination manually.
After previous changes `sudo` is not required anymore.
Solution: disable sudo.
Solution: enable coverall on linux only.
Solution: As Travis enables 2 cores let's use them both
Solution: do not try to install cmake on OSX and add clang as used
compiler explicitly
@coveralls
Copy link

coveralls commented May 13, 2018

Pull Request Test Coverage Report for Build 92

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+45.3%) to 100.0%

Totals Coverage Status
Change from base Build 91: 45.3%
Covered Lines: 2
Relevant Lines: 2

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 92

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+45.3%) to 100.0%

Totals Coverage Status
Change from base Build 91: 45.3%
Covered Lines: 2
Relevant Lines: 2

💛 - Coveralls

@bluca bluca merged commit 6fe3d54 into zeromq:master May 13, 2018
@sigiesec
Copy link
Member

@kurdybacha Cool that you added CI for Mac OS X :) That gives us and users on Mac OS X more confidence :)

I am somewhat unsure about the other changes you made here. Basically, you removed CI for libzmq 4.2.0, so we cannot make any propositions about that anymore :( It doesn't matter for me personally, but it might restrict users that are for some reason confined to an outdated libzmq version. My original idea was to stepwise add CI for even older versions. But maybe that's too much work for too little benefit. I didn't manage to write the mail regarding the design goals yet, maybe it's a good idea to seek feedback on the importance of support for older libzmq versions along with that.

@kurdybacha
Copy link
Contributor Author

thanks for the feedback @sigiesec. Let me explain in more details why I decided to drop 4.2.0 support.
It worked before only because libzmq was installed in the system. It seems that ZeroMQ cmake config is broken in 4.2.0 and you can not use it without installing target. As CI exercises only cmake builds we should not claim that we support something that does not work (or work partially - when you install libzmq target). If we would like to support 4.2.0 properly then that requires more investigation.

@kurdybacha kurdybacha deleted the osx-build branch May 13, 2018 20:31
@sigiesec
Copy link
Member

I can't follow your argument.

It must be possible to consume an installed libzmq, without relying on the CMake libzmq files. This is what will be provided by most Linux distributions that have a libzmq package, e.g. And we should have at least one CI job that tests this setup.

Your assumption that "ZeroMQ cmake config is broken in 4.2.0" is not accurate. Actually libzmq does not claim that it can be consumed as a CMake package at all. There was some fix for this, which is why it happens to work with 4.2.4 and 4.2.5, but it might easily break again since this is not tested on the libzmq side.

So the default way should not rely on the CMake-specific mechanism, but use pkgconfig to locate libzmq.

However, it would probably be good to be able to specify a custom location, in addition to the default of using the pkgconfig mechanism (which requires make install). This should also be tested then.

@kurdybacha
Copy link
Contributor Author

I think you follow it just right.

I hope you are kidding about CMake and libzmq. Any package that provides CMake build option should support basic use cases that any user might expect (and libzmq does after 4.2.0). Please give me one example where someone uses CMake but it can not be consumed as CMake package.

Of course we should test pkgconfig case but let's do it as clear, separate build type.

@sigiesec
Copy link
Member

Sorry I somehow missed your comment here, but I saw your PR fixing this on the cppzmq side again. Thanks for that!

I am not kidding about libzmq, but I agree it should be supported, i.e. tested. Maybe you could add a test in the libzmq builds that consumes libzmq as a CMake package?

@kurdybacha
Copy link
Contributor Author

No problem. I will take a look.

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

4 participants