handbrake: 1.0.7 -> 1.1.0, according changes #39512

Anton-Latukha · 2018-04-25T23:22:23Z

Motivation for this change

Switched from fetchFromGitHub to fetchurl due to upstream change of storage and usage of metadata of versions.
Instead of just version, current version.txt now stores:
```
URL=https://github.com/HandBrake/HandBrake.git
HASH=b463d33a4ed4c9da5fb6432e7fb7e08422fc1aad
SHORTHASH=b463d33a4
TAG=1.1.0
TAG_HASH=b463d33a4ed4c9da5fb6432e7fb7e08422fc1aad
REV=0
BRANCH=
REMOTE=https://github.com/HandBrake/HandBrake.git
DATE=2018-04-05 19:31:47 +0200
```
This info is available in a tarball from site.
GitHub tarball does not have this file. And upstream leaves to package managers to populate this information. So during discussion I and upstream saw getting tarball from official site (fetchurl) as a more convenient way for now.
HandBrake upstream has a LibAV patch that adds sdtp (sample dependency) into movenc. Compilation of 1.1.0 relies on this patch.
Removed substituteInPlace make/configure.py since it is no longer needed, now date information seems to be taken from version.txt
Updated HandBrake to 1.1.0

Things done

Tested using sandboxing
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

jtojnar · 2018-04-25T23:28:57Z

pkgs/development/libraries/libav/default.nix

