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

mixxx: install udev rules for HID controllers #92854

Closed
wants to merge 1 commit into from
Closed

mixxx: install udev rules for HID controllers #92854

wants to merge 1 commit into from

Conversation

ejpcmac
Copy link
Contributor

@ejpcmac ejpcmac commented Jul 10, 2020

Motivation for this change

Some udev rules are needed to use Mixxx with a HID controller. This PR makes the rules available so one can add services.udev.packages = [ pkgs.mixxx ]; to its configuration.nix and get the controllers working without any more tweak.

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.

@jtojnar
Copy link
Contributor

jtojnar commented Jul 10, 2020

Ideally, upstream would install the udev file for every distro:

https://github.com/mixxxdj/mixxx/blob/bff45d7d3a5ecb4e0856af521e70b3406c4e424f/SConscript#L399-L400

and to the correct path:

https://github.com/mixxxdj/mixxx/blob/bff45d7d3a5ecb4e0856af521e70b3406c4e424f/SConscript#L384

It should be in libdir (a.k.a. unix_lib_path), /etc is reserved for administrators, see https://www.freedesktop.org/software/systemd/man/udev.html#Rules%20Files

@ejpcmac
Copy link
Contributor Author

ejpcmac commented Jul 11, 2020

Mmm. I’m not sure to understand completely the SConscript (still in my wake up process): should it work on NixOS? Is it possible for it to work on every distro? As for the postInstall thing and the path, I’ve taken inspiration from the openocd derivation:

nixpkgs/pkgs/development/tools/misc/openocd/default.nix

Lines 46 to 54 in 84cf00f

postInstall = lib.optionalString stdenv.isLinux ''
mkdir -p "$out/etc/udev/rules.d"
rules="$out/share/openocd/contrib/60-openocd.rules"
if [ ! -f "$rules" ]; then
echo "$rules is missing, must update the Nix file."
exit 1
fi
ln -s "$rules" "$out/etc/udev/rules.d/"
'';

Do you want I add a comment to state it is a workaround until upstream has been fixed?

@jtojnar
Copy link
Contributor

jtojnar commented Jul 11, 2020

It would need to be modified. I suggest opening an issue (or better pull request) upstream and adding a comment linking to that here.

@ejpcmac
Copy link
Contributor Author

ejpcmac commented Jul 11, 2020

I’ve added a comment, changed the location and added an optionalString to patch this only for Linux. However, what has to be done upstream is not really clear for me—I’ve never written a SConscript—, so I’m not able to do a quick PR nor write a useful issue. If you have some time, can you please do it as it seems to be clear for you?

@jtojnar
Copy link
Contributor

jtojnar commented Jul 11, 2020

Looks like they added CMake script, which installs that by default: https://github.com/mixxxdj/mixxx/blob/e0bca241793926e4d57ad9e8c817fd5d7e0e5c54/CMakeLists.txt#L1146-L1151

@ejpcmac
Copy link
Contributor Author

ejpcmac commented Jul 11, 2020

Oh, that’s great news! I’ve updated my comment then, to state the workaround should not be needed for Mixxx 2.3.x.

@SuperSandro2000 SuperSandro2000 marked this pull request as draft November 29, 2020 03:13
@SuperSandro2000 SuperSandro2000 added the 2.status: wait-for-upstream Waiting for upstream fix (or their other action). label Nov 29, 2020
@prusnak prusnak mentioned this pull request Jun 29, 2021
Merged
11 tasks
@prusnak
Copy link
Member

prusnak commented Jun 29, 2021 •
edited

Mixxx 2.3.0 was just released - PR #128696

@ejpcmac can you please test whether that PR installs the udev rules correctly?

@ejpcmac
Copy link
Contributor Author

ejpcmac commented Jun 30, 2021

@prusnak Sure! I’m a bit busy at the moment so I can’t promise to do this before the week-end, but I’ll try for sure, and update the PR accordingly. Thank you for the update :)

@prusnak
Copy link
Member

prusnak commented Jun 30, 2021

The issue should be fixed via #128696 which has been just merged. Please open an issue if that is not the case and mixxx is still missing udev rules. Thanks!

@prusnak prusnak closed this Jun 30, 2021
This was referenced Jul 18, 2021
Closed
Merged
@ejpcmac ejpcmac deleted the mixxx-udev-rules branch August 27, 2021 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aszlig aszlig Awaiting requested review from aszlig

@bfortz bfortz Awaiting requested review from bfortz

@cillianderoiste cillianderoiste Awaiting requested review from cillianderoiste

Assignees
No one assigned
Labels
2.status: merge conflict 2.status: wait-for-upstream Waiting for upstream fix (or their other action). 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ejpcmac @jtojnar @prusnak @SuperSandro2000