liblo-devel Mailing List for LibLO

On Mon, Feb 15, 2016 at 2:18 PM, John Emmas <jo...@ti...> wrote:
> On 15/02/2016 15:15, Stephen Sinclair wrote:
>> Can you review the most recent commit?
>>
>> I decided to just remove the whole "#if 0" thing in all build systems.
>>
>
> Hi Steve,
>
> Prior to compiling, my build runs a small script.  I needed to adapt it
> slightly (to take account of the newly added @THREADS_INCLUDE@). But
> after doing that, everything still builds okay.

Great! I'm considering keeping this change, but I'll leave it on the
back-burner for a while and see if it causes any problems..

Steve

On 15/02/2016 15:15, Stephen Sinclair wrote:
> Can you review the most recent commit?
>
> I decided to just remove the whole "#if 0" thing in all build systems.
>

Hi Steve,

Prior to compiling, my build runs a small script.  I needed to adapt it 
slightly (to take account of the newly added @THREADS_INCLUDE@). But 
after doing that, everything still builds okay.

John

Can you review the most recent commit?

I decided to just remove the whole "#if 0" thing in all build systems.

Steve


On Mon, Feb 15, 2016 at 11:07 AM, John Emmas <jo...@ti...> wrote:
> On 15/02/2016 13:34, Stephen Sinclair wrote:
>> Basically from what I understand, in the automake build,
>> @ENABLE_THREADS@ gets set to 0 or 1, so the #if is fine either way.
>> In the CMake build, ENABLE_THREADS gets set to 1, or "", the latter
>> case making "#if" be not well-formed.  So changing it to "#if +0"
>> makes it work.  Now, I don't know CMake very well, maybe it would be
>> possible to set ENABLE_THREADS=0 instead of ""
>>
>
> Thanks for the quick reply Stephen.  To be honest, I don't know CMake
> either but I noticed this in 'cmake/CMakeLists.txt':-
>
>      if(THREADING)
>          set(ENABLE_THREADS 1)
>
>          // Some other stuff
>      endif()
>
> Maybe it just needs to look like this?
>
>      if(THREADING)
>          set(ENABLE_THREADS 1)
>
>          // Some other stuff
>      else()
>          set(ENABLE_THREADS 0)
>      endif()
>
> flv0 (Patric?) is probably the best person to know...  Regards,
>
> John
>
> ------------------------------------------------------------------------------
> Site24x7 APM Insight: Get Deep Visibility into Application Performance
> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> Monitor end-to-end web transactions and take corrective actions now
> Troubleshoot faster and improve end-user experience. Signup Now!
> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
> _______________________________________________
> liblo-devel mailing list
> lib...@li...
> https://lists.sourceforge.net/lists/listinfo/liblo-devel

On 15/02/2016 13:34, Stephen Sinclair wrote:
> Basically from what I understand, in the automake build,
> @ENABLE_THREADS@ gets set to 0 or 1, so the #if is fine either way.
> In the CMake build, ENABLE_THREADS gets set to 1, or "", the latter
> case making "#if" be not well-formed.  So changing it to "#if +0"
> makes it work.  Now, I don't know CMake very well, maybe it would be
> possible to set ENABLE_THREADS=0 instead of ""
>

Thanks for the quick reply Stephen.  To be honest, I don't know CMake 
either but I noticed this in 'cmake/CMakeLists.txt':-

     if(THREADING)
         set(ENABLE_THREADS 1)

         // Some other stuff
     endif()

Maybe it just needs to look like this?

     if(THREADING)
         set(ENABLE_THREADS 1)

         // Some other stuff
     else()
         set(ENABLE_THREADS 0)
     endif()

flv0 (Patric?) is probably the best person to know...  Regards,

John

On Mon, Feb 15, 2016 at 8:26 AM, John Emmas <jo...@ti...> wrote:
> I just updated from git master and discovered this puzzling change at
> line 34 of 'liblo/lo/lo.h.in':-
>
>        #if @ENABLE_THREADS@+0
>
> Previously it just said this:-
>
>        #if @ENABLE_THREADS@
>
> where ENABLE_THREADS was typically defined to be either 0 or 1 (I think).
>
> It doesn't seem to be causing any problems for me (I'm building with
> MSVC) but I'm just curious to know why the extra "+0" got added to the
> end of the line...


Thanks for reviewing!

The +0 got added in commit 666517 during flv0's development of the
CMake build.  It was a concession to allow him to make a change
outside the "cmake" directory, but it seemed necessary, and in my
estimation, harmless..?  Anyways, no better way to test than to put it
on master and see if it breaks anything.

Basically from what I understand, in the automake build,
@ENABLE_THREADS@ gets set to 0 or 1, so the #if is fine either way.
In the CMake build, ENABLE_THREADS gets set to 1, or "", the latter
case making "#if" be not well-formed.  So changing it to "#if +0"
makes it work.  Now, I don't know CMake very well, maybe it would be
possible to set ENABLE_THREADS=0 instead of "" in the case that it is
disabled, and therefore this would be an unnecessary change.

In any case I wasn't completely sold on the necessity of introducing
CMake to the project, but it seemed something that several users
wanted, so I said just keep it confined to the "cmake" directory and
it should be fine.  It's true that this is one change that is maybe
not necessary, I'm not sure.  Theoretically we could even remove the
whole #if conditional around lo_serverthread.h, since defining the
function prototypes doesn't hurt actually, even if they aren't
compiled into the library.  Another possibility, maybe cleaner, would
be setting the @ENABLE_THREADS@ replacement to be "#include
<lo_serverthread.h>", and simply not have that line at all if it is
disabled.

Steve

I just updated from git master and discovered this puzzling change at 
line 34 of 'liblo/lo/lo.h.in':-

       #if @ENABLE_THREADS@+0

Previously it just said this:-

       #if @ENABLE_THREADS@

where ENABLE_THREADS was typically defined to be either 0 or 1 (I think).

It doesn't seem to be causing any problems for me (I'm building with 
MSVC) but I'm just curious to know why the extra "+0" got added to the 
end of the line...

John