-    patches = []
+    patches = [
+      # From Handbrake
+      ./A21-mp4-sdtp.patch


Could you add a link to where the patch comes from, or even better use fetchurl to obtain it. Additionally, could you link to a libav issue tracking the merge of the patch into upstream libav?

@jtojnar
I searched and could not find a LibAV upstream issue about this patch.
I can ask HandBrake upstream about this if needed.

Is it better to use URL and hash of master branch of upstream, or a URL or that commit tree?
In first case it would fail with hash error on any patch change. But error would be clear.
On second case - it would work until it is going to not.

I could not find anything either. Asked about it on the PR: HandBrake/HandBrake#1006

Without an upstream bug report, it definitely should not be applied to main libav package. Nixpkgs tries to be as close to upstream as possible to simplify maintenance, adding random patches is widely frowned upon.

You could use (libav.overrideAttrs (super: {patches = (super.patches or []) ++ [(fetchurl {url = https://github.com/HandBrake/HandBrake/raw/7a49ab6e54a1fe6f33cf1e1ccbf46c8dd59015d1/contrib/ffmpeg/A21-mp4-sdtp.patch; sha256 = "xxx"; })];})), but personally, I find it more acceptable to be on Handbrake developers’ bad side than have one more libav package with dubious patches to please abhorrent upstream policies.

Or add patch to libav but disable it by default similarly to the fdk_aac codec:

patches = stdenv.lib.optionals withRandomHandbreakPatches [ # taken from: https://github.com/HandBrake/HandBrake/pull/1006 # upstream bug: xxx (fetchurl { url = https://github.com/HandBrake/HandBrake/raw/7a49ab6e54a1fe6f33cf1e1ccbf46c8dd59015d1/contrib/ffmpeg/A21-mp4-sdtp.patch; sha256 = "xxx"; }) ];

and use it with (libav.override { withRandomHandbreakPatches = true; })

@jtojnar
Ok. I would parse that line tomorrow.
I thought to use fetchpatch. But since filename is the same - fetchurl.

Also I can gather HandBrake patches into this package overrides.
For example: https://github.com/NixOS/nixpkgs/search?utf8=%E2%9C%93&q=handbrake+patch&type=
And also find patches from HandBrake upstream itself.

I saw how other HandBrake patches was done in NixPkgs and did the same way.

I am learning.

Thank you.

Unfortunately, not everything you see in nixpkgs is a good code. The patches in particular were often randomly added by someone in the past and nobody knows why were they added in the first place. I personally looked up and removed or added upstream links to almost a hundred patches.

fetchpatch is the same as fetchurl, only it also filters the patches using filterdiff. It needs to be used on patches that are generated by a web service and their layout is not static (e.g. GitHub commit patches). It is not needed for a static files.

@jtojnar
Also HandBrake in the next stable (1.2.0) would switch to FFMPEG, ETA is unknown.

So. Do I try to gather some of the patches originated from HandBrake and scattered across NixPkgs in overrides to package of origin in the next PRs?

Cool.

It would be nice, if you can spare the time.

Anton-Latukha · 2018-04-26T21:48:59Z

Chromium on Hydra fails the checks:

evaluation aborted with the following error message: 'Package ‘chromium-66.0.3359.117’ in /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/eval-1-lassulus.ewr1.nix.ci/pkgs/applications/networking/browsers/chromium/browser.nix:48 has an invalid meta attrset:
	 - key 'timeout' is unrecognized; expected one of: 
	      ['available', 'badPlatforms', 'branch', 'broken', 'description', 'downloadPage', 'downloadURLRegexp', 'executables', 'homepage', 'hydraPlatforms', 'isBuildPythonPackage', 'isFcitxEngine', 'isGutenprint', 'isIbusEngine', 'knownVulnerabilities', 'license', 'longDescription', 'maintainers', 'name', 'outputsToInstall', 'platforms', 'position', 'priority', 'repositories', 'schedulingPriority', 'tag', 'updateWalker', 'version'], refusing to evaluate.

Related to Slow chromium build is blocking channels #39476

Anton-Latukha · 2018-04-26T21:50:11Z

It all builds locally perfectly. And my package builds only libav_12 -> handbrake.

Anton-Latukha · 2018-04-26T23:04:00Z

Thank you Jan Tojnar @jtojnar for teaching and guidance.

Anton-Latukha · 2018-04-27T09:10:35Z

This is ready to be merged.

Anton-Latukha · 2018-04-27T18:43:54Z

@Mic92
I see you are merging stuff.
This is ready. And it's a self-contained PR.
I become a maintainer of this package.

7c6f434c · 2018-04-27T18:53:49Z

@GrahamcOfBorg build handbrake

GrahamcOfBorg · 2018-04-27T18:54:12Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: handbrake

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-04-27T18:56:22Z

Success on x86_64-linux (full log)

Attempted: handbrake

Partial log (click to expand)

Wrapping program /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0/bin/HandBrakeCLI
Wrapping program /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0/bin/ghb
shrinking RPATHs of ELF executables and libraries in /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0
shrinking /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0/bin/.HandBrakeCLI-wrapped
shrinking /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0/bin/.ghb-wrapped
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0/bin
patching script interpreter paths in /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0
checking for references to /build in /nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0...
/nix/store/0q4ipzra882s55hf386y0sc01jnfrxn1-handbrake-1.1.0

GrahamcOfBorg · 2018-04-27T19:02:27Z

Success on aarch64-linux (full log)

Attempted: handbrake

Partial log (click to expand)

Wrapping program /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0/bin/ghb
Wrapping program /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0/bin/HandBrakeCLI
shrinking RPATHs of ELF executables and libraries in /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0
shrinking /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0/bin/.HandBrakeCLI-wrapped
shrinking /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0/bin/.ghb-wrapped
strip is /nix/store/j7d4mr0ikv974ig7yzhknpsq288js4bs-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0/bin
patching script interpreter paths in /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0
checking for references to /build in /nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0...
/nix/store/zm9xip28llyxzizn51mfd9cm8akr858d-handbrake-1.1.0

Anton-Latukha · 2018-04-27T19:20:13Z

Thank you.

Anton-Latukha changed the title ~~handbrake: 1.0.7 -> 1.1.0 & according changes~~ handbrake: 1.0.7 -> 1.1.0, according changes Apr 25, 2018

jtojnar reviewed Apr 25, 2018

View reviewed changes

GrahamcOfBorg added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Apr 25, 2018

Anton-Latukha mentioned this pull request Apr 26, 2018

HandBrake 1.1.0 build fails in Nix HandBrake/HandBrake#1286

Closed

Anton-Latukha changed the title ~~handbrake: 1.0.7 -> 1.1.0, according changes~~ [WIP] handbrake: 1.0.7 -> 1.1.0, according changes Apr 26, 2018

jtojnar mentioned this pull request Apr 26, 2018

[RFC] Structured patches #39392

Open

Anton-Latukha force-pushed the handbrake-upd branch 2 times, most recently from 18a507d to 20c3ccc Compare April 26, 2018 21:03

handbrake: 1.0.7 -> 1.1.0 & according changes

f3ff2b1

Anton-Latukha force-pushed the handbrake-upd branch from 20c3ccc to f3ff2b1 Compare April 26, 2018 21:37

jtojnar approved these changes Apr 26, 2018

View reviewed changes

Anton-Latukha mentioned this pull request Apr 26, 2018

Chromium meta error broke the bot testing pipeline #39575

Closed

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Apr 26, 2018

Anton-Latukha changed the title ~~[WIP] handbrake: 1.0.7 -> 1.1.0, according changes~~ handbrake: 1.0.7 -> 1.1.0, according changes Apr 26, 2018

7c6f434c merged commit 547a41a into NixOS:master Apr 27, 2018

Anton-Latukha mentioned this pull request May 10, 2018

Cleanup of HandBrake related patches #40280

Merged

8 tasks

Anton-Latukha deleted the handbrake-upd branch May 30, 2018 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

handbrake: 1.0.7 -> 1.1.0, according changes #39512

handbrake: 1.0.7 -> 1.1.0, according changes #39512

Anton-Latukha commented Apr 25, 2018 •

edited

jtojnar Apr 25, 2018

Anton-Latukha Apr 25, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

Anton-Latukha Apr 26, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

Anton-Latukha Apr 26, 2018 •

edited

jtojnar Apr 26, 2018

Anton-Latukha commented Apr 26, 2018

Anton-Latukha commented Apr 26, 2018

Anton-Latukha commented Apr 26, 2018

Anton-Latukha commented Apr 27, 2018

Anton-Latukha commented Apr 27, 2018

7c6f434c commented Apr 27, 2018

GrahamcOfBorg commented Apr 27, 2018

GrahamcOfBorg commented Apr 27, 2018

GrahamcOfBorg commented Apr 27, 2018

Anton-Latukha commented Apr 27, 2018

handbrake: 1.0.7 -> 1.1.0, according changes #39512

handbrake: 1.0.7 -> 1.1.0, according changes #39512

Conversation

Anton-Latukha commented Apr 25, 2018 • edited

Motivation for this change

Things done

jtojnar Apr 25, 2018

Choose a reason for hiding this comment

Anton-Latukha Apr 25, 2018 • edited

Choose a reason for hiding this comment

jtojnar Apr 26, 2018 • edited

Choose a reason for hiding this comment

jtojnar Apr 26, 2018 • edited

Choose a reason for hiding this comment

Anton-Latukha Apr 26, 2018 • edited

Choose a reason for hiding this comment

jtojnar Apr 26, 2018 • edited

Choose a reason for hiding this comment

Anton-Latukha Apr 26, 2018 • edited

Choose a reason for hiding this comment

jtojnar Apr 26, 2018

Choose a reason for hiding this comment

Anton-Latukha commented Apr 26, 2018

Anton-Latukha commented Apr 26, 2018

Anton-Latukha commented Apr 26, 2018

Anton-Latukha commented Apr 27, 2018

Anton-Latukha commented Apr 27, 2018

7c6f434c commented Apr 27, 2018

GrahamcOfBorg commented Apr 27, 2018

GrahamcOfBorg commented Apr 27, 2018

GrahamcOfBorg commented Apr 27, 2018

Anton-Latukha commented Apr 27, 2018

Anton-Latukha commented Apr 25, 2018 •

edited

Anton-Latukha Apr 25, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

Anton-Latukha Apr 26, 2018 •

edited

jtojnar Apr 26, 2018 •

edited

Anton-Latukha Apr 26, 2018 •

edited