<p dir="ltr"><br>
On 6 May 2015 09:41, "Andrey Pokrovskiy" <<a href="mailto:wonder.mice@gmail.com">wonder.mice@gmail.com</a>> wrote:<br>
><br>
> > Oh, it's not part of this single patchbomb then?<br>
> > commit f1b125442be619c43bff45791a3f6fd5726099b5<br>
><br>
> No, that was a separate patch:<br>
> <a href="https://github.com/warmcat/libwebsockets/commit/41802c7a98a1ff559fba9e7b1165fd9a8e35e99d">https://github.com/warmcat/libwebsockets/commit/41802c7a98a1ff559fba9e7b1165fd9a8e35e99d</a><br>
><br>
> The patch you are talking about goes afterwards and removes<br>
> CMAKE_BUILD define from the CMakaLists.txt since it's no longer<br>
> needed.<br>
><br>
> But I agree that you should ask to split that patch :)</p>
<p dir="ltr">Yeah... it's nice of you to agree you should have been asked to do your patches better.</p>
<p dir="ltr">So who is going to do the work to fix this?  I know how to fix it by reverting the changes and then putting anything critical back in individually.  But it's clearly better to split the cmake includes if someone is going to do it.</p>
<p dir="ltr">-Andy</p>
<p dir="ltr">> On Tue, May 5, 2015 at 6:04 PM, Andy Green <<a href="mailto:andy.green@linaro.org">andy.green@linaro.org</a>> wrote:<br>
> ><br>
> > On 6 May 2015 08:33, "Andrey Pokrovskiy" <<a href="mailto:wonder.mice@gmail.com">wonder.mice@gmail.com</a>> wrote:<br>
> >><br>
> >> > Otherwise I think we were better off how it was.<br>
> >> > I assume user code prior to this with CMAKE_BUILD will just continue to<br>
> >> > work fine.<br>
> >><br>
> >> To use the previous version of the library correctly CMAKE_BUILD<br>
> >> _must_ be defined. It it worked without CMAKE_BUILD defined you just<br>
> >> got lucky.<br>
> >><br>
> >> If CMAKE_BUILD _must_ be defined, then it doesn't make sense to check<br>
> >> whether it defined or not.<br>
> ><br>
> > What I mean is that code already exists out there.  So whatever we do if it<br>
> > meets CMAKE_BUILD defined and include the current filepath, it should<br>
> > continue to act well.<br>
> ><br>
> >> > I think the root cause of the current situation is I accepted some<br>
> >> > patchbombs I should have asked to be split up.<br>
> >><br>
> >> It already was a _separate_ patch :)<br>
> ><br>
> > Oh, it's not part of this single patchbomb then?<br>
> ><br>
> > commit f1b125442be619c43bff45791a3f6fd5726099b5<br>
> > Author: wonder-mice <<a href="mailto:wonder.mice@gmail.com">wonder.mice@gmail.com</a>><br>
> > Date:   Wed Apr 22 10:52:13 2015 -0700<br>
> ><br>
> >     Subject: [PATCH] Multiple changes in the build process<br>
> ><br>
> >     * Default CMAKE_BUILD_TYPE to Release<br>
> >     * Use CMAKE_BUILD_TYPE to define _DEBUG (NDEBUG is more standard though,<br>
> >       but cmake defines it)<br>
> >     * Drop LWS_WITHOUT_DEBUG (use CMAKE_BUILD_TYPE for that)<br>
> >     * Drop LWS_NO_EXTERNAL_POLL (was not used)<br>
> >     * Drop CMAKE_BUILD (CMake is the only build system now)  <<<<------<br>
> >     * Add LWS_WITH_STATIC and LWS_WITH_SHARED to choose what version(s) to<br>
> >       build<br>
> >     * Add LWS_XXX_LIBRARIES and LWS_XXX_INCLUDE_DIRS for each library<br>
> >       dependency (zlib, openssl, libev, cyassl)<br>
> >     * Support setting of XXX_LIBRARIES, XXX_LIBRARIES and XXX_INCLUDE_DIRS<br>
> >       by parent project (when included with add_subdirectory())<br>
> >     * Rename LWS_USE_EXTERNAL_ZLIB to LWS_USE_BUNDLED_ZLIB and default it to<br>
> >       NO (since it's Windows only)<br>
> >     * Default LWS_WITHOUT_DAEMONIZE to NO (since network lib shouldn't know<br>
> >       how to do it anyway)<br>
> >     * Rename config.h.cmake to <a href="http://lws_config.h.in">lws_config.h.in</a><br>
> >     * Rename shared library to websockets_shared so linker will not be<br>
> >       confused<br>
> >     * Fix inline keyword detection in clang<br>
> >     * Explicitly set MACOSX_RPATH to YES on MacOS<br>
> ><br>
> > Yes it's a *single patch*, that's the issue: it should have been 14 patches.<br>
> ><br>
> > -Andy<br>
> ><br>
> >><br>
> >> On Tue, May 5, 2015 at 5:20 PM, Andy Green <<a href="mailto:andy.green@linaro.org">andy.green@linaro.org</a>> wrote:<br>
> >> ><br>
> >> > On 6 May 2015 07:40, "Andrey Pokrovskiy" <<a href="mailto:wonder.mice@gmail.com">wonder.mice@gmail.com</a>> wrote:<br>
> >> >><br>
> >> >> You have to ALWAYS include lws_config.h in the beginning of<br>
> >> >> libwebsockets.h because that's how libwebsockets.h knows about options<br>
> >> >> with which the library was compiled. If you look into libwebsockets.h<br>
> >> >> you will see it uses defines from lws_config.h, such as<br>
> >> >> LWS_NO_EXTENSIONS, LWS_OPENSSL_SUPPORT, LWS_USE_LIBEV.<br>
> >> >><br>
> >> >> Before that patch it was basically broken and to use it correctly you<br>
> >> >> have to define CMAKE_BUILD in your project manually to workaround<br>
> >> >> that.<br>
> >> >><br>
> >> >> But I agree that it's completely inappropriate to have such generic<br>
> >> >> defines in library public header. I even added a comment for that:<br>
> >> >><br>
> >> >> /* That's a bad idea since it will leak all internal defines outside */<br>
> >> >> #include "lws_config.h"<br>
> >> ><br>
> >> > Yeah.  It's better if you don't say, "it's a bad idea to stick my hand<br>
> >> > in<br>
> >> > the fire" and then stick your hand in the fire as if that absolved<br>
> >> > anyone<br>
> >> > from feeling the pain though.<br>
> >> ><br>
> >> >> I suppose the right solution is to split lws_config.h on two - public<br>
> >> >> (LWS_xxx things) and private (all other things). Or just prefix<br>
> >> >> everything with "LWS_". But any solution probably will have some<br>
> >> >> "compatibility" issues, so chances are things will remain the way they<br>
> >> >> are.<br>
> >> ><br>
> >> > I think the root cause of the current situation is I accepted some<br>
> >> > patchbombs I should have asked to be split up.<br>
> >> ><br>
> >> >> On Tue, May 5, 2015 at 1:19 PM, Roger Light <<a href="mailto:andy.green@linaro.org">andy.green@linaro.org</a>><br>
> >> >> wrote:<br>
> >> >> > Hi there,<br>
> >> >> ><br>
> >> >> > I believe that a recent changeset introduced some undesirable changes<br>
> >> >> > to libwebsockets. I'm not sure what the best solution is, hence this<br>
> >> >> > email.<br>
> >> >> ><br>
> >> >> > The change is:<br>
> >> >> ><br>
> >> >> ><br>
> >> >> ><br>
> >> >> > <a href="https://github.com/warmcat/libwebsockets/commit/04da2ccd1e8c5b582c4e2a77ee53f929ae8f22a0">https://github.com/warmcat/libwebsockets/commit/04da2ccd1e8c5b582c4e2a77ee53f929ae8f22a0</a><br>
> >> >> ><br>
> >> >> > "Always include lws_config.h since now we have only CMake build"<br>
> >> >> ><br>
> >> >> > This means that libwebsockets.h always #includes lws_config.h,<br>
> >> >> > whereas<br>
> >> >> > before it was only included if CMAKE_BUILD was defined - this means<br>
> >> >> > only when the library itself was being built in practice. From what I<br>
> >> >> > can see, lws_config.h includes a mix of information that is useful to<br>
> >> >> > people using lws - such as whether it was compiled with cyassl<br>
> >> >> > support<br>
> >> >> > - and lots of things that aren't really of any interest.<br>
> >> >> ><br>
> >> >> > One of these things is the "#define VERSION" macro. This means that<br>
> >> >> > lws is claiming ownership of the VERSION macro, which seems a bit<br>
> >> >> > impolite for a library to do :) The shame of it is that the changeset<br>
> >> >> > acknowledges that the change is going to cause problems.<br>
> >> >> ><br>
> >> >> > I think that these is a lot of leftover cruft from autoconf days, but<br>
> >> >> > I don't know everything that can be cut. I think that if it is<br>
> >> >> > possible to have lws_config.h for public information about the lws<br>
> >> >> > build, and lws_config_private.h for things that lws needs for its own<br>
> >> >> > build that would be a reasonable solution.<br>
> >> ><br>
> >> > It sounds good to me if someone will implement it.  Otherwise I think we<br>
> >> > were better off how it was.<br>
> >> ><br>
> >> > I assume user code prior to this with CMAKE_BUILD will just continue to<br>
> >> > work<br>
> >> > fine.<br>
> >> ><br>
> >> > -Andy<br>
> >> ><br>
> >> >> > Any thoughts?<br>
> >> >> ><br>
> >> >> > Cheers,<br>
> >> >> ><br>
> >> >> > Roger<br>
> >> >> > _______________________________________________<br>
> >> >> > Libwebsockets mailing list<br>
> >> >> > <a href="mailto:Libwebsockets@ml.libwebsockets.org">Libwebsockets@ml.libwebsockets.org</a><br>
> >> >> > <a href="http://ml.libwebsockets.org/mailman/listinfo/libwebsockets">http://ml.libwebsockets.org/mailman/listinfo/libwebsockets</a><br>
</p>