Skip to content

gnuradio: Add pyzmq as dependency to gr-zeromq#158070

Closed
charles-dyfis-net wants to merge 1 commit intoNixOS:masterfrom
charles-dyfis-net:gnuradio-pyzmq-dep
Closed

gnuradio: Add pyzmq as dependency to gr-zeromq#158070
charles-dyfis-net wants to merge 1 commit intoNixOS:masterfrom
charles-dyfis-net:gnuradio-pyzmq-dep

Conversation

@charles-dyfis-net
Copy link
Contributor

@charles-dyfis-net charles-dyfis-net commented Feb 4, 2022

Motivation for this change

GNU Radio ships with support for the 0mq message bus. For C++, this dependency is properly recognized; however, trying to execute a flowgraph containing Python 0mq blocks in GNU Radio Companion causes an import exception.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested review from bjornfor, doronbehar and fpletz February 4, 2022 01:07
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 4, 2022
@doronbehar
Copy link
Contributor

Indeed there's at least 1 python file in https://github.com/gnuradio/gnuradio/tree/maint-3.8/gr-zeromq/python/zeromq in the maint-3.{8,9,10} branches that import zmq. This issue should be reported upstream, or could it be that this is not a required dependency, but an optional one for only some flowgraphs? I might not be accurate with the terminology so I apologize in advance.

@charles-dyfis-net
Copy link
Contributor Author

I've talked to upstream (via https://chat.gnuradio.org/#/room/#gnuradio:gnuradio.org) only about 3.10, where they indicated that it should be a dependency (and is in fact one for the official Ubuntu packages).

That said, you're right that it's only used by a subset of blocks, and thus that one could reasonably choose to make those blocks optional.

@charles-dyfis-net
Copy link
Contributor Author

@doronbehar, I'm just now discovering something now that I presume you already knew (as the context makes your prior comment make a great deal more sense to me).

pyzmq is listed as a mandatory dependency in the upstream Debian packaging: https://github.com/gnuradio/gnuradio/blob/maint-3.10/.packaging/debian/control#L59

...but not listed at all in the upstream Fedora packaging at https://github.com/gnuradio/gnuradio/blob/maint-3.10/.packaging/fedora/gnuradio.spec (at least as of present date).

This smells to me like a bug in upstream's Fedora packaging, but I'm trying to get an explicit response as to whether one or the other is canonically correct.

@charles-dyfis-net
Copy link
Contributor Author

...also an explicit, mandatory dependency of the condaforge package: https://github.com/conda-forge/gnuradio-feedstock/blob/e3104cd7ae56f6aa4c6e99ef60b49c31c3328724/recipe/meta.yaml#L496 (thanks to its maintainer, @ryanvolz, for pointing this out).

@doronbehar
Copy link
Contributor

@charles-dyfis-net sorry for not being responsive, it's an exam period for me nowadays. It might be nice to open an issue upstream just to ask them if they would consider this dependency a mandatory one, and perhaps point them towards the difference between Fedora and Debian, they are rather responsive there from my experience.

I'd like to stick to their preference regarding this, and I'd also like their cmake files to check for the existence of this dependency for the gr-zeromq module. If pyzmq is considered optional by upstream for this module, remember you can always use the gnuradio wrapper derivation available in Nixpkgs which was created back in #99685 just for this kind of scenarios - you can use the extraPythonPackages when you override the gnuradio package and that will not cause a rebuild of the whole package, but just the wrapper.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 12, 2022
@doronbehar
Copy link
Contributor

I'm closing, as this is not a critical issue, and it is easily fixable by overriding gnuradio with an pyzmq to extraPythonPackages. Also there are merge conflicts now. Upstream also should enforce this policy in their cmake files. If they'll do that in their next release, we'll notice that as the build would fail.

@doronbehar doronbehar closed this Jun 28, 2023
@doronbehar
Copy link
Contributor

After trying to make the tests work, I decided to add pyzmq by default, this is done in #257809 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants