handbrake: 1.3.1 -> 1.3.2

[Anton-Latukha]

Motivation for this change

Starting to use GitHub CDN.
Every time there is any HandBrake release - it gets published on their website after a timeout of arbitrary days. And I want to be able to respond to notifications of release right away, and not postpone, stagger, keep checking wait website tarball and remember that I still need to update.

Also update to 1.3.2.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[peterhoeg]

What happens when you build from the checkout as opposed to the release tarball?

[peterhoeg]

and add autoreconfHook to nativeBuildInputs.

[doronbehar]

Also, some parts of the preConfigure are not needed since:

HandBrake/HandBrake#2690

HandBrake/HandBrake#2712

[Anton-Latukha]

@peterhoeg:

and add autoreconfHook to nativeBuildInputs.

Results:

source root is HandBrake-1.3.2
setting SOURCE_DATE_EPOCH to timestamp 1588534233 of file HandBrake-1.3.2/version.txt
patching sources
autoreconfPhase
autoreconf: 'configure.ac' or 'configure.in' is required
builder for '/nix/store/dvg6dlm5pwrxq8pfbljjqjfky6v5hsra-handbrake-1.3.2.drv' failed with exit code 1

[Anton-Latukha]

@peterhoeg:

What happens when you build from the checkout as opposed to the release tarball?

Git checkout

I was discussing this issue with them upstream a couple of years ago. They have invented their-own versioning inside a project:

nix-shell . -A handbrake
unpackPhase
cd source
patchPhase
scripts/repo-info.sh    # source: https://github.com/HandBrake/HandBrake/blob/8e2a08267705f09e92f06289820ad6ebbfc9040e/scripts/repo-info.sh

Output:

URL=
HASH=ccb2e7234e01a2a07f537512045f7511583deaf9
SHORTHASH=ccb2e7234e0
REV=224188
BRANCH=handbrake-1.3.2
REMOTE=
DATE=2020-05-05 12:44:02 +0300

And this returned information results into probe: repo info...(fail) code 1

Results in build log:

source root is source
patching sources
configuring
...
probe: repo info...(fail) code 1
probe: version.txt...(fail)
ERROR: HandBrake is missing version information it needs to build properly.
Clone the official git repository at https://github.com/HandBrake/HandBrake.git
or download an official source archive from https://handbrake.fr
; configure stop.
compute: project data...
builder for '/nix/store/vwispj413x66pgh419awlgd6z9vj3nnv-handbrake-1.3.2.drv' failed with exit code 1

I got no description on what this situation is about, threads I've participated in or read resolve things replying "use website tarball".

Tarball package

Running same script in itL

scripts/repo-info.sh

Also results into probe: repo info...(fail) code 1, but now of the other kind

fatal: not a git repository (or any of the parent directories): .git
fatal: not a git repository (or any of the parent directories): .git
Not a valid repository.

And that situation "works" in the tarball:

probe: repo info...(fail) code 1
probe: version.txt...(pass)
...

If to give the long answer.

As I understand that there does not exist the case to succesfully go there proselytize and rip-out their internal distribution & versioning - I do not worry about this too much.

[Anton-Latukha]

Also, some parts of the preConfigure are not needed since:

HandBrake/HandBrake#2690

HandBrake/HandBrake#2712

@doronbehar

Reduced the patches.

[doronbehar]

BTW, I'm getting this when running it:

** Message: 14:08:09.629: Couldn't initialize gstreamer. Disabling live preview.

I think you can fix it with something similar to:

https://github.com/Anton-Latukha/nixpkgs/blob/f9a7d6c4ed32bb78d0df7d25dd1a3b72f658dbe4/pkgs/applications/audio/transcribe/default.nix#L46-L47

While some gst-plugins are located in the build inputs so wrapGAppsHook will pick them up.

But it's also possible that gappsWrapperArgs+=( is not needed...

[Anton-Latukha]

BTW, I'm getting this when running it:

** Message: 14:08:09.629: Couldn't initialize gstreamer. Disabling live preview.

@doronbehar

