Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

firefoxen: remove unused gstreamer dependencies #50373

Merged
merged 1 commit into from Nov 16, 2018
Merged

firefoxen: remove unused gstreamer dependencies #50373

merged 1 commit into from Nov 16, 2018

Conversation

pbogdan
Copy link
Member

@pbogdan pbogdan commented Nov 14, 2018

Firefox and, by extension, Tor browser don't support gstreamer any more, this
removes what are effectively unused dependencies.

https://bugzilla.mozilla.org/show_bug.cgi?id=1234092 "Remove gstreamer support"

Motivation for this change

Working towards #22372 and #39975 (removal of old gstreamer and ffmpeg versions).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

Built firefox, firefox-esr, tor-browser-bundle and tor-browser-bundle-bin and verified media playback works in all (with mediaSupport enabled for the tor browsers).

/cc @joachifm @oxij for the tor browser changes

Not sure who, if anyone, to ping for the Firefox changes.

Firefox and, by extension, Tor browser don't support gstreamer any more, this
removes what are effectively unused dependencies.

https://bugzilla.mozilla.org/show_bug.cgi?id=1234092 "Remove gstreamer support"
@@ -19,7 +19,7 @@

, alsaSupport ? stdenv.isLinux, alsaLib
, pulseaudioSupport ? stdenv.isLinux, libpulseaudio
, ffmpegSupport ? true, gstreamer, gst-plugins-base
, ffmpegSupport ? true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this flag be removed entirely? I am wondering because how is ffmpeg pulled in if enabled?

The tor-browser-bundle has an explicit reference to ffmpeg.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT it bundles the needed headers and the libraries are loaded at runtime via dlopen so the package isn't needed at build time. For runtime the libraries are added to the LD_LIBRARY_PATH in the wrapper (wrapper.nix next to this file) based on whether the browser that is being wrapped was built with ffmpeg support.

@joachifm
Copy link
Contributor

LGTM

@andir andir merged commit b4f2c7b into NixOS:master Nov 16, 2018
@andir
Copy link
Member

andir commented Nov 16, 2018

Thanks! 👍 @pbogdan

@pbogdan pbogdan mentioned this pull request Nov 22, 2018
13 tasks
@pbogdan pbogdan deleted the ff-no-gs branch December 3, 2019 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants