[Bug 1069257] Review Request: fparser - Function parser library for C++

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1069257



--- Comment #10 from Till Hofmann <hofmann@xxxxxxxxxxxxxxxxxxx> ---
(In reply to Michael Schwendt from comment #6)
> 
>   | %files
>   | %doc docs/gpl.txt docs/lgpl.txt
> 
> The spec file doesn't comment on that. What's the full story here? Including
> the GPLv3 with the package only adds confusion, IMO.
> 

I'm not sure why upstream includes both license files, but the headers
reference both, too. I could just remove gpl.txt since it's not really used, I
just figured I'd put it in the package since they reference it in the headers.

> 
> > %package        devel
> > …
> > Requires:       %{name} = %{version}-%{release}
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
> 

fixed

> 
> > %package        devel
> > …
> > Requires:       cmake, mpfr-devel, gpm-devel
> 
> It's a good habit to add comments to explicit dependencies. Here the
> dependency on "cmake" raises the question why it has been added? Not
> everyone uses CMake. 

I've removed cmake and added comments to the other two. I also fixed a typo,
gpm-devel should be gmp-devel. MPFR and GMP are not necessary but provide extra
features, so I'm not sure if I should include them as requirement? 
>From their website:
> Different numerical types are supported: double, float, long double, long int,
> std::complex (of types double, float and long double), multiple-precision 
> floating point numbers using the MPFR library, and arbitrary precision 
> integers using the GMP library. (Note that it's not necessary for these two 
> libraries to exist in the system in order to use the Function Parser library 
> with the other numerical types. Support for these libraries is optionally 
> compiled in using preprocessor settings.)
> 

> > %install
> > rm -rf $RPM_BUILD_ROOT
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

fixed

> 
> 
> > %files devel
> > …
> > %{_libdir}/cmake/%{name}-%{version}/*
> 
> This would mean that the directory %{_libdir}/cmake/%{name}-%{version} is
> not included (aka being an "unowned" directory issue):

fixed

> 
> 
> > -shared -Wl,-soname,libfparser4.5.1.so.4.5.1 -o libfparser4.5.1.so.4.5.1
> 
> Uh oh! That's a very fragile SONAME, which will require rebuilds of
> dependencies for all minor version updates.

You're right. I'm not sure what a good solution would look like, I've stripped
all versions from the file names (e.g. the cmake files are now in
%{_libdir}/cmake/fparser) and set 4.5 as SO version, which results in
libfparser.4.5 .

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]