I specifically tested that live preview works.

And with https://github.com/Anton-Latukha/nixpkgs/blob/f9a7d6c4ed32bb78d0df7d25dd1a3b72f658dbe4/pkgs/applications/audio/transcribe/default.nix#L46-L47 ghb still throws that same warning. And just as before - in ghb preview still works despite of that warning.

[doronbehar]

@doronbehar
I specifically tested that live preview works.
And with https://github.com/Anton-Latukha/nixpkgs/blob/f9a7d6c4ed32bb78d0df7d25dd1a3b72f658dbe4/pkgs/applications/audio/transcribe/default.nix#L46-L47 ghb still throws that same warning. And just as before - in ghb live preview still works despite of that warning.

OK I guess then it's not that bad (I'm not using Handbrake actively, I only tested it once in the past). If someone will notice some missing functionality that may be related to this message, he'll put the effort to debug this.

[Anton-Latukha]

@doronbehar

It makes in Preview the Live Preview button appear.
Now I remember, it encodes some seconds of video and then allows to play/watch it in the internal player window.
Like here: https://handbrake.fr/docs/en/latest/workflow/preview-settings.html

I also do not use HandBrake mutch these days, Preview is needed, I almost never run Live Preview, run the encoding for some %s, and then looked at that sample.

Wrapper configuration of GST_PLUGIN_SYSTEM_PATH is not successful in enabling feature.
I may dig into it, make a deeper assessment, maybe look how they --prefix LD_LIBRARY_PATH : "${libPath}" and assemble its libPath variable.

Thank you.

[Anton-Latukha]

I do not understand why if one maintainer changes to the package, and already went through the review process - why other same-level maintained should "mandatory" review it.

The Mertens for example probably or too busy or do not care for it long time ago.

The review is done.

[doronbehar]

Yea this is how things are in this repo unfortunately. Sometimes a maintainer with a merge bit goes through the reviewed pulls sorted from old to new and they may encounter this PR at some point, but don't hold your breath for this to happen.

[wmertens]

Sorry, I missed that it was ok to merge. Indeed, I don't use handbrake any more, so feel free to sign yourself as a maintainer

[Anton-Latukha]

@wmertens I am also a maintainer of this package for 2-3 years already.

We chatted with you back in the day about the package.

I am talking that I am a maintainer, process requires review from the higher-order maintainer, like doronbehar.

But I am maintainer that passes review.
I am just puzzled why "process" requires review from same-level package maintainers.

People tend to not remove themself from maintainer fields and not care and not show up.
And so - there are packages that have 5-6 "maintainers" but no one shows up to maintain it. So why that package should have all 5-6 maintainers sign-off between each other every time? It is not like this is Linux Kernel development, and as I know they do multistage reviews, but do not require such multicross peer review signoff all the time, because nothing would get done then.

[Anton-Latukha]

@doronbehar

Thanks to you a lot. You are a great guy.

How is auto update bot there.

I see that it still not pipelined to autotrack and automerge

Why? It seems really strange.

I think most people, and you, would've preferred for minor updates to go without bothering them.

[doronbehar]

Thanks to you a lot. You are a great guy.

💚

It is not like this is Linux Kernel development, and as I know they do multistage reviews, but do not require such multicross peer review signoff all the time, because nothing would get done then.

How is auto update bot there.
I see that it still not pipelined to autotrack and automerge
Why? It seems really strange.
I think most people, and you, would've preferred for minor updates to go without bothering them.

Your phrasing is not that good but I think I understand you. See:

• [RFC 0050] Merge bot for maintainers rfcs#50
• https://discourse.nixos.org/t/new-rfc-50-merge-bot-for-maintainers/3795
• https://discourse.nixos.org/t/steps-towards-even-more-pr-automation/5634/13
• https://discourse.nixos.org/t/suggestion-to-help-maintainers-with-nixpkgs-prs-bloat/4794/20
• [RFC 0051] Mark stale nixpkgs issues rfcs#51
• [RFC 0030] Formalize review workflow rfcs#30