-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
saleae-logic: 1.2.10 -> 1.2.28 #53844
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
Conversation
]; | ||
|
||
in | ||
|
||
assert stdenv.system == "x86_64-linux"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stdenv.system is deprecated since NixOS 18.09 (e.g. see 2c2f1e3). Here we should replace it with stdenv.hostPlatform.system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -91,7 +92,7 @@ stdenv.mkDerivation rec { | |||
description = "Software for Saleae logic analyzers"; | |||
homepage = http://www.saleae.com/; | |||
license = licenses.unfree; | |||
platforms = [ "x86_64-linux" "i686-linux" ]; | |||
platforms = subtractLists platforms.i686 platforms.linux; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't seen this before, so I evaluated it in nix repl. Turns out that platforms.i686 is of a different type than platforms.linux, so nothing gets subtracted:
$ nix repl '<nixpkgs>'
nix-repl> (lib.subtractLists lib.platforms.i686 lib.platforms.linux) == lib.platforms.linux
true
Actually, I'm confused about platforms.linux no longer being a simple list of strings (["x86_64-linux" ...]). (I'm not that active in nixpkgs development.)
I guess I'd just do platforms = [ "x86_64-linux" ]
for now, unless someone can tell us what the proper way to limit to 64-bit linux with platforms.* attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the "x86_64-linux" assert at the top of the expression, I guess using "platforms = platforms.linux" is fine as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline comments. Otherwise, looks good.
@RostakaGmfun are you able to address the comments mentioned above? |
005bda7
to
4ac83f9
Compare
Apologies for the delay and thank you for the review! @bjornfor, please re-review the patch. |
It fails to start for me:
(Doing the same on master branch works fine.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ nix-build -A saleae-logic && ./result/bin/saleae-logic
/nix/store/qii77f2aqakzgdb6i1wlmsflp1k8qllr-saleae-logic-1.2.28
/nix/store/qii77f2aqakzgdb6i1wlmsflp1k8qllr-saleae-logic-1.2.28/Logic: error while loading shared libraries: libGL.so.1: cannot open shared object file: No such file or directory
@bjornfor, it's great you have noticed this. Turns out my I have added the libGL to library paths, and verified that I am able to run the binary with LD_LIBRARY_PATH set to "". |
The older 1.2.10 version does not support new Saleae devices well. i686 platform was removed because Saleae stopped providing 32-bit builds since 1.2.11.
4ac83f9
to
8434b51
Compare
@RostakaGmfun: I also have |
Thank you @bjornfor! |
@RostakaGmfun: Hi, I found that the .28 version is a beta version. The latest stable is .18. I made a PR to use the latest stable version. (I feel more comfortable backporting this to 19.03 if we use stable version.) |
Motivation for this change
The older 1.2.10 version does not support new Saleae devices well, like Saleae Logic Pro 16.
Things done
i686 platform was removed because Saleae stopped providing 32-bit
builds since 1.2.11.
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)