Bug 245665 - multimedia/makemkv: update to 1.15.1 and allow package creation
Summary: multimedia/makemkv: update to 1.15.1 and allow package creation
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: Kurt Jaeger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-16 11:12 UTC by Felix Palmen
Modified: 2020-05-11 18:50 UTC (History)
2 users (show)

See Also:


Attachments
Update to 1.15.1, allow package creation (5.59 KB, patch)
2020-04-16 11:12 UTC, Felix Palmen
no flags Details | Diff
Update to 1.15.1, allow package creation (6.95 KB, patch)
2020-05-10 10:00 UTC, Felix Palmen
no flags Details | Diff
Update to 1.15.1, allow package creation (7.45 KB, patch)
2020-05-10 23:49 UTC, Felix Palmen
no flags Details | Diff
Update to 1.15.1, allow package creation (7.51 KB, patch)
2020-05-11 12:54 UTC, Felix Palmen
felix: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Felix Palmen 2020-04-16 11:12:42 UTC
Created attachment 213460 [details]
Update to 1.15.1, allow package creation

Upstream version 1.15.1 is available.

I need some guidance with the following things please:

1. I received a PGP signed email from the upstream author, Mike Chen, in which he allowed to redistribute a modified makemkvcon binary for this port. Therefore I changed the Makefile and LEGAL accordingly -- the MakeMKV EULA now has "auto-accept" property, and I replaced RESTRICTED with NO_CDROM (as selling a package is still forbidden) and also added LEGAL_TEXT to explain some more. Are these changes correct, and should I provide the original email from Mike Chen somewhere?


2. I changed a few things to satisfy portlint, but something that was a warning before is now a fatal:

| FATAL: Makefile: LDFLAGS is already passed in CONFIGURE_ENV via bsd.port.mk.
| If you need to override the default value, alter LDFLAGS in the Makefile
| instead with LDFLAGS+=...

The override in CONFIGURE_ENV is needed to remove rpath magic added by USE_GCC=yes, because it shouldn't be applied when cross-compiling for Linux. Is there a better way to solve this?


Apart from these issues, the poudriere testport succeeds on my 12.1-RELEASE amd64 and the package created works as intended. Is this enough for me to set my maintainer-approval flag?
Comment 1 Rodrigo Osorio freebsd_committer 2020-04-19 13:43:48 UTC
Hi Felix,

I finally manage to find the details about what is required when a special licence grant is allowed for the project. This chapter gives you the
details of the process:

https://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-guide/tracking.license.grants.html

You are the persone in touch with the author, so I think you can do
the process in your own. Just in case my, add me on CC, so I can
keep an eye on the discussions ;).

This is not a requirement to commit the changes so I will start looking
this PR.

Cheers,
-- rodrigo
Comment 2 Felix Palmen 2020-04-20 08:13:57 UTC
Hi Rodrigo,

thanks for the hint, I sent a (hopefully complete) mail yesterday, we will see!

about the portlint issue:

- building the GNU cross toolchain requires GCC, and the rpath magic from bsd.gcc.mk is needed here, that's why I used USE_GCC=yes
- building the makemkv libs is done with the cross GCC and shouldn't include these rpath flags
- I have no way to keep them out of LDFLAGS because they are added after evaluating the port Makefile
- An alternative could be to not USE_GCC and add the dependency on GCC manually for building the cross toolchain, but I think this might be somewhat fragile?

Do you think it makes sense asking freebsd-ports@ for opinions here?

BR, Felix
Comment 3 Kurt Jaeger freebsd_committer 2020-05-09 07:30:01 UTC
testbuilds@work
Comment 4 Kurt Jaeger freebsd_committer 2020-05-09 08:04:07 UTC
Testbuild fails on cur-i386:

https://people.freebsd.org/~pi/logs/multimedia__makemkv-curi-1589009393.txt

Port says it would build with i386.
Comment 5 Felix Palmen 2020-05-09 08:25:08 UTC
Uhhh, so, seems it doesn't build on i386 any more, or does this only happen on current? I'll probably need a virtual machine first for trying to reproduce this.
Comment 6 Kurt Jaeger freebsd_committer 2020-05-09 08:59:40 UTC
I'll test on 12.1i. But you are right, if you want test complex ports, you
normally need a full suite of poudriere jails.
Comment 7 Kurt Jaeger freebsd_committer 2020-05-09 10:51:37 UTC
12.1-i368 fails as well:

https://people.freebsd.org/~pi/logs/multimedia__makemkv-12i.txt
Comment 8 Felix Palmen 2020-05-09 10:55:43 UTC
Thanks, and it's the same kind of error, so it's tied to this old glibc not building correctly on i386.

