Created attachment 166210 [details]
Use newer compiler flags and delete 2 patches

Created attachment 166212 [details]
Change compiler and delete 3 patches

Comment on attachment 166212 [details]
Change compiler and delete 3 patches

Looks like the compiler can recognize more options that can obsolete some more patches. I'm testing deleting more patches.

Created attachment 166217 [details]
Change compiler, remove obsoleted patches, add lv2 support

root@jlow:/usr/ports/audio/audacity # make stage-qa
====> Running Q/A tests (stage-qa)
Warning: 'bin/audacity' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD}
/!\ To run stage-qa automatically add DEVELOPER=yes to your environment /!\
root@jlow:/usr/ports/audio/audacity # make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
===> Checking for items in pkg-plist which are not in STAGEDIR
===> No pkg-plist issues found (check-plist)

Comment 6 Kubilay Kocak 2016-01-28 03:53:01 UTC

One should very careful applying optimizations for the common case, in particular as it relates to binary package builds. 

Additionally, this enforces a dependency on GCC, where instead the behaviour ought to be that the software builds with the compiler its given, provided of course, the compiler supports the required language constructs.

I'm -1 on those changes.

Also, this port should depend on (and use) ports versions of libraries rather than those that come bundled in the sources. This goes for at least audio/libmad, which precludes the optimization issue referenced above as well.

Comment 7 Kubilay Kocak 2016-01-28 03:53:59 UTC

Also, the binary should be stripped, either by way of an install-strip target as stage-qa mentions, or via $STRIP_CMD, with the former preferred

The optimizations were from the upstream, the previous patches removed them as c11 didn't understand the flags and lv2 support did not compile with clang. I changed the compiler to gcc so the patches to remove the upstream's optimizations weren't needed.

I have
INSTALL_TARGET= install-strip
in the Makefile and don't understand why it's still giving that 
Warning: 'bin/audacity' is not stripped

audacity has issues building with non bundled specific newer versions (especially ffmpeg and portaudio). I'll have to spend some time later to see if any libs can work with non-bundled versions.

Comment 9 Kubilay Kocak 2016-01-28 04:10:56 UTC

