Bug 710904 (oct-communications)

I will take the review.

Ping José? Are you going to do the review or not?

Looks like José is MIA. Taking over review.

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

Ping Thomas??

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.

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?

(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

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.

Very good. This package has been

APPROVED

Thanks a lot!

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

Git done (by process-git-requests).

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

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

octave-communications-1.1.0-2.fc16 has been pushed to the Fedora 16 testing repository.

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.

octave-communications-1.1.0-2.fc16 has been pushed to the Fedora 16 stable repository.