Bug 489686

Summary: Review Request: armadillo - fast C++ matrix library with interfaces to LAPACK and ATLAS
Product: [Fedora] Fedora Reporter: C Sand <conradsand.fb>
Component: Package ReviewAssignee: Chitlesh GOORAH <chitlesh>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: conradsand.fb, denis.arnaud_fedora, fedora-package-review, notting, rc040203, rdieter, susi.lehtola
Target Milestone: ---Flags: chitlesh: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://arma.sourceforge.net
Whiteboard:
Fixed In Version: 0.6.12-2.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-08-01 23:57:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description C Sand 2009-03-11 12:59:49 UTC
Hello, 

This is my first package, and hance I am kindly seeking a sponsor.  I'd like to get a review so the package can be included in Fedora.

Spec URL: http://arma.sourceforge.net/fedora/armadillo.spec
SRPM URL: http://arma.sourceforge.net/fedora/armadillo-0.5.2-1.src.rpm

Description: 
Armadillo is a streamlined C++ linear algebra library (matrix maths)
aiming towards a good balance between speed and ease of use.
Integer, floating point and complex numbers are supported,
as well as a subset of trigonometric and statistics functions.
Optional integration with LAPACK and ATLAS libraries is also provided.
A delayed evaluation approach is employed (during compile time)
to combine several operations into one and reduce (or eliminate) 
the need for temporaries. This is accomplished through recursive
templates and template meta-programming.  This library is useful
if C++ has been decided as the language of choice (due to speed
and/or integration capabilities), rather than another language
like Octave.  It is distributed under a license that is useful 
in both open-source and commercial contexts.

Comment 1 Ralf Corsepius 2009-03-11 16:18:21 UTC
This submission has several issues:

a) Package doesn't honor %{_libdir} (it installs its lib's to /usr/lib on x86_64)

b) Please read (closely related to a))
https://fedoraproject.org/wiki/Packaging:Cmake

c) Split the package into a run-time package and a *-devel package

d) Many of the BuildRequires are redundant (BR: *-devel normally automatically pulls in the corresponding run-time packages)

e) This construct:
[ "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
is being frowned upon in Fedora.

Please use
rm -rf $RPM_BUILD_ROOT
instead

f) Whether a separate *-doc package is necessary/useful is arguable.
Technically, there is no need to have one.

Comment 2 C Sand 2009-03-11 17:11:40 UTC
(In reply to comment #1)

Thanks for the feedback -- I will address these issues. I have a further question & clarification with respect to point (c):

> 
> c) Split the package into a run-time package and a *-devel package
> 

The library code is entirely in C++ header files, however routines from LAPACK, BLAS and ATLAS are called.

As such there is a convenience component in the form of a dummy run-time library (libarmadillo.so), which simply pulls in the LAPACK, BLAS and ATLAS libraries.  The idea is that a user can simply link against libarmadillo.so, rather than individually linking against LAPACK etc.

In other words, the user can do this:
 g++ -o my_prog my_prog.cpp -larmadillo

instead of a longer:
 g++ -o my_prog my_prog.cpp -L/usr/lib/atlas -lblas -llapack -llapack_atlas

Hence I am not sure whether splitting the package into run-time and -devel packages is the best way to go.  Suggestions ?

