Bug 464432 - Review Request: octopus - a TDDFT code
Summary: Review Request: octopus - a TDDFT code
Keywords:
Status: CLOSED DEFERRED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dominik 'Rathann' Mierzejewski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-28 20:54 UTC by Susi Lehtola
Modified: 2009-09-08 21:34 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-05-27 20:46:07 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
octopus-3.0.1 source file license check report (8.36 KB, text/plain)
2008-10-01 23:26 UTC, Dominik 'Rathann' Mierzejewski
no flags Details

Description Susi Lehtola 2008-09-28 20:54:02 UTC
Spec URL: http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus.spec
SRPM URL: http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus-3.0.1-1.fc9.src.rpm

Description: 
Octopus is a scientific program aimed at the ab initio virtual 
experimentation on a hopefully ever increasing range of systems 
types. Electrons are described quantum-mechanically within 
Density-Functional Theory (DFT), in its time-dependent form 
(TDDFT) when doing simulations in time. Nuclei are described
classically as point particles. Electron-nucleus interaction
is described within the Pseudopotential approximation.

rpmlint output at
http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus.lint

Comment 1 Susi Lehtola 2008-09-28 22:43:58 UTC
Release 2:
Implemented out-of-tree builds.

Spec file at
http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus.spec

SRPM at
http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus-3.0.1-2.fc9.src.rpm

rpmlint output:
octopus.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libstring_f.a
octopus.x86_64: W: devel-file-in-non-devel-package /usr/include/liboct_parser.h
octopus.x86_64: W: devel-file-in-non-devel-package /usr/include/xc_funcs.h
octopus.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libxc.a
octopus.x86_64: W: devel-file-in-non-devel-package /usr/include/string_f.h
octopus.x86_64: W: devel-file-in-non-devel-package /usr/lib64/liboct_parser.a
octopus.x86_64: W: devel-file-in-non-devel-package /usr/include/xc.h
octopus.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 46, tab: line 1)
2 packages and 1 specfiles checked; 0 errors, 8 warnings.

Comment 2 Susi Lehtola 2008-09-29 13:51:11 UTC
Release 3:
* Mon Sep 29 2008 Jussi Lehtola - 3.0.1-3
- Branch headers and .a files to -devel.
- Branch MPI binaries to MPI package.
- Add gcc-gfortran >= 4.3 to build reqs.

rpmlint output:

octopus-devel.x86_64: W: no-documentation
octopus-mpi.x86_64: W: no-documentation
SPECS/octopus.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 38, tab: line 1)
4 packages and 1 specfiles checked; 0 errors, 3 warnings.

http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus.spec
http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus-3.0.1-3.fc9.src.rpm

Comment 3 Dominik 'Rathann' Mierzejewski 2008-09-29 14:56:25 UTC
Very well, I'll review this one, too. Quick comment: if there are static libraries in -devel, then the subpackage must have
Provides: %{name}-static = %{version}-%{release}

Comment 4 Susi Lehtola 2008-09-29 15:25:27 UTC
Thanks, corrected.
http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus.spec
http://theory.physics.helsinki.fi/~jzlehtol/rpms/octopus-3.0.1-4.fc9.src.rpm

rpmlint output:
octopus-devel.x86_64: W: no-documentation
octopus-mpi.x86_64: W: no-documentation
SPECS/octopus.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 40, tab: line 1)
4 packages and 1 specfiles checked; 0 errors, 3 warnings.



PS. Reviewed bugs 

https://bugzilla.redhat.com/show_bug.cgi?id=437192
and
https://bugzilla.redhat.com/show_bug.cgi?id=452413

Comment 5 Dominik 'Rathann' Mierzejewski 2008-10-01 23:25:24 UTC
> PS. Reviewed bugs 
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=437192
> and
> https://bugzilla.redhat.com/show_bug.cgi?id=452413

You missed the desktop file guidelines.

Comment 6 Dominik 'Rathann' Mierzejewski 2008-10-01 23:26:56 UTC
Created attachment 319179 [details]
octopus-3.0.1 source file license check report

Comment 7 Dominik 'Rathann' Mierzejewski 2008-10-02 00:18:45 UTC
OK'd items omitted.

