Bug 206864

Summary: multimedia/x265: Update to 1.9 (and exp-run)
Product: Ports & Packages Reporter: Mikhail Teterin <mi>
Component: Individual Port(s)Assignee: Mikhail Teterin <mi>
Status: Closed FIXED    
Severity: Affects Only Me CC: antoine, jbeich, mi, riggs
Priority: --- Keywords: needs-qa, patch-ready
Version: LatestFlags: jbeich: exp-run?
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=204990
Attachments:
Description Flags
Update to 1.9, add test-target
none
Use "do-test" instead of "regression-test", enable tests through flag, use FreeBSD's md5
none
v3: update, bump, commit message
none
v3.1: update, bump, commit message
none
Include <sys/types.h> before <md5.h>, as the man-page specifies
none
Incorporate most of the earlier suggestions none

Description Mikhail Teterin freebsd_committer freebsd_triage 2016-02-02 21:51:14 UTC
Created attachment 166461 [details]
Update to 1.9, add test-target

The update itself is trivial, but, because of the sizable number of depending ports, an exp-run would be advised.

Should not be much different from Bug 204990.
Comment 1 Jan Beich freebsd_committer freebsd_triage 2016-02-02 22:02:28 UTC
According to freshports.org no port depends on it by default, so you need the following in make.conf:

  OPTIONS_UNSET+= X265

My quick check revealed very few consumers:

  $ git grep -lF X265 '*/Makefile*'
  graphics/libbpg/Makefile
  multimedia/avidemux/Makefile.common
  multimedia/ffmpeg/Makefile
  multimedia/vlc/Makefile
  x11/xpra/Makefile

