Bug 213768 - editors/libreoffice: Fix build with lang/gcc5 on 11.0-RELEASE
Summary: editors/libreoffice: Fix build with lang/gcc5 on 11.0-RELEASE
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: i386 Any
: --- Affects Only Me
Assignee: FreeBSD Office Team
URL:
Keywords: easy, needs-qa, patch
Depends on:
Blocks:
 
Reported: 2016-10-25 12:37 UTC by Kenneth Salerno
Modified: 2017-11-04 22:35 UTC (History)
4 users (show)

See Also:
koobs: maintainer-feedback? (office)
koobs: merge-quarterly?


Attachments
remove extra curly brace in patch (1.97 KB, patch)
2016-10-25 12:37 UTC, Kenneth Salerno
no flags Details | Diff
remove extra curly brace in patch (1.91 KB, patch)
2016-10-25 13:39 UTC, Kenneth Salerno
no flags Details | Diff
adding patch for include/tools/stream.hxx (496 bytes, patch)
2016-10-25 13:40 UTC, Kenneth Salerno
no flags Details | Diff
patch set to build editor/libreoffice with lang/gcc5 (3.48 KB, patch)
2016-10-26 00:49 UTC, Kenneth Salerno
no flags Details | Diff
patch set to build editor/libreoffice with lang/gcc5 (3.48 KB, patch)
2016-10-26 00:50 UTC, Kenneth Salerno
no flags Details | Diff
devel/liborcus patch to build with lang/gcc5 (945 bytes, patch)
2016-10-26 00:53 UTC, Kenneth Salerno
no flags Details | Diff
My make.conf used for ports, world and kernel compiled with GCC5.x (world's base.txz was used to build my poudriere jail) (4.63 KB, text/plain)
2016-10-26 19:13 UTC, Kenneth Salerno
no flags Details
patch to successfully build editor/libreoffice with lang/gcc5 (424 bytes, patch)
2016-10-27 20:57 UTC, Kenneth Salerno
no flags Details | Diff
make.conf for those who want everything built with GCC 5.x (5.11 KB, text/plain)
2016-10-27 21:00 UTC, Kenneth Salerno
no flags Details
fix for lang/gcc5 (396 bytes, patch)
2016-10-29 13:43 UTC, Kenneth Salerno
no flags Details | Diff
Fixes for lang/gcc5 (4.38 KB, patch)
2016-10-31 13:06 UTC, Kenneth Salerno
no flags Details | Diff
Fixes for lang/gcc5 (4.20 KB, patch)
2016-11-02 11:32 UTC, Kenneth Salerno
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Salerno 2016-10-25 12:37:35 UTC
Created attachment 176138 [details]
remove extra curly brace in patch

Patch from r395799 breaks lang/gcc5 build:

------------------------------------------------------------------------
r395799 | jkim | 2015-09-01 21:26:44 +0000 (Tue, 01 Sep 2015) | 9 lines

Update LibreOffice to 5.0.1.

Unfortunately, LibreOffice project completely dropped support for pre-C++11
compilers and libraries since 4.4.  Therefore, we cannot easily build it on
FeeBSD 9.x any more unless the system was rebuilt with WITH_CLANG_IS_CC and
WITH_LIBCPLUSPLUS.  If user is unable to upgrade the system for some reason,
the old port, i.e., 4.3.7, is still available from devel/libreoffice4 (with
no language packs).

------------------------------------------------------------------------

Specifically, there was an extra '}' added in the patch that caused this error when compiling libreoffice with gcc5:

In file included from /wrkdirs/usr/ports/editors/libreoffice/work/libreoffice-5.
0.6.3/bridges/source/cpp_uno/gcc3_linux_intel/cpp2uno.cxx:33:0:
/wrkdirs/usr/ports/editors/libreoffice/work/libreoffice-5.0.6.3/bridges/source/c
pp_uno/gcc3_linux_intel/share.hxx:131:1: error: expected declaration before '}' 
token
 }
 ^

