Author Topic: Unhappy with fixes for mac-compilation  (Read 4144 times)

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Unhappy with fixes for mac-compilation
« on: May 03, 2015, 12:03:40 pm »
I'm unhappy with some of the most recent patches for mac:
The first one https://cgit.jenslody.de/codeblocks/commit/?id=632970a8d8c4c0b798960c7152bb6c0a57714c1d doubles code for wx_osx, but only for hunspell, what about the other conditionals ?
Does it work, if we use the same conditionals for wx_gtk, wx_osx, wx_mac, etc ?
Code: diff
diff --git a/configure.ac b/configure.ac
index f0ebdd4..dbaf6e3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -237,6 +237,27 @@ case $WX_BASENAME in
AC_CHECK_PROG(SETFILE, SetFile, SetFile, /Developer/Tools/SetFile)
MACSETFILE="\$(SETFILE)"
;;
+ *wx_osx*)
+ AC_MSG_RESULT(wxOSX)
+
+ if test "x$BUILD_SPELLCHECKER_TRUE" = "x" ; then
+
+ AC_CHECK_PROG(HAVE_PKG_CONFIG, pkg-config, yes, no)
+ ifdef([PKG_CHECK_MODULES],[],[
+ define([PKG_CHECK_MODULES],
+ [ echo "You don't have pkg.m4 properly installed" >&2
+ exit 1
+ ])
+ ])
+ PKG_PROG_PKG_CONFIG
+
+ if test "x$HAVE_PKG_CONFIG" = "xyes"; then
+ PKG_CHECK_MODULES([HUNSPELL], [hunspell])
+ fi
+ fi
+ CB_HUNSPELL_CFLAGS="$HUNSPELL_CFLAGS"
+ CB_HUNSPELL_LIBS="$HUNSPELL_LIBS"
+ ;;
*)
AC_MSG_RESULT(other)
;;
This one removes flags that are not supported by clang (on mac?, which version). If I remember correctly, they are no problem on linux with clang.
A proper solution would be to remove them only for compilers, that do not support them.
https://cgit.jenslody.de/codeblocks/commit/?id=9613c554588da2d4851d6de9c16a0b5bce8b579c
Code: diff
diff --git a/acinclude.m4 b/acinclude.m4
index 1a6d145..ed0820b 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -69,12 +69,10 @@ AC_ARG_ENABLE(debug, [AC_HELP_STRING([--enable-debug], [turn on debugging (defau
if test "x$enable_debug" = "xyes"; then
CFLAGS="-g -DDEBUG -DCB_AUTOCONF $CFLAGS"
CXXFLAGS="-g -DDEBUG -DCB_AUTOCONF $CXXFLAGS"
- LDFLAGS="-Wl,--no-undefined"
AC_MSG_RESULT(yes)
else
CFLAGS="-O2 -ffast-math -DCB_AUTOCONF $CFLAGS"
CXXFLAGS="-O2 -ffast-math -DCB_AUTOCONF $CXXFLAGS"
- LDFLAGS="-Wl,--no-undefined"
AC_MSG_RESULT(no)
fi
])
I do not have a recent mac to test, I only have SnowLeopard on a VM, so that's the only stuff I can test.
But Apple changed/restricted so much in the last years (as far as I know), that it might work here, but not with newer releases.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Unhappy with fixes for mac-compilation
« Reply #1 on: May 03, 2015, 12:13:27 pm »
See this https://sourceforge.net/p/codeblocks/tickets/9/#e4a8 for the no-undefined issue.

The person that has reporting the problems for osx is very active and can test patches on osx.
So if you can think of changes that are better just post patches in the respective tickets and he'll happily test them, I think.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: Unhappy with fixes for mac-compilation
« Reply #2 on: May 03, 2015, 03:05:50 pm »
Well the rationale to apply the first patch was:
We cannot test them as we don't have Mac devs.
It works obviously as stated in the mentioned ticket.
It does not harm the other build process

The rationale to apply the second patch them was:
Its not something that we need at runtime, its a check to verify that we did actually nothing wrong. So the correct way would be to fix the build in the first place and not build something that throws when you did it wrong. Its basically a Linux only thing, as on Windows this is of on use, too.
So to "re-implement" it I would encapsulate it into a Linux-only environment. However, the second remark in the ticket (that it overrides LDFLAGS completely) is certainly true. So there was an error in doing it the way it was anyway.

Also, on recent Mac's there is no GCC any longer, so sooner or later we have to face clang or basically abandon Mac. Itsnot a nice situation, but if Mac people find out that something works I think we should seriously consider to apply it... at least if it does no harm to the rest. :-)
« Last Edit: May 03, 2015, 03:07:21 pm by MortenMacFly »
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: Unhappy with fixes for mac-compilation
« Reply #3 on: May 03, 2015, 07:46:56 pm »
This patch should (hopefully) fix it correctly (without overriding LDFLAGS):

diff removed
« Last Edit: May 04, 2015, 12:23:48 am by jens »

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: Unhappy with fixes for mac-compilation
« Reply #4 on: May 03, 2015, 07:54:03 pm »
Well the rationale to apply the first patch was:
We cannot test them as we don't have Mac devs.
It works obviously as stated in the mentioned ticket.
It does not harm the other build process
But it blows up the code (or autoconf-files in this case) and is probably not needed, or we should add am own check-function for all the stuff we check in the wx_gtk-section.
This should be not so hard and less error-prone.

The rationale to apply the second patch them was:
Its not something that we need at runtime, its a check to verify that we did actually nothing wrong. So the correct way would be to fix the build in the first place and not build something that throws when you did it wrong. Its basically a Linux only thing, as on Windows this is of on use, too.
So to "re-implement" it I would encapsulate it into a Linux-only environment. However, the second remark in the ticket (that it overrides LDFLAGS completely) is certainly true. So there was an error in doing it the way it was anyway.

Also, on recent Mac's there is no GCC any longer, so sooner or later we have to face clang or basically abandon Mac. Itsnot a nice situation, but if Mac people find out that something works I think we should seriously consider to apply it... at least if it does no harm to the rest. :-)

Clang is indeed an option, no matter what Apple does.
But much more important is to get the wx3 stuff working because at least Debian 8 (actual stable) does no longer provide wx2.8 and I guess more distributions will follow (sooner or later).

So we should try to make a release as soon as possible and seriously start fixing the wx3 issues.

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: Unhappy with fixes for mac-compilation
« Reply #5 on: May 04, 2015, 12:26:20 am »
New patch, that includes the check-link macro, as it is not installed on all platforms, requires at least autoconf 2.64:
Code: diff
commit ed4eaf496c3c1297119d3888ecc3152d32bcb458
Author: None <github@jenslody.de>
Date:   Mon May 4 00:12:48 2015 +0200

    - check whether the linker accepts "--no-undefined", should fix https://sourceforge.net/p/codeblocks/tickets/9 correctly

Index: acinclude.m4
===================================================================
--- acinclude.m4
+++ acinclude.m4
@@ -59,6 +59,23 @@
 AM_CONDITIONAL(CODEBLOCKS_DARWIN, test x$darwin = xtrue )
 ])
 
+dnl copied and renamed from autoconf-archive AX_CHECK_LINK_FLAG
+AC_DEFUN([CODEBLOCKS_CHECK_LINK_FLAG],
+[AC_PREREQ(2.64)dnl for _AC_LANG_PREFIX and AS_VAR_IF
+AS_VAR_PUSHDEF([CACHEVAR],[ax_cv_check_ldflags_$4_$1])dnl
+AC_CACHE_CHECK([whether the linker accepts $1], CACHEVAR, [
+  ax_check_save_flags=$LDFLAGS
+  LDFLAGS="$LDFLAGS $4 $1"
+  AC_LINK_IFELSE([m4_default([$5],[AC_LANG_PROGRAM()])],
+    [AS_VAR_SET(CACHEVAR,[yes])],
+    [AS_VAR_SET(CACHEVAR,[no])])
+  LDFLAGS=$ax_check_save_flags])
+AS_VAR_IF(CACHEVAR,yes,
+  [m4_default([$2], :)],
+  [m4_default([$3], :)])
+AS_VAR_POPDEF([CACHEVAR])dnl
+])dnl CODEBLOCKS_CHECK_LINK_FLAG
+
 dnl check whether to enable debugging
 AC_DEFUN([CODEBLOCKS_CHECK_DEBUG],
 [
Index: configure.ac
===================================================================
--- configure.ac
+++ configure.ac
@@ -21,8 +21,6 @@
 AM_CONDITIONAL([LINUX], [test `uname` = "Linux"])
 AM_CONDITIONAL([DARWIN], [test `uname` = "Darwin"])
 
-CODEBLOCKS_CHECK_DEBUG
-
 dnl Checks for programs.
 AC_PROG_CXX
 AC_PROG_CPP
@@ -33,6 +31,10 @@
 AC_PROG_MAKE_SET
 AC_PROG_AWK
 
+CODEBLOCKS_CHECK_LINK_FLAG([-Wl,--no-undefined],[LDFLAGS="-Wl,--no-undefined $LDFLAGS"])
+
+CODEBLOCKS_CHECK_DEBUG
+
 CODEBLOCKS_SETUP_FOR_TARGET
 
 AC_DISABLE_STATIC