Bug 215675 - graphics/rawtherapee: more cleanup
Summary: graphics/rawtherapee: more cleanup
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Matthias Andree
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2016-12-30 20:20 UTC by Jan Beich
Modified: 2017-01-15 13:21 UTC (History)
1 user (show)

See Also:
mandree: maintainer-feedback+


Attachments
v0 (9.23 KB, patch)
2016-12-30 20:20 UTC, Jan Beich
no flags Details | Diff
v0 (9.23 KB, patch)
2016-12-30 20:24 UTC, Jan Beich
no flags Details | Diff
v0.1 (9.30 KB, patch)
2016-12-30 20:44 UTC, Jan Beich
mandree: maintainer-approval-
Details | Diff
v0.2 (10.47 KB, patch)
2016-12-31 11:33 UTC, Jan Beich
no flags Details | Diff
v0.3 (10.25 KB, patch)
2017-01-06 10:01 UTC, Jan Beich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2016-12-30 20:20:06 UTC
Created attachment 178405 [details]
v0

- Fix Clang build (via upstream)
- Fix sound theme support via libcanberra
- Pacify stage-qa but prune unused deps via -Wl,--as-needed
- Apply -mtune=generic only when CPUTYPE isn't defined
- Convert OPTIMIZED_CFLAGS to option helpers but split off SSE
- Drop obsolete files and flags
- Convert to USES=localbase
- Sort LIB_DEPENDS
Comment 1 Jan Beich freebsd_committer freebsd_triage 2016-12-30 20:24:10 UTC
Created attachment 178406 [details]
v0

Typo in ${CFLAGS:M...}
Comment 2 Jan Beich freebsd_committer freebsd_triage 2016-12-30 20:41:46 UTC
Comment on attachment 178406 [details]
v0

Build logs:
93i386 - http://sprunge.us/SdeN
93amd64 - http://sprunge.us/cjDR
101i386 - http://sprunge.us/hfJc
103amd64 - http://sprunge.us/YYXA
110i386 - http://sprunge.us/JVff
110amd64 - http://sprunge.us/XOgj
head-amd64 - http://sprunge.us/hAEI
clang34 - http://sprunge.us/GeJf
Comment 3 Jan Beich freebsd_committer freebsd_triage 2016-12-30 20:44:54 UTC
Created attachment 178407 [details]
v0.1

Add missing SSE2_DESC
Comment 4 Matthias Andree freebsd_committer freebsd_triage 2016-12-31 10:04:57 UTC
I think C++11 is a universal requirement on RawTherapee, and OpenMP is an additional requirement that rules out our base compiler on 10.3, clang 3.4.

The other hard requirement is that the libc++ must match the default Standard C++ and Template Libraries of the system as a whole (due to ABI)

Note that I will pull the plug on support for FreeBSD < 10.3 tomorrow for all my ports, especially the C++ based ones, and since we won't complete another review round today, you will no longer need to deal with 10.1 or 9.3 compat.

