LMS-Community / slimserver-vendor

Third-party software used with Lyrion Music Server
https://lyrion.org
42 stars 68 forks source link

Feature/icu 59.1 #26

Closed fsbruva closed 7 years ago

fsbruva commented 7 years ago

Good evening-

Despite the petulant end to the discussion (and in response to the lack of proposed patch) for Issue #16 , this will fast forward the ICU library to version 59.1. The collation data files use the CLDR rule syntax which is compatible across ICU versions, so they were simply renamed as part of the upgrade. Additionally, the FreeBSD library names are created (correctly) automatically, so the symlinks are no longer necessary.

I'd request this be tested on Linux, Mac OS and Windows. If compatibility issues arise, I am happy to revise the branch to make the new version of ICU conditional on FreeBSD for versions 11 and up.

Sincerely, Matt

fsbruva commented 7 years ago

More research and debugging required.

michaelherger commented 6 years ago

@fsbruva - we have a problem here. I can't get ICU 59 to build on eg. Ubuntu 17.10. At first I thought your check failed, as it's running gcc 7.2.0 (your check requires 4.2). But disabling the check still fails the build:

ICU for C/C++ 58.2 is ready to be built.
=== Important Notes: ===
Data Packaging: archive
 This means: ICU data will be stored in a single .dat file.
 To locate data: ICU will look in /home/rock64/git/slimserver-vendor/CPAN/build/share/icu/58.2 which is the installation location. Call u_setDataDirectory() or use the ICU_DATA environment variable to override.
Building ICU: Use a GNU make such as make to build ICU.
checking the version of "make"... 4.1 (we wanted at least 3.80)
ok
C++ apps may want to build with CXXFLAGS =  --std=c++0x

If the result of the above commands looks okay to you, go to the directory
source in the ICU distribution to build ICU. Please remember that ICU needs
GNU make to build properly...
rebuilding config/icucross.mk
rebuilding config/icucross.inc
cd ./config; \
    /usr/bin/make -f pkgdataMakefile
make[1]: Entering directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/config'
rm -rf pkgdata.inc
make[1]: Leaving directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/config'
rm -rf config/icu-config
/usr/bin/install -c ./config/icu-config-top config/icu-config
chmod u+w config/icu-config
LC_ALL=C sed -f ./config/make2sh.sed < ./config/Makefile.inc | grep -v '#M#' | uniq >> config/icu-config
LC_ALL=C sed -f ./config/make2sh.sed < ./config/mh-linux | grep -v '#M#' | uniq >> config/icu-config
cat ./config/icu-config-bottom >> config/icu-config
chmod u-w config/icu-config
config/icu-uc.pc updated.
config/icu-i18n.pc updated.
config/icu-io.pc updated.
Note: rebuild with "/usr/bin/make VERBOSE=1 " to show all compiler parameters.
/usr/bin/make[0]: Making `all' in `stubdata'
make[1]: Entering directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/stubdata'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/stubdata'
/usr/bin/make[0]: Making `all' in `common'
make[1]: Entering directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/common'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/common'
/usr/bin/make[0]: Making `all' in `i18n'
make[1]: Entering directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/i18n'
   g++   ...  digitlst.cpp
digitlst.cpp:67:13: fatal error: xlocale.h: No such file or directory
 #   include <xlocale.h>
             ^~~~~~~~~~~
compilation terminated.
*** Failed compilation command follows: ----------------------------------------------------------
g++ -D_REENTRANT -DU_HAVE_ELF_H=1 -DU_HAVE_ATOMIC=1 -DU_HAVE_STRTOD_L=1 -I. -I../common -DU_ATTRIBUTE_DEPRECATED= -DU_I18N_IMPLEMENTATION -O3 -fPIC -DU_USING_ICU_NAMESPACE=0 -W -Wall -pedantic -Wpointer-arith -Wwrite-strings -Wno-long-long --std=c++0x -c -o digitlst.ao digitlst.cpp
--- ( rebuild with "/usr/bin/make VERBOSE=1 all" to show all parameters ) --------
../config/mh-linux:49: recipe for target 'digitlst.ao' failed
make[1]: *** [digitlst.ao] Error 1
make[1]: Leaving directory '/home/rock64/git/slimserver-vendor/CPAN/icu/source/i18n'
Makefile:143: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 2
make failed

Could you please try to make this part optional? Or fix the build parameters required to make this work?...

fsbruva commented 6 years ago

59? Or 58.2?

I withdrew this because of compiler issues.

fsbruva commented 6 years ago

I see the error capture has 58.2. I am installing gcc 7.3 in freebsd, and am spooling up an Ubuntu 17.10 vm now.

mherger commented 6 years ago

The check fails because of changed parameters for gcc: -dumpversion only returns the major number. See https://stackoverflow.com/questions/45168516/gcc-7-1-1-on-fedora-26-dumpversion-now-only-includes-major-version-by-default. The following change would pass the test, and should be backwards compatible with older versions:

diff --git a/CPAN/buildme.sh b/CPAN/buildme.sh
index a4b5abb..8f2c219 100755
--- a/CPAN/buildme.sh
+++ b/CPAN/buildme.sh
@@ -160,7 +160,7 @@ if [[ "$CC_TYPE" =~ "clang" ]]; then
     CC_IS_CLANG=true
 elif [[ "$CC_TYPE" =~ "gcc" || "$CC_TYPE" =~ "GCC" ]]; then
     CC_IS_GCC=true
-    CC_VERSION=`$GCC -dumpversion | sed "s#\ *)\ *##g" | sed -e 's/\.\([0-9][0-9]\)/\1/g' -e 's/\.\([0-9]\)/0\1/g' -e 's/^[0-9]\{3,4\}$/&00/'`
+    CC_VERSION=`$GCC -dumpfullversion -dumpversion | sed "s#\ *)\ *##g" | sed -e 's/\.\([0-9][0-9]\)/\1/g' -e 's/\.\([0-9]\)/0\1/g' -e 's/^[0-9]\{3,4\}$/&00/'`
 else
     echo "********************************************** ERROR ***************************************"
     echo "*"

Build is still running...

fsbruva commented 6 years ago

Glad you tracked that down.

Using public/7.9, ICU builds with GCC 7.3.0 on FreeBSD amd64. I wonder if the FreeBSD patches might fix the build issue you documented? I am having a little trouble getting the Ubuntu virtualmachine working to replicate your build problem.

fsbruva commented 6 years ago

Seems to be an Ubuntu issue, related to the fact they removed xlocale.h from glibc 2.26.

It seems that using "locale.h" works just fine. However, I have no idea how many places within the ICU source tree mention xlocale.h.

The problem is that the test about whether or not to use xlocale is inferred, based on platform: If it's Windows or Cygwin, use regular locale.h, otherwise, use xlocale.h. I looked at the latest version of ICU (60.2), and the test is corrected to say, If xlocale exists, use it - otherwise use locale.

The downside of upping the ICU version (again):

  1. If you compiled binaries that use 58.2, the 58.dat files would need to persist (forever) in the -server source tree,
  2. The minimum compiler version for GCC would be come 4.8, because we would need support for c++11.

The upside of upping the ICU version:

  1. We don't chase our tails on this problem as other platforms have xlocale issues.
  2. We raise the minimum compiler version above gcc 4.2.1, which resolves a bunch of known issues with later versions of FFmpeg.
michaelherger commented 6 years ago

A symlink to /usr/include/locale.h fixed that missing xlocale.h issue.