- MUST: rpmlint must be run on every package. The output should be posted in the review.

$ rpmlint /var/lib/mock//fedora-rawhide-i386/result
octopus.i386: W: dangling-relative-symlink /usr/lib/debug/.build-id/f8/9fbfbbed296d4a83448b1549262a7c9a7ae490 ../../../../bin/oct-test_mpi
octopus.i386: W: hidden-file-or-dir /usr/lib/debug/.build-id
octopus.i386: W: hidden-file-or-dir /usr/lib/debug/.build-id
octopus.i386: W: dangling-relative-symlink /usr/lib/debug/.build-id/32/6af5072f170e31d82e0f48aab7f5686283d55c ../../../../bin/octopus_mpi
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-cross-section.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-cross-section.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-rsf.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-rsf.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/octopus_mpi.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/octopus_mpi.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-test_mpi.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-test_mpi.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-help.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-help.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/octopus.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/octopus.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-test.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-test.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-center-geom.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-center-geom.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-xyz-anim.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-xyz-anim.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-vibrational.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-vibrational.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-harmonic-spectrum.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-harmonic-spectrum.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-broad.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-broad.debug
octopus.i386: W: unstripped-binary-or-object /usr/lib/debug/usr/bin/oct-oscillator-strength.debug
octopus.i386: E: statically-linked-binary /usr/lib/debug/usr/bin/oct-oscillator-strength.debug
octopus-mpi.i386: W: no-documentation
octopus.src: W: mixed-use-of-spaces-and-tabs (spaces: line 40, tab: line 1)
octopus-devel.i386: W: no-documentation
5 packages and 0 specfiles checked; 13 errors, 20 warnings.

