Bug 710904 - (oct-communications) Review Request: octave-communications - Communications for Octave
Review Request: octave-communications - Communications for Octave
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Susi Lehtola
Fedora Extras Quality Assurance
:
Depends On: oct-signal
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-05 12:23 EDT by Thomas Sailer
Modified: 2011-12-29 19:58 EST (History)
4 users (show)

See Also:
Fixed In Version: octave-communications-1.1.0-2.fc16
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-12-29 19:57:37 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
susi.lehtola: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Thomas Sailer 2011-06-05 12:23:07 EDT
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 11:21:41 EDT
I will take the review.
Comment 2 Susi Lehtola 2011-08-12 13:45:58 EDT
Ping José? Are you going to do the review or not?
Comment 3 Susi Lehtola 2011-08-16 03:04:20 EDT
Looks like José is MIA. Taking over review.
Comment 4 Susi Lehtola 2011-08-17 07:18:44 EDT
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 18:36:27 EST
Ping Thomas??
Comment 6 Thomas Sailer 2011-12-16 12:40:17 EST
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 04:39:59 EST
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 04:55:08 EST
(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 07:30:50 EST
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 08:25:13 EST
Very good. This package has been

APPROVED
Comment 11 Thomas Sailer 2011-12-17 10:08:41 EST
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 16:31:38 EST
Git done (by process-git-requests).
Comment 13 Fedora Update System 2011-12-18 18:30:03 EST
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 18:30:58 EST
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 11:59:50 EST
octave-communications-1.1.0-2.fc16 has been pushed to the Fedora 16 testing repository.
Comment 16 Fedora Update System 2011-12-29 19:57:37 EST
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-29 19:58:56 EST
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.