Fix BSD Builds. Builds and the SRT library on FreeBSD-11, TrueOS-12, … #588

jlsantiago0 · 2019-02-26T20:59:43Z

Fix BSD Builds. Builds and the SRT library on FreeBSD-11, TrueOS-12, and OpenBSD-6.4 and my tests are working. NOTE: the support applications currently do not build on OpenBSD, but do on FreeBSD. I have not done a lot of testing. All testing was with my application and the library sending/receiving TS over SRT. With and without encryption was tested. This commit also adds the ability to disable the SUPPORT applications in the top level of the build. Added a CMAKE variable DISABLE_SUPPORT which is OFF by default. I have not tested DragonFlyBSD or NetBSD.

…and OpenBSD-6.4 and my tests are working.

ethouris · 2019-02-27T07:36:08Z

CMakeLists.txt

@@ -77,6 +78,7 @@ endif()
 # options
 option(CYGWIN_USE_POSIX "Should the POSIX API be used for cygwin. Ignored if the system isn't cygwin." OFF)
 option(ENABLE_CXX11 "Should the c++11 parts (srt-live-transmit) be enabled" ON)
+option(DISABLE_SUPPORT_APPLICATIONS "Should the Support Applications be Disabled?" OFF)


In this system there's an enable/disable symmetry in configure, therefore all boolean enabler options must have ENABLE_ prefix. Also, for options please use as short name as possible - ENABLE_APPS should be enough.

ENABLE_APPS or ENABLE_SAMPLE_APPS

Other parts of the code refers to them as support applications. Maybe there needs to be some consistency about what to call them. Either Sample Applications or Support Applications.

Committed a change to use ENABLE_APPS

ethouris · 2019-02-27T07:36:15Z

CMakeLists.txt

+endif()
+
+if ( ENABLE_CXX11
+   AND NOT DISABLE_SUPPORT_APPLICATIONS )


For me this looks like two separate variables for the same purpose. My proposal is: make the ENABLE_CXX11 option control the other, and here only the other option shall be in the condition.

@jlsantiago0
Modify line 248 (add set(ENABLE_APPS 0)):

if (NOT ENABLE_CXX11) set(ENABLE_APPS 0) message(WARNING "Parts that require C++11 support will be disabled (srt-live-transmit)") endif()

It feels to me that using a variable ENABLE_CXX11 to determine select building of the sample/support applications is the problem. Not the ENABLE_APPS or ENABLE/DISABLE_SUPPORT/SAMPLE_APPLICATIONS/APPS. In otherwords the decision to ENABLE_APPS depends on ENABLE_CXX11, and then the guard to actually do the building of the applications should be using ENABLE_APPS or whatever.

I just checked in a change to use the ENABLE_APPS as the guard to build the applications that is set to OFF when ENABLE_CXX11 is OFF.

I agree that ENABLE_CXX11 is now redundant since enabling APPS or TESTS implicitly means CXX11. We should probably have a statement for backward compatibility where disabling CXX automatically means disabling APPS and TESTS, but, otherwise, ENABLE_CXX11 should not be used anywhere in the make logic.

OK. I committed a change to resolve this issue.

ethouris · 2019-02-27T07:40:10Z

srtcore/srt_compat.c

@@ -22,7 +22,8 @@ written by
 #include <stdio.h>
 #include <errno.h>
 #if !defined(_WIN32) \
-   && !defined(__MACH__)
+   && !defined(__MACH__) \
+   && !(defined(__unix__) && defined(BSD))


Well, isn't that kinda simpler - that is, Mac and Linux only?

What about Solaris or other operating systems that do have features.h? There are more things in heaven and earth Horatio than fit into your philosphy. So maybe it can be simplified by checking for defined(__unix__) and not defined(BSD) for now. I assume OSX and its variants define(BSD)? I dont know. There is probably a portable way to determine if features.h is available in the OS. But I dont know what that is.

I commited a change to simplify this to check for __unix__ and not BSD. BSD seems to be defined on all the BSD derived OSes including OSX. I am not sure however whether that is the case for iOS, Watch, and AppleTV OSes tho. But I assume it is. Like for instance Android != linux, but does defined __unix__ and __linux__ . I tested and it builds correctly on OSX.

Important thing for it is not that it should be enabled on every system that features features.h, but to be enabled on every system where we want something specific from its features.h. We might have some "sensible default portable" method for systems that this project wasn't tested against, but this should simply NOT try to include this file at all, even if it exists.

ethouris · 2019-02-27T07:47:45Z

CMakeLists.txt