(In reply to xxjack12xx from comment #8)

The root cause solution for the optimizations (requiring the patch), is for upstream to make their configure script check for the presence of support for those flags:

standard checks (compile tests with -<flag>) or the following are options:

www.gnu.org/s/autoconf-archive/ax_check_compile_flag.html

I'd recommend leaving the patch in place until that occurs (at least report the issue) in favour of requiring users install another compilers.

Regarding the strip issue, check using the file(1) command to confirm whether or not it's stripped (after install) to exclude an issue with stripped file detection in the framework. If it isn't stripped, look to the audacity build to figure out why.

At the present time the change as currently proposed is (IMO) not ready to commit. Fixing the strip issue 'can' be deferred, but I recommend reverting the patch removal and compiler changes given the existing port version works now

I will make a report to the upstream that they should check for compiler flag support before trying to use them. The flags that currently aren't recognized are:
-fthread-jumps -fcse-follow-jumps -fcse-skip-blocks -fregmove -fschedule-insns2 -fcheck-new -Wno-unused-local-typedefs

The binary says it is not stripped according to file, I'll look into that.

I'll look into how to get lv2 support to compile with the current compiler also, probably more missing compiler flag checks. I'll make a new patch for that one.

When using clang to build lv2, it results in a strange error but works fine with gcc. Not sure how or why it occurs and how to fix this.

gmake[4]: Entering directory '/usr/home/jack/obj/usr/usr/ports/audio/audacity/work/audacity-Audacity-2.1.2/lib-src/lv2'
./build --srcdir=.
gmake[4]: ./build: Command not found
Makefile:4: recipe for target 'all' failed
gmake[4]: *** [all] Error 127
gmake[4]: Leaving directory '/usr/home/jack/obj/usr/usr/ports/audio/audacity/work/audacity-Audacity-2.1.2/lib-src/lv2'
Makefile:1008: recipe for target 'all-recursive' failed
gmake[3]: *** [all-recursive] Error 1
gmake[3]: Leaving directory '/usr/home/jack/obj/usr/usr/ports/audio/audacity/work/audacity-Audacity-2.1.2/lib-src'
Makefile:726: recipe for target 'all-recursive' failed
gmake[2]: *** [all-recursive] Error 1
gmake[2]: Leaving directory '/usr/home/jack/obj/usr/usr/ports/audio/audacity/work/audacity-Audacity-2.1.2'
===> Compilation failed unexpectedly.

Fixed the binary stripping warning in
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206702

I've emailed audacity upstream to add support for compiler flag checking.

Comment 13 Kubilay Kocak 2016-01-28 06:08:23 UTC

(In reply to xxjack12xx from comment #12)

I take it from the separate issue you're happy for that to be committed now while the changes in this issue are re-worked, or would you prefer to revert the compiler changes here and integrate the strip'ping bugfix in this attachment? :)

I found the lv2 issue, seems the shebagfix didn't fix the file "lib-src/lv2/build" so looks like will need some manual patching for now. Let's commit that other bug and leave this one about the optimization flag patches. What do you suggest?

Should we have people use another compiler to keep the upstream configures/Makefiles unpatched or use the built in compiler with the patches to remove their flags they didnt check for?

Comment 15 Kubilay Kocak 2016-01-28 06:20:10 UTC

(In reply to xxjack12xx from comment #14)

Let's commit bug 206702 first, then with this issue:

* Revert the compiler changes / patch removal
* Patch for LV2

Don't forget to run QA again, and please update the Summary here to match the new intent

Great work!

Ok, will update this bug when i can get lv2 support to compile.

Created attachment 166249 [details]
Add lv2 support and fix shebangs

The patch to add lv2 support is now ready along with shebang fixes.

Created attachment 166250 [details]
Building audio/audacity for 9.3-STABLE

2.1.2 couldn't build on 9.3-STABLE.

I'm trying with

.include <bsd.port.pre.mk>
.if ! ${COMPILER_FEATURES:Mlibc++}
LIB_DEPENDS+=   libc++.so.1:${PORTSDIR}/devel/libc++
CXXFLAGS+=      -std=c++11
CXXFLAGS+=      -stdlib=libc++
CXXFLAGS+=      -nostdinc++ -isystem ${LOCALBASE}/include/c++/v1
.endif
.include <bsd.port.post.mk>

But, it is not yet completed.

Created attachment 166260 [details]
patch for 9.3-STABLE

Finally, I have succeeded to build audacity with this patch.

Created attachment 166261 [details]
log applied attachment 166260 [details]

i will test in 10 and 11. Thanks!

The new patches compiles fine on 10 and 11

Comment 24 Thomas Zander 2016-01-31 23:21:08 UTC

My suggestion to keep the patch for the strip issue in Bug 206702 as short as possible and you re-submit the patch for this bug to address the shebangs, lv2 and midi all at once. Hope this is not too much trouble.

Ok, will have this sorted out momentarily.

Created attachment 166425 [details]
Fix shebangs, add lv2 support, strip binary

*** Bug 206702 has been marked as a duplicate of this bug. ***

This new attachment fixes the shebangs, strips binary, removes the required midi option, and adds lv2 plugin support.

Comment 29 Kubilay Kocak 2016-02-02 11:19:58 UTC

MFH candidate

Comment 30 commit-hook 2016-02-02 23:13:28 UTC

A commit references this bug:

Author: riggs
Date: Tue Feb  2 23:13:08 UTC 2016
New revision: 407904
URL: https://svnweb.freebsd.org/changeset/ports/407904

Log:
  Add LV2 OPTION, remove MIDI OPTION, strip binary, fix build on 9.3

  PR:		206694
  Submitted by:	xxjack12xx@gmail.com (maintainer)
  MFH:		2016Q1

Changes:
  head/audio/audacity/Makefile
  head/audio/audacity/files/patch-lib-src__libsoxr__configure
  head/audio/audacity/files/patch-lib-src__lv2__Makefile
  head/audio/audacity/files/patch-lib-src_lv2_configure
  head/audio/audacity/files/patch-src_effects_vamp_LoadVamp.cpp
  head/audio/audacity/files/patch-src_import_ImportFLAC.cpp

Comment 31 Thomas Zander 2016-02-02 23:18:46 UTC

(In reply to xxjack12xx from comment #28)

Committed with a small change: When the build framework chooses gcc, we currently need -std=c++11 to avoid compile errors (e.g. on 9.3 i386).

Comment 32 Thomas Zander 2016-02-02 23:29:22 UTC

2016Q1 is on a previous upstream version and according to upstream changelog, no security issues have been addressed.

Do we need to increment the PORTREVISION?

Comment 34 commit-hook 2016-02-03 22:45:28 UTC

A commit references this bug:

Author: riggs
Date: Wed Feb  3 22:45:23 UTC 2016
New revision: 408004
URL: https://svnweb.freebsd.org/changeset/ports/408004

Log:
  Bump PORTREVISION

  Double-checked package content: Not identical to previous revision,
  hence r407904 should have bumped PORTREVISION.

  PR:		206694
  Submitted by:	xxjack12xx@gmail.com (maintainer)

Changes:
  head/audio/audacity/Makefile

Comment 35 Thomas Zander 2016-02-03 22:49:48 UTC

Thanks! I double-checked the package, you are correct. PORTREVISION bumped.