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.

Sign up for GitHub

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

Jump to bottom

gst-plugins-good: Remove jack2 by default #63677

Merged
merged 1 commit into from Jul 23, 2019
Merged

gst-plugins-good: Remove jack2 by default #63677

merged 1 commit into from Jul 23, 2019

Conversation

averelld
Copy link
Contributor

@averelld averelld commented Jun 23, 2019 •
edited

Motivation for this change

This should be a simple typo introduced in 8d495e0#diff-c366b8ce64df5be37708454f340e869eR72
which created the enableJack flag but didn't remove the dependency from the previous line.

/cc @nh2

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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from matthewbauer June 23, 2019 00:18
@jtojnar
Copy link
Contributor

jtojnar commented Jun 23, 2019

Alternately, we could always build it, since I have already split ffado in #63393

@averelld
Copy link
Contributor Author

averelld commented Jun 23, 2019 •
edited

Fine with me too. Actually zbar in gst-pluglins-bad is very similar, and also pulls in big qt dependencies (that are currently broken), see https://hydra.nixos.org/build/95265067
That broken qtbase can be reached via ffado or zbar, while those packages build fine separately for some reason.

@nh2
Copy link
Contributor

nh2 commented Jun 24, 2019 •
edited

@averelld Yes, definitely a typo on my side here, as having it both as optional and non-optional doesn't make sense:

  ++ optionals stdenv.isLinux [ libv4l libpulseaudio libavc1394 libiec61883 libgudev jack2 ]
  ++ optionals (stdenv.isLinux && enableJack) [
    jack2
  ];

Alternately, we could always build it, since I have already split ffado in #63393

@jtojnar This PR still looks correct though; if we want to always build jack support, we should simply set enableJack ? true, I think.

Do you agree?

@jtojnar
Copy link
Contributor

jtojnar commented Jun 24, 2019

I would be fine with dropping the parameter and always building it. But since nothing seems to use it, it is not very urgent.

@averelld averelld mentioned this pull request Jul 23, 2019
Merged
10 tasks
@worldofpeace
Copy link
Contributor

Don't think we built this before so it shouldn't be added unconditionally, whether to enable it by default is a separate topic.

@worldofpeace worldofpeace merged commit 42f527e into NixOS:master Jul 23, 2019
@averelld averelld deleted the gst-plugins-good-build branch July 24, 2019 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nh2 nh2 nh2 approved these changes

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@averelld @jtojnar @nh2 @worldofpeace