patch attached to patch the patch :)
Comment 1 Kenneth Salerno 2016-10-25 13:39:40 UTC
Created attachment 176139 [details]
remove extra curly brace in patch
Comment 2 Kenneth Salerno 2016-10-25 13:40:22 UTC
Created attachment 176140 [details]
adding patch for include/tools/stream.hxx
Comment 3 Kenneth Salerno 2016-10-25 13:45:48 UTC
Adding patch to include "stdio.h" for missing "EOF" and *printf declarations that should be included in include/tools/stream.hxx.

(I also edited original patch submitted to not have absolute path to my "/usr/local/poudriere/ports/local" in diff.)
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2016-10-25 13:49:37 UTC
Thank you for your report and patches Kenneth.

Could you please:

1) Generate the patch to sources (include/tools/stream.hxx) using the method (makepatch) outlined below, specifically 4.4. Patching:

https://www.freebsd.org/doc/en/books/porters-handbook/slow-patch.html

This will automatically produce a correctly named patch-* file in the files/ directory

2) Replace these two individual changes with a single unified diff (preferably `svn diff`) against the port directory.

3) Confirm these changes pass QA (portlint and poudriere). For details and instructions see: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/testing.html
Comment 5 Jung-uk Kim freebsd_committer freebsd_triage 2016-10-25 16:51:00 UTC
I'll take it.
Comment 6 Jung-uk Kim freebsd_committer freebsd_triage 2016-10-25 20:08:36 UTC
(In reply to Kenneth Salerno from comment #2)
Can you please show me error messages without the patch for stream.hxx?
Comment 7 Kenneth Salerno 2016-10-26 00:49:11 UTC
Created attachment 176168 [details]
patch set to build editor/libreoffice with lang/gcc5

Requires you build devel/liborcus with gcc5 as well (patch attached here for those who are curious). Note that trying to build devel/boost-libs with gcc5 is a losing battle so don't bother.
Comment 8 Kenneth Salerno 2016-10-26 00:50:55 UTC
Created attachment 176169 [details]
patch set to build editor/libreoffice with lang/gcc5

Requires you build devel/liborcus with gcc5 as well (patch attached here for those who are curious). Note that trying to build devel/boost-libs with gcc5 is a losing battle so don't bother.
Comment 9 Kenneth Salerno 2016-10-26 00:52:16 UTC
One of the patches is brute-force because I couldn't figure out why /usr/local/lib/gcc5/include/c++/string wasn't being preferred over another version somewhere else. Need help making that more elegant/correct.
Comment 10 Kenneth Salerno 2016-10-26 00:53:17 UTC
Created attachment 176170 [details]
devel/liborcus patch to build with lang/gcc5

Unpatched, liborcus fails to detect -lboost_program_options
Comment 11 Kenneth Salerno 2016-10-26 14:21:58 UTC
(In reply to Kenneth Salerno from comment #10)
Just a little more detail about this patch: the actual failure comes from the -lboost_program_options link test conftest.cpp of autoconf's `configure'.

The issue isn't that ld on conftest.o link doesn't find /usr/local/lib/libboost_program_options.so, the problem is yet another C++ header include issue/grief of using lang/gcc5 gnu++11 /usr/local/lib/gcc5/include on a std=c++11 /usr/include system. There is an undefined function logged in config.log and configure bombs. The attached patch is a brute-force fix that technically doesn't break anything for clang or other arches, but it doesn't get to the root of the issue.

What I would like to find out is what -I flags and -D flags I should add to CXXFLAGS in my make.conf to fix this fragile C++ environment.
Comment 12 Kenneth Salerno 2016-10-26 14:29:45 UTC
(In reply to Jung-uk Kim from comment #6)
Hello. Yes, sure.

I apologize for not adding a note about this as this patch is ambiguous! The patch to add "#include <stdio.h>" to stream.hxx is not to fix that particular header - the issue is in the 4 or 5 *.cxx files that _include_ stream.hxx. They require a definition of *printf functions and "EOF".

Not sure where clang++'s system includes are pulling in stdio.h, but without explicitly including it for g++5, my build failed.
Comment 13 Kenneth Salerno 2016-10-26 14:32:01 UTC
(In reply to Kenneth Salerno from comment #12)
(i.e. I would rather patch stream.hxx once than patch the other 5 individual *.cxx files.)
Comment 14 Jung-uk Kim freebsd_committer freebsd_triage 2016-10-26 18:27:50 UTC
I just committed a share.hxx fix.  However, GCC is not supported ATM.  In fact, it will not be easy to support GCC5 without ugly hacks.  Re-assign to office@.
Comment 15 Kenneth Salerno 2016-10-26 19:12:14 UTC
(In reply to Jung-uk Kim from comment #14)
Thank you. I know gcc5 isn't officially supported to compile ports, it just produces much better binaries compared to clang (anyone impressed with how fast clang can produce binaries should think about how many optimization passes its missing compared with gcc...).

I'm attaching my make.conf so you see how I needed to get around certain issues without patching any of the other ports besides devel/liborcus and editors/libreoffice to successfully build 1,065 ports with lang/gcc5.
Comment 16 Kenneth Salerno 2016-10-26 19:13:44 UTC
Created attachment 176197 [details]
My make.conf used for ports, world and kernel compiled with GCC5.x (world's base.txz was used to build my poudriere jail)
Comment 17 Kenneth Salerno 2016-10-27 02:42:44 UTC
(In reply to Jung-uk Kim from comment #14)
The patches attached to this bug report will build libreoffice successfully, runs great actually. However I understand having made it work and supporting it are two different things.

But I agree my fix to assertion_traits.cxx and formulagroupcl.cxx are ugly, especially my if !clang then gcc statement... Sorry about that, just don't know how to make a proper test for g++5 there.
Comment 18 Kenneth Salerno 2016-10-27 20:57:07 UTC
Created attachment 176227 [details]
patch to successfully build editor/libreoffice with lang/gcc5

Great news! I was able to build and test libreoffice compiled with lang/gcc5 with only one small change now. I figured out a better way to have g++5 declare std::to_string and the various things from stdio.h without touching libreoffice code at all.
Comment 19 Kenneth Salerno 2016-10-27 21:00:17 UTC
Created attachment 176228 [details]
make.conf for those who want everything built with GCC 5.x

In the event my new proposed patch to editors/libreoffice/Makefile does not get approval, I am attaching my make.conf for those who find this bug on the web and want to get it working on their own.
Comment 20 Jung-uk Kim freebsd_committer freebsd_triage 2016-10-27 21:07:47 UTC
(In reply to Kenneth Salerno from comment #18)
This patch is even worse, i.e., unconditionally include stdio.h for all C++ sources.  That is not acceptable even if it lets you compile LibreOffice.  BTW, C++ sources should include cstdio, not stdio.h.
Comment 21 Kenneth Salerno 2016-10-27 21:44:36 UTC
(In reply to Jung-uk Kim from comment #20)
Ok, I could test with -include cstdio instead. It should already be included for every preprocess, that's one of the two major problems here with the gcc5 port.

The first problem is that the lang/gcc5 includes for C++ seem to be missing a lot of things the system /usr/include/c++/v1 have, things that should be brought in as internal includes for named includes in the source, which is why I need to explicitely include stdio/cstdio here.

The second problem is g++5 expects C99 defined to enter the include bodies, which is addressed in the patch as well.

If the supported compiler for x86 ports is clang, what's the harm in this patch as part of the if !clang? At worst it's redundant for other arches with a supported g++.
Comment 22 Kenneth Salerno 2016-10-29 13:43:50 UTC
Created attachment 176274 [details]
fix for lang/gcc5

1. converted stdio.h to cstdio.

2. Note this was added to the 'else' of the condition of using clang:
   .if ${COMPILER_TYPE} == "clang"
   By default, no one on x86 using a supported configuration even sees this.
Comment 23 Kenneth Salerno 2016-10-30 03:17:38 UTC
(In reply to Jung-uk Kim from comment #20)
I gave it some thought and realized I am being lazy and should submit a patch to the individual files only where necessary - it is only in five places anyway.

You are right that I should not ram cstdio into every source file, it could even potentially increase the executable size if the linker is not discarding unused symbols.

I will also make sure to surround anything I am adding in #if (__GNUC__ == 5 && __GNUC_MINOR__ == 4)
Comment 24 Kenneth Salerno 2016-10-31 13:06:27 UTC
Created attachment 176341 [details]
Fixes for lang/gcc5
Comment 25 Jung-uk Kim freebsd_committer freebsd_triage 2016-11-01 19:54:47 UTC
Please see:

https://lists.freebsd.org/pipermail/freebsd-ports/2016-October/105590.html

I think it is wrong to hack every port for lang/gcc5 bug.
Comment 26 Gerald Pfeifer freebsd_committer freebsd_triage 2016-11-01 20:26:06 UTC
(In reply to Kenneth Salerno from comment #23)
> #if (__GNUC__ == 5 && __GNUC_MINOR__ == 4)

If a condition like this shall be used, I suggest to omit the second
part or any version update of lang/gcc5 will break things again.
Comment 27 Kenneth Salerno 2016-11-01 21:39:54 UTC
(In reply to Jung-uk Kim from comment #25)
I figured as much, but in building 1,065 ports with gcc5, I only ran into an issue with 15 of them in total (two of which are not even related to this C99 issue). I suppose that just means the vast majority of ports are written in C and not C++, and the ones that are do not conform c++11 anyway.

> pkg stat | head -3
Local package database:
        Installed packages: 1066
        Disk space occupied: 8 GiB

> ls ports-patches/
patch-avogadro-kpsalerno-gcc5
patch-boost-libs-kpsalerno-gcc5
patch-brasero-kpsalerno-gcc5
patch-firefox-kpsalerno-gcc5
patch-gegl-kpsalerno-gcc5
patch-ghostscript9-agpl-base-kpsalerno-gcc5
patch-gnome-desktop-kpsalerno-gcc5
patch-jsoncpp-kpsalerno-gcc5
patch-kmplot-kpsalerno-gcc5
patch-libGL-kpsalerno-gcc5
patch-libreoffice-kpsalerno-gcc5
patch-marble-kpsalerno-gcc5
patch-podofo-kpsalerno-gcc5
patch-scribus-kpsalerno-gcc5
patch-webkit2-gtk3-kpsalerno-gcc5
Comment 28 Kenneth Salerno 2016-11-02 11:32:44 UTC
Created attachment 176418 [details]
Fixes for lang/gcc5

Tested lang/gcc6 and confirmed C99 define bug only exists with lang/gcc5, though still need cstdio even for lang/gcc6.

Removed __GNUC_MINOR__ test and made adjustments for lang/gcc6 as well.
Comment 29 Curtis Hamilton 2016-12-10 15:24:08 UTC
I've discovered instead of patching multiple files, it's easier to make the below change in the Makefile.

Change from:

.if ${COMPILER_FEATURES:Mlibstdc++}
BROKEN=		Build with system libstdc++ is unsupported
.endif

Change to:

.if ${COMPILER_FEATURES:Mlibstdc++}
MAKE_ENV+=	WITH_LIBCPLUSPLUS=yes
CXXFLAGS+=	-D_GLIBCXX_USE_C99=1
.endif

This worked for me and I was able to buld LO5 (r426579 2016-11-20).
Comment 30 Gerald Pfeifer freebsd_committer freebsd_triage 2017-11-04 22:35:28 UTC
We got this addressed upstream in GCC 6 and later, and since the default
version of GCC used in the FreeBSD Ports Collection is now GCC 6, this
problem has thus been addressed -- both in GCC and by using a newer version
of GCC.