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

Jump to bottom

mixx: Build with libshout 2.4.1 #82624

Merged
merged 1 commit into from Mar 29, 2020
Merged

mixx: Build with libshout 2.4.1 #82624

merged 1 commit into from Mar 29, 2020

Conversation

matthiasbeyer
Copy link
Contributor

Because libshout 2.4.2 and newer seem to break streaming in mixxx, build
it with 2.4.1 instead.

This actually fixes the problem.

Motivation for this change

DJing without streaming is just half the fun, especially in times of Corona.

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.

Should probably be backported to the stable channel 19.09

@matthiasbeyer
Copy link
Contributor Author

CC @haslersn

@aszlig
Copy link
Member

aszlig commented Mar 16, 2020

Since libshout is a bundled dependency, maybe it makes sense to use the bundled one instead, so that we avoid such incompatibilities in the future?

Other than that, which error did you get? I'm asking because maybe we can supply an upstream patch and use that one instead.

@matthiasbeyer
Copy link
Contributor Author

I was unable to connect to my streaming server. It told me that the connection was unavailable or the password/user was wrong. That was all I got, the console output of mixxx did not tell me anything about the actual problem.

aszlig
aszlig approved these changes Mar 17, 2020
View reviewed changes
Copy link
Member

@aszlig aszlig left a comment

Choose a reason for hiding this comment

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

Mhm, as a temporary measure, I'd say it's okay, but I think it makes sense to add a comment on the libshout definition to elaborate why we're using an older version.

@matthiasbeyer
Copy link
Contributor Author

I'll do this. Maybe not today anymore.

@ofborg ofborg bot requested a review from aszlig March 18, 2020 09:41
etu
etu requested changes Mar 28, 2020
View reviewed changes
Copy link
Contributor

@etu etu left a comment

Choose a reason for hiding this comment

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

Wouldn't it be easier and cleaner to include libshout and just make an overrideAttrs to override the specific attrs to build it with an older version rather than duplicating the entire derivation?

@matthiasbeyer
Copy link
Contributor Author

More like this?

I'll squash if you consider this ready.

@etu
Copy link
Contributor

etu commented Mar 28, 2020

@matthiasbeyer I was thinking to still have it in the let, and call it libshout' or something. Then the buildInputs look cleaner as well :)

@etu
Copy link
Contributor

etu commented Mar 28, 2020

@matthiasbeyer Looks good! Feel like squashing it?

@matthiasbeyer
mixx: Build with libshout 2.4.1
3a3860f 
Because libshout 2.4.2 and newer seem to break streaming in mixxx, build
it with 2.4.1 instead.

This actually fixes the problem.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
@etu etu merged commit 74b2eae into NixOS:master Mar 29, 2020
@matthiasbeyer matthiasbeyer deleted the mixxx-with-old-libshout branch March 29, 2020 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etu etu etu requested changes

@cillianderoiste cillianderoiste Awaiting requested review from cillianderoiste

@bfortz bfortz Awaiting requested review from bfortz

@aszlig aszlig Awaiting requested review from aszlig

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@matthiasbeyer @aszlig @etu