Tempting to just remove i386 as a supported architecture :) But I'll still install a VM soon to be able to test (and hopefully track down) this myself.
Comment 9 Kurt Jaeger freebsd_committer 2020-05-09 10:58:03 UTC
testbuilding on 11.4-BETA1-i386 right now...
Comment 10 Kurt Jaeger freebsd_committer 2020-05-09 11:32:36 UTC
The results for 11.4-BETA1-i386:

https://people.freebsd.org/~pi/logs/multimedia__makemkv-11i.txt
Comment 11 Felix Palmen 2020-05-09 11:39:17 UTC
Exact same issue again. It will take me a while to get infrastructure up and running for debugging this :(

Would it be an option to mark it broken on i386 *for now*?
Comment 12 Kurt Jaeger freebsd_committer 2020-05-09 12:21:58 UTC
If I change

CONFIGURE_ENV= [...]
               LDFLAGS="${LDFLAGS:N-Wl,-rpath=*}"

to
               LDFLAGS+="${LDFLAGS:N-Wl,-rpath=*}"

at least portlint does no longer report a FATAL.

Because the build is very elaborate, does this change the outcome ?
Comment 13 Felix Palmen 2020-05-09 12:31:32 UTC
(In reply to Kurt Jaeger from comment #12)
Thanks for this suggestion!

The intention here is to *remove* -rpath magic added to LDFLAGS by bsd.gcc.mk as it would be wrong for cross-building to Linux. My immediate concern is that writing it that way might just double all LDFLAGS except for the -rpath part (if += works at all for setting the environment). But I will give it a try soon anyways and report back :)
Comment 14 Felix Palmen 2020-05-10 10:00:28 UTC
Created attachment 214344 [details]
Update to 1.15.1, allow package creation

As I suspected, the suggested change gives wrong results like this:

$ patchelf --print-rpath /usr/local/lib/makemkv/libmakemkv.so.1
/usr/local/lib/gcc9

I'm not sure whether this creates problems at runtime, but it is definitely wrong to have Linux binaries with an rpath pointing to FreeBSD libraries.

But it gave me another idea: provide a custom do-configure target instead. This is a bit clumsy, but the whole port is clumsy already (basically bootstrapping a GNU environment in pre-configure) and it "pets" portlint.

I also removed i386 for now, adding a specific IGNORE message with the reason. Rationale: The closed-source binary is only available for i386 and amd64, and probably, the majority of users is on amd64, so providing a working version for them is better than nothing and gives me time to setup infrastructure and analyze the problem on i386.

Please tell me your thoughts about these changes.
Comment 15 Felix Palmen 2020-05-10 23:49:35 UTC
Created attachment 214368 [details]
Update to 1.15.1, allow package creation

Fixed it on i386!

The key was to change to building for i686-unknown-linux-gnu. I guess it's ok to assume i686, as I've seen that in base as well? It's probably the only option anyways, I see in the makemkv-bin Makefile that makemkvcon needs at least i586.

Tested on 13-CURRENT i386 so far, I will set maintainer-approval as soon as I have tested builds on all 13, 12.1, 11.3 / amd64, i386 :)
Comment 16 Felix Palmen 2020-05-11 12:54:38 UTC
Created attachment 214374 [details]
Update to 1.15.1, allow package creation

Further testing revealed i486 is enough to make the build succeed, therefore the port now uses the following targets on i386:

- i686-unknown-linux-gnu on 13.0+
- i486-unknown-linux-gnu otherwise

* Build successfully tested on 13-CURRENT, 12.1-RELEASE and 11.3-RELEASE, amd64 and i386
* 12.1-RELEASE/amd64 package tested for correct operation on my Desktop
* portlint FATAL avoided

Therefore, I'd suggest this is finally ready for commit.
Comment 17 Kurt Jaeger freebsd_committer 2020-05-11 17:14:42 UTC
testbuilds@work
Comment 18 commit-hook freebsd_committer 2020-05-11 18:50:22 UTC
A commit references this bug:

Author: pi
Date: Mon May 11 18:50:19 UTC 2020
New revision: 534928
URL: https://svnweb.freebsd.org/changeset/ports/534928

Log:
  multimedia/makemkv: update to 1.15.1 and allow package creation

  - Added support for AACS v76 (for those poor souls without LibreDrive)
  - Improved handling for discs with mastering errors
  - Many internal improvements and small bugfixes
  - For some HD audio streams frames were dropped incorrectly on
    segment boundaries

  PR:		245665
  Submitted by:	Felix Palmen <felix@palmen-it.de> (maintainer)
  Reviewed by:	rodrigo

Changes:
  head/LEGAL
  head/multimedia/makemkv/Makefile
  head/multimedia/makemkv/distinfo
  head/multimedia/makemkv/pkg-descr
Comment 19 Kurt Jaeger freebsd_committer 2020-05-11 18:50:39 UTC
Committed, thanks!