Bug 710904 (oct-communications) - Review Request: octave-communications - Communications for Octave
Summary: Review Request: octave-communications - Communications for Octave
Keywords:
Status: CLOSED ERRATA
Alias: oct-communications
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: oct-signal
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-06-05 16:23 UTC by Thomas Sailer
Modified: 2011-12-30 00:58 UTC (History)
4 users (show)

Fixed In Version: octave-communications-1.1.0-2.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-12-30 00:57:37 UTC
Type: ---
susi.lehtola: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Thomas Sailer 2011-06-05 16:23:07 UTC
Spec URL: http://sailer.fedorapeople.org/octave-communications.spec
SRPM URL: http://sailer.fedorapeople.org/octave-communications-1.0.10-1.fc15.src.rpm
Description: Digital Communications, Error Correcting Codes (Channel Code), Source Code functions, Modulation and Galois Fields

Comment 1 José Matos 2011-06-13 15:21:41 UTC
I will take the review.

Comment 2 Susi Lehtola 2011-08-12 17:45:58 UTC
Ping José? Are you going to do the review or not?

Comment 3 Susi Lehtola 2011-08-16 07:04:20 UTC
Looks like José is MIA. Taking over review.

Comment 4 Susi Lehtola 2011-08-17 11:18:44 UTC
Please add comments about the functionality of Patch0 and Patch1.

**

Why aren't you using %{octave_pkg_build}?

**

You need to add the -v flag to the mkoctfile commend so that build flags can be seen.

**

The package fails to build:

+ '[' -e /builddir/build/BUILDROOT/octave-communications-1.0.10-1.fc15.x86_64/usr/share/octave/packages/communications-1.0.10/packinfo/on_uninstall.m ']'
+ echo 'function on_uninstall (desc)'
/var/tmp/rpm-tmp.7FEKFz: line 44: /builddir/build/BUILDROOT/octave-communications-1.0.10-1.fc15.x86_64/usr/share/octave/packages/communications-1.0.10/packinfo/on_uninstall.m: No such file or directory
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.7FEKFz (%install)
    Bad exit status from /var/tmp/rpm-tmp.7FEKFz (%install)
Child returncode was: 1

Comment 5 Susi Lehtola 2011-12-15 23:36:27 UTC
Ping Thomas??

Comment 6 Thomas Sailer 2011-12-16 17:40:17 UTC
Spec URL: http://sailer.fedorapeople.org/octave-communications.spec
SRPM URL: http://sailer.fedorapeople.org/octave-communications-1.1.0-1.fc16.src.rpm

Pong...

I've updated the package to the current version.

I can't use %{octave_pkg_build}, because dependency checking does not work for me.

Comment 7 Susi Lehtola 2011-12-17 09:39:59 UTC
Does not build:

Making dvi comms.dvi
/bin/sh: line 4: texi2dvi: command not found
make: *** [comms.dvi] Error 127

Adding BR /usr/bin/texi2dvi fixes this problem.

**

BuildRequires:  octave-devel octave-signal >= 1.0.0 octave-image >= 0.0.0 hdf5-devel

is somewhat complicated. I would prefer the BRs once per line, especially if you state EVR requirements.

**

Please take a habit of preserving permissions while copying sources, e.g.
 cp -p %{SOURCE1} %{SOURCE2} .

Also, if you give executable rights to the source files, you don't need to run chmod in the spec. 

**

You do know that

pushd doc
make
popd

can be written simply as

make -C doc

**

Please get rid of the %attr lines, they shouldn't be necessary.

**

Didn't you file a bug against octave for the octave_cmd bit not working?

Comment 8 Susi Lehtola 2011-12-17 09:55:08 UTC
(In reply to comment #7)
> Also, if you give executable rights to the source files, you don't need to run
> chmod in the spec. 

Scrap this, it will cause an rpmlint warning.

**

rpmlint output:

octave-communications.x86_64: W: obsolete-not-provided octave-forge
octave-communications.x86_64: W: hidden-file-or-dir /usr/share/octave/packages/communications-1.1.0/packinfo/.autoload
octave-communications.x86_64: E: zero-length /usr/share/octave/packages/communications-1.1.0/packinfo/.autoload
octave-communications.x86_64: E: script-without-shebang /usr/share/octave/packages/communications-1.1.0/@galois/fft.m
octave-communications.x86_64: E: script-without-shebang /usr/share/octave/packages/communications-1.1.0/comms.info
octave-communications.x86_64: W: dangerous-command-in-%preun rm
3 packages and 0 specfiles checked; 3 errors, 5 warnings.

These are expected for Octave packages.


MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
MUST: The spec file for the package is legible and macros are used consistently. ~OK
- Please address the issues raised above.

MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
- License is GPLv2+.

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
$ md5sum communications-1.1.0.tar.gz ../SOURCES/communications-1.1.0.tar.gz 
1ec83d2757d5aa7d65be4a4c29741eba  communications-1.1.0.tar.gz
1ec83d2757d5aa7d65be4a4c29741eba  ../SOURCES/communications-1.1.0.tar.gz

MUST: The package MUST successfully compile and build into binary rpms. NEEDSWORK
- Add the missing BR.

MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK

MUST: Permissions on files must be set properly. OK
- Remove the spurious %attr lines.

MUST: Large documentation files must go in a -doc subpackage. N/A
MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK
- Licenses and so on are already installed.

MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned, architecture dependent dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK

SHOULD: The package builds in mock. NEEDSWORK
EPEL: Clean section exists. NEEDSWORK
EPEL: Buildroot cleaned before install. OK
EPEL: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A

Comment 9 Thomas Sailer 2011-12-17 12:30:50 UTC
Thanks for the review Jussi!

Spec URL: http://sailer.fedorapeople.org/octave-communications.spec
SRPM URL:
http://sailer.fedorapeople.org/octave-communications-1.1.0-2.fc16.src.rpm

Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3590036

I've changed the spec file according to your comments.

> Also, if you give executable rights to the source files, you don't need to run
> chmod in the spec. 

As you noticed, rpmlint doesn't like that, so I kept the chmod.

> Please get rid of the %attr lines, they shouldn't be necessary.

Ok, I added chmod statements in the install section and dropped the %attr's.

Two .m files are distributed with the executable bit set, so either chmod or %attr is necessary to silence rpmlint.

Comment 10 Susi Lehtola 2011-12-17 13:25:13 UTC
Very good. This package has been

APPROVED

Comment 11 Thomas Sailer 2011-12-17 15:08:41 UTC
Thanks a lot!

New Package SCM Request
=======================
Package Name: octave-communications
Short Description: Communications for Octave
Owners: sailer
Branches: f15 f16

Comment 12 Gwyn Ciesla 2011-12-18 21:31:38 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2011-12-18 23:30:03 UTC
octave-communications-1.1.0-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/octave-communications-1.1.0-2.fc15

Comment 14 Fedora Update System 2011-12-18 23:30:58 UTC
octave-communications-1.1.0-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/octave-communications-1.1.0-2.fc16

Comment 15 Fedora Update System 2011-12-21 16:59:50 UTC
octave-communications-1.1.0-2.fc16 has been pushed to the Fedora 16 testing repository.

Comment 16 Fedora Update System 2011-12-30 00:57:37 UTC
octave-communications-1.1.0-2.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2011-12-30 00:58:56 UTC
octave-communications-1.1.0-2.fc16 has been pushed to the Fedora 16 stable repository.


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