Bug 484360 - Review Request: gedit-vala - Vala Toys for gEdit
Summary: Review Request: gedit-vala - Vala Toys for gEdit
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Richard W.M. Jones
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 484359
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-02-06 14:09 UTC by Michel Lind
Modified: 2009-03-04 16:20 UTC (History)
3 users (show)

Fixed In Version: 0.3.2-3.fc10.1
Clone Of:
Environment:
Last Closed: 2009-03-04 16:20:18 UTC
Type: ---
Embargoed:
rjones: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
Log file of rpmbuild on Fedora 10 (31.81 KB, text/plain)
2009-02-09 16:28 UTC, Richard W.M. Jones
no flags Details
Patch to gedit-vala.spec (703 bytes, patch)
2009-02-09 18:12 UTC, Richard W.M. Jones
no flags Details | Diff

Description Michel Lind 2009-02-06 14:09:32 UTC
Spec URL: http://salimma.fedorapeople.org/specs/gnome/vtg.spec
SRPM URL: http://salimma.fedorapeople.org/specs/gnome/vtg-0.3.2-1.fc11.src.rpm
Description:
Vala Toys for gEdit is an experimental collection of plugins that
extends the gEdit editor to make it a better programmer editor for the
Vala language.

Vtg tries to make less compromises as possible so, for now, its scope
is narrowed only to support Vala.

It is written in Vala and it is currently composed of just one plugin
with four modules:

    * Bracket completion module
    * Symbol completion module
    * Project Manager - based on the gnome build framework library
    * Project builder / executer

Comment 1 Richard W.M. Jones 2009-02-09 14:33:13 UTC
First some general comments:

I've done some vala packages before and they were all named
"vala-*" or "*-vala".  eg. bug 214227, bug 454668, vala-tools, etc.
I suspect we need Vala-specific packaging guidelines in this area.

> vscsymbolcompletion.c:63:38: error: vala/valastructvaluetype.h:
> No such file or directory

We are missing a dependency somehow.  vala-devel package does
not have this header file.  It helps if you could do a Koji
scratch build of your package, since that will ensure that all
missing BRs have been found.

  koji build --scratch dist-f11 yourpackage.src.rpm

> %setup -q -n vtg-%{version}

-n argument not required here (but will be if the package gets
a different name ...)

> %configure --docdir=%{_datadir}/doc/%{name}-%{version}

Is this right?  I would have written: --docdir=%{_docdir}/%{name}-%{version}

> %doc %{_datadir}/%{name}-%{version}

Comment 2 Michel Lind 2009-02-09 15:20:03 UTC
$ rpm -qf /usr/include/vala-1.0/vala/valastructvaluetype.h 
vala-devel-0.5.6-1.fc11.x86_64

The file is in 0.5.6 which is in updates-testing. Unfortunately,
I can't do a Koji build until gtksourcecompletion lands.

%setup:
I put the -n because I'm not sure about the name either. vala-vtg or gedit-vtg? I'd say gedit-vala, since it seems safe to assume the project will be gedit-specific. Thoughts?

docdir: agreed, thanks.

Let me know which name seems preferable and I'll update the spec.