Something has gone wrong here.
%{_libdir}/*
seems to be the culprit.


- MUST: The package must meet the Packaging Guidelines .

%post
install-info %{_infodir}/octopus.info &> /dev/null

%preun
install-info --remove octopus &> /dev/null

See
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Texinfo


The tarball contains a number of bundled libraries that have separate upstreams. You should package them separately and modify (or work with upstream to modify) octopus to use the external, system-wide packages instead.


- MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines .

Some files are under GPLv2+ and some (libxc) are under LGPLv3+. There are many files without licensing information, so please ask upstream to fix that. See attached log (attachment #319179 [details]) from licensecheck for more details.

external_libs/expokit is non-free and must be removed from distributed sources unless you can convince its authors to relicense it under a free license.

external_libs/metis-4.0 is non-free.

external_libs/qshep is non-free as well.


- MUST: The License field in the package spec file must match the actual license.

License: GPLv2+
Should be GPLv3+. IIUC LGPLv3+ libraries can only be linked in if GPLv2+ sources are "upgraded" to GPLv3. I'll re-check this, but I think I'm correct.


- MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.

Source URL is bad:

$ spectool -g octopus.spec 
--2008-10-01 23:47:15--  http://www.tddft.org/programs/octopus/download/octopus-3.0.1.tar.gz
Resolving www.tddft.org... 193.137.208.200
Connecting to www.tddft.org|193.137.208.200|:80... connected.
HTTP request sent, awaiting response... 404 Not Found
2008-10-01 23:47:16 ERROR 404: Not Found.

SRPM source file doesn't match upstream:
54e00d2eb2af7fbd902876bef32b409e  octopus-3.0.1.tar.gz
e17887506f2596e1826d2d09bc75214f  octopus-3.0.1.tar.gz.srpm


- MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.

From configure output:
checking for ATL_xerbla in -latlas... no

Maybe add BuildRequires: atlas-devel?

checking for netcdf... no
configure: WARNING: Could not find netcdf library.
                *** Will compile without netcdf support

BuildRequires: netcdf-devel seems not enough, you need to fix configure, too.


- MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines .

%install
rm -rf %{buildroot}

cd single
make install DESTDIR=%{buildroot}
cd ..

cd mpi
make install DESTDIR=${RPM_BUILD_ROOT}
cd ..

%clean
rm -rf ${RPM_BUILD_ROOT}

Inconsistent usage, please fix.


One last thing: there's a testsuite in the package. Please consider adding

%check
make check

Comment 8 Dominik 'Rathann' Mierzejewski 2008-10-02 00:29:14 UTC
(In reply to comment #7)
> - MUST: The License field in the package spec file must match the actual
> license.
> 
> License: GPLv2+
> Should be GPLv3+. IIUC LGPLv3+ libraries can only be linked in if GPLv2+
> sources are "upgraded" to GPLv3. I'll re-check this, but I think I'm correct.

https://fedoraproject.org/wiki/Licensing#GPLCompatibilityMatrix

I'm not sure, but I think it's not linking, but distributing libxc as part of octopus (or is that "mere aggregation"?) that affects the license. However, the license tag could definitely stay at GPLv2+ if you package libxc separately and only link against it.

Comment 9 Susi Lehtola 2008-10-02 09:00:03 UTC
> external_libs/expokit is non-free and must be removed from distributed sources
> unless you can convince its authors to relicense it under a free license.
> 
> external_libs/metis-4.0 is non-free.
> 
> external_libs/qshep is non-free as well.

Oh my, you are right. This is a showstopper. I'm contacting upstream to see whether they have any plans to migrate to free implementations.

Is there an automatic tool to check the licenses of the source files?

> - MUST: The License field in the package spec file must match the actual
> license.
> 
> License: GPLv2+
> Should be GPLv3+. IIUC LGPLv3+ libraries can only be linked in if GPLv2+
> sources are "upgraded" to GPLv3. I'll re-check this, but I think I'm correct.

OK. Funny, I based my SPEC on the SRPM available from the Octopus website. You'd think the developers had their licenses right..

> SRPM source file doesn't match upstream:
> 54e00d2eb2af7fbd902876bef32b409e  octopus-3.0.1.tar.gz
> e17887506f2596e1826d2d09bc75214f  octopus-3.0.1.tar.gz.srpm

Used the one from upstream SRPM. My bad.

Comment 10 Dominik 'Rathann' Mierzejewski 2008-10-02 10:47:14 UTC
(In reply to comment #9)
> Is there an automatic tool to check the licenses of the source files?

I'm using licensecheck.pl from debian-utils. Perhaps it could be added to fedora-rpmdevtools.

> > - MUST: The License field in the package spec file must match the actual
> > license.
> > 
> > License: GPLv2+
> > Should be GPLv3+. IIUC LGPLv3+ libraries can only be linked in if GPLv2+
> > sources are "upgraded" to GPLv3. I'll re-check this, but I think I'm correct.
> 
> OK. Funny, I based my SPEC on the SRPM available from the Octopus website.
> You'd think the developers had their licenses right..

See comment #8.

> > SRPM source file doesn't match upstream:
> > 54e00d2eb2af7fbd902876bef32b409e  octopus-3.0.1.tar.gz
> > e17887506f2596e1826d2d09bc75214f  octopus-3.0.1.tar.gz.srpm
> 
> Used the one from upstream SRPM. My bad.

Maybe ask upstream why they are different?

Comment 11 Susi Lehtola 2008-10-13 06:55:00 UTC
Upstream replies:

"It turns out that one of the octopus developers (Xavier Andrade) has
decided to try to remove METIS, substituting it by a free alternative
(ZOLTAN). We also removed expokit, and as I said qshep is also
non-essential. So it may very well be that next release of octopus
(unknown date) will be free of license problems."

Until a license problem free version is available, octopus cannot be packaged.

Comment 12 Dominik 'Rathann' Mierzejewski 2008-10-13 11:37:25 UTC
(In reply to comment #11)
> Upstream replies:
> 
> "It turns out that one of the octopus developers (Xavier Andrade) has
> decided to try to remove METIS, substituting it by a free alternative
> (ZOLTAN). We also removed expokit, and as I said qshep is also
> non-essential. So it may very well be that next release of octopus
> (unknown date) will be free of license problems."

That's encouraging.

> Until a license problem free version is available, octopus cannot be packaged.

Or you could try backporting whatever changes they made to remove the non-free code, but that's up to you.

Comment 13 Susi Lehtola 2009-05-27 20:46:07 UTC
Nothing has still happened, closing the bug as deferred.


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