gnuradio: [3.7.10.1->3.7.11] [Enable Darwin support] #26144

lukeadams · 2017-05-27T02:55:57Z

Motivation for this change

GNU Radio only required a few small tweaks to enable compilation on Darwin. Updated to 3.7.11 which is compatible with 3.10.1 and includes bugfixes.

UHD builds on Darwin with no changes, links and works
qwt6_qt4 was already Darwin enabled, but was missing AGL

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 (using nix-shell --pure)
- Linux
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.

This is my first pull request so I apologize if it is preferred to submit a separate pull per package.
There are child gnuradio packages which are still linux-only (small changes for darwin support), but I wanted to improve the PYTHONPATH situation before submitting them.
This package produces python outputs, so I propose changing this to a wrapper package which would depend on gnuradio-core and gnuradio-python. gnuradio-python would create a python package using buildPythonPackage.
I had to use DYLD_FRAMEWORK_PATH due to Python27 linking to a pure CoreFoundation, causing QWT/QT to segfault.

Attached is the output log of nox-review:
gnuradio_qwt6-qt4_uhd_darwin_buildoutput.txt

mention-bot · 2017-05-27T02:55:59Z

@lukeadams, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bjornfor, @wkennington and @FRidh to be potential reviewers.

fpletz

Cool! Thanks!

LnL7 · 2017-05-27T08:25:13Z

pkgs/applications/misc/gnuradio/default.nix

+  '';
+
+  # Check needs to run after install due to dependencies on nix store paths
+  phases = [ "unpackPhase" "patchPhase" "configurePhase"  "buildPhase" "installPhase" "fixupPhase" "checkPhase" "distPhase" ];


You should be able to use the installCheckPhase instead of reordering all the phases.

👍
I didn't even see that phase! fixed

LnL7 · 2017-05-27T08:29:01Z

pkgs/applications/misc/gnuradio/default.nix

  postInstall = ''
    printf "backend : Qt4Agg\n" > "$out/share/gnuradio/matplotlibrc"

    for file in "$out"/bin/* "$out"/share/gnuradio/examples/*/*.py; do
        wrapProgram "$file" \
            --prefix PYTHONPATH : $PYTHONPATH:$(toPythonPath "$out") \
-            --set MATPLOTLIBRC "$out/share/gnuradio"
+            --set MATPLOTLIBRC "$out/share/gnuradio" ${stdenv.lib.optionalString stdenv.isDarwin " --set DYLD_FRAMEWORK_PATH /System/Library/Frameworks"}


Can you add a comment pointing to #24693, when that's fixed we should be able to get rid of the DYLD_FRAMEWORK_PATH stuff.

lukeadams · 2017-05-28T04:15:33Z

pkgs/development/libraries/qwt/6_qt4.nix

-  buildInputs = [ qt4 ];
+  buildInputs = [ 
+    qt4 
+  ] ++ stdenv.lib.optional stdenv.isDarwin [ AGL ];


Need to change this to stdenv.lib.optionals which returns a list given a list

lukeadams · 2017-05-28T04:31:58Z

~~I ran nix-collect-garbage -d, and now running nix-build '<nixpkgs>' -p gnuradio -v 4 --pure --cores 8 --no-out-link~~

~~results in this~~

