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=459088 Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtasaka@xxxxxxxxxxxxxxxxxxx --- Comment #21 from Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> 2008-10-19 14:52:35 EDT --- Some notes for 2.0.2-1: A. %description stage * License - is actually BSD (also see CHANGES.txt) Perhaps you want to change the license of .pc.in file. * (Build)Requires * For -devel subpackage - "Requires: protobuf" is redundant because protobuf-compiler already requires protobuf. * For -python subpackage - Would you explain why -python subpackage has "Requires python-setuptools"? ------------------------------------------------------ Additional remark about python subpackage: The -python subpackage should not depend on the base package or any other packages because it is a pure python implementation. ------------------------------------------------------ - Well, for technical discussion, does this mean that there will be no problem even if the installed version of protobuf and protobuf-python differ? (if you don't write Requires this can happen). This discussion can be applied for -java subpackage. * For -java subpackage - About "BuildRequires: java-devel >= 1.6.0" -- If this means that Java binding needs OpenJDK to build, then this line must be -------------------------------------------------------- BuildRequires: java-devel >= 1:1.6.0 -------------------------------------------------------- java-devel vitrual Provides by java-1.6.0-openjdk-devel has Epoch 1 for historical reason (see: https://fedoraproject.org/wiki/Packaging/Java#BuildRequires_and_Requires ) -- If either OpenJDK or icedtea can be used to build, then "BuildRequires: java-devel >= 1.7" is sufficient (Note that 1.7 < 1:1.6) * For -javadoc subpackage - "Requires: %{name}-%{version}-%{release}-java" is perhaps a typo. * Shipping -static subpackage - Please explain why this package is needed where -devel subpackage is provided which includes .so symlink libraries. Usually static archives must be removed unless the package does not provide shared libraries. B. %prep, %build, %instll, scriptlets stage: * Timestamps - When using "cp" or "install" commands, add "-p" option to keep timestamps on installed files - Also this package uses "install-sh" when installing files. In such case -------------------------------------------------------- make install DESTDIR=$RPM_BUILD_ROOT CPPROG="cp -p" -------------------------------------------------------- is useful to keep timestamps on installed files as much as possible. ('INSTALL="install -p"' option is usually tricks for Makefiles based on Makefiles generated by more recent Makefiles and not using install-sh) * Internal libraries - This package uses "gtest" library to build. This is already provided as "gtest" rpm on Fedora system-widely so this package must be patched to use system-wide gtest library. * Unneeded /sbin/ldconfig call - There is no need that -devel subpackage should call /sbin/ldconfig on post/postun scriptlets. C. %files entry * Macros - Please refer to https://fedoraproject.org/wiki/Packaging/RPMMacros %_prefix/include must be %_includedir. * For -devel subpackage -------------------------------------------------------- [tasaka1@localhost ~]$ rpm -qf /usr/include/google/protobuf/ protobuf-devel-2.0.2-1.fc10.i386 [tasaka1@localhost ~]$ rpm -qf /usr/include/google/ file /usr/include/google is not owned by any package -------------------------------------------------------- - Well, it seems that some other packages also install files under %_includedir/google, however when installing this -devel subpackage (and its dependency rpms), %_includedir/google is not owned by any packages. So for now %_includedir/google must also be owned by this rpm. * For -python subpackage -------------------------------------------------------- %files python -f python/INSTALLED_FILES -------------------------------------------------------- - This method (i.e. listing %files entry by using INSTALLED_FILES created by setup.py) is forbidden on Fedora. With this method directory ownership issue is frequently ignored (and actually for this case): https://fedoraproject.org/wiki/Packaging/Python#System_Architecture https://fedoraproject.org/wiki/Packaging/UnownedDirectories !!For -vim subpackage ! Neither of %_datadir/vim/vimfiles/{ftdetect,syntax} are owned by any packages, however I will ask vim maintainer about this. * For -java subpackage - %{_datadir}/maven2/poms, %{_mavendepmapfragdir} are already owned by jpackage-utils and this package should not own these directories (in this point packaging guidelines is a bit wrong). -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review