Fix Handbrake build on Darwin by turning off GUI support #63643

bdesham · 2019-06-22T03:31:23Z

Motivation for this change

Fix cairo build on macOS by disabling Xlib support.
Fix ffmpeg-full build on macOS by making sure that the nvenc option is not used.
Fix handbrake build by adding a dependency on lzma (for all platforms) and adding some Darwin-specific libraries and patches.
Fix mp4v2 build by building against the C++03 standard. The package has some code that is not valid under the newer standards.
Add video-transcoding, a set of scripts for transcoding, inspecting, and converting videos.

Things done

cc @codyopel @Fuuzetsu @Anton-Latukha @wmertens

Anton-Latukha · 2019-06-22T12:28:01Z

pkgs/applications/video/handbrake/default.nix

+  VideoToolbox ? null,
+  # GTK. This is not available on Darwin, since libappindicator-gtk3 is not
+  # supported there.
+  useGtk ? !stdenv.isDarwin, wrapGAppsHook ? null,


Explanation in Pull Request message is quite, sends in a different direction, that it is in the code.

Can you explain adding !stdenv.isDarwin here.

If I understand correctly, code now says if system is Darwin - then !stdenv.isDarwin equals false, which disables derivation dependencies:

{ buildInputs = [] ++ lib.optionals useGtk [ glib gtk3 libappindicator-gtk3 libnotify gst_all_1.gstreamer gst_all_1.gst-plugins-base dbus-glib udev libgudev hicolor-icon-theme ]; }

And disables GTK:

(if useGtk then "--disable-gtk-update-checks" else "--disable-gtk")

during build.