Comment 3 Ralf Corsepius 2009-03-11 17:43:52 UTC
(In reply to comment #2)
> (In reply to comment #1)

> In other words, the user can do this:
>  g++ -o my_prog my_prog.cpp -larmadillo

> Hence I am not sure whether splitting the package into run-time and -devel
> packages is the best way to go.

This is a classic devel-package use-case

>  Suggestions ?
Move the headers and lib*.so into *-devel
and let the main-package only contain lib*.x.y.z


BTW: Unless I am mistaken, this package builds its libs stripped.
In Fedora, libs are supposed to be built unstripped (rpm strips them later).

Comment 4 C Sand 2009-03-12 13:36:59 UTC
(In reply to comment #3)
> Move the headers and lib*.so into *-devel
> and let the main-package only contain lib*.x.y.z

Hello,

I've followed your recommendations and redone the spec file, as well as CMakeLists.txt.  %{_libdir} should (hopefully) be honoured.

The updated files are:

Spec URL: http://arma.sourceforge.net/fedora/armadillo.spec
SRPM URL: http://arma.sourceforge.net/fedora/armadillo-0.5.3-1.src.rpm


> BTW: Unless I am mistaken, this package builds its libs stripped.
> In Fedora, libs are supposed to be built unstripped (rpm strips them later).  

I don't have any explicit stripping, but the output from rpmbuild has these two lines:

+ /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
+ /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump

However, the above messages are amongst other stuff regarding java and python bytecompile, which has nothing to do with this package.  If a stripped library build is occuring, is "/brp-strip-static-archive" responsible?  If so, how is it turned off ?

Comment 5 Michael Schwendt 2009-03-16 15:33:15 UTC
> %dir %{_docdir}/%{name}-%{version}
> %dir %{_docdir}/%{name}-%{version}/docs_user
> %dir %{_docdir}/%{name}-%{version}/docs_tech
> %dir %{_docdir}/%{name}-%{version}/examples
> %{_docdir}/%{name}-%{version}/*.txt
> %{_docdir}/%{name}-%{version}/index.html
> %{_docdir}/%{name}-%{version}/docs_user/*
> %{_docdir}/%{name}-%{version}/docs_tech/*
> %{_docdir}/%{name}-%{version}/examples/*

Since you create and fill this directory yourself, you can simply replace above lines with this single line:

%{_docdir}/%{name}-%{version}/

The trailing slash is not important, but makes it more explicit that you include a directory recursively.


> %dir %{_includedir}/armadillo_bits
> %{_includedir}/armadillo_bits/*

Same here:
%{_includedir}/armadillo_bits/


> Requires:       atlas, lapack, blas, boost

Examine your built package's list of dependencies (rpm --query --requires ...). If there are automatic dependencies on the atlas/lapack/blas/boost library SONAMEs, drop above explicit Requires from the spec file. We rely on rpmbuild's automatic SONAME dependencies:
https://fedoraproject.org/wiki/PackagingDrafts/ExplicitRequires

Comment 6 C Sand 2009-03-24 05:03:55 UTC
Hello,

There are indeed automatic dependencies on the abovementioned libraries. As such, the spec file has been modified as requested.

The updated files are:

Spec URL: http://arma.sourceforge.net/fedora/armadillo.spec
SRPM URL: http://arma.sourceforge.net/fedora/armadillo-0.6.0-1.src.rpm

Comment 7 C Sand 2009-03-25 10:07:36 UTC
(In reply to comment #6)

Updated spec and source RPM for 0.6.1 release:

Spec URL: http://arma.sourceforge.net/fedora/armadillo.spec
SRPM URL: http://arma.sourceforge.net/fedora/armadillo-0.6.1-1.src.rpm

Comment 8 C Sand 2009-05-07 09:42:19 UTC
(In reply to comment #5)

spec and src rpm updated:

http://arma.sourceforge.net/fedora/armadillo.spec
http://arma.sourceforge.net/fedora/armadillo-0.6.10-1.src.rpm 

Anyone out there ?

Comment 9 Susi Lehtola 2009-05-07 12:15:10 UTC
Chitlesh: are you reviewing the package and sponsoring Conrad?

Comment 10 Susi Lehtola 2009-05-07 12:19:28 UTC
A few notes:

- gcc-c++, libstdc++-devel are not needed. First, BR libstdc++-devel is redundant since it's required by gcc-c++, second gcc-c++ is already in the standard kit installed in the buildroot: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

- Also blas-devel is unnecessary since it's pulled in by lapack-devel.

- The section
 mkdir -p $RPM_BUILD_ROOT/%{_docdir}/%{name}-%{version}
 cp -r -p README.txt LICENSE.txt index.html examples docs_user docs_tech licenses $RPM_BUILD_ROOT/%{_docdir}/%{name}-%{version}
is unnecessary. Just insert
 %doc README.txt LICENSE.txt index.html examples docs_user docs_tech licenses
in the %files section of the main package. Also, note that if the documentation is large, it should be branched in its own package, which may be the case here.

Comment 11 Susi Lehtola 2009-05-07 12:30:02 UTC
Also, you should add %{?dist} to the Release: tag.

Comment 12 Rex Dieter 2009-05-07 12:55:28 UTC
Re: comment #9 
I can help too (with review/sponsoring) if chitlesh isn't able.

Comment 13 Chitlesh GOORAH 2009-05-07 21:34:49 UTC
(In reply to comment #9)
> Chitlesh: are you reviewing the package and sponsoring Conrad?  

 Yes, today I've assigned this bug under my name

Comment 14 Chitlesh GOORAH 2009-05-07 21:53:30 UTC
#1
(In reply to comment #10)
> - Also blas-devel is unnecessary since it's pulled in by lapack-devel.

Conrad, here blas-devel is already required by lapack-devel, you can verify this with

chitlesh $ rpm -qR lapack-devel
blas-devel = 3.1.1-4.fc10             <----------- here it is
lapack = 3.1.1-4.fc10
liblapack.so.3
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1

Hence, if it is not important to add it in the spec file. For your next package, use this "rpm -qR XXXX" command to verify where you have unnecessary added redundant dependencies.

#2 verify your rpms
there is a package caled "rpmlint". It helps you verify the quality of your rpms. Try rpmlint -i XXXX.rpm for each generated rpms before uploading for review. Any warning or errors should be corrected. The solutions of some common rpmlint warnings are listed on the fedora wiki.

Comment 15 C Sand 2009-05-08 02:10:07 UTC
(In reply to comment #10)
> A few notes:
> 
> - gcc-c++, libstdc++-devel are not needed.

Thanks -- I've removed the explicit build dependencies.

> http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

Based on rpm -qR, boost-devel doesn't pull in libstdc++-devel.  Armadillo header files certainly need the header files provided by libstdc++-devel, and I'd assume boost-devel would also.

As such, should there be a dependency on libstdc++-devel in the -devel package?

The packaging guidelines only state that gcc-c++ (and hence libstdc++-devel) can be assumed to exist for "BuildRequires". Is this also the case for the "Requires" section ?


> Just insert
> %doc README.txt LICENSE.txt ... in the %files section of the main package. 

I've adapted this as follows:

%files
...
%doc LICENSE.txt licenses

%files devel
...
%doc README.txt index.html examples docs_user docs_tech

The reasoning is that the licensing applies whether or not the header files are present.  "README.txt" and "index.html" refer to files which are only present when both the base and -devel package are installed (due to the layout in the original .tar.gz archive).

> Also, note that if the documentation is large, it should be 
> branched in its own package, which may be the case here.  

I'd prefer to keep it in the -devel package to keep thing simpler, and hence less error prone. (I'd also prefer not to go overboard in splitting packages like Debian does).  The documentation goes hand in hand when one does development using this library, so I'm not sure if the added complexity and effort of an extra doc package is worth it.

Comment 16 C Sand 2009-05-21 03:42:16 UTC
(In reply to comment #14)

The spec file has been improved and the rpms checked through rpmlint. There are two minor warnings:

# rpmlint armadillo-0.6.11-2.fc10.x86_64.rpm
armadillo.x86_64: W: unstripped-binary-or-object /usr/lib64/libarmadillo.so.0.6.11
armadillo.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

# rpmlint armadillo-devel-0.6.11-2.fc10.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

I'm not sure why there is the unstripped-binary-or-object warning. According to Comment #3 (Ralf Corsepius), "In Fedora, libs are supposed to be built unstripped (rpm strips them later)".  Is this a problem worth worrying about ?

New spec file and source rpm are located at:

http://arma.sourceforge.net/fedora/armadillo.spec
http://arma.sourceforge.net/fedora/armadillo-0.6.11-2.fc10.src.rpm

Comment 17 Ralf Corsepius 2009-05-21 04:09:03 UTC
(In reply to comment #16)

> Comment #3 (Ralf Corsepius), "In Fedora, libs are supposed to be built
> unstripped (rpm strips them later)".  Is this a problem worth worrying about ?
Generally speaking, without having checked details of the current package:
Yes, it is a blocker and must be fixed.

Comment 18 Ralf Corsepius 2009-05-21 05:13:18 UTC
OK, your debuginfo issue is caused by not passing CXXFLAGS/CFLAGS to cmake, which causes libarmadillo.so.* to be built with inappropriate CFLAGS:

From build.log:
...
/usr/bin/c++   -Darmadillo_EXPORTS -fPIC -I/users/corsepiu/src/rpms/BUILD/armadillo-0.6.11/include 
  -o CMakeFiles/armadillo.dir/src/pull_libs.o -c /users/corsepiu/src/rpms/BUILD/armadillo-0.6.11/src/pull_libs.cpp

Note: All mandatory RPM_OPT_FLAGS are missing.


Using %cmake instead of "cmake ..." will likely resolve your issue.

Comment 19 Chitlesh GOORAH 2009-05-21 14:26:54 UTC
#1: Ralf is right here, use the macro %cmake instead of cmake.

#2: Also before creating the SRPM, change the permissions of both the spec file and the sources to 0644, currently :
chitlesh(rpmbuild)[1]$rpmlint /home/chitlesh/rpmbuild/SRPMS/armadillo-0.6.11-2.fc10.src.rpm
armadillo.src: W: strange-permission armadillo-0.6.11.tar.gz 0640
armadillo.src: W: strange-permission armadillo.spec 0640

#3: Swap the docs_user and LICENSE.txt from %name-devel to %name as they are user specific.

#4: 
remove /usr/share/doc/armadillo-devel-0.6.11/examples/Makefile.cmake after make install

Comment 20 C Sand 2009-05-22 10:24:58 UTC
(In reply to comment #18)
> 
> Using %cmake instead of "cmake ..." will likely resolve your issue.  

I tried this, and rpmlint still generates a warning.  

It appears that "/usr/lib/rpm/brp-strip" is supposed to strip the library, however it's not doing its job.

Putting a line like "strip $RPM_BUILD_ROOT/%{_libdir}/*.so*" in the "%install" part seems to fix the warning, however I'm not sure whether this is the most appropriate fix.

Comment 21 Chitlesh GOORAH 2009-05-23 16:31:53 UTC
My scratch build with %cmake does not seem to have such rpmlint warning
http://koji.fedoraproject.org/scratch/chitlesh/task_1368413/

Comment 22 C Sand 2009-05-25 08:11:04 UTC
(In reply to comment #21)
> My scratch build with %cmake does not seem to have such rpmlint warning
> http://koji.fedoraproject.org/scratch/chitlesh/task_1368413/  

This is pretty weird.

I have a pretty much stock-standard Fedora 10 installation.  The rpm building process (via "rpmbuild -ba armadillo.spec"), causes cmake to install files into "/root/rpmbuild/BUILDROOT/armadillo-0.6.11-4.fc10.x86_64/usr/".  Afterwards the following scripts are run:

+ /usr/lib/rpm/brp-compress
+ /usr/lib/rpm/brp-strip
+ /usr/lib/rpm/brp-strip-static-archive
+ /usr/lib/rpm/brp-strip-comment-note

Looking at "/usr/lib/rpm/brp-strip" (as installed on my system), we have this line: 

  grep -v ' shared object,' | \

... which means it explicitly does not strip shared objects. A more appropriate "/usr/lib/rpm/brp-strip-shared" exists, but this isn't being run (why?)

Comparing your build log for x86_64 against mine, both use CXXFLAGS that contain "-O2 -g".  The main differences appear after cmake installs files into $BUILDROOT.  Specifically, the following scripts are run:

+ /usr/lib/rpm/find-debuginfo.sh --strict-build-id /builddir/build/BUILD/armadillo-0.6.11
extracting debug info from /builddir/build/BUILDROOT/armadillo-0.6.11-2.fc11.x86_64/usr/lib64/libarmadillo.so.0.6.11
symlinked /usr/lib/debug/usr/lib64/libarmadillo.so.0.6.11.debug to /usr/lib/debug/usr/lib64/libarmadillo.so.0.debug
symlinked /usr/lib/debug/usr/lib64/libarmadillo.so.0.6.11.debug to /usr/lib/debug/usr/lib64/libarmadillo.so.debug
52 blocks
+ /usr/lib/rpm/check-buildroot
+ /usr/lib/rpm/redhat/brp-compress
+ /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
+ /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
+ /usr/lib/rpm/brp-python-bytecompile
+ /usr/lib/rpm/redhat/brp-python-hardlink
+ /usr/lib/rpm/redhat/brp-java-repack-jars

So what's going on here ?

Comment 23 Chitlesh GOORAH 2009-05-26 11:40:08 UTC
(In reply to comment #20)
> (In reply to comment #18)
> > 
> > Using %cmake instead of "cmake ..." will likely resolve your issue.  
> 
> I tried this, and rpmlint still generates a warning.  

Does it ? can you post your SRPM please.

My scratch build for F-10 gives me only these warnings:
http://koji.fedoraproject.org/scratch/chitlesh/task_1376830/

chitlesh(~)[0]$ls *.rpm
armadillo-0.6.11-2.fc10.i386.rpm    armadillo-debuginfo-0.6.11-2.fc10.i386.rpm    armadillo-devel-0.6.11-2.fc10.ppc64.rpm
armadillo-0.6.11-2.fc10.ppc64.rpm   armadillo-debuginfo-0.6.11-2.fc10.ppc64.rpm   armadillo-devel-0.6.11-2.fc10.ppc.rpm
armadillo-0.6.11-2.fc10.ppc.rpm     armadillo-debuginfo-0.6.11-2.fc10.ppc.rpm     armadillo-devel-0.6.11-2.fc10.x86_64.rpm
armadillo-0.6.11-2.fc10.x86_64.rpm  armadillo-debuginfo-0.6.11-2.fc10.x86_64.rpm
armadillo-0.6.11-2.fc11.src.rpm     armadillo-devel-0.6.11-2.fc10.i386.rpm

chitlesh(~)[0]$rpmlint *.rpm
armadillo.i386: W: no-documentation
armadillo.ppc64: W: no-documentation
armadillo.ppc: W: no-documentation
armadillo.x86_64: W: no-documentation
armadillo.src: W: strange-permission armadillo-0.6.11.tar.gz 0640
armadillo.src: W: strange-permission armadillo.spec 0640
13 packages and 0 specfiles checked; 0 errors, 6 warnings.

Comment 24 Denis Arnaud 2009-05-28 23:34:06 UTC
rpmlint complains on the absence of documentation for the non -devel package. I would alter the %file section as per following:
%files
%defattr(-,root,root,-)
%{_libdir}/*.so.*
%doc README.txt LICENSE.txt

It effectively duplicates the README.txt and LICENSE.txt files, but I think it is normal to repeat both of those on the two generated packages. The guideline (https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files) may not be clear enough on that part.

Comment 25 Susi Lehtola 2009-05-29 08:03:00 UTC
(In reply to comment #24)
> rpmlint complains on the absence of documentation for the non -devel package. I
> would alter the %file section as per following:
> %files
> %defattr(-,root,root,-)
> %{_libdir}/*.so.*
> %doc README.txt LICENSE.txt
> 
> It effectively duplicates the README.txt and LICENSE.txt files, but I think it
> is normal to repeat both of those on the two generated packages. The guideline
> (https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files) may not
> be clear enough on that part.  

I'd place at least the LICENSE.txt in the main package, since it applies to the main package too. The development documentation, however, should go with -devel (unless it is so big that it should be branched into a separate -doc package).

As the -devel package requires the main package, the documentation of the main package is also available whenever -devel is installed, and thus there is no need to duplicate the file(s).

Comment 26 Chitlesh GOORAH 2009-06-06 19:48:29 UTC
%doc README.txt LICENSE.txt
should only be placed once and preferably on the %{name}/main package. It is not important that all -devel should entail documentation.

Comment 27 Mamoru TASAKA 2009-06-06 19:59:56 UTC
Conrad, if you are still confronting with unstripped-binary-or-object
rpmlint warning, would you check if you have "redhat-rpm-config"
surely installed?

Comment 28 C Sand 2009-06-08 12:45:01 UTC
(In reply to comment #27)
> Conrad, if you are still confronting with unstripped-binary-or-object
> rpmlint warning, would you check if you have "redhat-rpm-config"
> surely installed?  

Installing "redhat-rpm-config" appears to have fixed the problem -- many thanks!

Comment 29 C Sand 2009-06-08 13:33:03 UTC
(In reply to comment #26)
> %doc README.txt LICENSE.txt
> should only be placed once and preferably on the %{name}/main package. It is
> not important that all -devel should entail documentation.  

I've redone the spec file -- rpmlint now gives no warnings and no errors.

http://arma.sf.net/fedora/armadillo.spec
http://arma.sf.net/fedora/armadillo-0.6.11-4.fc10.src.rpm

The -devel package needs to contain all the original text files and documentation, as they are referred to in the upstream README.txt file. The documentation is included here rather than the main package, as it is only required when developing programs that use the library.

The main package contains the library and LICENSE.txt (+ associated license subdir).  It didn't make sense to include README.txt in the main package, as it refers to files that are only present in the -devel package.

I believe the package is now ready for inclusion into Fedora -- many thanks to everyone for their help.

Comment 30 Susi Lehtola 2009-06-08 14:14:10 UTC
You still shouldn't duplicate LICENSE.txt and licenses in the -devel package, as they are already in the main package.

Comment 31 C Sand 2009-06-08 14:21:38 UTC
(In reply to comment #30)
> You still shouldn't duplicate LICENSE.txt and licenses in the -devel package,
> as they are already in the main package.  

Not including LICENSE.txt in -devel will break the references/links from README.txt and "index.html" (present in the upstream .tar.gz). Including README.txt only makes sense in the -devel package.

Not including LICENSE.txt in the main package will cause rpmlint to give warnings.

Suggestions ?

Comment 32 Susi Lehtola 2009-06-08 14:30:12 UTC
(In reply to comment #31)
> (In reply to comment #30)
> > You still shouldn't duplicate LICENSE.txt and licenses in the -devel package,
> > as they are already in the main package.  
> 
> Not including LICENSE.txt in -devel will break the references/links from
> README.txt and "index.html" (present in the upstream .tar.gz). Including
> README.txt only makes sense in the -devel package.

In this case duplication may be alright, another option is to sed the link to point to %{_datadir}/doc/%{name}-%{version}/LICENSE.txt.

Comment 33 C Sand 2009-06-08 16:48:08 UTC
(In reply to comment #32)
> In this case duplication may be alright, another option is to sed the link to
> point to %{_datadir}/doc/%{name}-%{version}/LICENSE.txt.  

We have two possible solutions:
(1) keep the spec file as is
(2) remove "LICENSE.txt" and "licenses" from -devel and add the following lines to the spec file:

%post devel
/sbin/ldconfig
ln -s %{_docdir}/%{name}-%{version}/LICENSE.txt %{_docdir}/%{name}-devel-%{version}/LICENSE.txt
ln -s %{_docdir}/%{name}-%{version}/licenses %{_docdir}/%{name}-devel-%{version}/licenses

%preun devel
rm %{_docdir}/%{name}-devel-%{version}/LICENSE.txt
rm -r %{_docdir}/%{name}-devel-%{version}/licenses


Solution (2) saves about 43 Kb in disk space, but it's messier (post-install and pre-uninstall scriptlets).

I'd vote for solution (1) as it's cleaner, but I'm also happy to go with (2) if that's what people prefer.

Comment 34 Susi Lehtola 2009-06-08 16:56:24 UTC
You can also just create the symlinks in the install phase, which is a lot neater than creating them in %post:

%install
...
mkdir symlink
ln -s %{_datadir}/doc/%{name}-%{version}/LICENSE.txt symlink/
ln -s %{_datadir}/doc/%{name}-%{version}/licenses symlink/

%files devel
%doc symlink/*

Comment 35 Michael Schwendt 2009-06-08 17:03:14 UTC
rm -rf symlink ; mkdir symlink

as not removing an existing symlink directory would break --short-circuit builds (and especially rpmbuild -bi --short-circuit foo.spec is very helpful).

Comment 36 C Sand 2009-06-08 17:24:49 UTC
(In reply to comment #34)
> You can also just create the symlinks in the install phase,
> which is a lot neater than creating them in %post:
> ...

(In reply to comment #35)
> rm -rf symlink ; mkdir symlink

ok, spec file updated with the cleaner solution.

http://arma.sf.net/fedora/armadillo.spec
http://arma.sf.net/fedora/armadillo-0.6.11-5.fc10.src.rpm

rpmlint now gives two warnings, but they're just a harmless byproduct:

armadillo-devel.x86_64: W: dangling-symlink /usr/share/doc/armadillo-devel-0.6.11/licenses /usr/share/doc/armadillo-0.6.11/licenses
armadillo-devel.x86_64: W: dangling-symlink /usr/share/doc/armadillo-devel-0.6.11/LICENSE.txt /usr/share/doc/armadillo-0.6.11/LICENSE.txt

(just to make it clear to anyone puzzled: as the -devel package is dependant on the main package, the symlinks will always work).

Comment 37 Chitlesh GOORAH 2009-06-09 09:06:29 UTC
Actually there is no need to include the licenses again in the sub -devel package.
as I said in comment #26 

Remove these
rm -rf symlink; mkdir symlink
ln -s %{_docdir}/%{name}-%{version}/LICENSE.txt symlink/
ln -s %{_docdir}/%{name}-%{version}/licenses symlink/

Comment 38 C Sand 2009-06-09 09:18:06 UTC
(In reply to comment #37)
> Actually there is no need to include the licenses 
> again in the sub -devel package.

They need to be there, otherwise README.txt and index.html (from upstream archive) refer to stuff that isn't there (i.e. we'd have broken references/links).

Comment 39 Michael Schwendt 2009-06-09 09:34:00 UTC
Well, sometimes it's worth the little bit of extra effort and install all documentation files into a common %{_docdir}/%{name}-%{version}/ via %install instead of using %doc for subpackages.

[The more subpackages include %doc files, the more docdirs get created. That doesn't help with keeping /usr/share/doc browsable. Some packages with multiple subpackages spread their %doc files over more than four different docdirs.]

Comment 40 C Sand 2009-06-09 10:09:17 UTC
(In reply to comment #39)
> Well, sometimes it's worth the little bit of extra effort
> and install all documentation files into a common 
> %{_docdir}/%{name}-%{version}/ via %install instead 
> of using %doc for subpackages.

I concur.  Updated:

http://arma.sf.net/fedora/armadillo.spec
http://arma.sf.net/fedora/armadillo-0.6.11-6.fc10.src.rpm

rpmlint gives no warnings on the source and binary packages.

(I hope this is the last iteration...)

Comment 41 Susi Lehtola 2009-06-09 10:23:26 UTC
Now you need to:

- mark the documentation files as %doc

and

- add
 %dir %{_docdir}/%{name}-%{version}/
to the files of the main package. (I don't think it owns the directory yet.)

Comment 42 C Sand 2009-06-09 11:13:47 UTC
(In reply to comment #41)
> Now you need to:
> 
> - mark the documentation files as %doc
> 
> and
> 
> - add
>  %dir %{_docdir}/%{name}-%{version}/
> to the files of the main package. (I don't think it owns the directory yet.)  

Agreed on "%dir %{_docdir}/%{name}-%{version}/"

However, is %doc guaranteed to always place files into %{_docdir}/%{name}-%{version}/ ?

If not, this is going to conflict with the -devel package which lists the documentation as "%{_docdir}/%{name}-%{version}/README.txt ... " 

I can't use %doc for the -devel package, as it's going to create a separate directory.  Michael Schwendt (Comment #39) suggested to avoid the use of %doc for the devel package in order to keep everything in one place.  If we're using direct file specification instead of %doc in the devel package, why not also do this in the main package ?

Comment 43 Susi Lehtola 2009-06-09 11:42:40 UTC
(In reply to comment #42)
> Agreed on "%dir %{_docdir}/%{name}-%{version}/"
> 
> However, is %doc guaranteed to always place files into
> %{_docdir}/%{name}-%{version}/ ?
> 
> If not, this is going to conflict with the -devel package which lists the
> documentation as "%{_docdir}/%{name}-%{version}/README.txt ... " 

Nope, it just tags the files as documentation in the RPM database, see:
http://www.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html

Normally one doesn't use this, but in this case you should.

Comment 44 Michael Schwendt 2009-06-09 11:45:56 UTC
%doc for absolute paths is just an attribute, whereas for local files it copies them into a subpkg's default docdir (after emptying that docdir). Your %files entries with %{_docdir}/%{name}-%{version} at the front are absolute paths. You can add the %doc attribute to them without problems.

Comment 45 C Sand 2009-06-09 12:04:39 UTC
(In reply to comment #43)
> Nope, it just tags the files as documentation in the RPM database, see:
> http://www.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html
> 
> Normally one doesn't use this, but in this case you should.  

ok, done:

http://arma.sf.net/fedora/armadillo.spec
http://arma.sf.net/fedora/armadillo-0.6.11-7.fc10.src.rpm

7th time lucky ?

Comment 46 Denis Arnaud 2009-06-13 16:16:47 UTC
1. The following line:
  Requires:       libstdc++-devel, atlas-devel, lapack-devel, boost-devel
should be removed, as the corresponding dependencies are already taken into account by the 'BuildRequires:' line, and as the 'Requires:' dependencies are automatically calculated: https://fedoraproject.org/wiki/Packaging/Guidelines#Requires .

2. I would add a "-f" (force) flag in the following line:
 rm examples/Makefile.cmake
But it may be due to my paranoia :)

3. Since you are the main upstream developer, I would suggest (but that is not at all a Fedora requirement) to burry the header files in a specific directory, resulting, after the installation, into:
 %{_includedir}/armadillo/armadillo
 %{_includedir}/armadillo/bits
 %{_includedir}/armadillo/armadillo_itpp
That way, you would have to add the armadillo directory prefix when you include Armadillo headers. For instance:
#include <armadillo/bits/Mat_meat.hpp>
instead of
#include <armadillo_bits/Mat_meat.hpp>
But that is only a suggestion!

Comment 47 Michael Schwendt 2009-06-13 18:14:47 UTC
Re: Comment 46

> 1.

Unusual. Typically, explicit Requires for a -devel package are added because building with the -devel package requires files contained within additional -devel packages (e.g. headers which are included by default). At least the Boost headers are included by the files in armadillo-devel -- and there is no pkg-config file and therefore no automatic dependency-chain based on the Requires found in the pkg-config file.

Comment 48 Denis Arnaud 2009-06-13 20:11:03 UTC
(In reply to comment #47)
> 
> Unusual. Typically, explicit Requires for a -devel package are added because
> building with the -devel package requires files contained within additional
> -devel packages (e.g. headers which are included by default). At least the
> Boost headers are included by the files in armadillo-devel -- and there is no
> pkg-config file and therefore no automatic dependency-chain based on the
> Requires found in the pkg-config file.  

With the version without the 'Requires:' line, after installation of the RPMs, I get:
# rpm -qpR armadillo-0.6.11-8.fc10.x86_64.rpm 
[...]
libblas.so.3()(64bit)  
libc.so.6(GLIBC_2.2.5)(64bit)  
libcblas.so.3()(64bit)  
libgcc_s.so.1()(64bit)  
liblapack.so.3()(64bit)  
liblapack_atlas.so.3()(64bit)  
libstdc++.so.6(GLIBCXX_3.4)(64bit)  

# ldd -r /usr/lib64/libarmadillo.so.0.6.11 
[...]
liblapack_atlas.so.3 => /usr/lib64/atlas/liblapack_atlas.so.3 (0x00007fd825c3a000)
libcblas.so.3 => /usr/lib64/atlas/libcblas.so.3 (0x00007fd825a1c000)
libblas.so.3 => /usr/lib64/atlas/libblas.so.3 (0x00007fd825067000)
liblapack.so.3 => /usr/lib64/atlas/liblapack.so.3 (0x00007fd8248ab000)
libstdc++.so.6 => /usr/lib64/libstdc++.so.6 (0x00007fd8245a0000)
libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007fd824104000)
libc.so.6 => /lib64/libc.so.6 (0x00007fd823d91000)
libf77blas.so.3 => /usr/lib64/atlas/libf77blas.so.3 (0x00007fd823b74000)
libatlas.so.3 => /usr/lib64/atlas/libatlas.so.3 (0x00007fd8231f4000)
libgfortran.so.3 => /usr/lib64/libgfortran.so.3 (0x00007fd822f19000)

Indeed, the Boost libraries are not needed, apparently because the ARMA_USE_BOOST environment variable is not set to true when calling cmake. That way, Armadillo does not use Boost, even when it is present on the building platform. In the header files delivered with the -devel sub-package, Boost is not used as, but through a specific wrapper, namely arma_boost, which does not need Boost when the ARMA_USE_BOOST variable is set to false.

We may try to alter the specification file, so as to define the ARMA_USE_BOOST variable to true. In that case, the RPM building process should detect the dependency on Boost, I guess.

Or did I miss something?

Comment 49 Denis Arnaud 2009-06-13 20:17:40 UTC
I forgot, following is the version I used:
-----------------------------------
Spec URL: http://denisarnaud.fedorapeople.org/armadillo/0611/8/armadillo.spec
SRPM URL: http://denisarnaud.fedorapeople.org/armadillo/0611/8/armadillo-0.6.11-8.fc10.src.rpm
-----------------------------------

Comment 50 Denis Arnaud 2009-06-13 21:01:38 UTC
The following is an attempt to build with Boost:
-----------------------------------
Spec URL: http://denisarnaud.fedorapeople.org/armadillo/0611/9/armadillo.spec
SRPM URL:
http://denisarnaud.fedorapeople.org/armadillo/0611/9/armadillo-0.6.11-9.fc10.src.rpm
-----------------------------------  

Boost is detected by CMake, but it seems that the Boost libraries are still not needed (most of Boost is made of just headers). However, as when ARMA_USE_BOOST is set to true, some Boost headers (e.g., Boost Date-Time) are included, it is indeed required to add the dependency on boost-devel for the -devel sub-package. That is fine.

But, as lapack-devel and blas-devel include only libraries (and not header files), I would exclude the correspondent dependency from the 'Requires:' part. The same way, I would exclude dependency on 'libstdc++-devel'.

So, following is a suggestion for the corresponding part in the RPM specification file:
-------------------
# The header files of %{name} include some Boost header files, delivered within
# the boost-devel sub-package. However, since there is no dependency on any
# Boost library (most of Boost is delivered as header files only), the RPM
# building process does not detect that dependency. That dependency must
# therefore be added manually.
Requires:       boost-devel
-------------------

Comment 51 C Sand 2009-06-14 04:54:50 UTC
(In reply to comment #46)
> 2. I would add a "-f" (force) flag in the following line:
>  rm examples/Makefile.cmake

ok

> 3. (...) I would suggest (but that is not at all a Fedora 
> requirement) to burry the header files in a specific 
> directory, resulting, after the installation, into:
>  %{_includedir}/armadillo/armadillo
>  %{_includedir}/armadillo/bits
>  %{_includedir}/armadillo/armadillo_itpp
> That way, you would have to add the armadillo directory 
> prefix when you include Armadillo headers. For instance:
> #include <armadillo/bits/Mat_meat.hpp>

The files from the "armadillo_bits" directory are never directly included by the user (it wouldn't work). Instead, the user includes only one file, i.e.
#include <armadillo>
which then includes everything from "armadillo_bits".

(In reply to comment #50)
> ...
> But, as lapack-devel and blas-devel include only libraries 
> (and not header files), I would exclude the correspondent 
> dependency from the 'Requires:' part.

I see what you're getting at, but I'm not totally clear on this. Users will be linking their armadillo programs with -larmadillo, which in turn should automatically pull in the lapack and blas libraries.  However, aren't "/usr/lib/liblapack.so" and "/usr/lib/libblas.so" required for this to work?  (they're present in lapack-devel and blas-devel).

> The same way, I would exclude dependency on 'libstdc++-devel'.

Armadillo explicitly needs the header files from "libstdc++-devel", unless we can assume that a c++ compiler will always be installed ?  (I didn't want to make a dependency on gcc-c++, as people may possibly want to use another compiler).

> So, following is a suggestion for the corresponding part in the RPM
> specification file:
> -------------------
> # The header files of %{name} include some Boost header files, delivered within
> # the boost-devel sub-package. However, since there is no dependency on any
> # Boost library (most of Boost is delivered as header files only), the RPM
> # building process does not detect that dependency. That dependency must
> # therefore be added manually.
> Requires:       boost-devel
> -------------------  

ok.

Comment 52 Susi Lehtola 2009-06-14 06:24:34 UTC
(In reply to comment #51)
> (In reply to comment #50)
> > ...
> > But, as lapack-devel and blas-devel include only libraries 
> > (and not header files), I would exclude the correspondent 
> > dependency from the 'Requires:' part.
> 
> I see what you're getting at, but I'm not totally clear on this. Users will be
> linking their armadillo programs with -larmadillo, which in turn should
> automatically pull in the lapack and blas libraries.  However, aren't
> "/usr/lib/liblapack.so" and "/usr/lib/libblas.so" required for this to work? 
> (they're present in lapack-devel and blas-devel).

The armadillo library is linked against blas and lapack, this is picked up by rpm which adds dependencies on the soname'd libraries as Denis said in #48. One doesn't need blas-devel and lapack-devel to use armadillo, just blas and lapack which are automatically pulled in.

> > The same way, I would exclude dependency on 'libstdc++-devel'.
> 
> Armadillo explicitly needs the header files from "libstdc++-devel", unless we
> can assume that a c++ compiler will always be installed ?  (I didn't want to
> make a dependency on gcc-c++, as people may possibly want to use another
> compiler).

This, on the other hand I'd keep, since nothing picks this up automatically.

Comment 53 Michael Schwendt 2009-06-14 08:07:02 UTC
Re: comment #48

> With the version without the 'Requires:' line, after installation
> of the RPMs, I get:
> # rpm -qpR armadillo-0.6.11-8.fc10.x86_64.rpm 

Since you suggested to delete "Requires" lines from the armadillo-devel (!) subpackage, you need to query the armadillo-devel package, not the main one.

Comment 54 Denis Arnaud 2009-06-14 10:24:08 UTC
(In reply to comment #51)
> Armadillo explicitly needs the header files from "libstdc++-devel", unless we
> can assume that a c++ compiler will always be installed ?  (I didn't want to
> make a dependency on gcc-c++, as people may possibly want to use another
> compiler).

As stated on https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 , there is no need to explicitly set a dependency on gcc-c++ . And, if we have a look at the specification file of gcc (generating the gcc-c++ sub-package: http://cvs.fedoraproject.org/viewvc/rpms/gcc/F-11/gcc.spec?revision=1.49&view=markup), we see that the gcc-c++ gets an explicit dependency on libstdc++-devel package:
 %package c++
 Summary: C++ support for GCC
 Group: Development/Languages
 Requires: gcc = %{version}-%{release}
 Requires: libstdc++ = %{version}-%{release}
 Requires: libstdc++-devel = %{version}-%{release}

And that seems normal to me, as anyone developing in C++ would like to be able to use the STL (and, hence, the STL header files).

So, to recap, I would just list boost-devel within the 'Requires:' dependency, and add the result of that discussion within the Wiki (for example by adding an "Exception" sub-section to https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires, but I have not the right to alter that page).

Comment 55 C Sand 2009-06-14 11:15:24 UTC
(In reply to comment #54)
> there is no need to explicitly set a dependency on gcc-c++ 
> ...
> So, to recap, I would just list boost-devel within
> the 'Requires:' dependency, and add the result of 
> that discussion within the Wiki 

The intent was never to put a dependency on gcc-c++, but specifically only on libstdc++-devel.  Jussi Lehtola (comment #52) seems to agree with this.

According to
http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
"There is no need to include the following packages or their dependencies as BuildRequires", followed by a list which includes gcc-c++.

However, these exceptions are for "BuildRequires" and not "Requires".  So we can't assume that gcc-c++ is installed on the user's machine, and hence we can't assume that libstc++-devel is installed.

It can be stated that Armadillo is not usable without a C++ compiler, however we leave the choice of the compiler upto the user.  The question is whether libstdc++-devel is specific to g++, or whether it can be used by other compilers (including g++ derivatives). If it can be used by other compilers, I'd prefer to have the "Requires" as follows for armadillo-devel:

Requires: boost-devel, libstdc++-devel

Comment 56 Chitlesh GOORAH 2009-06-14 11:26:32 UTC
(In reply to comment #46)
> 1. The following line:
>   Requires:       libstdc++-devel, atlas-devel, lapack-devel, boost-devel
> should be removed, as the corresponding dependencies are already taken into
> account by the 'BuildRequires:' line, and as the 'Requires:' dependencies are
> automatically calculated:
> https://fedoraproject.org/wiki/Packaging/Guidelines#Requires .

Aren't you confused here ? The Requires is for the -devel package and not for the main package.


Denis, I would appreciate in the future, if you could just post the lines what need attention rather a separate SRPM. I nearly mistook your spec for the review.

The package from my point of view is fine. Any objection before I approve it ?
http://arma.sf.net/fedora/armadillo-0.6.11-7.fc10.src.rpm

Comment 57 Chitlesh GOORAH 2009-06-14 11:41:39 UTC
Conrad what is your FAS username ? I'll use it to sponsor you.

Comment 58 C Sand 2009-06-14 12:13:11 UTC
(In reply to comment #57)
> Conrad what is your FAS username ? I'll use it to sponsor you.  

Thanks.  FAS username sent to your email

Comment 59 Michael Schwendt 2009-06-14 12:17:48 UTC
Re: comment #55

No, that's a misinterpretation of the guidelines. The spec file's BuildRequires become the src.rpm's Requires. The gcc-c++ package is mandatory in the "development-tools" as well as "buildsys-build" comps groups. And in turn libstdc++-devel is assumed to be available, too, and need not be specified as a Requires/BuildRequires in any package.

Comment 60 Susi Lehtola 2009-06-14 12:48:29 UTC
(In reply to comment #56)
> The package from my point of view is fine. Any objection before I approve it ?
> http://arma.sf.net/fedora/armadillo-0.6.11-7.fc10.src.rpm  

IMO the Requires: atlas-devel, lapack-devel of armadillo-devel are probably unnecessary. If one builds with
 g++ -larmadillo
not with
 g++ -llapack -latlas -larmadillo
then one doesn't need atlas-devel or lapack-devel and these can be safely dropped, since armadillo acts as a wrapper for the atlas and lapack libraries.

Comment 61 Denis Arnaud 2009-06-14 21:29:06 UTC
(In reply to comment #60, comment #59 and comment #56)

1. Thanks Michael for having shed light on this: that was what I was trying to say, but less successfully :)

2. OK, Chitlesh, I will not interfere with my own versions (which I use when reviewing packages), but will only suggest simple alterations. You are right, that's cleaner.

3. 
> IMO the Requires: atlas-devel, lapack-devel of armadillo-devel are probably
> unnecessary. If one builds with
>  g++ -larmadillo
> not with
>  g++ -llapack -latlas -larmadillo
> then one doesn't need atlas-devel or lapack-devel and these can be safely
> dropped, since armadillo acts as a wrapper for the atlas and lapack libraries.  

https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRequires properly states "It is also a safety feature that prevents builds with that would not otherwise fail, but would be missing crucial features. For example, a graphical application may exclude PNG support after its configure script detects that libpng is not installed."
And, in the Armadillo CMake configure script (CMakeLists.txt), the following excerpt:
-----------------------------
  IF(LAPACK_FOUND)
    SET(ARMA_USE_LAPACK true)
  ENDIF(LAPACK_FOUND)
  
  IF(BLAS_FOUND)
    SET(ARMA_USE_BLAS true)
  ENDIF(BLAS_FOUND)

IF(Boost_FOUND)
  IF(Boost_MAJOR_VERSION GREATER 0)
    IF(Boost_MINOR_VERSION GREATER 33)
      SET(ARMA_USE_BOOST      true)
    ENDIF(Boost_MINOR_VERSION GREATER 33)
  ENDIF(Boost_MAJOR_VERSION GREATER 0)
ENDIF(Boost_FOUND)
-----------------------------
suggests that, if Lapack, Blas and/or Boost are not found, then they are not used (and probably replaced by internal/workaround code).
Hence, they should be listed as dependencies... of the building process, i.e., as 'BuildRequires:'.

And, as mentioned by Michael, boost-devel should be present in the 'Requires:' line as well, as most of Boost is delivered as header files only, which cannot be detected properly by the building process scripts.


In summary, we would have:
--------------
BuildRequires:  cmake, atlas-devel, lapack-devel, boost-devel
%package devel
# Comment justifying the use of a 'Requires:' line
Requires:       boost-devel
--------------

Is that fine? Or did I miss something?

Comment 62 Susi Lehtola 2009-06-15 06:10:06 UTC
(In reply to comment #61)
> In summary, we would have:
> --------------
> BuildRequires:  cmake, atlas-devel, lapack-devel, boost-devel
> %package devel
> # Comment justifying the use of a 'Requires:' line
> Requires:       boost-devel
> --------------
> 
> Is that fine? Or did I miss something?  

No, you missed boost-devel which is needed if armadillo headers use boost classes / something else only available in the boost header files.

Comment 63 C Sand 2009-06-15 08:17:18 UTC
(In reply to comment #60)
> IMO the Requires: atlas-devel, lapack-devel of armadillo-devel are probably
> unnecessary. If one builds with
>  g++ -larmadillo
> not with
>  g++ -llapack -latlas -larmadillo
> then one doesn't need atlas-devel or lapack-devel and these can be safely
> dropped, since armadillo acts as a wrapper for the atlas and lapack libraries.  

Agreed to omitting lapack-devel.

However, we need atlas-devel. Atlas headers are included by Armadillo headers.
(line 85 of upstream include/armadillo, where a workaround for Ubuntu and other systems is present).

Comment 64 C Sand 2009-06-15 08:35:00 UTC
Updated spec and SRPM:

http://arma.sf.net/fedora/armadillo.spec
http://arma.sf.net/fedora/armadillo-0.6.11-8.fc10.src.rpm

Spec file summary:

main package:
BuildRequires:  cmake, boost-devel, lapack-devel, atlas-devel

devel package:
Requires:       boost-devel, atlas-devel
(and a note explaining why they're needed)

install section:
rm -f examples/Makefile.cmake

I think these satisfy everyone's concerns.

Comment 65 Denis Arnaud 2009-06-15 19:50:44 UTC
The following shows that both lapack-devel and blas-devel contain only symbolic links on libraries and static libraries, but no header files:
----------------------------------------
$ rpm -ql lapack-devel
 /usr/lib64/liblapack.a
 /usr/lib64/liblapack.so
 /usr/lib64/liblapack_pic.a
$ rpm -ql blas-devel
 /usr/lib64/libblas.a
 /usr/lib64/libblas.so
----------------------------------------

So, if I am not mistaken, armadillo-devel does not need to list those development packages in the 'Requires:' statement. Of course, they are needed when building the Armadillo packages, but not any longer after that (they are not needed by someone wanting to develop with armadillo-devel).

Comment 66 Susi Lehtola 2009-06-15 20:09:12 UTC
Whoa, this review is unusually long. A lot of static, though.

(In reply to comment #65)
> The following shows that both lapack-devel and blas-devel contain only symbolic
> links on libraries and static libraries, but no header files:

clip
 
> So, if I am not mistaken, armadillo-devel does not need to list those
> development packages in the 'Requires:' statement. Of course, they are needed
> when building the Armadillo packages, but not any longer after that (they are
> not needed by someone wanting to develop with armadillo-devel).  

And they are not, as you could have seen in #64. The only packages required by -devel are boost-devel and atlas-devel, which both contain headers that are used by the armadillo headers. libstdc++-devel should probably be required too.

Comment 67 Denis Arnaud 2009-06-16 09:40:25 UTC
(In reply to comment #66)
> And they are not, as you could have seen in #64. The only packages required by
> -devel are boost-devel and atlas-devel, which both contain headers that are
> used by the armadillo headers.

Sorry, it was my mistake: I read blas-devel instead of atlas-devel!

So, that is fine for me with boost-devel and atlas-devel. Moreover, as stated in comment #59 by Michael, "libstdc++-devel is assumed to be available, too, and need not be specified as a Requires/BuildRequires in any package".

Comment 68 C Sand 2009-06-21 05:19:21 UTC
(In reply to comment #67)
> So, that is fine for me with boost-devel and atlas-devel. Moreover, as stated
> in comment #59 by Michael, "libstdc++-devel is assumed to be available, too,
> and need not be specified as a Requires/BuildRequires in any package".  

ok, I take it to mean the package is now all okay by everyone.

Chitlesh, what are your thoughts on approval ?

Comment 69 Chitlesh GOORAH 2009-07-05 12:42:17 UTC
Give me your latest spec & srpm, I'll do a full review.

Comment 70 C Sand 2009-07-06 03:07:43 UTC
(In reply to comment #69)
> Give me your latest spec & srpm, I'll do a full review.  

http://arma.sourceforge.net/fedora/armadillo.spec
http://arma.sourceforge.net/fedora/armadillo-0.6.12-1.fc10.src.rpm

rpmlint reports no errors and no warnings on the .src.rpm

Upon generation of the binary packages, there is only one very minor warning on "armadillo-devel-0.6.12-1.fc10.x86_64.rpm":

armadillo-devel.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/armadillo-0.6.12/README.txt

This is due to DOS end-of-line encoding used for that file (alone) in the upstream package. The DOS encoding was specifically used so the README file is readable using the brain-damaged Notepad under Windows (which is the default program for opening text files).

Under Linux, the DOS end-of-line encoding has no ill effects for all practical purposes. The file is viewed without any problems using viewers/editors such as "more", "less", "gedit", "kwrite", "nano", etc.

Comment 71 Susi Lehtola 2009-07-06 07:46:32 UTC
(In reply to comment #70)
> Upon generation of the binary packages, there is only one very minor warning on
> "armadillo-devel-0.6.12-1.fc10.x86_64.rpm":
> 
> armadillo-devel.x86_64: W: wrong-file-end-of-line-encoding
> /usr/share/doc/armadillo-0.6.12/README.txt
> 
> This is due to DOS end-of-line encoding used for that file (alone) in the
> upstream package. The DOS encoding was specifically used so the README file is
> readable using the brain-damaged Notepad under Windows (which is the default
> program for opening text files).
> 
> Under Linux, the DOS end-of-line encoding has no ill effects for all practical
> purposes. The file is viewed without any problems using viewers/editors such as
> "more", "less", "gedit", "kwrite", "nano", etc.  

Still, you need to fix it. This is quite trivial: run

for file in README; do
 sed 's/\r//' $file >$file.new && \
 touch -r $file $file.new && \
 mv $file.new $file
done

in the %setup phase to convert it.

(If you try catting the file it doesn't work properly.)

Comment 72 C Sand 2009-07-06 08:06:30 UTC
(In reply to comment #71)
> Still, you need to fix it. This is quite trivial: run
> (...)
> in the %setup phase to convert it.

Can do this, or how about just "dos2unix" ?  Seems less hassle than embedding a shell script.

Comment 73 Susi Lehtola 2009-07-06 08:14:11 UTC
Well, you can do just a sed oneliner
 sed -i 's/\r//' README
but this loses the time stamp. Same with dos2unix. That's why it's best to use the former version.

Comment 74 Chitlesh GOORAH 2009-07-06 08:16:12 UTC
https://fedoraproject.org/wiki/Common_Rpmlint_issues#wrong-file-end-of-line-encoding

Use the fix Jussi gave, as it preserves timestamps.

Comment 75 Michael Schwendt 2009-07-06 08:40:48 UTC
You can tell dos2unix/unix2dos to preserve the timestamp, too.

Comment 76 Chitlesh GOORAH 2009-07-06 08:44:59 UTC
Let's stick to the Fedora packaging guidelines's equivalent, for consistency.

Comment 77 C Sand 2009-07-06 09:18:00 UTC
(In reply to comment #74)
> 
> Use the fix Jussi gave, as it preserves timestamps.  

Done.

http://arma.sourceforge.net/fedora/armadillo.spec
http://arma.sourceforge.net/fedora/armadillo-0.6.12-2.fc10.src.rpm

rpmlint reports no errors on any of the rpms (source and binary)

Comment 78 Chitlesh GOORAH 2009-07-06 15:35:52 UTC
- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The spec file name matches the base package %{name}
- MUST: The package meets the Packaging Guidelines.
- MUST: The package is licensed (LGPLv3+) with an open-source compatible license
and meet other legal requirements as defined in the legal section of Packaging
Guidelines.
- MUST: The License field in the package spec file matches the actual license.
- MUST: the source package includes the text of the license(s) in its own file,
then that file, containing the text of the license(s) for the package is
included in %doc.
- MUST: The spec file must be written in American English.
- MUST: The spec file for the package is be legible.
- MUST: The sources used to build the package must matches the upstream source,
as provided in the spec URL.
- MUST: The package successfully compiles and builds into binary rpms on at
least i386.
- MUST: All build dependencies is listed in BuildRequires.
- MUST: The spec file handles locales properly.
- MUST: If the package does not contain shared library files located in the
dynamic linker's default paths
- MUST: the package is not designed to be relocatable
- MUST: the package owns all directories that it creates.
- MUST: the package does not contain any duplicate files in the %files listing.
- MUST: Permissions on files are set properly.
- MUST: The package has a %clean section, which contains rm -rf %{buildroot}
(or $RPM_BUILD_ROOT).
- MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines.
- MUST: The package contains code, or permissable content. This is described in
detail in the code vs. content section of Packaging Guidelines.
- MUST: There are no Large documentation files
- MUST: %doc does not affect the runtime of the application. To summarize: If
it is in %doc, the program must run properly if it is not present.
- MUST: There are no Header files or static libraries
- MUST: The package does not contain library files with a suffix
- MUST: Package does NOT contain any .la libtool archives
- MUST: Package containing GUI applications includes a %{name}.desktop file,
and that file must be properly installed with desktop-file-install in the
%install section.
- MUST: Package does not own files or directories already owned by other
packages

SHOULD Items:

 - SHOULD: The source package does include license text(s) as LICENSE.txt
 - SHOULD: mock builds succcessfully in i386.
 - SHOULD: The reviewer tested that the package functions as described. A
package should not segfault instead of running, for example.
 - SHOULD: No scriptlets were used, those scriptlets must be sane.
 - SHOULD: No subpackages present.

APPROVED

Comment 79 Chitlesh GOORAH 2009-07-06 15:37:55 UTC
Complete the packaging process as from "Add_Package_to_CVS_and_Set_Owner" section

https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner

Comment 80 C Sand 2009-07-08 02:14:02 UTC
New Package CVS Request
=======================
Package Name: armadillo
Short Description: fast C++ matrix library with interfaces to LAPACK and ATLAS
Owners: conrads
Branches: F-10 F-11

Comment 81 Jason Tibbitts 2009-07-08 16:31:38 UTC
CVS done.

Comment 82 Fedora Update System 2009-07-10 04:27:33 UTC
armadillo-0.6.12-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/armadillo-0.6.12-2.fc10

Comment 83 Fedora Update System 2009-07-10 04:43:16 UTC
armadillo-0.6.12-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/armadillo-0.6.12-2.fc11

Comment 84 Fedora Update System 2009-07-16 07:20:26 UTC
armadillo-0.6.12-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update armadillo'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-7645

Comment 85 Fedora Update System 2009-07-16 07:36:10 UTC
armadillo-0.6.12-2.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update armadillo'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-7695

Comment 86 Fedora Update System 2009-08-01 23:57:16 UTC
armadillo-0.6.12-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 87 Fedora Update System 2009-08-02 00:01:18 UTC
armadillo-0.6.12-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 88 C Sand 2010-07-15 08:24:34 UTC
Package Change Request
======================
Package Name: armadillo
New Branches: EL-6
Owners: conrads

version 0.9.52 is mature enough to be part of EPEL

Comment 89 Kevin Fenzi 2010-07-16 17:26:53 UTC
CVS done (by process-cvs-requests.py).

Comment 90 Susi Lehtola 2011-01-23 15:18:40 UTC
Might you be bothered to build for EL5 as well? The rawhide spec builds as is...

Comment 91 C Sand 2011-01-25 08:13:18 UTC
(In reply to comment #90)
> Might you be bothered to build for EL5 as well?
> The rawhide spec builds as is...

I don't have access to a RHEL 5 machine, so can't test the library properly.

The run-time component might build as is, but that doesn't really test the internals (as Armadillo is a template library). The run-time component is just an alias for LAPACK + BLAS + ATLAS + etc.

Also, I don't know what version of GCC is in RHEL 5. The library was only tested with 4.2, 4.3 and 4.4.  It should work with 4.0 and 4.1, but I don't have the time to incorporate possible workarounds for earlier compilers (the library hits templates pretty hard).

I also don't know what version of ATLAS is in RHEL 5. Anything earlier than 3.8 is full of bugs.