Bug 433642 (gnuradio-review) - Review Request: gnuradio - Software defined radio framework
Summary: Review Request: gnuradio - Software defined radio framework
Alias: gnuradio-review
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Lubomir Rintel
QA Contact: Fedora Extras Quality Assurance
Whiteboard: NotReallyEasyFix
Depends On: 433949
TreeView+ depends on / blocked
Reported: 2008-02-20 16:14 UTC by Marek Mahut
Modified: 2014-08-08 11:56 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2008-04-24 07:03:50 UTC
lkundrak: fedora-review+
gwync: fedora-cvs+

Attachments (Terms of Use)
gnuradio-3.1.1-1.fc8 rpmlint (13.19 KB, text/plain)
2008-02-27 18:27 UTC, Lubomir Kundrak
no flags Details

Description Marek Mahut 2008-02-20 16:14:09 UTC
Spec URL: http://mmahut.fedorapeople.org/reviews/gnuradio/gnuradio.spec
SRPM URL: http://mmahut.fedorapeople.org/reviews/gnuradio/gnuradio-3.1.1-1.fc8.src.rpm
Description: GNU Radio is a collection of software that when combined with minimal 
hardware, allows the construction of radios where the actual waveforms 
transmitted and received are defined by software. What this means is 
that it turns the digital modulation schemes used in today's high 
performance wireless devices into software problems.

Comment 1 Marek Mahut 2008-02-20 16:15:06 UTC
Does not build on ppc64 because sdcc package is not available for ppc64 arch yet
(but it will be after Jesse's rebuild for gcc34).

Comment 2 Lubomir Kundrak 2008-02-27 16:38:57 UTC
Review for release 1.fc8:
* RPM name is OK
* Source gnuradio-3.1.1.tar.gz is the same as upstream

Needs work:
* Spec file: some paths are not replaced with RPM macros
  (wiki: Packaging/Guidelines#macros)
* Build failed in mock

--- this I tried in fedora-8-x86_64, seemed like you were lacking

checking for Python.h... no
configure: error: cannot find usable Python headers
error: Bad exit status from /var/tmp/rpm-tmp.17800 (%build)

(18 checks have been run)


%files usrp

Is usrp subpackage empty?

I'll check once I manage to build it; mock builds are running right now

Comment 3 Lubomir Kundrak 2008-02-27 18:27:00 UTC
Created attachment 296101 [details]
gnuradio-3.1.1-1.fc8 rpmlint

Marek, Marek.

1.) Seems like you are missing the following:

BuildRequires:	boost-devel
BuildRequires:	python-devel
BuildRequires:	swig
BuildRequires:	doxygen

2.) This won't get packaged on 64bits, as your python modules are architecture

Please replace python_sitelib definition with

%{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}

and its occurences with python_sitearch

3.) gnuradio-usrp sumpackage is really empty. Should it be?

4.) RPMLint doesn't like you, see the attachment.

These definitely have to be addressed:
gnuradio.x86_64: W: non-executable-in-bin ...
gnuradio.x86_64: W: devel-file-in-non-devel-package ...

5.) You install libtool files which you should have %excluded:

For example

Comment 4 Marek Mahut 2008-03-03 18:58:38 UTC
I hit a problem and trying to solve it with upstream. I'll let you know soon.

Comment 5 Marek Mahut 2008-03-04 17:40:11 UTC
Spec URL: http://mmahut.fedorapeople.org/reviews/gnuradio/gnuradio.spec
Koji build rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=490947
Koji build F8: http://koji.fedoraproject.org/koji/taskinfo?taskID=491008

- F8 does not build under ppc, because dependency sdcc is missing, will be
solved by https://admin.fedoraproject.org/updates/F8/pending/sdcc-2.6.0-11.fc8

- gnuradio-3.1.1-gcc34.patch fixes build problems under gcc 3.4

- gnuradio-3.1.1-templates.tar.gz includes files that aren't in official tar
ball, upstream just "forgot" :)

- rpmlint is kinda sane now

Comment 6 Marek Mahut 2008-03-04 17:54:28 UTC
Koji build F8: http://koji.fedoraproject.org/koji/taskinfo?taskID=491021

Comment 7 Lubomir Kundrak 2008-03-04 21:32:29 UTC
Wow, this is much better.

1.) I can not believe this worked: :)

