[Bug 459088] Review Request: protobuf - Protocol Buffers - Google's data interchange format

[bugzilla at redhat.com]

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 at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp




--- Comment #21 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  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 at localhost ~]$ rpm -qf /usr/include/google/protobuf/
protobuf-devel-2.0.2-1.fc10.i386
[tasaka1 at 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.