sdr-j-fm: 3.16-unstable-2023-12-07 → 3.20-2025-10-07#448315
sdr-j-fm: 3.16-unstable-2023-12-07 → 3.20-2025-10-07#448315doronbehar merged 5 commits intoNixOS:masterfrom
Conversation
Sigmanificient
changed the title
nixpkgs-ci
bot
added
10.rebuild-linux: 1-10
|
doronbehar
left a comment
doronbehar
left a comment
There was a problem hiding this comment.
Thanks for the update! Looks good overall.
pkgs/by-name/sd/sdr-j-fm/package.nix
Outdated
| wrapProgram $out/bin/fmreceiver \ | ||
| --prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath finalAttrs.runtimeDependencies} |
There was a problem hiding this comment.
This is surprising. Do you have an idea why is this needed? Could you share what happens without this?
There was a problem hiding this comment.
There was a problem hiding this comment.
Sure, see https://github.com/JvanKatwijk/sdr-j-fm/blob/283d362241da94ea935da6476cc08bc4cf2e7564/devices/rtlsdr-handler/rtlsdr-handler.cpp#L113, without it nothing works.
This link explains only librtlsdr.so, not all of the rest of the runtimeDependencies. I think that this is also something that should be avoided. Usually open source packages don't need such LD_LIBRARY_PATH wrapping. In the worst case, we can use substituteInPlace and put there the full Nix path of librtlsdr.so.
There was a problem hiding this comment.
This link explains only librtlsdr.so, not all of the rest of the runtimeDependencies.
The same for other devices: https://github.com/search?q=repo%3AJvanKatwijk%2Fsdr-j-fm%20dlopen&type=code
There was a problem hiding this comment.
In the worst case, we can use substituteInPlace and put there the full Nix path of librtlsdr.so.
I think maintaining such patch would be harder.
There was a problem hiding this comment.
Yea you might be right at the moment on this subject. I suspect that something in upstream's CMakeLists.txt should be modified to help us avoid this, so I'm consulting with a LLM for this.
There was a problem hiding this comment.
without it nothing works.
@sikmir what exactly doesn't work? I built and ran this binary with and without this wrapping and it didn't fail. I got many times this message:
ALSA lib pcm_dmix.c:1000:(snd_pcm_dmix_open) unable to open slave
And many more... But no substantial failure.
There was a problem hiding this comment.
Try to test with real hardware. I've tested with rtl-sdr. You will get failed to open librtlsdr.so.
There was a problem hiding this comment.
sikmir
force-pushed
the
sdr-j-fm
branch
4 times, most recently
from
a7c8225 to
5af3e39
Compare
sikmir
force-pushed
the
sdr-j-fm
branch
2 times, most recently
from
1826094 to
9ecfa50
Compare
|
Let's give upstram a few days to reply. Also, the commit they merged regarding Qt::Xml is much larger, then the patch we currently pull. Perhaps if they'll fix the linking issue we might want to update to an unstable version. |
doronbehar
changed the title
|
I decided I won't mind this getting merged as is. However I changed the commit log a bit, and added a comment stating that the |
|
Taking your 👍 reaction as an approval, merging. |
| stdenv.mkDerivation (finalAttrs: { | ||
| pname = "sdr-j-fm"; | ||
| version = "3.20"; | ||
| version = "3.20-2025-10-07"; |
There was a problem hiding this comment.
3 participants
JvanKatwijk/sdr-j-fm@8e3a67f...3.20
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.