Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 204975 - Review Request: vigra - Generic Programming for Computer Vision
Summary: Review Request: vigra - Generic Programming for Computer Vision
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-09-01 19:50 UTC by Bruno Postle
Modified: 2010-03-19 08:49 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-14 09:45:08 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Check this SPEC file with some corrections (deleted)
2006-09-05 13:22 UTC, Parag AN(पराग)
no flags Details
Updated SPEC for you (deleted)
2006-09-05 13:26 UTC, Parag AN(पराग)
no flags Details

Description Bruno Postle 2006-09-01 19:50:45 UTC
Spec URL: http://bugbear.blackfish.org.uk/~bruno/apt/SPECS/vigra.spec
SRPM URL: http://bugbear.blackfish.org.uk/~bruno/apt/fedora/linux/5/x86_64/SRPMS.panorama/vigra-1.4.0-1.src.rpm
Description:

VIGRA stands for "Vision with Generic Algorithms". It's a novel computer vision
library that puts its main emphasis on customizable algorithms and data
structures. By using template techniques similar to those in the C++ Standard
Template Library, you can easily adapt any VIGRA component to the needs of your
application, without thereby giving up execution speed.

Note 1: I've been maintaining this and other packages in a 3rd party
repository which I'd like to migrate to Extras, so I need a sponsor.

Note 2: vigra is a dependency for the lprof package.

Note 3: This library gets statically linked to libtiff/jpeg/png.  I'd
appreciate pointers for getting it to link dynamically.

Comment 1 Parag AN(पराग) 2006-09-05 11:30:15 UTC
{Not Official Reviewer}

- rpmlint on SOURCE rpm is NOT silent
rpmlint -iv vigra-1.4.0-1.src.rpm 
I: vigra checking
W: vigra invalid-license MIT/X11
The value of the License tag is invalid.  Valid values are:
"Academic Free License", "Adaptive Public License", "Apache License", "Apache
Software License", "Apple Public Source License", "Artistic", "Attribution
Assurance License", "BSD", "Computer Associates Trusted Open Source License",
"CDDL", "CPL", "CUA Office Public License", "EU DataGrid Software License",
"Eclipse Public License", "Educational Community License", "Eiffel Forum
License", "Entessa Public License", "Fair License", "Frameworx License",
"GPL", "LGPL", "Historical Permission Notice and Disclaimer", "IBM Public
License", "Intel Open Source License", "Jabber Open Source License", "Lucent
Public License", "MIT", "CVW License", "Motosoto License", "MPL", "NASA Open
Source Agreement", "Naumen Public License", "Nethack General Public License",
"Nokia Open Source License", "OCLC Research Public License", "Open Group Test
Suite License", "Open Software License", "PHP License", "Python license",
"Python Software Foundation License", "QPL", "RealNetworks Public Source
License", "Reciprocal Public License", "Ricoh Source Code Public License",
"Sleepycat License", "Sun Industry Standards Source License", "Sun Public
License", "Sybase Open Watcom Public License", "University of Illinois/NCSA
Open Source License", "Vovida Software License", "W3C License", "wxWindows
Library License", "X.Net License", "Zope Public License", "zlib License",
"Design Public License", "GFDL", "LaTeX Project Public License", "OpenContent
License", "Open Publication License", "Public Domain", "Ruby License", "SIL
Open Font License", "Charityware", "Commercial", "Distributable", "Freeware",
"Non-distributable", "Proprietary", "Shareware".
If the license is close to an existing one, you can use '<license> style'.

=> I think you better write MIT only in License tag

W: vigra rpm-buildroot-usage %build    
--docdir=%{buildroot}/usr/share/doc/%{name}-%{version}
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

E: vigra configure-without-libdir-spec
A configure script is run without specifying the libdir. configure
options must be augmented with something like --libdir=%{_libdir}.

Many things need to be considered in this package SPCE file. I have modified
SPEC file will post it 

Comment 2 Parag AN(पराग) 2006-09-05 13:22:45 UTC
Created attachment 135554 [details]
Check this SPEC file with some corrections

Update SRPM and post here

Comment 3 Parag AN(पराग) 2006-09-05 13:26:52 UTC
Created attachment 135555 [details]
Updated SPEC for you