Comment 3 Richard W.M. Jones 2009-02-09 15:28:58 UTC
(In reply to comment #2)
> $ rpm -qf /usr/include/vala-1.0/vala/valastructvaluetype.h 
> vala-devel-0.5.6-1.fc11.x86_64
> 
> The file is in 0.5.6 which is in updates-testing. Unfortunately,
> I can't do a Koji build until gtksourcecompletion lands.

OK got this update.  Now I get this error:

No translations found for vtg in /home/rjones/rpmbuild/BUILDROOT/vtg-0.3.2-1.fc10.x86_64

Sorry, I'm having to do this on Fedora 10 because my Rawhide machine
has popped its clogs.

> %setup:
> I put the -n because I'm not sure about the name either. vala-vtg or gedit-vtg?
> I'd say gedit-vala, since it seems safe to assume the project will be
> gedit-specific. Thoughts?

Personally I don't care - it's something that might need to
be raised with packaging committee though.  In any case it's
not a blocking issue.

> docdir: agreed, thanks.
> 
> Let me know which name seems preferable and I'll update the spec.

Comment 4 Michel Lind 2009-02-09 15:58:13 UTC
Bizarre:
rpm -ql vtg | grep LC_MESSAGES

/usr/share/locale/es_AR/LC_MESSAGES/vtg.mo
/usr/share/locale/it/LC_MESSAGES/vtg.mo

and the intltool dependency (which should pull gettext and gettext-devel; I just dropped the redundant gettext dependency from my .spec) should ensure that these are buildable.

Could you check the content of vtg-0.3.2/po and try running 'make' there?

We'll find out once GtkSourceCompletion goes in and we can use Koji in any case.

Comment 5 Michel Lind 2009-02-09 15:58:24 UTC
Bizarre:
rpm -ql vtg | grep LC_MESSAGES

/usr/share/locale/es_AR/LC_MESSAGES/vtg.mo
/usr/share/locale/it/LC_MESSAGES/vtg.mo

and the intltool dependency (which should pull gettext and gettext-devel; I just dropped the redundant gettext dependency from my .spec) should ensure that these are buildable.

Could you check the content of vtg-0.3.2/po and try running 'make' there?

We'll find out once GtkSourceCompletion goes in and we can use Koji in any case.

Comment 6 Michel Lind 2009-02-09 16:11:26 UTC
Just noticed a test for LC_MESSAGES in ./configure that might be relevant: it tried compiling a file that includes locale.h, which is in glibc-headers.

This is not listed as something that is explicitly pulled in by Koji in the packaging guidelines, but in my experience it's always installed anyway (it can be uninstalled cleanly if you also remove sectool and sectool-gui).

Could you check if glibc-headers is installed? Thanks.

Comment 7 Richard W.M. Jones 2009-02-09 16:28:14 UTC
Yes, I've got gettext, gettext-devel and glibc-headers.

Running 'make install' explicitly in the po directory does
install the correct files.

Configure indicates that it has found all the right tools.
I will attach the build log anyway in case you can see anything.

Comment 8 Richard W.M. Jones 2009-02-09 16:28:56 UTC
Created attachment 331329 [details]
Log file of rpmbuild on Fedora 10

Comment 9 Michel Lind 2009-02-09 17:48:08 UTC
checking for VTGPLUGIN... no

Ah, of course, I missed a BR on gnome-build-devel.

http://salimma.fedorapeople.org/specs/gnome/gedit-vala.spec
http://salimma.fedorapeople.org/specs/gnome/gedit-vala-0.3.2-2.fc11.src.rpm

Comment 10 Richard W.M. Jones 2009-02-09 18:12:35 UTC
Created attachment 331341 [details]
Patch to gedit-vala.spec

It was missing GConf2-devel.  There are some other
problems too which I've fixed in this patch.

Comment 11 Richard W.M. Jones 2009-02-09 18:13:39 UTC
rpmlint output:

gedit-vala.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 16)

Comment 12 Richard W.M. Jones 2009-02-09 18:17:57 UTC
+ rpmlint output

But better to fix the small problem in comment 11.

+ package name satisfies the packaging naming guidelines

Discussed on fedora-packaging mailing list here:
https://www.redhat.com/archives/fedora-packaging/2009-February/msg00022.html

+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license

----------------------------------
This is as far as I got with the review.  There is some
confusing stuff going on with the documentation in this
package.  It should appear in /usr/share/doc/%{name}-%{version}
but instead appears directly under /usr/share.