But building them locally would still be costly due to
https://github.com/freebsd/poudriere/issues/319
Comment 2 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-02 22:05:36 UTC
(In reply to Jan Beich from comment #1)
> According to freshports.org no port depends on it by default

In that case, no revision-bumping is warranted at all. I'll just commit the update, I suppose.

People like myself, who build by hand with non-default options, will be taken care of by portupgrade.

> OPTIONS_UNSET+= X265

? Why would I be unsetting it?
Comment 3 Jan Beich freebsd_committer freebsd_triage 2016-02-02 22:14:05 UTC
(In reply to Jan Beich from comment #1)
>  OPTIONS_UNSET+= X265

Typo, should be: OPTIONS_SET+= X265

>  $ git grep -lF X265 '*/Makefile*'

Another typo: s/X265/libx265.so/ may find more consumers.

(In reply to Mikhail Teterin from comment #2)
> In that case, no revision-bumping is warranted at all. I'll just commit the update, I suppose.

According to Porter's Handbook

  PORTREVISION must be increased each time a change is made to the port that changes the generated package in any way. That includes changes that only affect a package built with non-default options.

https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/makefile-naming.html#makefile-portrevision
Comment 4 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-02 22:22:33 UTC
(In reply to Jan Beich from comment #3)
> PORTREVISION must be increased each time a change is made to the port
> that changes the generated package in any way.
> That includes changes that only affect a package built with
> non-default options.

This would only make sense if applicable to the port itself, not its dependencies...

There is an example in the Handbook, that explicitly talks about shared library bump of a dependency, but, according to the example provided there, that does not apply to non-default options:

> someone trying to install the old package after installing a newer version
> of the dependency will fail since it will look for the old libfoo.x instead
> of libfoo.(x+1)

this would not happen with official binary packages (because they don't link with libx265).
Comment 5 Jan Beich freebsd_committer freebsd_triage 2016-02-02 22:31:45 UTC
(In reply to Jan Beich from comment #3)
> will be taken care of by portupgrade.

portmaster doesn't rebuild consumers unless asked to i.e., PORTREVISION is bumped. No clue about synth.

IIRC, portmanager (removed) a few years ago tried to differentiate itself from portupgrade by rebuilding ports more often.

(In reply to Mikhail Teterin from comment #4)
> this would not happen with official binary packages (because they don't link with libx265).

Ports users are affected. portmaster is still recommended in the Handbook despite an attempt to deprecate it in ports r407270.

Hopefully, you'll at least wait for exp-run results before landing.
Comment 6 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-02 22:41:20 UTC
(In reply to Jan Beich from comment #5)
> portmaster doesn't rebuild consumers unless asked to i.e.,
> PORTREVISION is bumped

That really is a bug in portmaster -- it should be detecting changes in dependencies automatically. Then PORTREVISION can be delegated to signaling the following message: "You should rebuild this port to fix a bug and/or gain new functionality".

The automatic bumps like what we are discussing should not be necessary because the automatic tools must take care it themselves.

> portupgrade by rebuilding ports more often

Portupgrade will preserve the older shared libraries still used by other ports.

> Hopefully, you'll at least wait for exp-run results before landing

Yes, I shall. Thank you very much for your help.
Comment 7 Jan Beich freebsd_committer freebsd_triage 2016-02-02 22:48:23 UTC
Comment on attachment 166461 [details]
Update to 1.9, add test-target

>+test regression-test check: build
>+	${WRKSRC}/test/TestBench

Please, use the new testing framework. See ports r398125 until someone documents it in Porter's Handbook.

  do-test:
  	${WRKSRC}/test/TestBench

>+@@ -582,8 +582,3 @@
>+ endif(ENABLE_CLI)
>+ 
>+-if(ENABLE_ASSEMBLY AND NOT XCODE)
>+-    option(ENABLE_TESTS "Enable Unit Tests" OFF)
>+-    if(ENABLE_TESTS)
>+-        add_subdirectory(test)
>+-    endif()
>+-endif()
>++add_subdirectory(test)

Do you really need to patch out a conditional? Try

  CMAKE_ARGS=	-DENABLE_TESTS=on
Comment 8 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-02 23:32:22 UTC
Created attachment 166463 [details]
Use "do-test" instead of "regression-test", enable tests through flag, use FreeBSD's md5

(In reply to Jan Beich from comment #7)
> do-test:

Can I just add do-test to the list of aliases (check, test)?

> CMAKE_ARGS=	-DENABLE_TESTS=on

Oh, I tried "yes" and I tried "true" to no avail -- only then did I just patch out the conditional. But "on" seems to work, thank you!
Comment 9 Jan Beich freebsd_committer freebsd_triage 2016-02-03 01:45:52 UTC
Created attachment 166471 [details]
v3: update, bump, commit message

I have trouble building your v2 on 11.0-CURRENT (r295114) amd64

  In file included from source/encoder/search.cpp:27:
  In file included from source/common/picyuv.h:28:
  In file included from /usr/include/md5.h:40:
  /usr/include/sys/md5.h:37:3: error: unknown type name 'u_int32_t'
    u_int32_t state[4];   /* state (ABCD) */
    ^
  /usr/include/sys/md5.h:38:3: error: unknown type name 'u_int32_t'
    u_int32_t count[2];   /* number of bits, modulo 2^64 (lsb first) */
    ^

(In reply to Jan Beich from comment #1)
> According to freshports.org no port depends on it by default...

Mea culpa...

  This port is required by:   <----- easy to miss, maybe use different color
  for Libraries

      multimedia/gstreamer1-plugins-x265
      x11/xpra

(In reply to Mikhail Teterin from comment #8)
> Can I just add do-test to the list of aliases (check, test)?

test is already implemented. regression-test was a special target only run by tinderbox but not poudriere. So, if you want to keep using legacy aliases try

  regression-test check: test
  do-test:
  	${WRKSRC}/test/TestBench


  $ make clean check

As upstream only builds tests for x86 you may need to conditionalize it. However, Tier2 builds are broken for unrelated reasons.
http://beefy8.nyi.freebsd.org/data/head-armv6-default/p407727_s295126/logs/errors/x265-1.8.log
http://beefy7.nyi.freebsd.org/data/head-mips-default/p407727_s295126/logs/errors/x265-1.8.log

And USES=pathfix can be applied since ports r405449 instead of a custom post-patch.
Comment 10 Jan Beich freebsd_committer freebsd_triage 2016-02-03 01:48:55 UTC
Created attachment 166472 [details]
v3.1: update, bump, commit message

Bump multimedia/gstreamer1-plugins-x265 as well.
Comment 11 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-03 02:50:03 UTC
(In reply to Jan Beich from comment #9)
>      multimedia/gstreamer1-plugins-x265
>      x11/xpra

Ok, so for this two the bump is warranted. Sort of -- though I still think, automatic things of this nature should be done automatically.

> As upstream only builds tests for x86 you may need to conditionalize it

I'd be most interested in seeing, whether -- and how -- things break for other platforms. This may be a reason to patch the upstream's CMakeLists.txt to attempt testing unconditionally.

I suppose, I'll make myself the maintainer for a while -- to get the fallout e-mails, if any.

> And USES=pathfix can be applied since ports r405449 instead of a
> custom post-patch.

That's nice!
Comment 12 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-03 02:58:29 UTC
Created attachment 166474 [details]
Include <sys/types.h> before <md5.h>, as the man-page specifies

(In reply to Jan Beich from comment #9)
>   /usr/include/sys/md5.h:37:3: error: unknown type name 'u_int32_t'

Khmm, haven't seen this on 10.2. How about the attached version of patch-md5 -- which provides for <sys/types.h> before <md5.h>?
Comment 13 Jan Beich freebsd_committer freebsd_triage 2016-02-03 05:56:15 UTC
Comment on attachment 166474 [details]
Include <sys/types.h> before <md5.h>, as the man-page specifies

(In reply to Mikhail Teterin from comment #11)
> I'd be most interested in seeing, whether -- and how -- things break for other
> platforms. 

comment 9 had armv6 and mips logs that can be reproduced using poudriere + qemu-user-static. Periodically someone does aarch64 builds but logs tend to not live long (e.g. dependencies of bug 201763).

> This may be a reason to patch the upstream's CMakeLists.txt to attempt
> testing unconditionally.

Look inside test/CMakeLists.txt or test/checkasm-a.asm. I'd be surprised
if it works anywhere else besides x86.

> I suppose, I'll make myself the maintainer for a while -- to get the fallout
> e-mails, if any.

I think, you need to subscribe explicitly as failure mails from non-Tier1 platforms are disabled.
https://lists.freebsd.org/pipermail/freebsd-ports/2015-June/099438.html

(In reply to Mikhail Teterin from comment #12)
> Khmm, haven't seen this on 10.2.

Also happens with poudriere 10.1R i386 - http://sprunge.us/MVZQ

> How about the attached version of patch-md5 -- which provides for <sys/types.h> before <md5.h>?

It does help, thanks. Tested by commenting out the following hack

  CFLAGS+=       -Du_int32_t=uint32_t # sys/md5.h
Comment 14 Kubilay Kocak freebsd_committer freebsd_triage 2016-02-03 06:00:55 UTC
@Jan, which attachment needs the exp-run?

Can we obsolete the one that has been superseded
Comment 15 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-03 06:30:11 UTC
Created attachment 166484 [details]
Incorporate most of the earlier suggestions

(In reply to Jan Beich from comment #13)
> However, Tier2 builds are broken for unrelated reasons.

There used to be a list of machines, where a committer could login to to test things. Are there mips and arm boxes so accessible?

(In reply to Kubilay Kocak from commment #14)
> @Jan, which attachment needs the exp-run?

Please, use this one. Thank you!
Comment 16 Antoine Brodin freebsd_committer freebsd_triage 2016-02-05 06:47:11 UTC
multimedia/avidemux-plugins fails to package with X265 option on,  the rest looks fine

Failure log:

http://package22.nyi.freebsd.org/data/101amd64-default-PR206864/2016-02-04_18h37m19s/logs/errors/avidemux-plugins-2.6.11.log
Comment 17 Mikhail Teterin freebsd_committer freebsd_triage 2016-02-05 07:18:56 UTC
(In reply to Antoine Brodin from comment #16)
> multimedia/avidemux-plugins fails to package with X265 option on

It, actually, fails to configure... From the log:

-- Checking for x265
-- *****************
-- Found x265.h
-- Found x265_config.h
--   core version: 79
-- Found x265.h
-- Found x265 library
-- Could not find x265_encoder_open_79 in /usr/local/lib/libx265.so
-- Change Dir: /wrkdirs/usr/ports/multimedia/avidemux-plugins/work/.build/CMakeFiles/CMakeTmp

Run Build Command:"/usr/local/bin/gmake" "cmTC_d5b9c/fast"
gmake[1]: Entering directory '/wrkdirs/usr/ports/multimedia/avidemux-plugins/work/.build/CMakeFiles/CMakeTmp'
gmake -f CMakeFiles/cmTC_d5b9c.dir/build.make CMakeFiles/cmTC_d5b9c.dir/build
gmake[2]: Entering directory '/wrkdirs/usr/ports/multimedia/avidemux-plugins/work/.build/CMakeFiles/CMakeTmp'
Building C object CMakeFiles/cmTC_d5b9c.dir/CheckFunctionExists.c.o
/usr/local/bin/gcc48  -DCHECK_FUNCTION_EXISTS=x265_encoder_open_79  -O2 -pipe  -DLIBICONV_PLUG -fno-omit-frame-pointer -fstack-protector -Wl,-rpath=/usr/local/lib/gcc48 -fno-strict-aliasing -fmessage-length=0 -fmessage-length=0    -o CMakeFiles/cmTC_d5b9c.dir/CheckFunctionExists.c.o   -c /wrkdirs/usr/ports/multimedia/avidemux-plugins/work/avidemux_2.6.11/cmake_compile_check/CheckFunctionExists.c
Linking C executable cmTC_d5b9c
/usr/local/bin/cmake -E cmake_link_script CMakeFiles/cmTC_d5b9c.dir/link.txt --verbose=1
/usr/local/bin/gcc48  -O2 -pipe  -DLIBICONV_PLUG -fno-omit-frame-pointer -fstack-protector -Wl,-rpath=/usr/local/lib/gcc48 -fno-strict-aliasing -fmessage-length=0 -fmessage-length=0     CMakeFiles/cmTC_d5b9c.dir/CheckFunctionExists.c.o  -o cmTC_d5b9c  /usr/local/lib/libx265.so -ldl -Wl,-rpath,/usr/local/lib 
/usr/local/bin/ld: cannot find -ldl
collect2: error: ld returned 1 exit status
CMakeFiles/cmTC_d5b9c.dir/build.make:98: recipe for target 'cmTC_d5b9c' failed
gmake[2]: Leaving directory '/wrkdirs/usr/ports/multimedia/avidemux-plugins/work/.build/CMakeFiles/CMakeTmp'
gmake[2]: *** [cmTC_d5b9c] Error 1
Makefile:126: recipe for target 'cmTC_d5b9c/fast' failed
gmake[1]: Leaving directory '/wrkdirs/usr/ports/multimedia/avidemux-plugins/work/.build/CMakeFiles/CMakeTmp'
gmake[1]: *** [cmTC_d5b9c/fast] Error 2

Could not find x265

And I don't think, it worked with x265 1.8 (the current version) either -- because Avidemux' adds the stupid -ldl on ALL platforms except Windows.

riggs@, could you confirm (or deny) whether you have avidemux-plugins building properly with X265 option on? Thank you!
Comment 18 commit-hook freebsd_committer freebsd_triage 2016-02-10 20:08:55 UTC
A commit references this bug:

Author: mi
Date: Wed Feb 10 20:08:40 UTC 2016
New revision: 408649
URL: https://svnweb.freebsd.org/changeset/ports/408649

Log:
  Upgrade x265 from 1.8 to 1.9. Add the test-target to utilize upstream's bundled
  tests and make myself the maintainer to deal with fallout, if any.

  Bump PORTREVISION of the two other ports, which depend on x265 by default.

  PR:		206864

Changes:
  head/multimedia/gstreamer1-plugins-x265/Makefile
  head/multimedia/x265/Makefile
  head/multimedia/x265/distinfo
  head/multimedia/x265/files/
  head/multimedia/x265/files/patch-md5
  head/multimedia/x265/pkg-plist
  head/x11/xpra/Makefile