scanning for references inside ‘/nix/store/q343nsw48n3abjyx80c8z5gkm74kj048-gnuradio-3.7.11’
building path(s) ‘/nix/store/06vnp56jbw8w97c24997kmk5mic9xlm7-shell’
build input 4 does not exist
builder for ‘/nix/store/7cmldwrfp0i6lcbl8zjknvvfasvjap9j-shell.drv’ failed with exit code 1
error: build of ‘/nix/store/7cmldwrfp0i6lcbl8zjknvvfasvjap9j-shell.drv’ failed`

~~not sure what is causing it since nix-shell works fine~~

edit: it was because i had -v 4 for increased verbosity in nix-shell. Oops. I just re-ran it with nix-shell -p nox --run "nox-review wip --against master^" and all was well.

LnL7 · 2017-05-28T10:14:32Z

You should use nix-build '<nixpkgs>' -A gnuradio -p only makes sense for nix-shell, same for --pure but I think that's just ignored. You can also force a rebuild by adding --check without having to run a garbage collect first.

qwt6_qt4: enableParallelBuilding

- Add CoreAudio dependency (Darwin) for Gnuradio - Use qwt6_qt4 instead of older qwt5 - qwt is not being linked by full path so install_name_tool is used on Darwin - Clean up build inputs with link to source - Enclose Darwin modifications in `optionalString` - Enable testing - Use installCheckPhase instead of (post-build) checkPhase - Add link to DYLD_LIBRARY_PATH GH issue

LnL7 · 2017-05-29T05:40:56Z

I'm getting some test failures https://gist.github.com/LnL7/34bc91c98e28cafdc40e09013bc39e45

lukeadams · 2017-05-30T00:32:05Z

@LnL7 Test 166 seems to be one of a handful which intermittently fail due to generating (and capturing) large amounts of data. I can compile a list of these as we may want to disable them to prevent spurious failures.

Regarding 196, that is what the install_name_tool line in the postBuild is for, as CMake isn't properly linking the qwt libs on Darwin, despite finding the correct file (placing a relative path instead of a full path to the nix store library file). I'm not sure if you still have the log from it laying around, but I've had it fail due to a segfault as a result of python2 and qwt4, where qwt4 uses an impure CoreFoundation framework. (which was fixed by DYLD_FRAMEWORK_PATH) see next comment

notes:
~~- it may be beneficial to resolve the qwt linking errors with a patch. gnuradio-3.8 is in the works (introduces many breaking changes), I can check if they fixed it~~

is there a way to wrap the binaries with a specific python interpreter, but using build-output python libraries? (to avoid use of PYTHONPATH)

lukeadams · 2017-05-31T04:13:42Z

Apparently the qwt framework name was a relative path. Using install_name_tool -id to repair the Darwin qwt framework (in the qwt* packages) seems to fix the linking issue, removing the need for install_name_tool -change in the gnuradio builder! I'll be pushing the fixes to my branch soonish, and they should permanently resolve the 196 test failure (and resolve the issue in future for qwt-dependents on Darwin). 😄

I'm wondering if it might be helpful to create a darwin post-build hook that searches for issues like this, something like find $out $ -name '*.dylib' $ -print -exec sh -c 'otool -L {} | awk "!/nix/" ' ";" (although the -name '*.dylib' section needs to account for unnamed (i.e. in *.framework files) or misnamed (i.e gnuradio's use of *.so for shared libraries on Darwin) library files

unrelated to above:

since I couldn't really tell from the contributing docs, would you prefer if I pushed all of these little changes as their own commits, or should I rebase them before pushing to my branch? I been rebasing before pushing previously, but can see how that may make it more difficult for reviewers to tell that I made a change...
if bin/gnuradio-companion fails with KeyError 'options', remove ~/.gnuradio/config.conf
again with Python: somehow create pythonEnv and patch the shebang on the examples, since wrapping *.py files causes imports to fail

lukeadams · 2017-06-26T16:55:04Z

I'm going to close this until i have a chance to polish it. (next week)
Still needs to be done for this version:

Possibly bump to 3.7.11->3.7.12
Fix PYTHONPATH issues: using makeWrapper breaks .py imports on examples, etc. Need to use pythonEnv
Add dynamic PYTHONPATH/GRC_BLOCKS_PATH so gnuradio-* extensions can be used via nix-shell -p gnuradio-* without needing gnuradio-wrapper to bundle all of it together

Since GR-3.8 is coming soon with some breaking changes, I may be able to work on that too. Of note

Use of qwt6 with qt5
Python 3
Possible changes to plugins

lukeadams · 2017-07-03T19:38:59Z

@fpletz : I noticed you merged 3.7.11, which is awesome!
Regarding PythonPath, is it acceptable to make gnuradio-* act like pythonpackages?
Essentially, "plugins" would be used by calling nix-shell -p gnuradio -p gnuradio-osmosdr for example (which would dynamically set GRC_BLOCKS_PATH and PYTHONPATH). I have this functionality working on my local copy but wanted to get your thoughts.

This method prevents the need for any interpreter patching and eliminates potential user profile conflicts (e.g. GNUr-enabled python 2 being linked in the user profile), and also makes all gnuradio executables and example scripts "just work" when used within the shell.

FRidh · 2017-07-03T19:49:07Z

Something like nix-shell -p gnuradio -p gnuradio-osmosdr can only work with a setupHook. That wouldn't be run when installing these individual items in a profile. For that, you would need to perform a composition in some other way.

lukeadams changed the title ~~Gnuradio update~~ gnuradio: [3.7.10.1->3.7.11] [Enable Darwin support] May 27, 2017

LnL7 added the 6.topic: darwin Running or building packages on Darwin label May 27, 2017

fpletz approved these changes May 27, 2017

View reviewed changes

LnL7 reviewed May 27, 2017

View reviewed changes

lukeadams force-pushed the gnuradio-update branch from cd836c0 to 48d6e8a Compare May 28, 2017 04:09

lukeadams commented May 28, 2017

View reviewed changes

lukeadams force-pushed the gnuradio-update branch from 48d6e8a to 4ca0f44 Compare May 28, 2017 04:22

pSub added the 8.has: package (update) label May 28, 2017

Luke Adams added 3 commits May 28, 2017 15:22

UHD: Darwin build

ae9478d

qwt6_qt4: Add AGL input on Darwin

86ec731

qwt6_qt4: enableParallelBuilding

Luke Adams added 2 commits May 30, 2017 23:42

qwt6_qt4: use full path in library name on Darwin

b4ef3b0

gnuradio: no longer need qwt fixes on darwin

eadc23b

lukeadams force-pushed the gnuradio-update branch from 4ca0f44 to eadc23b Compare May 31, 2017 04:47

lukeadams closed this Jun 26, 2017

lukeadams mentioned this pull request Jul 12, 2017

Gnuradio: Enable Darwin Support #27344

Merged

8 tasks

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gnuradio: [3.7.10.1->3.7.11] [Enable Darwin support] #26144

gnuradio: [3.7.10.1->3.7.11] [Enable Darwin support] #26144

lukeadams commented May 27, 2017

mention-bot commented May 27, 2017

fpletz left a comment

LnL7 May 27, 2017

lukeadams May 28, 2017

LnL7 May 27, 2017

lukeadams May 28, 2017

lukeadams May 28, 2017

lukeadams May 28, 2017

lukeadams commented May 28, 2017 •

edited

LnL7 commented May 28, 2017

LnL7 commented May 29, 2017

lukeadams commented May 30, 2017 •

edited

lukeadams commented May 31, 2017 •

edited

lukeadams commented Jun 26, 2017

lukeadams commented Jul 3, 2017 •

edited

FRidh commented Jul 3, 2017

gnuradio: [3.7.10.1->3.7.11] [Enable Darwin support] #26144

gnuradio: [3.7.10.1->3.7.11] [Enable Darwin support] #26144

Conversation

lukeadams commented May 27, 2017

Motivation for this change

Things done

mention-bot commented May 27, 2017

fpletz left a comment

Choose a reason for hiding this comment

LnL7 May 27, 2017

Choose a reason for hiding this comment

lukeadams May 28, 2017

Choose a reason for hiding this comment

LnL7 May 27, 2017

Choose a reason for hiding this comment

lukeadams May 28, 2017

Choose a reason for hiding this comment

lukeadams May 28, 2017

Choose a reason for hiding this comment

lukeadams May 28, 2017

Choose a reason for hiding this comment

lukeadams commented May 28, 2017 • edited

LnL7 commented May 28, 2017

LnL7 commented May 29, 2017

lukeadams commented May 30, 2017 • edited

lukeadams commented May 31, 2017 • edited

lukeadams commented Jun 26, 2017

lukeadams commented Jul 3, 2017 • edited

FRidh commented Jul 3, 2017

lukeadams commented May 28, 2017 •

edited

lukeadams commented May 30, 2017 •

edited

lukeadams commented May 31, 2017 •

edited

lukeadams commented Jul 3, 2017 •

edited