However, please either prove that we can build with OpenMP disabled or leave the [gcc-]c++11-lib as universal requirement.  I'm not sure we need gcc- if we can get libomp support with clang, and I'm not sure if I want a radical cleanup when we should instead see to getting rawtherapee-devel up to speed so that it can replace the aging rawtherapee code.
Comment 5 Jan Beich freebsd_committer freebsd_triage 2016-12-31 10:24:51 UTC
(In reply to Matthias Andree from comment #4)
> please either prove that we can build with OpenMP disabled

What else do you need besides clang34 build log I've provided in comment 2? I also did very basic runtime check inside jail to catch startup crashes.

> I'm not sure we need gcc- if we can get libomp support with clang

Some FreeBSD users and DragonFly prefer GCC. And good luck getting devel/openmp to build on i386, not to mention Tier2 architectures.

> I will pull the plug on support for FreeBSD < 10.3 tomorrow

Prepare to be spammed by pkg-fallout@ if you do it before portmgr upgrades package builders.
Comment 6 Jan Beich freebsd_committer freebsd_triage 2016-12-31 11:33:33 UTC
Created attachment 178416 [details]
v0.2

Build logs for OPENMP=off + OPTIMIZED_CFLAGS=off:
9.3 i386  (GCC 4.2)     - http://sprunge.us/ZGhe
9.3 amd64 (GCC 4.2)     - http://sprunge.us/TMfO
10.1 i386 (Clang 3.4)   - http://sprunge.us/fRDB
10.3 amd64 (Clang 3.4)  - http://sprunge.us/KiFO
11.0 i386  (Clang 3.8)  - http://sprunge.us/bFKI
11.0 amd64 (Clang 3.8)  - http://sprunge.us/XXUB
head-amd64 (Clang 3.9)  - http://sprunge.us/hDYE

(In reply to Matthias Andree from comment #4)
> I think C++11 is a universal requirement on RawTherapee

This version doesn't use any of C++11 features, so it builds fine with GCC 4.2.1. A quick check on 9.3 i386 shows it doesn't crash on startup but I haven't tested beyond that.

> libc++ must match the default Standard C++ and Template Libraries

Leaf ports can use libstdc++ just fine as long as dependencies don't pull libc++. Mixing both is possible[1] but not supported by libstdc++ from any of  lang/gcc*.

[1] https://wiki.freebsd.org/NewC++Stack#Mixing_Libraries_using_Libc.2B-.2B-_and_Libstdc.2B-.2B-

> getting rawtherapee-devel up to speed so that it can replace the aging rawtherapee code.

Apologies for misleading. I don't use either port, so only interested in cleanup. -devel port didn't exist long enough and has some style issues of its own.
Comment 7 Matthias Andree freebsd_committer freebsd_triage 2017-01-05 02:15:25 UTC
After some discussion with Ingo Weyrich on IRC, I'm not going down the route of splitting SSE and SSE2, instead, I've committed a change that upgrades things from -msse to -msse2 on i386 (both are an implicit default on AMD64) because most of the benificial SIMD stuff in RawTherapee is SSE2-based.

And there really isn't much point in running RT on machines that don't support SSE2, it's just too painful on the users.
Comment 8 Jan Beich freebsd_committer freebsd_triage 2017-01-06 10:01:05 UTC
Created attachment 178571 [details]
v0.3

- Rebase after ports r430611
- Restore SSE2 by default on i386
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-01-14 19:40:01 UTC
A commit references this bug:

Author: mandree
Date: Sat Jan 14 19:39:01 UTC 2017
New revision: 431493
URL: https://svnweb.freebsd.org/changeset/ports/431493

Log:
  Further cleanup, 11-RELEASE build fixes.

  Prune unused dependencies via -Wl,--as-needed.

  Build with as few requirements as works for 10.3-RELEASE and
  11.0-RELEASE, i386 and amd64. This should use the base CC on anything
  but 10.3-RELEASE amd64, where we use GCC 4.9 for OpenMP support.

  Specifically, 11.0-RELEASE compilations do *not* work with GCC 4.9 and
  the result fails with SIGBUS with apparent bogus SSE code generation and
  misalignment.

  Fix sound theme support via libcanberra (change Linux ifdefs to !Apple
  ifdefs).

  Apply -DPROC_TARGET_NUMBER="1" (i. e. build an executable with generic
  tuning) only when building packages and unless -march is in CFLAGS.

  USE_GNOME upgraded from gtkmm20 to gtkm24, and add devel/openmp as
  requisite on amd64 (rather than implicit clang or gcc libs), bumping
  PORTREVISION.

  Silence cmake developer warnings, and be sure to export linker flags to
  cmake (so it finds -lomp in its configuration phase)
  [-DCMAKE_POLICY_DEFAULT_CMP0056:STRING=NEW]

  Cleanups: leverage USES=localbase, and drop/avoid duplicate definitions
  and dead code from the Makefile and support for unsupported older
  FreeBSD releases.

  Based in part on v0.3 patch of...
  PR:		215675
  Submitted by:	jbeich@
  MFH:		2017Q1

Changes:
  head/graphics/rawtherapee/Makefile
  head/graphics/rawtherapee/files/patch-CMakeLists.txt
  head/graphics/rawtherapee/files/patch-rtengine_dcraw.cc
  head/graphics/rawtherapee/files/patch-rtengine_improcfun.h
  head/graphics/rawtherapee/files/patch-rtengine_safegtk.cc
  head/graphics/rawtherapee/files/patch-rtgui_soundman.cc
  head/graphics/rawtherapee/files/rawtherapee.in
  head/graphics/rawtherapee/pkg-message
Comment 10 Matthias Andree freebsd_committer freebsd_triage 2017-01-14 19:41:34 UTC
Specifically, I've skipped the "add ull suffix" patch because none of the four {i386, amd64} x {10.3-RELEASE, 11.0-RELEASE} builds seem to require it and I expect that the compiler determines the required constant width by itself, in line with relevant standards.

Thanks for the sound fixes and several optimization and cleanups and ideas.
Comment 11 commit-hook freebsd_committer freebsd_triage 2017-01-15 13:21:28 UTC
A commit references this bug:

Author: mandree
Date: Sun Jan 15 13:20:25 UTC 2017
New revision: 431558
URL: https://svnweb.freebsd.org/changeset/ports/431558

Log:
  MFH: r430611 r431493

  Upgrade CPU flags to use SSE2 by default on i386.

  Reported by:	Ingo Weyrich

  Further cleanup, 11-RELEASE build fixes.

  Prune unused dependencies via -Wl,--as-needed.

  Build with as few requirements as works for 10.3-RELEASE and
  11.0-RELEASE, i386 and amd64. This should use the base CC on anything
  but 10.3-RELEASE amd64, where we use GCC 4.9 for OpenMP support.

  Specifically, 11.0-RELEASE compilations do *not* work with GCC 4.9 and
  the result fails with SIGBUS with apparent bogus SSE code generation and
  misalignment.

  Fix sound theme support via libcanberra (change Linux ifdefs to !Apple
  ifdefs).

  Apply -DPROC_TARGET_NUMBER="1" (i. e. build an executable with generic
  tuning) only when building packages and unless -march is in CFLAGS.

  USE_GNOME upgraded from gtkmm20 to gtkm24, and add devel/openmp as
  requisite on amd64 (rather than implicit clang or gcc libs), bumping
  PORTREVISION.

  Silence cmake developer warnings, and be sure to export linker flags to
  cmake (so it finds -lomp in its configuration phase)
  [-DCMAKE_POLICY_DEFAULT_CMP0056:STRING=NEW]

  Cleanups: leverage USES=localbase, and drop/avoid duplicate definitions
  and dead code from the Makefile and support for unsupported older
  FreeBSD releases.

  Based in part on v0.3 patch of...
  PR:		215675
  Submitted by:	jbeich@

  Approved by:	ports-secteam (junovitch@)

Changes:
_U  branches/2017Q1/
  branches/2017Q1/graphics/rawtherapee/Makefile
  branches/2017Q1/graphics/rawtherapee/files/patch-CMakeLists.txt
  branches/2017Q1/graphics/rawtherapee/files/patch-rtengine_dcraw.cc
  branches/2017Q1/graphics/rawtherapee/files/patch-rtengine_improcfun.h
  branches/2017Q1/graphics/rawtherapee/files/patch-rtengine_safegtk.cc
  branches/2017Q1/graphics/rawtherapee/files/patch-rtgui_soundman.cc
  branches/2017Q1/graphics/rawtherapee/files/rawtherapee.in
  branches/2017Q1/graphics/rawtherapee/pkg-message