Bug 520322 - Review Request: mm-common - common files for GNOME C++ bindings
Summary: Review Request: mm-common - common files for GNOME C++ bindings
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 532096 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-08-30 16:09 UTC by Krzesimir Nowak
Modified: 2009-12-20 11:34 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-09-19 17:20:56 UTC
mtasaka: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Krzesimir Nowak 2009-08-30 16:09:19 UTC
Spec URL: http://wips.pl/~kudi/rpm/mm-common/mm-common.spec
SRPM URL: http://wips.pl/~kudi/rpm/mm-common/mm-common-0.7.1-1.fc11.src.rpm

Description:
The mm-common module provides the build infrastructure and utilities
shared among the GNOME C++ binding libraries.  It is only a required
dependency for building the C++ bindings from the gnome.org version
control repository.  An installation of mm-common is not required for
building tarball releases, unless configured to use maintainer-mode.

This is my first package, so I'm seeking for a sponsor. Thanks.

Comment 1 Mamoru TASAKA 2009-09-02 18:40:47 UTC
Some notes:

* Version
  - It seems that the latest version is 0.7.2.

* License
  - As http://gcc.gnu.org/onlinedocs/libstdc++/ says libstdc++ documents
    are under GFDL, the license tag should be "GPLv2+ and GFDL".

* About BR and its releated comments
---------------------------------------------------------
# mm-common uses curl or wget to download the most recent libstdc++.tag.
---------------------------------------------------------
  - ... but during build process nothing is to be downloaded.
    ( and koji (Fedora buildsystem) does not allow to download anything
      during rpmbuild )
    Would you clarify what you mention here?

  - By the way does this package need rebuild every time the (libstdc++)
    upstream doctags/libstdc++.tag is updated?
    ( Again on koji, no net connection is supported during rpmbuild
      process )

  - Would you check if autoconf/automake is really needed for
    BuildRequires? They don't seem to be needed.


* Macros
  - Please use macros properly. /usr/bin should be %{_bindir}.
    https://fedoraproject.org/wiki/Packaging/RPMMacros

  ! Note
    - For consistency I would suggest that "Requires: libxslt" should be
      used instead of "Requires: %{_bindir}/xsltproc", however I guess
      you want to suppress rpmlint "error" about explicit-lib-dependency.
      So if you want to use "Requires: %{_bindir}/xsltproc", I won't
      object to it.