? %doc includes license file
? spec file written in American English
? spec file is legible
? upstream sources match sources in the srpm
? package successfully builds on at least one architecture
? ExcludeArch bugs filed
? BuildRequires list all build dependencies
? %find_lang instead of %{_datadir}/locale/*
? binary RPM with shared library files must call ldconfig in %post and %postun
? does not use Prefix: /usr
? package owns all directories it creates
? no duplicate files in %files
? %defattr line
? %clean contains rm -rf $RPM_BUILD_ROOT
? consistent use of macros
? package must contain code or permissible content
? large documentation files should go in -doc subpackage
? files marked %doc should not affect package
? header files should be in -devel
? static libraries should be in -static
? packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
? libfoo.so must go in -devel
? -devel must require the fully versioned base
? packages should not contain libtool .la files
? packages containing GUI apps must include %{name}.desktop file
? packages must not own files or directories owned by other packages
? %install must start with rm -rf %{buildroot} etc.
? filenames must be valid UTF-8

Optional:

? if there is no license file, packager should query upstream
? translations of description and summary for non-English languages, if available
? reviewer should build the package in mock
? the package should build into binary RPMs on all supported architectures
? review should test the package functions as described
? scriptlets should be sane
? pkgconfig files should go in -devel
? shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin

Comment 13 Michel Lind 2009-02-09 19:48:43 UTC
vtg provides vala-gen-project, which generates a skeleton Vala project; the files in /usr/share/vtg/licenses are for the use of the project generator.

That being said, there were a couple of errors I introduced during the name change, and there were some tabs in the file too. Fixed now at the same URL.

Comment 14 Richard W.M. Jones 2009-02-09 20:42:22 UTC
(In reply to comment #13)
> vtg provides vala-gen-project, which generates a skeleton Vala project; the
> files in /usr/share/vtg/licenses are for the use of the project generator.

No that's not what I meant.  I meant this directory:

/usr/share/gedit-vala-0.3.2

which is seemingly [Fedora] documentation, not data.  The root
cause of the problem seems to be this:

%files
[...]
%doc %{_datadir}/%{name}-%{version}

This should all be in %{_docdir}/%{name}-%{version}
(ie. /usr/share/doc/gedit-vala-0.3.2)

Comment 15 Michel Lind 2009-02-09 20:59:37 UTC
Ah, my mistake. I was fixing the %configure flag, but the --docdir switch there is actually ignored, and forgot about the actually-effective directive in %install.

Fixed now. Per Toshio's email to packaging, is there any preference between gedit-vtg and gedit-vala? For searchability, I'd say gedit-vala makes more sense, but gedit-vtg will be closer to the upstream name.

Comment 16 Richard W.M. Jones 2009-02-09 21:14:10 UTC
(In reply to comment #15)
> Ah, my mistake. I was fixing the %configure flag, but the --docdir switch there
> is actually ignored, and forgot about the actually-effective directive in
> %install.

No there still seems to be a problem there.

rpm -qlp shows:
[...]
/usr/share/gedit-vala-0.3.2
/usr/share/gedit-vala-0.3.2/AUTHORS
/usr/share/gedit-vala-0.3.2/COPYING
/usr/share/gedit-vala-0.3.2/COPYING-GPL
/usr/share/gedit-vala-0.3.2/COPYING-LGPL
/usr/share/gedit-vala-0.3.2/ChangeLog
/usr/share/gedit-vala-0.3.2/INSTALL
/usr/share/gedit-vala-0.3.2/NEWS
/usr/share/gedit-vala-0.3.2/README
[...]

Can you bump the release number and post some new URLs
just in case there is some caching problem at this end.

> Fixed now. Per Toshio's email to packaging, is there any preference between
> gedit-vtg and gedit-vala? For searchability, I'd say gedit-vala makes more
> sense, but gedit-vtg will be closer to the upstream name.

I really don't care either way.

Comment 17 Michel Lind 2009-02-09 22:10:52 UTC
Ah, yes. Not a caching problem -- I use scripts to maintain my reviewed packages, and previously had had to manually push new SRPMS. I simply forgot to upload the new SPRM.

Bumped release here:
http://salimma.fedorapeople.org/specs/gnome/gedit-vala-0.3.2-3.fc11.src.rpm

Comment 18 Richard W.M. Jones 2009-02-09 23:00:46 UTC
Continuing the review from comment 12 ...

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
  132006e82ed6cce5b5ed6ee10b2b63ab / 477402

+ package successfully builds on at least one architecture
  x86_64
n/a ExcludeArch bugs filed
+ BuildRequires list all build dependencies
+ %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and %postun
n/a does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a -devel must require the fully versioned base
+ packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
- reviewer should build the package in mock
? the package should build into binary RPMs on all supported architectures
- review should test the package functions as described
+ scriptlets should be sane 
n/a pkgconfig files should go in -devel
n/a shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin


----------------------------------------
This package is APPROVED by rjones
----------------------------------------

Comment 19 Michel Lind 2009-02-10 04:18:20 UTC
New Package CVS Request
=======================
Package Name: gedit-vala
Short Description: Vala Toys for gEdit
Owners: salimma
Branches: F-9 F-10 EL-5
InitialCC:

Comment 20 Kevin Fenzi 2009-02-10 22:32:04 UTC
cvs done.

Comment 21 Fedora Update System 2009-02-24 20:55:41 UTC
gedit-vala-0.3.2-3.fc10.1 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 gedit-vala'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2019

Comment 22 Fedora Update System 2009-03-04 16:20:11 UTC
gedit-vala-0.3.2-3.fc10.1 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.


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