Do you want to make Handbrake on macOS headless (in favour of this shows

(if stdenv.isDarwin then "--disable-xcode" else ""

, which disables GUI build). Or it somehow uses a different GUI toolkit?

You stated that this "Fix handbrake build by adding a dependency on lzma (for all platforms) and adding some Darwin-specific libraries and patches.", it seems quite misleading.

Did you tried to work-around lack of libappindicator-gtk3 and what were the results?

AFAIR libappindicator-gtk3 is a tiny functionality and an optional dependency that I kept in closure, I only work in the Linux space. Are you positive that because of GTK build on macOS is not possible and we required to throw away baby with bathwater (of course I also refer to official build documentation?

Sorry for such a long list of remarks and questions, try to provide closer description to changes.

Also:

# GTK. This is not available on Darwin, since libappindicator-gtk3 is not supported there.

Does it mean that this code block is not available, or GTK is not available?

To sum-up:

Is libappindicator-gtk3 is really major required dependency of build on macOS, then we required to make only headless build?

Maybe I missed or do not know some information - then please provide more information.

Maybe I did throw the baby out with the bathwater 🙂 Your concerns are totally fair and I apologize for not including more detail in the pull request.

I did try to make the libappindicator-gtk3 dependency optional, but then I faced the problem that Handbrake still had a transitive dependency on systemd (which is not available on macOS). I’m not sure which of Handbrake’s dependencies introduced this transitive dependency but I did find that completely disabling GTK support on macOS took care of the problem. (It’s not clear to me whether Handbrake supports being built without libappindicator-gtk3.)

I also could not figure out how to build the native macOS GUI. Removing the --disable-xcode flag let the package build but the only file in the resulting derivation was still HandbrakeCLI. (In retrospect, I’m not sure what prompted me to add this flag in the first place—the package now seems to build equally well regardless of whether or not it’s specified. But the native GUI is not present in either case.)

My assumption was that most Nix users on macOS would prefer a headless Handbrake, at least compared to the status quo, which was that the package was completely unavailable on macOS. I decided that the best way forward would be to completely disable the GTK portion of the package when building under macOS.

I can think of the following ways to resolve this. (I’m a newer Nix user, though, so it’s completely possible that there’s something I’ve missed or misunderstood!)

Remove all Handbrake changes from this PR.

Update the PR so that the only change to Handbrake is to add lzma and the Darwin libraries as dependencies. Users on macOS will need to set useGtk = false in config.nix in order to install Handbrake.

Leave the PR as is. Users on macOS will only be able to install headless Handbrake.

Hold the PR until it offers the native Handbrake GUI on macOS.

Hold the PR until it offers the GTK GUI on macOS. (Is this possible?)

I’ll leave it to you to decide which resolution would be best.

@Anton-Latukha Any thoughts on this?

then I faced the problem that Handbrake still had a transitive dependency on systemd (which is not available on macOS). I’m not sure which of Handbrake’s dependencies introduced this transitive dependency

This derivation needs to be found. Consider using nix why-depends.

The dependency chain seems to be Handbrake → various parts of GTK+3 → libatk → at-spi2 → dbus → systemd.

nix why-depends --all nixpkgs.handbrake nixpkgs.systemd

/nix/store/5kainpmfmb0i12a73i6zn08piqyxvv7m-handbrake-1.2.2 ╚═══bin/.ghb-wrapped: …IL_56.LIBAVFORMAT_58./nix/store/hg8r2zrmpkx3al9hsvnkrxvr4rifl54k-gtk+3-3.24.8/lib:/nix/store/164… => /nix/store/hg8r2zrmpkx3al9hsvnkrxvr4rifl54k-gtk+3-3.24.8pkx3al9hsvnkrxvr4rifl54k-gtk+3-3.24.8/share/gsettings-sc… bin/ghb: …xport XDG_DATA_DIRS='/nix/store/hg8r2zrmpkx3al9hsvnkrxvr4rifl54k-gtk+3-3.24.8/share/gsettings-sc… ╚═══bin/gtk-launch: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-am-et.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-cedilla.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-cyrillic-translit.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-inuktitut.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-ipa.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-multipress.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-thai.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-ti-er.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-ti-et.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-viqr.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-wayland.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-waylandgtk.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/immodules/im-xim.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/printbackends/libprintbackend-cups.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/printbackends/libprintbackend-file.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/gtk-3.0/3.0.0/printbackends/libprintbackend-lpr.so: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… => /nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.08yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… lib/libgtk-3.so.0.2404.4: …w2w23-atk-2.32.0/lib:/nix/store/hp9m8yl53dmgydini31l71x8g3407gmq-at-spi2-atk-2.32.0/lib:/nix/sto… ╚═══lib/libatk-bridge-2.0.so.0.0.0: …j8lg-glib-2.60.3/lib:/nix/store/jlnmaraahxbpnygmhjm1qzsyn8n2b9m8-at-spi2-core-2.32.1/lib:/nix/st… => /nix/store/jlnmaraahxbpnygmhjm1qzsyn8n2b9m8-at-spi2-core-2.32.1 ╚═══libexec/at-spi-bus-launcher: …a boolean but got %s./nix/store/k02nmqccdqhyk00k4a6hr20kv1f5v6pq-dbus-1.12.16/bin/dbus-daemon...… => /nix/store/k02nmqccdqhyk00k4a6hr20kv1f5v6pq-dbus-1.12.16 ╚═══etc/systemd/user/dbus.socket: …t/bus.ExecStartPost=-/nix/store/hach8csknszq07jfv8bdgqhgwmj7bcz7-systemd-242/bin/systemctl --use… => /nix/store/hach8csknszq07jfv8bdgqhgwmj7bcz7-systemd-242

@bdesham

Ooops, really sorry for the long pause.

Preface: I myself learned quickly that small PRs are way better then complex epic ones. If you can avoid making an epic or complex PR.

It would be easier for you to merge changes, if you branch-out and separately PR changes to different packages. If your package fixes rely on each-other - you can keep the main develop branch for yourself, and cherry-pick your main develop branch changes into different branches.

Some while back I touched ATK and at-spi-core2, since they implement accessibility specification and use dbus I wonder why they require systemd.

I am currently researching the question about dependency on systemd, also feel free to pursue yourself.

I had picked the ffmpeg and mp4v2 stuff around a week ago.

Situation of dependency on systemd is covered in 3 comments here #62421 (comment)

@bdesham Thanks for sharing story of experience.
Since now the source of problem is known - current changes are reasonable. Would fix GUI when upstream issue for it is resolved.

I don't think that libappindicator-gtk3 is needed by handbrake at all! I can't find it in any build script. Perhaps we can just remove that everywhere.

libgudev & udev are optional dependencies, but can be just added on linux. Otherwise gtk support should just work.

pkgs/development/libraries/cairo/default.nix

matthewbauer

cairo has worked on x86_64-darwin for a long time. You shouldn't need to make any changes to this

This involved several pieces: - Always disable GTK GUI support under Darwin. The gtk3 package depends transitively on dbus, which depends transitively on systemd, which is not currently supported on Darwin. (I gather that it may be possible to work around this in the future.) - Also disable the native GUI support under Darwin (using the --disable-xcode flag). Building this GUI would require using the Xcode build system, which I was not able to figure out how to do; for now, all builds on Darwin are command-line-only. - Add the lzma package as a dependency on all platforms. - Add dependencies on the AudioToolbox, Foundation, libobjc, and VideoToolbox packages on Darwin.

bdesham · 2019-07-19T17:22:20Z

I’ve force-updated the branch so that it’s based on the latest nixpkgs master branch. I removed all of the commits except for the Handbrake one, since @vcunat was kind enough to cherry-pick some of the other commits earlier and, as @matthewbauer points out, the cairo changes don’t actually seem to be necessary. (I’ll open a new PR later to add the video-transcoding scripts that this PR used to include.) I also added some notes to the Handbrake default.nix file and added some commentary to the commit message; I didn’t change any of the code itself.

I want to apologize to all of you for this PR turning into such a mess—I should have broken it up into several separate, orthogonal PRs. I will definitely keep that in mind next time 🙂 Thank you everyone for the kind feedback and suggestions on this!

matthewbauer

Looks good now! Ideally gtk backend would work, but it may not be necessary for most users

bdesham · 2019-08-26T17:00:10Z

This is good to be merged, right? Would someone with write access mind doing that?

aanderse · 2019-08-31T15:40:25Z

@GrahamcOfBorg build handbrake

aanderse · 2019-08-31T22:03:32Z

@bdesham can you please look into the aarch64-linux failure if you get a chance?

vcunat · 2019-09-01T09:21:26Z

I see

cp: cannot create regular file './internal_defaults.json': File exists

which does not seem related to this PR. It's probably some race in the build system, as on aarch64.nixos.community it was fixed by --cores 1. Perhaps it will be better to disable parallel building? It doesn't take too long.

bdesham · 2019-09-15T22:33:08Z

@vcunat Is there anything I can do to move this PR along? I don’t have write access and I’m not sure which parallel-building setting you’re referring to.

vcunat · 2019-09-16T06:32:13Z

I meant: aaf3881. (Easier done than explained.)

bdesham · 2019-09-16T14:06:39Z

Thanks @vcunat!

ofborg bot added 8.has: clean-up 8.has: package (new) labels Jun 22, 2019

ofborg bot requested review from wmertens and Fuuzetsu June 22, 2019 03:41

ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 11-100 labels Jun 22, 2019

knedlsepp approved these changes Jun 22, 2019

View reviewed changes

Anton-Latukha suggested changes Jun 22, 2019

View reviewed changes

bdesham mentioned this pull request Jul 12, 2019

ffmpeg-full does not build on macOS #64651

Closed

vcunat pushed a commit that referenced this pull request Jul 13, 2019

ffmpeg-full: Fix on Darwin (PR #63643)

Verified

This commit was signed with the committer’s verified signature.

vcunat Vladimír Čunát

GPG key ID: E747DF1F9575A3AA

Verified
Learn about vigilant mode

2d2a3dc

Anton-Latukha mentioned this pull request Jul 19, 2019

at-spi2-core has a large closure size #62421

Open

Anton-Latukha approved these changes Jul 19, 2019

View reviewed changes

matthewbauer reviewed Jul 19, 2019

View reviewed changes

pkgs/development/libraries/cairo/default.nix Outdated Show resolved Hide resolved

matthewbauer reviewed Jul 19, 2019

View reviewed changes

pkgs/development/libraries/cairo/default.nix Outdated Show resolved Hide resolved

matthewbauer requested changes Jul 19, 2019

View reviewed changes

bdesham force-pushed the video-transcoding branch from c35f9fb to 765cd8c Compare July 19, 2019 17:18

bdesham changed the title ~~Video transcoding~~ Fix Handbrake build on Darwin by turning off GUI support Jul 19, 2019

ofborg bot added the 6.topic: darwin label Jul 19, 2019

matthewbauer approved these changes Jul 19, 2019

View reviewed changes

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 11-100 labels Jul 19, 2019

vcunat added a commit that referenced this pull request Sep 16, 2019

Merge #63643: handbrake: fix build on Darwin

Verified

This commit was signed with the committer’s verified signature.

vcunat Vladimír Čunát

GPG key ID: E747DF1F9575A3AA

Verified
Learn about vigilant mode

48af02a

vcunat approved these changes Sep 16, 2019

View reviewed changes

vcunat merged commit 765cd8c into NixOS:master Sep 16, 2019

bdesham deleted the video-transcoding branch September 16, 2019 14:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Handbrake build on Darwin by turning off GUI support #63643

Fix Handbrake build on Darwin by turning off GUI support #63643

bdesham commented Jun 22, 2019

Anton-Latukha Jun 22, 2019 •

edited

Loading

bdesham Jun 24, 2019 •

edited

Loading

bdesham Jul 2, 2019

FRidh Jul 12, 2019

bdesham Jul 12, 2019

Anton-Latukha Jul 19, 2019 •

edited

Loading

vcunat Jul 19, 2019

Anton-Latukha Jul 19, 2019 •

edited

Loading

Anton-Latukha Jul 19, 2019

matthewbauer Jul 19, 2019 •

edited

Loading

matthewbauer left a comment

bdesham commented Jul 19, 2019

matthewbauer left a comment

bdesham commented Aug 26, 2019

aanderse commented Aug 31, 2019

aanderse commented Aug 31, 2019

vcunat commented Sep 1, 2019

bdesham commented Sep 15, 2019

vcunat commented Sep 16, 2019

bdesham commented Sep 16, 2019

Fix Handbrake build on Darwin by turning off GUI support #63643

Fix Handbrake build on Darwin by turning off GUI support #63643

Conversation

bdesham commented Jun 22, 2019

Motivation for this change

Things done

Anton-Latukha Jun 22, 2019 • edited Loading

Choose a reason for hiding this comment

bdesham Jun 24, 2019 • edited Loading

Choose a reason for hiding this comment

bdesham Jul 2, 2019

Choose a reason for hiding this comment

FRidh Jul 12, 2019

Choose a reason for hiding this comment

bdesham Jul 12, 2019

Choose a reason for hiding this comment

Anton-Latukha Jul 19, 2019 • edited Loading

Choose a reason for hiding this comment

vcunat Jul 19, 2019

Choose a reason for hiding this comment

Anton-Latukha Jul 19, 2019 • edited Loading

Choose a reason for hiding this comment

Anton-Latukha Jul 19, 2019

Choose a reason for hiding this comment

matthewbauer Jul 19, 2019 • edited Loading

Choose a reason for hiding this comment

matthewbauer left a comment

Choose a reason for hiding this comment

bdesham commented Jul 19, 2019

matthewbauer left a comment

Choose a reason for hiding this comment

bdesham commented Aug 26, 2019

aanderse commented Aug 31, 2019

aanderse commented Aug 31, 2019

vcunat commented Sep 1, 2019

bdesham commented Sep 15, 2019

vcunat commented Sep 16, 2019

bdesham commented Sep 16, 2019

Anton-Latukha Jun 22, 2019 •

edited

Loading

bdesham Jun 24, 2019 •

edited

Loading

Anton-Latukha Jul 19, 2019 •

edited

Loading

Anton-Latukha Jul 19, 2019 •

edited

Loading

matthewbauer Jul 19, 2019 •

edited

Loading