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
Lev Shamardin <shamardin(a)gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flag|needinfo?(shamardin(a)gmail.c |
|om) |
--- Comment #23 from Lev Shamardin <shamardin(a)gmail.com> 2008-10-31 10:07:22 EDT
---
(In reply to comment #21)
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.
Fixed this.
* For -python subpackage
- Would you explain why -python subpackage has "Requires
python-setuptools"?
This was a typo. Fixed.
------------------------------------------------------
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.
From my point of view, the only possible problem is that someone can
finish using newer protobuf-compiler with older python/java
bindings. Both java and python implementations are usable as a runtime
without any C++ code, you only need corresponding version of
protobuf-compiler for development.
* 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:
Unfortunately I'm not that familiar with java development and
packaging. The problem is that if I just add 'BuildRequires:
java-devel' the build fails at some point with errors like:
-------------------------------------------------------
1. ERROR in
/builddir/build/BUILD/protobuf-2.0.2/java/src/main/java/com/google/p
rotobuf/TextFormat.java (at line 438)
while (pos < matcher.regionStart()) {
^^^^^^^^^^^
The method regionStart() is undefined for the type Matcher
-------------------------------------------------------
On my Fedora 8 system BuildRequires: java-devel brings
java-1.5.0-gcj-devel-1.5.0.0-17.fc8.i386 in mock build.
I can successfully build the package with
java-1.6.0-sun-devel-1.6.0.3-1jpp without mock on my system, and
BuildRequires: java-devel >= 1.6 brings
java-1.7.0-icedtea-devel-1.7.0.0-0.19.b21.snapshot.fc8.i586 which
builds the package succesfully.
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)
I'm not sure if this is a good idea. This requirement in general makes
it impossible to build the package with
java-1.6.0-sun-devel-1.6.0.3-1jpp, while java-devel >= 1.6 brings
correct dependencies in mock and allows to build the package with sun
java 1.6
* For -javadoc subpackage
- "Requires: %{name}-%{version}-%{release}-java" is perhaps a typo.
Fixed.
* 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.
I just think this can be handy, and I generally disagree with the
policy of packaging only shared libraries for development. There are
some cases when you need static libraries, and for these cases having
static libraries packaged saves you from rebuilding the required
libraries yourself. I see no problems here, since normally developer
will install -devel and not -static subpackage.
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)
Fixed.
* 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.
Working on this. Will update the ticket as soon as it is ready.
* Unneeded /sbin/ldconfig call
- There is no need that -devel subpackage should call /sbin/ldconfig on
post/postun scriptlets.
This was a typo. Fixed.
C. %files entry
* Macros
- Please refer to
https://fedoraproject.org/wiki/Packaging/RPMMacros
%_prefix/include must be %_includedir.
Fixed.
* 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.
Done.
* 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
Fixed.
!!For -vim subpackage
! Neither of %_datadir/vim/vimfiles/{ftdetect,syntax} are owned
by any packages, however I will ask vim maintainer about this.
Any news on this item?
* 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).
Fixed.
So finally,
Updated SPEC:
http://shamardin.googlepages.com/protobuf.spec
New SRPM:
http://shamardin.googlepages.com/protobuf-2.0.2-2.fc8.src.rpm
Changes:
* Fri Oct 31 2008 Lev Shamardin <shamardin(a)gmail.com> - 2.0.2-2
- Use python_sitelib macro instead of INSTALLED_FILES.
- Fix the license.
- Fix redundant requirement for -devel subpackage.
- Fix wrong dependences for -python subpackage.
- Fix typo in requirements for -javadoc subpackage.
- Use -p option for cp and install to preserve timestamps.
- Remove unneeded ldconfig call for post scripts of -devel subpackage.
- Fix directories ownership.
I will update the SPEC to use the packaged gtest in the next few days, could
you please check for any other errors left meanwhile?
--
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.