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

bluez: add tools to output that we were building anyway #39813

Merged
merged 1 commit into from
May 10, 2018

Conversation

peterhoeg
Copy link
Member

@peterhoeg peterhoeg commented May 1, 2018

Motivation for this change

A bunch of the old bluez tools have been deprecated (hciconfig, hcitool) but we weren't distributing their (experimental) replacements making certain bluetooth operations rather difficult.

This PR just adds in what we were building anyway (lifted from Arch Linux) + a few minor style cleanups.

Cc: @globin @fpletz

I haven't run nox-review as it wants to build the ~~~entire KDE suite~~~ a ton of packages.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

@peterhoeg peterhoeg requested review from fpletz and globin May 1, 2018 14:07
@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 101-500 labels May 1, 2018
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: bluez

Partial log (click to expand)

/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.list-devices-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.test-sap-server-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.simple-agent-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.simple-endpoint-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.test-hfp-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.test-network-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.opp-client-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test/test/.simple-player-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
checking for references to /build in /nix/store/ac1mr87dfjdqnn9g32ah0jav4zk45v6p-bluez-5.49-test...
/nix/store/a3jw8vc44p72cl9w5si27jw8gx7zpwdl-bluez-5.49

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: bluez

Partial log (click to expand)

/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.list-devices-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.test-sap-server-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.simple-agent-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.simple-endpoint-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.test-hfp-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.test-network-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.opp-client-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test/test/.simple-player-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
checking for references to /build in /nix/store/0rfm4kl0p5p2a7cs9l5rcgmnb6n3afm9-bluez-5.49-test...
/nix/store/50b4vkhl62x6y02j7jz0jigw6nkr9dbs-bluez-5.49

@@ -73,14 +76,21 @@ stdenv.mkDerivation rec {
# Add extra configuration
mkdir $out/etc/bluetooth
ln -s /etc/bluetooth/main.conf $out/etc/bluetooth/main.conf

# Add missing tools, ref https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/bluez
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem like something we should be doing- but I guess if Arch thinks its a good idea. Could you try doing this instead though:

substituteInPlace Makefile.tools --replace noinst_PROGRAMS bin_PROGRAMS

Not sure if it will work but IMO much cleaner than the for loop.

Copy link
Member Author

@peterhoeg peterhoeg May 1, 2018

Choose a reason for hiding this comment

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

This doesn't seem like something we should be doing

Why not? Some of these tools might be experimental but I fail to see why making them available is bad in any way. It's not a change of behaviour.

The substituteInPlace didn't do the trick unfortunately (it would indeed have been much cleaner).

Copy link
Member

Choose a reason for hiding this comment

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

I guess the main question is: why didn't the maintainers have these installed by default? It could just be a bug but there might also be a valid reason. Definitely not a blocker for this - I am just skeptical of custom install scripts in general.

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on my reading of the ML archives, the bluez guys deprecated the existing tools before having the replacements ready and while these tools are experimental, they are needed for certain tasks (example: toggle an adapter to LE mode only) that were previously the responsibility of hciconfig and hcitool.

The existence of them doesn't mean that the bluetooth daemon starts doing stuff differently so everything should work as normal but we now make low-level manipulation of the bluetooth stack/adapters possible again.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: bluez

Partial log (click to expand)

/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.list-devices-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.test-sap-server-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.simple-agent-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.simple-endpoint-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.test-hfp-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.test-network-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.opp-client-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
/nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test/test/.simple-player-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/ydxpyrgfqj6z7lqnv059566l7kpn35j3-python-2.7.14/bin/python"
checking for references to /build in /nix/store/gfkqd7aqlcz13jhwfm08f3aw2y3r3dp0-bluez-5.49-test...
/nix/store/dgwlfhsh326sxp73b5bn4jlwphjm3wch-bluez-5.49

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: bluez

Partial log (click to expand)

/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.list-devices-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.test-sap-server-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.simple-agent-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.simple-endpoint-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.test-hfp-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.test-network-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.opp-client-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
/nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test/test/.simple-player-wrapped: interpreter directive changed from "/usr/bin/python" to "/nix/store/l9j2jsc9flrbmpf799nw9wdq59gpwms8-python-2.7.14/bin/python"
checking for references to /build in /nix/store/78qy6zhvby6x3k68h0y8wcxzlxn4460j-bluez-5.49-test...
/nix/store/w56d7v41x9vgzs0c56kf5crm78cnr7gs-bluez-5.49

@peterhoeg
Copy link
Member Author

Cc @andir

@andir
Copy link
Member

andir commented May 5, 2018

Thanks for working on this (@peterhoeg). I am currently building things and running a nox-review over night. It looks pretty cleanish. As Matthew already mentioned substituteInPlace would have be nicer but if that is not a working solution I am fine with the proposed one.

I'll give it some verification during the coming day. I expect no major breakage since we are just adding a few files.

@peterhoeg
Copy link
Member Author

It should be quite safe considering we're just adding a few binaries but not otherwise changing the build.

@peterhoeg
Copy link
Member Author

@andir, you're good with this?

@andir
Copy link
Member

andir commented May 10, 2018

@peterhoeg It looks alright. I didn't get to testing this so far. If you (both?) tested that I'm fine with it.

@peterhoeg
Copy link
Member Author

My interest was in btmgmt which works here - I haven't tested that it otherwise works as it will require a massive rebuild of packages here.

@andir
Copy link
Member

andir commented May 10, 2018

I backported it to my 18.03 laptop that I've with me and will try to test the bluetooth stuff once my rebuild is finished.

Copy link
Member

@andir andir left a comment

Choose a reason for hiding this comment

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

LGTM. I was able to pair with a new BT device.

Also the new tools seem to work for me.

@peterhoeg peterhoeg merged commit 10c9ec6 into NixOS:master May 10, 2018
@peterhoeg peterhoeg deleted the f/bluez branch May 10, 2018 23:15
@peterhoeg peterhoeg restored the f/bluez branch May 11, 2018 00:52
@peterhoeg peterhoeg deleted the f/bluez branch October 8, 2018 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 101-500
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants