Make it workable on OpenBSD #2690

Merged
merged 1 commit into from Mar 25, 2016

Projects

None yet

2 participants

@devnexen
Contributor
  • Additional LMMS_BUILD flag.
  • Disallow on plugins -Wl,-no-undefined which triggers undefined references.
  • Make sure X11 headers are found.
@tresf
Member
tresf commented Mar 20, 2016

No objections here...

I couldn't get a compile to fire due to such and old gcc version (4.2, failed due to the -std=c++-0x). What technique did you use to get on a modern gcc version? Also, OpenBSD 5.8 or 5.9?

Command log for reference:

#////// DEPS //////
su -
export PKG_PATH=http://ftp5.usa.openbsd.org/pub/OpenBSD/$(uname -r)/packages/$(machine -a)/
 # ^-- rit mirror, closest to me; placed in /root/.profile

pkg_add git cmake qt5 fftw3-float libogg libvorbis libsndfile libsamplerate jack sdl \
 fluidsynth fltk gcc-4.9.3p0 g++-4.9.3p0
 # ^-- couldn't find stk, but easy enough to dl/compile if needed
exit # or CTRL+D leave root

#////// BUILD //////
git clone -b master https://github.com/lmms/lmms
cd lmms; mkdir build target; cd build;

# gcc 4.9
cmake .. -DCMAKE_INSTALL_PREFIX=../target/ \
 -DCMAKE_C_COMPILER=egcc \
 -DCMAKE_CXX_COMPILER=eg++ \
 -DWANT_QT5=True \
 -DCMAKE_PREFIX_PATH=/usr/local/lib/qt5/cmake
 # ^-- pkgconfig couldn't find qt5, hence prefix

# -- OR --

# clang
cmake .. -DCMAKE_INSTALL_PREFIX=../target/ \
 -DCMAKE_C_COMPILER=clang \
 -DCMAKE_CXX_COMPILER=clang++ \
 -DWANT_QT5=True \
 -DCMAKE_PREFIX_PATH=/usr/local/lib/qt5/cmake
@devnexen
Contributor

There is either clang or gcc 4.9 (in this case compilers are egcc and eg++) port/package.

@devnexen
Contributor

As from kind @tresf request, and he mentioned the base compiler is far too old to compile even c++0x code ; hence this is how to test properly on OpenBSD (tested with the current stable), here is a little update of his command log reference

  • pkg_add git cmake qt5 fftw3-float libogg libvorbis libsndfile libsamplerate jack sdl fluidsynth fltk gcc-4.9.3p0 g++-4.9.3p0 (or the two last can be replaced by llvm which installs as well the clang frontends).
  • cmake .. -DCMAKE_INSTALL_PREFIX=../target/ -DCMAKE_C_COMPILER=(egcc or clang) -DCMAKE_CXX_COMPILER=(eg++ or clang++) -DWANT_QT5=True -DCMAKE_PREFIX_PATH=/usr/local/lib/qt5/cmake

Hope it is useful.

@tresf
Member
tresf commented Mar 21, 2016

@devnexen Much obliged. I'll try those this evening (western hemisphere) and post back. 👍

@tresf tresf and 1 other commented on an outdated diff Mar 22, 2016
plugins/zynaddsubfx/CMakeLists.txt
ADD_DEFINITIONS(-DOS_LINUX)
-ELSE(LMMS_BUILD_LINUX OR LMMS_BUILD_APPLE)
+ELSE(LMMS_BUILD_LINUX OR LMMS_BUILD_APPLE OR LMMS_BUILD_OPENBSD)
@tresf
tresf Mar 22, 2016 Member

Not incorrect, but this can be simplified to ELSE(), ENDIF(), etc.

@devnexen
devnexen Mar 22, 2016 Contributor

Is it correct with older version of cmake ?

@tresf
tresf Mar 22, 2016 Member

Is it correct with older version of cmake ?

Yes, this is a readability practice. e.g. if you have many nested IF()'s.

IF(FOO)
  MESSAGE("IN FOO")
  IF(BAR)
    MESSAGE("IN BAR")
  ENDIF(FOO)
ENDIF(BAR)

It's fine to leave, but we've been slowly cleaning them up in many places as we visit them as they start to loose their effectiveness when there are too many params.

IF(FOO OR BAR OR LOREM OR IPSUM OR DOLOR OR SIT OR AMET)
  MESSAGE("HELLO FOO BAR LOREM IPSUM DOLOR SIT AMET")
ENDIF(FOO OR BAR OR LOREM OR IPSUM OR DOLOR OR SIT OR AMET)

vs.

IF(FOO OR BAR OR LOREM OR IPSUM OR DOLOR OR SIT OR AMET)
  MESSAGE("HELLO FOO BAR LOREM IPSUM DOLOR SIT AMET")
ENDIF()
@devnexen
devnexen Mar 22, 2016 Contributor

fair point. updating.

@tresf tresf commented on the diff Mar 22, 2016
include/lmms_math.h
@@ -34,7 +34,7 @@
#include <cmath>
using namespace std;
-#if defined (LMMS_BUILD_WIN32) || defined (LMMS_BUILD_APPLE) || defined(LMMS_BUILD_HAIKU) || defined (__FreeBSD__)
+#if defined (LMMS_BUILD_WIN32) || defined (LMMS_BUILD_APPLE) || defined(LMMS_BUILD_HAIKU) || defined (__FreeBSD__) || defined(__OpenBSD__)
@tresf
tresf Mar 22, 2016 Member

