[zeromq-dev] zmq cmake patch

[George Neill]

Malo,

> Thanks for your time and all the explanations. See inline comments...

Certainly.   Thanks for paying attention to my noise! :)

>>> In attachment is patch against rev. 641 it adds only very basic support
>>> for
>>> cmake (only zmq and zmq_server is build on Linux).
>>> It is based on your work and should be seen as a model and starting
>>> point.
>>> I'm "few hours" cmake user and hence your experience is important for us,
>>> anyway here are my notes to cmake support.
>>>
>>> 1. At first we would like to use as many macros provided by cmake as
>>> possible.
>>> Searching for thread library can be done with find_package Threads.
>>> There's
>>> FindSDL.cmake and FindJava.cmake in Modules directory and we should
>>> presumably use those.
>>
>> While a handful of Find*.cmake packages are distributed with cmake; I
>> don't believe kitware maintains them.  I have found most of these
>> package scripts -are- useful if you plan on supporting just the
>> win/linux/mac platforms.  but in the end I have found myself copying
>> them local and modifying them as soon as i move on to support
>> different platforms.
>>
>> As an example, the default in FindThreads.cmake for HPUX is to blindly
>> use CMA threads and there's no way to override it without making a
>> local copying and changing.  IMO there probably should be a
>> FindNativeThreads.cmake and FindPosixThreads.cmake and
>> FindThreads.cmake should call either of those based on some flags.
>>
>
> For us this is quite big disappointment especially for such a basic thing
> like threads are and how cmake is trying to be multiplatform.
> Seems that you are right and this the right way.

Oh, I guess I don't see it as a disappointment.  I see it as an area
where they need help, and we can provide it.  Not too many people have
access to unusual hardware platforms.  The one thing I don't really
understand is why kitware doesn't own those.  I suppose if we asked
over on the cmake list they'd tell us why.

>> For SDL, FindSDL.cmake implicitly pulls in FindThreads even when they
>> may not be necessary.
>> For Java, FindJNI.cmake checks for far more than what is needed to
>> build zmq ... plus it will only work for win/mac/linux (even if you
>> set ENV vars)
>>
>> The unicap and socket find scripts didn't exist ... so as a starting
>> point, I created those.  The socket one, quite honestly could be
>> considered a platform check.
>>
>> In summary, I believe the worthiness of each Find*.cmake package
>> inclusion should be looked at, but I don't necessarily think we should
>> just implicitly use them.  I consider each more of a starting point.
>> Of course, in the end, i'd like to contribute back whatever we find to
>> the maintainers of each script.
>>
>
> And what about pkg-confing cmake interface? For the moment we
> did not have any problem with using pkg-config on ZMQ supported platforms at
> the moment.

There exists a FindPkgConfig.cmake, but I haven't really picked
through it ... so I can't really comment much on it ATM.

>>> 2. Add as less new files as possible.
>>> I have moved code from zmq_platform_checks.cmake and
>>> zmq_cmake_policies.cmake into main CMakeLists.txt file they are easy to
>>> spot
>>> and modiffy. Note that changes should be able to make even user which is
>>> not
>>> so familiar with cmake and with no so much time to explore all the cmake
>>> hierarchy.
>>
>> Your call.  As a developer, I believe they are _easier_ to work with
>> split in to separate files.  As the project grows, I believe you'll
>> find having those split out would be better (same with c++ or any
>> other language for that matter).  Your argument could be solved with
>> some decent documentation.
>
> Ok, let's do it our way now, we can split the files as the build system
> grows.

Sure.  I am okay with that.

>>> 3. If something is getting wrong stop the configure process.
>>> For example if you are on unsupported platform or some library was not
>>> found
>>> all the process should be stopped with some warning.
>>
>> I agree for the most part. I would argue it should only stop on
>> -required- items ... basically anything detrimental to zmq working.
>> So for example, one would just spit out a warning when trying to build
>> the camera example ... not stop the build.  But, again, a tunable
>> could be set -D"STOP_ON_WARNING=ON" or something like that.  Do what
>> you feel is necessary.
>
> In this we wont agree, our idea is when user is asking something lets say
> configuring ZMQ with camera example and we can not give it to him, we are
> not
> giving him anything. We would like to keep this behaviour.

Sure, I don't have a problem with that.

>>> 4. We prefer to build shared libraries by default.
>>
>> Ok.  That's fine, we can just change the default BUILD_SHARED_LIBS
>> variable to ON.
>>
>> BTW, if you have not realized it yet, this is probably one of the
>> biggest *potential* issues with converting over to use cmake.  I don't
>> think it's a big deal, but some probably do.  In order to build static
>> and shared libraries for the same target you can do one of two things.
>>
>> 1.  Do two builds, on with BUILD_SHARED_LIBS turned on and the other
>> with it off.
>> 2.  Give your shared/archive libraries a suffix (different target
>> names).  e.g.  libzmq_static.a,  libzmq_shared.so
>
> We prefer your 1. possibility here.

Same here.

>>> 5. Do not make it look ZMQ specific.
>>> Your are creating macros zmq_find_unicap.cmake which can be acctualy
>>> FindUnicap.cmake and used with find_package interface.
>>
>> I specifically chose -not- to use the FIND_PACKAGE command for these
>> because 1.  I wanted it to be clear these -are- specific to zmq  2. I
>> don't like the naming convention enforced and 3. I don't like mixing
>> camelhump and underbar notation.
>>
>> Silly, probably, but I don't feel you lose much by doing an INCLUDE
>> vs. a FIND_PACKAGE  (I could be wrong).
>
> Sure for you is the same but for somebody which is not so familiar
> with cmake and our potential future building system is more much more
> straightforward to see find_package (whatever) like to include (whatever).
> Maybe for this reason find_package interface has been created.

Sure, you can easily rename the scripts I have in there to
Findzmq_threads.cmake (or something else, though i'd recommend they
aren't the same name as the cmake distributed versions) and call them
with FIND_PACKAGE(zmq_threads) .... it'll work just the same.  Or we
could just create a ZMQ_FIND_PACKAGE(threads) ...

>>> Do not take an offence, maybe I have missed something and you have good
>>> reason to do thing the way you do it, we just need a little explanation.
>>> Anyway as soon as you are signing Copyright Assignment and License we
>>> will
>>> create you branch where we can continue work on cmake patch.
>>
>> No offense at all, infact I always try to be in learn mode ... so if
>> you find a better way to do it please share.  We'll disagree on some
>> but that's fine.  I have used cmake for about a year now, and still
>> consider myself a rookie with it.  The above are just my opinions and
>> I am not associated in any way with kitware or it's maintainers.
>
> You know there is not an universal "better way" if there is more
> then 1 person involved:)

Well one thing is for sure, I will always have more to learn.

Thanks again for your time and comments.

Later,
George.