# $ tar -cf gnuradio-3.1.1-templates.tar gnuradio-core/src/lib/gengen/*.t &&
gzip gnuradio-3.1.1-templates.tar
tar -xvvf %{SOURCE1}
mv *.t gnuradio-core/src/lib/gengen/

However, you could shorten the comment with -czf :)
And the proper way to unpack is to use %setup macro --- it can do that!
Also why do you move those *.t files? Doesn't tar preserve paths while
unpacking? And you don't uncompress the archive -- did that work? And why do you
pass two -v-s?

2.) PATH=/usr/libexec/sdcc:$PATH

%{_libexecdir} here?

3.) in %files:
Please replace the doc/ directory location with %{docdir} in  %{_datadir}/doc/usrp*

The package is brewing in my mock now, I'll proceed once it builds.
PS: Your gcc43 patch is awesome; I never seen that lon gcc43 patch yet :)

Comment 8 Lubomir Kundrak 2008-03-05 07:37:40 UTC
4.) you have the same %doc files in -doc and -examples

5.) Please look at the contents of -example package. Did you mean bytecode to go
there? Either make these executable and move to bin or libexec, or (better) move
them to doc.

6.) Nuke alsa-lib dependency. You don't depend on it. alsa-lib-devel does,
though, so this is more of a cosmetic change, it will get pulled in anyway :)

7.) You install libtool files. Exclude them.
example: /usr/lib64/python2.5/site-packages/gnuradio/gr/_gnuradio_swig_py_io.la

8.) The package should contain the text of the license
Have you asked upstream?

Comment 9 Lubomir Kundrak 2008-03-05 07:43:55 UTC
10.) gnuradio-devel:


Why do you pack shared libraries in -devel? -devel should not be required for
vital package functionality!

Hint: In most cases, *.so belong to -devel. Versioned ones never do.

11.) gnuradio-doc; maybe this is not a problem, just a question:
You have directory /usr/share/doc/usrp-3.1.1 
The package that you build is called gnuradio-usrp
Wouldn't it make sense to either rename the package to usrp, or the doc
directory to gnuradio-usrp-3.1.1?

(given what are the filenames in that package, I'd go for the first and would
put the usrp documentation into that package)

Comment 10 Marek Mahut 2008-03-05 15:58:50 UTC
Spec URL: http://mmahut.fedorapeople.org/reviews/gnuradio/gnuradio.spec

All 10 issues has been addresses. Regarding 11), you're right, debian project is
also using "usrp" as name of the package.

Comment 11 Lubomir Kundrak 2008-03-09 11:23:03 UTC
I am still wondering whether it makes sense to compile the example files:


Comment 12 Marek Mahut 2008-03-11 12:51:52 UTC
It does not really. Until bug 182498 is fixed I've excluded it from the spec
file manually.

Spec URL: http://mmahut.fedorapeople.org/reviews/gnuradio/gnuradio.spec

Comment 13 Lubomir Kundrak 2008-03-20 17:02:04 UTC
Several times looking at this over and over -- couldn't find anything more :)
I'm happy with this now, thanks for the package!


Comment 14 Marek Mahut 2008-03-22 19:57:18 UTC
Thank you Lubomir,

New Package CVS Request
Package Name: gnuradio
Short Description: Software defined radio framework
Owners: mmahut
Branches: F-8
InitialCC: astronomy-sig
Cvsextras Commits: yes

Comment 15 Kevin Fenzi 2008-03-23 01:02:25 UTC
cvs done.

Comment 16 Marek Mahut 2008-03-23 10:38:30 UTC
Thank you Kevin,
Thank you Lubomir,

Imported and build in rawhide and awaiting update for Fedora 8 (FEDORA-2008-2435).

Comment 17 Marek Mahut 2008-04-04 10:14:29 UTC
Package Change Request
Package Name: gnuradio
New Branches: EL-5

Comment 18 Kevin Fenzi 2008-04-04 15:28:33 UTC
cvs done.

Comment 19 Marek Mahut 2008-04-24 07:03:50 UTC
Thank you, tracking in Bug 443921

Comment 20 Jaroslav Škarvada 2014-08-08 10:57:02 UTC
Package Change Request
Package Name: gnuradio
New Branches: epel7
Owners: jskarvad

Comment 21 Gwyn Ciesla 2014-08-08 11:56:27 UTC
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.