Bug 241928 - multimedia/handbrake: fix build on GCC architectures
Summary: multimedia/handbrake: fix build on GCC architectures
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: freebsd-ports-bugs mailing list
URL:
Keywords:
Depends on:
Blocks: 241902
  Show dependency treegraph
 
Reported: 2019-11-12 21:06 UTC by Piotr Kubaj
Modified: 2019-11-25 02:06 UTC (History)
1 user (show)

See Also:
naito.yuichiro: maintainer-feedback+


Attachments
patch (6.33 KB, patch)
2019-11-12 21:06 UTC, Piotr Kubaj
no flags Details | Diff
v2 (6.38 KB, patch)
2019-11-17 19:38 UTC, Piotr Kubaj
pkubaj: maintainer-approval? (naito.yuichiro)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Kubaj freebsd_committer 2019-11-12 21:06:52 UTC
Created attachment 209112 [details]
patch

Use C11 compiler. Needed by ffmpeg.
Correct comment typo.
Remove two Linux-specific includes from ppc_cpudetect.c that cause build errors, since we don't have them.
Merge upstream patch for building x265 on non-x86.
Don't link to libc++ when building with GCC.
Comment 1 Yuichiro NAITO 2019-11-13 01:10:20 UTC
Would you tell me what kind of architecture for your target?

Please remember that upstream project clearly says they will support only for amd64 architecture.
HandBrake is not intended to work on other architectures.
But I respect FreeBSD support policy that i386 is Tier 1,
I added patches for i386 architecture.

If you keep maintain on your architecture, I will work with you.

I will reply on your comment.

> Use C11 compiler. Needed by ffmpeg.

  It's no problem for me.

> Correct comment typo.

  That's right, it's my mistake.

> Remove two Linux-specific includes from ppc_cpudetect.c that cause build errors, since we don't have them.

  HandBrake 1.3.0 changes to use libvpx from Ports.
  PR #241902 should obsolete this patch.

> Merge upstream patch for building x265 on non-x86.

  HandBrake 1.3.0 updates x265 to 3.2.1.
  As you say, it will be fixed by upstream and PR #241902 will fix this.

> Don't link to libc++ when building with GCC.- compiler:c11

  It has complicated reason.
  I think it is necessary for amd64/i386 when HandBrake is built by gcc.
  I would like to make this patch for specific architecture.

  The reason is shown in following GitHub issue.

  https://github.com/HandBrake/HandBrake/issues/1674
Comment 2 Piotr Kubaj freebsd_committer 2019-11-13 09:14:04 UTC
(In reply to Yuichiro NAITO from comment #1)
> Would you tell me what kind of architecture for your target?
powerpc64

> I think it is necessary for amd64/i386 when HandBrake is built by gcc.
amd64 and i386 on all supported releases use LLVM and I don't see any USE_GCC in the port's Makefile.
Comment 3 Yuichiro NAITO 2019-11-14 00:49:05 UTC
(In reply to Piotr Kubaj from comment #2)
>> Would you tell me what kind of architecture for your target?
>powerpc64

Thanks.
How about adding powerpc64 to ONLY_FOR_ARCHS?

I can't support all other architectures than amd64/i386.
Testing on powerpc64 is left to you.

>> I think it is necessary for amd64/i386 when HandBrake is built by gcc.
>amd64 and i386 on all supported releases use LLVM and I don't see any USE_GCC in the port's Makefile.

In case of CC=gcc.
Some people want to compare performance between clang and gcc.

Anyway, is it useful to you that changing your patch to following?

> LDFLAGS += $(if $(findstring gcc, $(GCC.gcc)), $(if $(findstring x86, $(GCC.archs)), $(GCC.LDFLAGS), ), )

GCC.LDFLAGS is exported if compiler is gcc and arch is x86 (includes x86_64).
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2019-11-14 04:58:05 UTC
Approved as mentor (builds on blackbird) pending maintainer review.
Comment 5 Piotr Kubaj freebsd_committer 2019-11-17 19:38:07 UTC
Created attachment 209211 [details]
v2

powerpc64 added to ONLY_FOR_ARCHS.

Your make/variant/freebsd.defs patch is ok, but note that powerpc64 soon switches on head to clang as well. Do you accept the attached patch?
Comment 6 Yuichiro NAITO 2019-11-18 03:59:09 UTC
(In reply to Piotr Kubaj from comment #5)

> Your make/variant/freebsd.defs patch is ok, but note that powerpc64 soon switches on head to clang as well. Do you accept the attached patch?

Thanks for the information.
I've changed my mind to check if clang is installed in base.

Could you please update your patch to following?

> LDFLAGS += $(if $(findstring gcc, $(GCC.gcc)), $(if $(shell /usr/bin/clang -dumpversion 2> /dev/null), $(GCC.LDFLAGS), ), )

I approve your patch with this fix and confirmed working on amd64 and i386.

And it's for my interest.
May I ask you the reason why you want to run HandBrake on powerpc64?
Comment 7 Piotr Kubaj freebsd_committer 2019-11-20 09:54:55 UTC
(In reply to Yuichiro NAITO from comment #6)
Yes, your patch works.

> May I ask you the reason why you want to run HandBrake on powerpc64?
I take care of making ports work on powerpc64.
Comment 8 commit-hook freebsd_committer 2019-11-24 11:42:54 UTC
A commit references this bug:

Author: pkubaj
Date: Sun Nov 24 11:42:25 UTC 2019
New revision: 518318
URL: https://svnweb.freebsd.org/changeset/ports/518318

Log:
  multimedia/handbrake: fix build on powerpc64

  Use C11 compiler, because of ffmpeg.
  Correct comment typo.
  Don't include asm/cputable.h and linux/auxvec.h in libvpx-1.7.0/vpx_ports/ppc_cpudetect.c - those headers are Linux-only.
  Merge upstream patch from x265 to fix compilation on non-x86.
  Don't link with libc++ when using GCC.

  PR:		241928
  Approved by:	naito.yuichiro@gmail.com (maintainer), linimon (mentor)

Changes:
  head/multimedia/handbrake/Makefile
  head/multimedia/handbrake/files/patch-contrib_libvpx_P05-freebsd-ppc.patch
  head/multimedia/handbrake/files/patch-contrib_x265_P01-freebsd-ppc.patch
  head/multimedia/handbrake/files/patch-make_variant_freebsd.defs