Bug 246046 - gtksourceview2 - GtkSourceView v2.x
gtksourceview2 - GtkSourceView v2.x
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity low
: ---
: ---
Assigned To: Toshio Kuratomi
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-27 21:51 EDT by Matthias Clasen
Modified: 2008-10-11 10:11 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-11 10:11:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
toshio: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Matthias Clasen 2007-06-27 21:51:57 EDT
The situation here is slightly messed up, since I already updated the
gtksourceview package in rawhide to 1.90 (ie v2) before realizing that there is
a reasonable number of packages still depending on v1, and that upstream is actually
planning to maintain the versions parallel-installable. 

To rectify this, I propose revert the gtksourceview package to the v1 version of
the library (which requires an epoch), and moving v2 to a new gtksourceview2
package.

Here are the specs and packages (I only include the v1 package for completeness,
the review request is about gtksourceview2).

http://people.redhat.com/mclasen/gtksourceview/gtksourceview.spec
http://people.redhat.com/mclasen/gtksourceview/gtksourceview-1.8.5-2.fc8.src.rpm

http://people.redhat.com/mclasen/gtksourceview/gtksourceview2.spec
http://people.redhat.com/mclasen/gtksourceview/gtksourceview2-1.90.1-1.fc8.src.rpm
Comment 1 Toshio Kuratomi 2007-06-28 22:59:26 EDT
Here's a few preliminary problems:

Cosmetic:
* The gnome_print_version macro isn't used in this package.

Blockers:
* gtksourceview2 should own the directories from %{_datadir}/gtk-doc and deeper.
* devhelp doesn't work.  It appears if you rename the directory you also need to
rename the *.devhelp? files.  Renaming to gtksourceview-2.0.devhelp
* Package won't build as is.  You need to specify -n gtksourceview to the %setup
macro.
Comment 2 Matthias Clasen 2007-06-29 10:03:53 EDT
> * The gnome_print_version macro isn't used in this package.

Removed

> * gtksourceview2 should own the directories from %{_datadir}/gtk-doc and deeper.

It won't make things any better, but if it makes you happy...imo it would be
best to hold of on random directory ownership changes until we know a good
solution.
 
> * devhelp doesn't work.  It appears if you rename the directory you also need 
> to rename the *.devhelp? files.  Renaming to gtksourceview-2.0.devhelp

Done

* Package won't build as is.  You need to specify -n gtksourceview to the %setup
macro.

It certainly built as is here, and it already has
 %setup -n gtksourceview-%{version}


New version:

http://people.redhat.com/mclasen/gtksourceview/gtksourceview2.spec
http://people.redhat.com/mclasen/gtksourceview/gtksourceview2-1.90.1-3.fc8.src.rpm
Comment 3 Toshio Kuratomi 2007-06-29 16:29:58 EDT
(In reply to comment #2)
> * Package won't build as is.  You need to specify -n gtksourceview to the %setup
> macro.
> 
> It certainly built as is here, and it already has
>  %setup -n gtksourceview-%{version}

I certainly wondered how that got all the way to review :-)

Good:
* spec name matches package name
* Licensed under the GPL which is an open source license.
* License text included
* Spec file is clear and easy to read
* Sources match upstream
* Builds in mock on x86_64 devel
* Uses %find_lang to find locale files.
* ldconfig called on %post
* Owns all created directories
* No duplicate files
* Permissions set correctly except as noted in the rpmlint section below
* %clean used
* Proper use of macros for directory paths
* Included doc files are reasonable for the packages.
* -devel package includes the headers and requires pkgconfig
* -devel has proper requires on the base package
* No *.la files and no static libraries

Rpmlint Output:
  E: gtksourceview2 non-executable-script
  /usr/share/gtksourceview-2.0/language-specs/check.sh 0644
  E: gtksourceview2 non-executable-script 
  /usr/share/gtksourceview-2.0/language-specs/convert.py 0644

These are a bit strange.  Upstream has these in their Makefile.am as::
  languages_DATA = $(LANGUAGES) language.rng language2.rng language.dtd \
                 check.sh convert.py

They appear to be scripts to aid developers in creating new *.lang files for
gtksourceview.  For now you can get rid of the rpmlint warning by chmod a+x the
files.  Longer term it seems like upstream should consider either not installing
them or installing them to a bin directory as something like
gtksourceview-check-lang and gtksourceview-convert-lang.