@@ -24,6 +24,7 @@ include(CheckFunctionExists)
 # Platform shortcuts
 set_if(DARWIN      ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 set_if(LINUX       ${CMAKE_SYSTEM_NAME} MATCHES "Linux")
+set_if(BSD         ${CMAKE_SYSTEM_NAME} MATCHES "[bB][sS][dD]$")


I know that CMake is poor as it comes to support case insensitive matching, but I really think this can be less clumsy:

string(TOLOWER ${CMAKE_SYSTEM_NAME} SYSNAME_LC) set_if(BSD ${SYSNAME_LC} MATCHES "bsd$")

Similarly in line 702:

AND ${SYSNAME_LC} MATCHES "openbsd$"

It seems clumsier to make a new variable. But I think that is just a measure of preference.

I agree with @jlsantiago0 , single line statement, even with "[bB][sS][dD]$" in it, makes my mind twist less than tracing all that variable spaghetti.

OK. I submitted a commit that uses a seperate variable for the lower case representation of the system name and use that for comparisons to support case variability.

ethouris · 2019-02-27T07:53:22Z

I have one more question: I can see lots of places which are enabled under conditions of OSX, TARGET_OS_IOS and TARGET_OS_TV, now BSD is added to this. Can't this be collected under one common label defined in cmake?

BTW. For the case when the apps have to be disabled per nonportable multicast entries, please make a separate issue for that.

… shorten the variable name as requested.

…ystems, but not in BSD based ones.

jlsantiago0 · 2019-02-27T18:45:57Z

I have one more question: I can see lots of places which are enabled under conditions of OSX, TARGET_OS_IOS and TARGET_OS_TV, now BSD is added to this. Can't this be collected under one common label defined in cmake?

I think that code can be changed to just check for #if defined(BSD) as it should be defined on any BSD derived OS. It is for OSX toolchain in my tests. I dont know about the toolchains for iOS, Watch, and AppleTV tho. So i will not make that change.

…g on OpenBSD.

jlsantiago0 · 2019-02-27T18:56:41Z

BTW. For the case when the apps have to be disabled per nonportable multicast entries, please make a separate issue for that.

Separate issue created #590

…n MATCHING the system name patterns so that we dont have to use regular expressions to handle case variability.

maxsharabayko

Just leaving the links for possible future references.

jlsantiago0 added 2 commits February 26, 2019 14:53

Fix BSD Builds. Builds and the SRT library on FreeBSD-11, TrueOS-12, …

b788e7f

…and OpenBSD-6.4 and my tests are working.

Use a better name for the variable to diable Support Applications.

04072e5

ethouris requested changes Feb 27, 2019

View reviewed changes

maxsharabayko added this to the v.1.3.3 milestone Feb 27, 2019

maxsharabayko added the [build] Area: Changes in build files label Feb 27, 2019

jlsantiago0 added 2 commits February 27, 2019 12:07

Switch variable from DISABLE to ENABLE support application builds and…

9aa39d4

… shorten the variable name as requested.

Simplify the feature.h check which should be in most unix operating s…

e585414

…ystems, but not in BSD based ones.

jlsantiago0 mentioned this pull request Feb 27, 2019

transmitmedia.cpp wont build on OpenBSD #590

Closed

Open a seperate issue for the sample/support applications not buildin…

42f367f

…g on OpenBSD.

Use a seperate variable for the lower case of the CMAKE_SYSTEM_NAME i…

5b86046

…n MATCHING the system name patterns so that we dont have to use regular expressions to handle case variability.

ethouris approved these changes Feb 28, 2019

View reviewed changes

maxsharabayko approved these changes Feb 28, 2019

View reviewed changes

rndi merged commit f865119 into Haivision:master Feb 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix BSD Builds. Builds and the SRT library on FreeBSD-11, TrueOS-12, … #588

Fix BSD Builds. Builds and the SRT library on FreeBSD-11, TrueOS-12, … #588

jlsantiago0 commented Feb 26, 2019 •

edited

Loading

ethouris Feb 27, 2019

maxsharabayko Feb 27, 2019

jlsantiago0 Feb 27, 2019

jlsantiago0 Feb 27, 2019

ethouris Feb 27, 2019

maxsharabayko Feb 27, 2019

jlsantiago0 Feb 27, 2019

jlsantiago0 Feb 27, 2019

rndi Feb 27, 2019

jlsantiago0 Feb 27, 2019

ethouris Feb 27, 2019

jlsantiago0 Feb 27, 2019 •

edited

Loading

jlsantiago0 Feb 27, 2019 •

edited

Loading

ethouris Feb 28, 2019

ethouris Feb 27, 2019

jlsantiago0 Feb 27, 2019

rndi Feb 27, 2019

jlsantiago0 Feb 27, 2019

ethouris commented Feb 27, 2019

jlsantiago0 commented Feb 27, 2019 •

edited

Loading

jlsantiago0 commented Feb 27, 2019

maxsharabayko left a comment •

edited

Loading

Fix BSD Builds. Builds and the SRT library on FreeBSD-11, TrueOS-12, … #588

Fix BSD Builds. Builds and the SRT library on FreeBSD-11, TrueOS-12, … #588

Conversation

jlsantiago0 commented Feb 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlsantiago0 Feb 27, 2019 • edited Loading

Choose a reason for hiding this comment

jlsantiago0 Feb 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethouris commented Feb 27, 2019

jlsantiago0 commented Feb 27, 2019 • edited Loading

jlsantiago0 commented Feb 27, 2019

maxsharabayko left a comment • edited Loading

Choose a reason for hiding this comment

jlsantiago0 commented Feb 26, 2019 •

edited

Loading

jlsantiago0 Feb 27, 2019 •

edited

Loading

jlsantiago0 Feb 27, 2019 •

edited

Loading

jlsantiago0 commented Feb 27, 2019 •

edited

Loading

maxsharabayko left a comment •

edited

Loading