[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 #32 from Michael Schwendt <bugs.michael@xxxxxxx> ---
What has changed is that two of the installed headers are wrong (= I copied all
the names from the topdir instead of checking that three of them should not be
installed). That's wrong, especially since the two gmp/mpfr headers could not
be included anyway because of a missing subdir:

Cmake based:
$ rpmls -p fparser-devel-4.5.1-4.fc20.x86_64.rpm |grep inc
drwxr-xr-x  /usr/include/fparser
-rw-r--r--  /usr/include/fparser/fparser.hh
-rw-r--r--  /usr/include/fparser/fpaux.hh
-rw-r--r--  /usr/include/fparser/fpconfig.hh
-rw-r--r--  /usr/include/fparser/fptypes.hh

 -> fpaux.hh, fptypes.hh, fpconfig.hh are internal headers only

Autotools based:
$ rpmls -p fparser-devel-4.5.1-6.fc20.x86_64.rpm |grep inc
drwxr-xr-x  /usr/include/fparser
-rw-r--r--  /usr/include/fparser/fparser.hh
-rw-r--r--  /usr/include/fparser/fparser_gmpint.hh
-rw-r--r--  /usr/include/fparser/fparser_mpfr.hh
-rw-r--r--  /usr/include/fparser/fpconfig.hh

 -> fpconfig.hh is an internal header only
 -> fparser_*.hh should not be installed if gmp/mpfr support is not compiled in


> Do you want me to include the possibility to turn MPFR/GMP on/off?

That depends on whether those features will be needed. Building with MPFR/GMP
would make the library depend on libmpfr/libgmp, but using the extra headers
would still be optional -> the case early in the review.

Anyway, if you say MPFR/GMP is not needed, only the three headers ought to be
dropped. Shipping internal headers is not a great idea. Eventually somebody
would start including them.

-- 
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]