W: gtksourceview2-devel no-documentation
Ignorable.  There actually is documentation here but not marked as such.  You
could mark the gtk-doc API docs as %doc if you want but we're also trying to get
a patch into rpm to mark those automatically just like man files and things in
%{_docdir}

Bad:
* Missing BuildRequire: pcre-devel
* Only a subset of the *.lang files pass the check.sh script.  Running
convert.py over those files yields a file that check.sh will validate.  We can 
convert all the non-validating files with commands like this:

  for baselang in R boo d docbook lua msil nemerle spec vbnet vhdl ; do
    ./convert.py $baselang.lang > $baselang
    mv $baselang $baselang.lang
  done

Maybe we should do this in the spec file for now and submit a patch for the
issue upstream?

Pending:
* Package does not follow the naming guidelines:
  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#MultiplePackages

  In this case, the Guidelines specify that the latest version should be
[basename] and previous versions should be [basename][version].  I sent a
message to fedora-packaging to try and get this restriction removed.
Comment 4 Matthias Clasen 2007-07-02 14:26:52 EDT
> These are a bit strange.  Upstream has these in their Makefile.am as::
>  languages_DATA = $(LANGUAGES) language.rng language2.rng language.dtd \
>                  check.sh convert.py

After consulting upstream, I've removed these.

> * Missing BuildRequire: pcre-devel

I've patched configure now to use the GRegex provided by GLib 2.13.x,
and it builds fine in mock without pcre-devel


> * Only a subset of the *.lang files pass the check.sh script. 

The check script fails for old-format language specs. The library can handle
both formats.


> In this case, the Guidelines specify that the latest version should be
> [basename] and previous versions should be [basename][version]. 

The guidelines are misguided here...

New versions up at 

http://people.redhat.com/mclasen/gtksourceview/gtksourceview2.spec
http://people.redhat.com/mclasen/gtksourceview/gtksourceview2-1.90.1-4.fc8.src.rpm

Comment 5 Toshio Kuratomi 2007-07-03 14:19:35 EDT
Thanks.  The latest version cleans up all the issues raised in the last review.
 Just waiting on accumulating enough votes to relax the naming guidelines.
Comment 6 Matthias Clasen 2007-07-07 10:49:30 EDT
Can I get a yay-or-nay on this one, please ? The gtksourceview situation in
rawhide is pretty broken right now...
Comment 7 Toshio Kuratomi 2007-07-07 14:09:05 EDT
I can give you a yay with a caveat :-(

Sufficient votes were obtained from the Packaging Committee but there wasn't
enough time between that and the FESCo meeting for it to be definitively
ratified.  I sent a message to FESCo with the report on Thursday which
probably[1]_ requires 3 business days for any objections to be mentioned so it's
not official until Tuesday morning.

If you feel comfortable with the fact that you could be asked to rename
gtksourceview/gtksourceview2 if FESCo vetoes the relaxation of the policy, you
can go ahead and request a branch.  If not, please hold off until Tuesday morning.

I'm marking this package APPROVED with that caveat.

.. [1] the policy stating this was discussed in a FESCo meeting but never
written up.  I'm writing up the policy before the next meeting.
Comment 8 Matthias Clasen 2007-07-07 15:47:34 EDT
Gah, soon we'll need to raise package review fees to finance all the overhead
Comment 9 Matthias Clasen 2007-07-10 13:03:15 EDT
New Package CVS Request
=======================
Package Name: gtksourceview2 
Short Description: A library for viewing source files
Owners: rstrode@redhat.com
Branches: 
InitialCC: 
Comment 10 Kevin Fenzi 2007-07-10 18:47:27 EDT
cvs done.
Comment 11 Matthias Clasen 2007-07-10 19:31:49 EDT
Package Change Request
======================
Package Name: gtksourceview2
Updated Fedora Owners: mclasen@redhat.com,rstrode@redhat.com

Gah, please add me to the owners, too, so that I can do the initial import. 
Ray is gone already...

Comment 12 Kevin Fenzi 2007-07-10 19:44:55 EDT
Yeah, I wondered about that... ;) 
cvs done. 
Comment 13 Matthias Clasen 2007-07-10 20:59:35 EDT
Thanks. Built in rawhide.
Comment 14 John (J5) Palmieri 2007-08-16 18:50:08 EDT
Package Change Request
======================
Package Name: gtksourceview2
Branches: OLPC-2
Owners: chris@void.printf.net, johnp@redhat.com
Comment 15 Peter Robinson 2008-10-11 10:11:11 EDT
Closing - in rawhide

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