Comment 4 Bruno Postle 2006-09-05 16:15:09 UTC
(In reply to comment #3)

> Updated SPEC for you

Thanks, I've applied the changes, fixed the license and a couple of typos:

Spec URL: http://bugbear.blackfish.org.uk/~bruno/apt/SPECS/vigra.spec
SRPM URL:
http://bugbear.blackfish.org.uk/~bruno/apt/fedora/linux/5/x86_64/SRPMS.panorama/vigra-1.4.0-2.fc5.src.rpm

I also put the documentation back in the -devel package as this is
really developer information and there is a lot of it - Should it
go in a -doc subpackage instead?

Comment 5 Parag AN(पराग) 2006-09-06 04:51:14 UTC
Now packaging looks ok

No let the documentation be in -devel package only

Comment 6 Kevin Fenzi 2006-09-08 02:24:14 UTC
Thanks for the prelim review Parag.

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (MIT)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
ea91f2fb4212a21d708aced277e6e85a  vigra1.4.0.tar.gz
ea91f2fb4212a21d708aced277e6e85a  vigra1.4.0.tar.gz.1
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Spec has needed ldconfig in post and postun
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
See below - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - -doc subpackage needed/used.
OK - Packages %doc files don't affect runtime.
OK - Headers/static libs in -devel subpackage.
OK - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - .la files are removed.
OK - Package doesn't own any directories other packages own.
OK - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
OK - Should build in mock.
OK - Should have sane scriptlets.
OK - Should have subpackages require base package with fully versioned depend.

Issues:

1. You might change your default attibute lines from:
%defattr(-, root, root)

to

%defattr(-, root, root,-)
(To keep directory permissions the same and not use umask)

2. This package appears to be able to use 'boost python'. Might
look into submitting boost and boost-python? 

Neither of those are blockers, you might fix the first before importing.

I am happy to APPROVE this package and sponsor you.
Note that your other submission was approved, so you should be able to 
import it as well. 

Continue from step 6 under (Get a Fedora Account):
http://www.fedoraproject.org/wiki/Extras/Contributors#head-
a89c07b5b8abe7748b6b39f0f89768d595234907

If you have any questions at all, feel free to email me
or catch me on irc in #fedora-extras (my nick is nirik).

Comment 7 Bruno Postle 2006-09-08 20:49:50 UTC
(In reply to comment #6)

Thanks for sponsoring me, I'm reading through the docs now.

> %defattr(-, root, root,-)

Done.

> 2. This package appears to be able to use 'boost python'. Might
> look into submitting boost and boost-python? 

I've looked into it and the --with-boostpython option is obsolete, there is now
a separate vigrapython package:

https://mailhost.informatik.uni-hamburg.de/pipermail/vigra/2006-March/000199.html

I've also extended the BuildRequires, as vigra needs fftw >= 3 and gcc-c++ isn't
installed on a standard system:

Spec URL: http://bugbear.blackfish.org.uk/~bruno/apt/SPECS/vigra.spec
SRPM URL:
http://bugbear.blackfish.org.uk/~bruno/apt/fedora/linux/5/x86_64/SRPMS.panorama/vigra-1.4.0-3.fc5.src.rpm



Comment 8 Kevin Fenzi 2006-09-09 03:53:34 UTC
In reply to comment #7:

ok on the defattr change. 

I just saw the --with-boostpython in the configure script, so I thought I would 
mention it. No problem if it's obsolete now. 

Requiring fftw >= 3 is fine, but gcc-c++ is part of the base build env: 
http://www.fedoraproject.org/wiki/Packaging/Guidelines#Exceptions
so it doesn't need to be listed there. 

Otherwise the package looks good to go to me. 
Let me know if you run into any problems with the procedure to import and 
build. 

Comment 9 Milan Crha 2010-03-18 12:36:27 UTC
As this is assigned to a review of vigra-1.6.0-2.1, then I'm using it for my question:

I see that .spec file has spec license MIT, same as is shown in the LICENSE.txt file, but the "License features" says "Source code: Adobe". I guess it should be also "Source code: MIT", shouldn't it?

Apart of this the .spec file looks good from my point of view.

Comment 10 Bruno Postle 2010-03-18 23:37:55 UTC
(In reply to comment #9)
> 
> I see that .spec file has spec license MIT, same as is shown in the LICENSE.txt
> file, but the "License features" says "Source code: Adobe". I guess it should
> be also "Source code: MIT", shouldn't it?

I'm confused, I can't find the string "License features" or any reference to Adobe in the vigra sourcecode or the vigra website.

Where do you see this?

Comment 11 Milan Crha 2010-03-19 08:49:15 UTC
(In reply to comment #10)
> I'm confused, I can't find the string "License features" or any reference to
> Adobe in the vigra sourcecode or the vigra website.
> 
> Where do you see this?    

Ouch, never mind, my fault, please ignore that.


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