Comment 2 Krzesimir Nowak 2009-09-02 19:56:59 UTC
(In reply to comment #1)
> Some notes:
> 
> * Version
>   - It seems that the latest version is 0.7.2.
> 

Yes, it was released today. Packaged it.

> * License
>   - As http://gcc.gnu.org/onlinedocs/libstdc++/ says libstdc++ documents
>     are under GFDL, the license tag should be "GPLv2+ and GFDL".
> 

Done.

> * About BR and its releated comments
> ---------------------------------------------------------
> # mm-common uses curl or wget to download the most recent libstdc++.tag.
> ---------------------------------------------------------
>   - ... but during build process nothing is to be downloaded.
>     ( and koji (Fedora buildsystem) does not allow to download anything
>       during rpmbuild )
>     Would you clarify what you mention here?
> 
>   - By the way does this package need rebuild every time the (libstdc++)
>     upstream doctags/libstdc++.tag is updated?
>     ( Again on koji, no net connection is supported during rpmbuild
>       process )
> 
>   - Would you check if autoconf/automake is really needed for
>     BuildRequires? They don't seem to be needed.
> 

Ok, here I messed completely (sort of compiling source from git vs. compiling source from tarball). So all BR are gone now, since none are needed. libstdc++.tag is downloaded when building from git (or in maintainer mode), so this tag file is already present in tarball. I'd leave keeping tag file updated task to upstream.

> * Macros
>   - Please use macros properly. /usr/bin should be %{_bindir}.
>     https://fedoraproject.org/wiki/Packaging/RPMMacros
> 
>   ! Note
>     - For consistency I would suggest that "Requires: libxslt" should be
>       used instead of "Requires: %{_bindir}/xsltproc", however I guess
>       you want to suppress rpmlint "error" about explicit-lib-dependency.
>       So if you want to use "Requires: %{_bindir}/xsltproc", I won't
>       object to it. 

Since rpmlint's error about explicit-lib-dependency is not important in this case, I changed it to libxslt. 

New paths:
SPEC: http://wips.pl/~kudi/rpm/mm-common/mm-common.spec
SRPM: http://wips.pl/~kudi/rpm/mm-common/mm-common-0.7.2-1.fc11.src.rpm

Comment 3 Mamoru TASAKA 2009-09-03 16:58:36 UTC
Okay.

- "Requires: perl" is redundant. This dependency is automatically
  detected by rpmbuild itself and is automatically added to binary
  rpm.

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

Comment 4 Krzesimir Nowak 2009-09-04 12:12:39 UTC
Removed `Requires: perl'. Spec in the same url as before.

I'll probably make some pre-reviews then, since now I don't have any other packages to make and request review. Also, seems that many packages in "no one is reviewing" actually have some sort of reviews.

Comment 5 Mamoru TASAKA 2009-09-05 17:43:11 UTC
Okay, then I will wait for your pre-review.

Comment 6 Krzesimir Nowak 2009-09-10 10:27:59 UTC
My prereview: https://bugzilla.redhat.com/show_bug.cgi?id=520550#c4

And new upstream release of mm-common has arrived. Now it puts an example skeleton of build system (configure.ac, Makefile.ams, example sources) for mm modules in documentation. Should I make a separate package or put it into %doc?

Comment 7 Mamoru TASAKA 2009-09-10 12:45:32 UTC
Well, usually it depends on how the packager thinks of it, however
if such documents are enough large compared to other parts, I recommend
to create a seperate package.

Comment 8 Krzesimir Nowak 2009-09-10 18:47:53 UTC
I decided to split the package into two - main one and docs one.

I noticed that README and skeleton are installed into /usr/share/doc/mm-common/ and rest, which are not normally installed (ChangeLog and COPYING) into /usr/share/doc/mm-common-0.7.3/. It doesn't look nice for me, but that probably is not a blocker, is it?

spec: http://wips.pl/~kudi/rpm/mm-common/mm-common.spec
srpm: http://wips.pl/~kudi/rpm/mm-common/mm-common-0.7.3-1.fc11.src.rpm

rpmlint outputs:

mm-common.spec:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

mm-common-0.7.3-1.fc11.src.rpm:
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

mm-common-0.7.3-1.fc11.noarch.rpm:
mm-common.noarch: E: explicit-lib-dependency libxslt
mm-common.noarch: W: devel-file-in-non-devel-package /usr/share/pkgconfig/mm-common-libstdc++.pc
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

mm-common-docs-0.7.3-1.fc11.noarch.rpm:
mm-common-docs.noarch: E: version-control-internal-file /usr/share/doc/mm-common/skeletonmm/skeleton/skeletonmm/private/.gitignore
mm-common-docs.noarch: E: version-control-internal-file /usr/share/doc/mm-common/skeletonmm/doc/reference/.gitignore
mm-common-docs.noarch: W: spurious-executable-perm /usr/share/doc/mm-common/skeletonmm/autogen.sh
mm-common-docs.noarch: E: zero-length /usr/share/doc/mm-common/skeletonmm/skeleton/src/skeleton_method.defs
mm-common-docs.noarch: E: zero-length /usr/share/doc/mm-common/skeletonmm/skeleton/src/skeleton_enum.defs
mm-common-docs.noarch: E: zero-length /usr/share/doc/mm-common/skeletonmm/skeleton/src/skeleton_extra.defs
mm-common-docs.noarch: E: version-control-internal-file /usr/share/doc/mm-common/skeletonmm/build/.gitignore
mm-common-docs.noarch: E: version-control-internal-file /usr/share/doc/mm-common/skeletonmm/skeleton/.gitignore
mm-common-docs.noarch: E: version-control-internal-file /usr/share/doc/mm-common/skeletonmm/.gitignore
mm-common-docs.noarch: W: doc-file-dependency /usr/share/doc/mm-common/skeletonmm/autogen.sh R
1 packages and 0 specfiles checked; 8 errors, 2 warnings.

This many warns and errors in doc package are because of example skeleton of mm module.

Oh, and I hope %changelog in spec file now is not important, so I'm not adding new entries to it, but changing them to new version and date instead, because the package did not hit Fedora Collection yet.

Comment 9 Mamoru TASAKA 2009-09-12 18:39:42 UTC
Well, for 0.7.3-1

* Timestamps
  - Now this package installs more files, keeping timestamps
    on installed files as much as possible is preferable.
    Please consider to use
------------------------------------------------------
make install DESTDIR=%{buildroot} INSTALL="insatll -p" CPPROG="cp -p"
------------------------------------------------------
    to keep timestamps on installed files.
    - Usually adding INSTALL="insatll -p" works for makefiles based
      on Makefiles generated from recent autotools
    - However this package also uses "install-sh" to install files.
      For this, adding CPPROG="cp -p" will (usually) keep timestamps
      on installed files.

* redundant %doc
  - Files under %{_mandir} are automatically marked as %doc.
  - Also files under %{_docdir} are automatically marked as %doc.

* Directory ownership issue
  - This is actually under discussion, however this package installs
    some files under %{_datadir}/aclocal/ and currently owned by
    "automake" rpm. So adding "Requires: automake" to this package is 
    preferable to satisfy directory ownership issue.

  - -docs package installs some files under %{_docdir}/%{name}/,
    however the directory %{_docdir}/%{name} is not owned by any packages,
    which should be owned by "this" (i.e. mm-common-docs) package:

    https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Forgetting_to_Include_a_Toplevel_Directory

! %changelog
  - I recommend to add entries into %changelog even this is still
    under review.

Comment 10 Krzesimir Nowak 2009-09-13 18:43:03 UTC
(In reply to comment #9)
> Well, for 0.7.3-1
> 
> * Timestamps
>   - Now this package installs more files, keeping timestamps
>     on installed files as much as possible is preferable.
>     Please consider to use
> ------------------------------------------------------
> make install DESTDIR=%{buildroot} INSTALL="insatll -p" CPPROG="cp -p"
> ------------------------------------------------------
>     to keep timestamps on installed files.
>     - Usually adding INSTALL="insatll -p" works for makefiles based
>       on Makefiles generated from recent autotools
>     - However this package also uses "install-sh" to install files.
>       For this, adding CPPROG="cp -p" will (usually) keep timestamps
>       on installed files.
> 

Done.

> * redundant %doc
>   - Files under %{_mandir} are automatically marked as %doc.
>   - Also files under %{_docdir} are automatically marked as %doc.
> 

Done.

> * Directory ownership issue
>   - This is actually under discussion, however this package installs
>     some files under %{_datadir}/aclocal/ and currently owned by
>     "automake" rpm. So adding "Requires: automake" to this package is 
>     preferable to satisfy directory ownership issue.
> 

Done.

>   - -docs package installs some files under %{_docdir}/%{name}/,
>     however the directory %{_docdir}/%{name} is not owned by any packages,
>     which should be owned by "this" (i.e. mm-common-docs) package:
> 
>    
> https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Forgetting_to_Include_a_Toplevel_Directory

I hope I got it right now. I added ownerships to %{_datadir}/%{name}/ and to %{_docdir}/%{name}/. 

> 
> ! %changelog
>   - I recommend to add entries into %changelog even this is still
>     under review.  

Ok.

spec: http://wips.pl/~kudi/rpm/mm-common/mm-common.spec
srpm: http://wips.pl/~kudi/rpm/mm-common/mm-common-0.7.3-2.fc11.src.rpm

Comment 11 Mamoru TASAKA 2009-09-14 18:42:47 UTC
Well,

- This package itself can be approved now.
- I checked your pre-review and it seems good to some extent
  for the first comments.

----------------------------------------------------------
   This package (mm-common) is APPROVED by mtasaka
----------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
After you request for sponsorship a mail will be sent to sponsor 
members automatically (which is invisible for you) which notifies 
that you need a sponsor. After that, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 10/11, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Comment 12 Krzesimir Nowak 2009-09-14 21:44:32 UTC
I requested for sponsorship. FAS name: krnowak

Thanks.

Comment 13 Mamoru TASAKA 2009-09-15 11:50:08 UTC
Okay, now I am sponsoring you. Please follow "Join" wiki again.

Comment 14 Krzesimir Nowak 2009-09-16 09:13:35 UTC
New Package CVS Request
=======================
Package Name: mm-common
Short Description: common files for GNOME C++ bindings
Owners: krnowak
Branches: F-10 F-11
InitialCC:

Comment 15 Jason Tibbitts 2009-09-16 21:39:32 UTC
CVS done.

Comment 16 Fedora Update System 2009-09-17 16:36:50 UTC
mm-common-0.7.3-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/mm-common-0.7.3-2.fc11

Comment 17 Fedora Update System 2009-09-17 16:36:55 UTC
mm-common-0.7.3-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/mm-common-0.7.3-2.fc10

Comment 18 Fedora Update System 2009-09-19 00:11:23 UTC
mm-common-0.7.3-2.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update mm-common'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-9742

Comment 19 Fedora Update System 2009-09-19 00:13:15 UTC
mm-common-0.7.3-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update mm-common'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-9753

Comment 20 Mamoru TASAKA 2009-09-19 17:20:56 UTC
Closing.

Comment 21 Mamoru TASAKA 2009-10-30 15:56:09 UTC
*** Bug 532096 has been marked as a duplicate of this bug. ***

Comment 22 Daniel Elstner 2009-12-20 10:23:22 UTC
(In reply to comment #1)
> * License
>   - As http://gcc.gnu.org/onlinedocs/libstdc++/ says libstdc++ documents
>     are under GFDL, the license tag should be "GPLv2+ and GFDL".

No, the API reference documentation is released under the same license as the libstdc++ source code.  It's not made very visible:

http://gcc.gnu.org/onlinedocs/libstdc++/manual/bk01pt01ch01s02.html

See the bottom of the page.

Comment 23 Mamoru TASAKA 2009-12-20 11:31:27 UTC
Please don't comment such a thing on review request bugs closed
more than one months ago and open a new bug against mm-common instead.

Comment 24 Mamoru TASAKA 2009-12-20 11:34:07 UTC
(In reply to comment #23)
> Please don't comment such a thing on review request bugs closed
> more than one months ago and open a new bug against mm-common instead.  

Actually more than 3 months ago.


Note You need to log in before you can comment on or make changes to this bug.