Probably time for us to roll IF(NOT CMAKE_COMPILER_IS_GNUCC) ... here, as this list is getting quite long and AFAIK, these are all GNU compat helpers. Nope. 😈

@tresf
tresf Mar 22, 2016 Member

On second thought, LMMS_BUILD_WIN32 may still turn up as a GCC compiler, so that one might need to be added if Travis shows it failing once its removed. Nope. 😈

@tresf
tresf Mar 22, 2016 Member

Ok, I can't seem to find much reason for the discrepancies between compilers on these helpers but I still feel we shouldn't need platform-specific #ifdef's. Can we try removing the #ifdefs's and just letting this block run on all platforms? Travis will let us know if it causes any compile errors well before merging, but they all seem to check first, which should be safe on all platforms.

@tresf tresf commented on an outdated diff Mar 22, 2016
plugins/LadspaEffect/tap/CMakeLists.txt
@@ -11,7 +11,7 @@ FOREACH(_item ${PLUGIN_SOURCES})
ENDIF(LMMS_BUILD_WIN32)
IF(LMMS_BUILD_APPLE)
SET_TARGET_PROPERTIES("${_plugin}" PROPERTIES LINK_FLAGS "${LINK_FLAGS} -Bsymbolic -lm")
- ELSE(LMMS_BUILD_APPLE)
+ ELSEIF(NOT LMMS_BUILD_APPLE AND NOT LMMS_BUILD_OPENBSD)
@tresf
tresf Mar 22, 2016 Member

NOT LMMS_BUILD_APPLE is a redundant expression here.

@tresf
Member
tresf commented Mar 22, 2016

No major issues with the PR. Nice work. I've left a few comments where we could do things slightly better. Feedback welcome.

Test results with GCC 4.9 pass. I've updated my snippet above to reflect your suggestions.

Next is clang... From your dragon avatar, I can only assume that's your compiler of choice? :)

image

@tresf
Member
tresf commented Mar 22, 2016

Ok, so clang is a bit shotty at the moment... Without -stdlib=libc++:

/usr/local/include/X11/qt5/QtCore/qflags.h:43:10: fatal error: 'initializer_list' file not found
#include <initializer_list>

With -stdlib=libc++:

/usr/local/include/X11/qt5/QtCore/qcompilerdetection.h:890:11: fatal error: 'utility' file not found
# include <utility>
          ^
1 error generated.
*** Error 1 in . (plugins/flp_import/CMakeFiles/flpimport.dir/build.make:79 'plugins/flp_import/CMakeFiles/flpimport.dir/unrtf.cpp.o': cd /h...)
*** Error 2 in . (CMakeFiles/Makefile2:1189 'plugins/flp_import/CMakeFiles/flpimport.dir/all')
*** Error 2 in /home/openbsd/lmms-clang/build (Makefile:137 'all')

That's as far as I got. If you're ok leaving out support for clang for now, so are we. 👍

@devnexen
Contributor

Hi and thanks for the feedback.

I admit for clang I tested with OpenBSD current which is clang 3.8, I think with OpenBSD 5.8 it is 3.5 so we can leave it for the moment.
About the ifdef I think it is because INT32_MAX was already defined if I remember well on OpenBSD.

@tresf tresf commented on an outdated diff Mar 25, 2016
plugins/zynaddsubfx/CMakeLists.txt
@@ -3,12 +3,12 @@ INCLUDE(BuildPlugin)
# definitions for ZynAddSubFX
IF(LMMS_BUILD_LINUX OR LMMS_BUILD_APPLE OR LMMS_BUILD_OPENBSD)
- FIND_PACKAGE(X11)
@tresf
tresf Mar 25, 2016 Member

Formatting looks slightly off here.

@tresf tresf commented on an outdated diff Mar 25, 2016
plugins/zynaddsubfx/CMakeLists.txt
ADD_DEFINITIONS(-DOS_LINUX)
-ELSE()
@tresf
tresf Mar 25, 2016 Member

Hmmm.. I'm restarting the Travis job... this just doesn't make any sense. We use this cmake syntax all over the place.

@tresf
tresf Mar 25, 2016 Member

Yeah, something strange with Travis was happening, it's passing now after resubmitting it, so the ELSE(FOO) versus ELSE() syntax change was perfectly legal.

image

@tresf
Member
tresf commented Mar 25, 2016

@devnexen thanks again for this. If you could touch up the formatting and squash your commits e.g. git rebase -i HEAD~6; git push --force (where 6 is the number of commits in the PR -- there's 5 now, assuming you'll add one more) then we'll have this merged. 👍

@devnexen devnexen Make it workable on OpenBSD
- Additional LMMS_BUILD flag.
- Disallow on plugins -Wl,-no-undefined which triggers undefined references.
- Make sure X11 headers are found.

Lib ossaudio is needed only for OpenBSD

redundant expression removal

simplify condition for detection OS 'kind'

seems the last commit brought an issue on OSx travis test ....
f4890ec
@devnexen
Contributor

Ok thanks for your patience :)

@tresf tresf merged commit 770c07f into LMMS:master Mar 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment