Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: <ppl-0.9> - <A modern C++ library providing numerical abstractions> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=227669 bugs.michael@xxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: <ppl-0.9> - |Review Request: <ppl-0.9> - |<A modern C++ library |<A modern C++ library |providing numerical |providing numerical |abstractions> |abstractions> OtherBugsDependingO| |177841 nThis| | ------- Additional Comments From bugs.michael@xxxxxxx 2007-02-09 19:27 EST ------- The spec file needs *a lot* of work. Let me try to cover many issues in this reply, although I'm afraid I will fail to catch all due to the sheer number of packaging problems. In particular, sub-packages are missing. Run "rpmlint" with option -i on your src.rpm (and also the binary rpms!): $ rpmlint ppl-0.9-1.src.rpm W: ppl summary-ended-with-dot The Parma Polyhedra Library: a C++ library for numerical abstractions. E: ppl no-changelogname-tag W: ppl invalid-license GPL v2 W: ppl hardcoded-packager-tag %{packager} W: ppl hardcoded-prefix-tag /usr W: ppl setup-not-quiet W: ppl mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 1) All findings are worth fixing. "setup-not-quiet" can be ignored, but when adding the -q option to the %setup line of the spec file, the build logs get more readable as the contents of the extracted source archive are not included. "no-changelogname-tag" means that your spec file is missing a standard %changelog section where you sum up important changes applied to the spec file. > %define builddir $RPM_BUILD_DIR/%{name}-%{version} This is unused in the spec file, so don't define it. > %define name ppl > %define version 0.9 > %define release 1 Don't. All macros are defined in the lines below. Here: > Name: %{name} > Version: %{version} > Release: %{release} Fill in the values in those lines, instead of redefining macros via macros. Do this: Name: ppl Version: 0.9 Release: 1 It defines %name, %version, and %release. You can move these lines to the top of the spec file and improve readability. > Vendor: ppl-devel@xxxxxxxxxxx > Packager: %{packager} Don't. Build systems need to be able to override these, so only define them in your local configuration. You don't want anybody, who builds broken binary rpms from your src.rpm, to mark them as coming from you. The included spec %changelog is an entirely different thing. > License: GPL v2 It's just "GPL". > Requires: gmp >= 4.1.3, gcc-c++ >= 4.0.2 Both are wrong. Instead, you want "BuildRequires: gmp-devel", provided that you want to compile with GNU MP. If so, the dependency on the GNU MP library soname is automatically inserted by rpmbuild. > Prefix: /usr This means that you want to mark the packages as relocatable. Whether it really is relocatable remains to be seen after bringing it into shape first. You can safely delete this line unless you insist on making it relocatable. > %setup -n %{name}-%{version} Just %setup -q is fine, since -n %{name}-%{version} is the default. > %build > CXXFLAGS="$RPM_OPT_FLAGS" ./configure --enable-shared \ [snip] Instead of this long command-line, simply run the %configure macro and add any options to it, which it doesn't set automatically. %configure --enable-shared Look at rpm --eval %configure to see what it does. For example, it sets CXXFLAGS for you, too. > %install > if [ -d $RPM_BUILD_ROOT ] > then > rm -rf $RPM_BUILD_ROOT > fi > mkdir -p $RPM_BUILD_ROOT Not needed and superfluous. Use just: %install rm -rf $RPM_BUILD_ROOT > make prefix=$RPM_BUILD_ROOT%{_prefix} bindir=$RPM_BUILD_ROOT%{_bindir} \ [snip] With standard GNU autotools packages, prefer this line make DESTDIR=$RPM_BUILD_ROOT install over the rather long command-line in your spec file. When it doesn't work (unlikely with well-maintained autotools code), there is the %makeinstall macro, which can be used instead. Look at rpm --eval %makeinstall to see what it would do. But prefer the DESTDIR install. > %files The main "ppl" package contains a mixture of files. Split off a "ppl-devel" package, which contains the files needed only for software development (include files, the libppl.so symlink, documentation for developers, ppl-config, and so on). The "ppl" package should only contain the main library files, the licence, and any files relevant to the library run-time. Don't include static libraries. If possible, disable them at configure time with --disable-static or just delete them in %install. Also don't include libtool archive files *.la. > %files c Same here. A "ppl-c-devel" package is missing. But doesn't it make sense to put C and C++ library and API into the "ppl" and "ppl-devel" packages? > %files gprolog > %defattr(-,root,root) > %{_bindir}/ppl_gprolog > %{_libdir}/ppl/ppl_gprolog.pl The directory %{_libdir}/ppl/ is not included. You can add %dir %{_libdir}/ppl in the right package which should own this directory entry. >%files sicstus Same here and the other sub-packages. They also contain an orphaned %_libdir/ppl directory. > %post > /sbin/ldconfig > %postun > /sbin/ldconfig > %post c > /sbin/ldconfig > %postun c > /sbin/ldconfig Better: %post -p /sbin/ldconfig %postun -p /sbin/ldconfig %post c -p /sbin/ldconfig %postun c -p /sbin/ldconfig That executes /sbin/ldconfig directly instead of via /bin/sh, and it creates a dependency on /sbin/ldconfig automatically. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review