Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=566171 --- Comment #5 from Michael Schwendt <mschwendt@xxxxxxxxx> 2010-02-26 17:33:37 EST --- A closer look at the package: > 2. I still don't understand why it was "GPLv3+", as the license of any autoconf/automake or libtool files typically is not included in the rpm's "License" tag. > 4. The comment "Force use of system libtool" is misleading. Actually, you don't want to enforce anything. You just want to build an initial autotools/libtool framework, because the svn snapshot doesn't include those files. Running the included autogen.sh script would work, but it would also execute ./configure directly. Copying from autogen.sh works, but should be done in %prep section after %setup, as it is a form of source preparation to be executed once only (just like applying patches). Doing it %prep would also be beneficial for --short-circuit builds. And instead of running all the tools manually, you could run just "autoreconf -i". > --enable-maintainer-mode Why do you enable it? (especially for this svn snapshot where you regenerate the files already) > rm -rf $RPM_BUILD_ROOT%{_libdir}/*.la The .la file in %{python_sitearch}/*/*.la should be deleted, too. Btw, it's broken anyway if you delete the other one in %_libdir. > sed -ie /pkgpy/s/libhid/hid/g Makefile # Use 'hid' module name instead 'libhid' > sed -ie /pkgpy/s/libhid/hid/g swig/Makefile # Use 'hid' module name instead 'libhid' The comment is insufficient. *Why* is this done? And it substitutes more than intended in swig/Makefile. Take a look at the diff against the original file. Both modifications ought to be applied as %patch files instead, which is less fragile. These two sed substitutions would fail silently, and your %files section is not explicit enough due to wildcards and would include _any_ Python module and not just 'hid'. * The diff against -1.fc12: > -BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) > +BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXX) Why that change? The new one doesn't match the guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag > -make CFLAGS="$CFLAGS" > - > +make CFLAGS="$CFLAGS" The extra CFLAGS definition here should not be necessary and would be the wrong place where to do it. As can be seen in "rpm --eval %configure", CFLAGS are defined and exported already when running the "configure "script. [If you needed to "fix" the CFLAGS usage, the proper place would be to fix the configure script and/or the Makefiles.] "make" should also be run as "make %{?_smp_mflags}": https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make > %defattr(-,root,root) https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > +%ifnarch i686 > %{python_sitearch}/* > +%endif I guess this is because rpmbuild warns about duplicate file entries on platforms (not limited to i686) where %{python_sitearch} and %{python_sitelib} are the same. But actually this is completely harmless _as long as_ you include the Python files in the same subpackage. And rpmbuild then does not include any file more than once. https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files is merely important for packages with multiple subpackages and multiple %files sections, where files may be included in _more than one_ subpackage by accident. If you have strong feelings about it, you could do %{python_sitelib}/* %ifarch x86_64 ppc64 sparc64 s390x %{python_sitearch}/* %endif to cover more platforms correctly, but it doesn't add much value. A brief comment instead would be fine. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review