Bug 459540 (mediawiki-imagemap) - Review Request: mediawiki-imagemap
Summary: Review Request: mediawiki-imagemap
Keywords:
Status: CLOSED NEXTRELEASE
Alias: mediawiki-imagemap
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-08-19 21:05 UTC by Ismael Olea
Modified: 2008-10-24 12:00 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-10-13 07:43:53 UTC
Type: ---
Embargoed:
lemenkov: fedora-review+
huzaifas: fedora-cvs+


Attachments (Terms of Use)

Description Ismael Olea 2008-08-19 21:05:13 UTC
Proposal of a MediaWiki extension:

Spec URL: http://olea.org/tmp/mediawiki-imagemap.spec

SRPM URL: http://olea.org/paquetes-rpm/xhtml2fo-style-xsl-20050106-1.src.rpm

SRPM URL: http://olea.org/paquetes-rpm/mediawiki-imagemap-rev39658-1.src.rpm
RPM URL: http://olea.org/paquetes-rpm/mediawiki-imagemap-rev39658-1.noarch.rpm

Description:

This extension lets to use image maps inside Mediawiki.

Comment 1 Jason Tibbitts 2008-08-19 21:41:55 UTC
What is the xhtml2fo-style-xsl srpm for?

Comment 2 Ismael Olea 2008-08-19 21:52:40 UTC
Sorry!! I've cut and pasted badly!!

Spec URL: http://olea.org/tmp/mediawiki-imagemap.spec

SRPM URL: http://olea.org/paquetes-rpm/mediawiki-imagemap-rev39658-1.src.rpm
RPM URL: http://olea.org/paquetes-rpm/mediawiki-imagemap-rev39658-1.noarch.rpm


The other package got its own bug report (https://bugzilla.redhat.com/show_bug.cgi?id=428793). It's not related with MediaWiki at all.

Comment 3 Peter Lemenkov 2008-08-23 08:18:10 UTC
I'll review it

Comment 4 Peter Lemenkov 2008-08-23 11:57:14 UTC
Notes:

* You should describe exact steps how to obtain source code and create tarball. E.g. something like

# cvs -d:pserver:anonymous.berlios.de:/cvsroot/sippy export -D 2008-06-27  sippy
# tar -cjvf sippy-20080627.tar.bz2 sippy
Source0:        sippy-20080627.tar.bz2

* Versioning scheme is ugly. If (then) project will release officialtraball, say ver. 1.0.0, then you will be forced to add %epoch to your spec, because 1.0.0 is earlier than rev[0-9].* . I suggest you to consider using the following versioning scheme

Version: 0
Release: 0.1.svn39658%{?dist}

Note leading zero. E.g. Release == 0.RPMREV.svnSVNREV%{?dist}
With this naming scheme you won't have any troubles then upstream will release official tarball.

Other things looks sane.

Comment 5 Ismael Olea 2008-08-24 11:35:26 UTC
I've follow your recomendations. I remade the package using the supposed last stable version for MediaWiki 1.13.

Updated info

http://olea.org/paquetes-rpm/mediawiki-imagemap-MW1.13-0.1.r37906.olea.src.rpm
http://olea.org/paquetes-rpm/mediawiki-imagemap-MW1.13-0.1.r37906.olea.noarch.rpm
Spec URL: http://olea.org/tmp/mediawiki-imagemap.spec

Comment 6 Peter Lemenkov 2008-08-24 12:08:41 UTC
> I remade the package using the supposed last stable version for MediaWiki 1.13.

OK. BTW Axel Thimm right now rebuilding MediaWiki 1.13 for F-8 and F-9.

REVIEW:

MUST Items:

- rpmlint warns aboun incoherent version in changelog. Easy to fix - just add notes about latest changes under actual version (MW1.13-0.1.r37906).
+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines .
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source:

[petro@Sulaco SPECS]$ md5sum ../SOURCES/ImageMap-MW1.13-r37906.tar.gz*
39e7f7fceb125cf6a707d4c057b15f57  ../SOURCES/ImageMap-MW1.13-r37906.tar.gz
39e7f7fceb125cf6a707d4c057b15f57  ../SOURCES/ImageMap-MW1.13-r37906.tar.gz.orig
[petro@Sulaco SPECS]$

+ The package successfully compiles and builds into binary rpms on at least one supported architecture (ppc).
+ All build dependencies are listed in BuildRequires.
+ A package owns all directories that it creates.
+ A package does not contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot}
+ The package consistently uses macros, as described in the macros section of Packaging Guidelines .
+ The package contains code, or permissable content.
+ Everything, the package includes as %doc, does not affect the runtime of the application.
+ Package does not own files or directories already owned by other packages. 
+ At the beginning of %install, the package runs rm -rf %{buildroot}
+ All filenames in rpm packages are valid UTF-8.

SHOULD Items:

- SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.


So, finally

* please add entry to %changelog to make rpmlint happy
* consider shortening %description - no need to add notes, describing how to download latest version (we should provide such support)
* find a minute to drop email to ustream about LICENSE inclusion

and this package is


APPROVED.

Comment 7 Ismael Olea 2008-08-24 17:46:01 UTC
(In reply to comment #6)

> MUST Items:
> 
> - rpmlint warns aboun incoherent version in changelog. Easy to fix - just add
> notes about latest changes under actual version (MW1.13-0.1.r37906).

Strange: I'm rpmlinting my packages and get zero comments :-m
I'm using this:

[olea@lisergia SPECS]$ rpm -q rpmlint
rpmlint-0.84-2.fc9.noarch
[olea@lisergia SPECS]$ rpmlint -ivv mediawiki-imagemap.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.


> SHOULD Items:
> 
> - SHOULD: If the source package does not include license text(s) as a separate
> file from upstream, the packager SHOULD query upstream to include it.

aha.
 
> So, finally
> 
> * please add entry to %changelog to make rpmlint happy

done.

> * consider shortening %description - no need to add notes, describing how to
> download latest version (we should provide such support)

Well, I'm confused here since you suggested to explain the way to get the sources (see https://bugzilla.redhat.com/show_bug.cgi?id=459540#c4).

You mean to write it in comments inside the spec file or to remove it since the sources are created through an upstream service?

> * find a minute to drop email to ustream about LICENSE inclusion

done


Thanks :-)

Comment 8 Peter Lemenkov 2008-08-24 17:56:20 UTC
(In reply to comment #7)

> > * consider shortening %description - no need to add notes, describing how to
> > download latest version (we should provide such support)
> 
> Well, I'm confused here since you suggested to explain the way to get the
> sources (see https://bugzilla.redhat.com/show_bug.cgi?id=459540#c4).
>
> You mean to write it in comments inside the spec file or to remove it since the
> sources are created through an upstream service?

I thought that you use custom made svn snapshot. In such cases instructions how to build such snapshot should be supplied in spec-file. See this one, as a reference:

http://cvs.fedoraproject.org/viewvc/rpms/flashrom/devel/flashrom.spec?view=markup


> Thanks :-)

And one lastminute fix - please change versioning scheme again! I don't think that using MW1.13 as "Version" is a good idea. As its just a svn-snapshot and no official release was made so far, you should use 0, e.g.

Version: 0
Release: 0.1.%{revision}%{?dist}

Comment 9 Ismael Olea 2008-08-25 06:15:36 UTC
(In reply to comment #8)

> I thought that you use custom made svn snapshot. 

It was at the first version, before discovering the WikiMedia tgz service :-)
  
> And one lastminute fix - please change versioning scheme again! I don't think
> that using MW1.13 as "Version" is a good idea. As its just a svn-snapshot and
> no official release was made so far, you should use 0, e.g.
> 
> Version: 0
> Release: 0.1.%{revision}%{?dist}

done: http://olea.org/tmp/mediawiki-imagemap.spec

Comment 10 Peter Lemenkov 2008-08-25 06:48:34 UTC
OK. All issues now resolved.
Don't forget to raise fedora-cvs flag to "?" :)

Comment 11 Ismael Olea 2008-09-17 20:34:38 UTC
New Package CVS Request
=======================
Package Name:       mediawiki-imagemap
Short Description:  The ImageMap extension for MediaWiki
Owners:             olea
Branches:           F-8 F-9
InitialCC:          mtasaka

Comment 12 Huzaifa S. Sidhpurwala 2008-09-18 04:05:29 UTC
cvs done

Comment 13 Peter Lemenkov 2008-09-27 17:53:32 UTC
Ismael, any news since mid September? Are you already sponsored by someone or not?

Comment 14 Mamoru TASAKA 2008-09-27 17:58:52 UTC
(In reply to comment #13)
> Ismael, any news since mid September? Are you already sponsored by someone or
> not?

I am sponsoring him.

Comment 15 Ismael Olea 2008-10-06 01:36:26 UTC
cvs updated
packages build: https://koji.fedoraproject.org/koji/builds?userID=724

Now I'm supposed to push them through bodhi?

Comment 16 Peter Lemenkov 2008-10-06 04:24:33 UTC
Exactly.

https://admin.fedoraproject.org/updates

Comment 17 Ismael Olea 2008-10-12 20:10:14 UTC
Done for mediawiki-imagemap-0-0.1.r37906.fc8 and mediawiki-imagemap-0-0.1.r37906.fc9

When pushing mediawiki-imagemap-0-0.1.r37906.fc10 I get an error:


-------------------------
500 Internal error

The server encountered an unexpected condition which prevented it from fulfilling the request.

Powered by CherryPy 2.3.0 
-------------------------

:-??????

Comment 18 Ismael Olea 2008-10-13 07:43:53 UTC
I think is fine to close now with the «NEXTRELEASE» tag.

Comment 19 Mamoru TASAKA 2008-10-24 12:00:35 UTC
Well, about today's F-8 broken dependency for mediawiki-imagemap
I tried to mail to Ismael but it is rejected (mail returned).
Would you know why?
So for now I write here about how to fix it. 

Hello, Ismael:

As you know today Michael Schwendt reported that the dependency
of mediawiki-imagemap is broken on F-8 ppc64 [1] as:
-------------------------------------------------
mediawiki-imagemap-0-0.1.r37906.fc8.noarch  requires  mediawiki >= 0:1.13
-------------------------------------------------

This is because on F-8 mediawiki does not support ppc64 at all [2]
(while on F-9+ mediawiki also supports ppc64). So
your last change to make dependency on mediawiki unversioned like:
-------------------------------------------------
-Requires: mediawiki >= 1.13
+Requires: mediawiki
-------------------------------------------------
does not work.
In this case you should use "ExcludeArch" like
-------------------------------------------------
BuildArch: noarch
%if 0%{?fedora} == 8
ExcludeArch: ppc64
%endif
Requires: mediawiki >= 1.13
-------------------------------------------------
This may seem strange because mediawiki-imagemap itself is noarch, however
Fedora package server treats this correctly, so this method is valid [3]

Also, please make it sure that F-8 package has EVR (Epoch-Version-Release)
not larger than F-9, F-10. Currently F-9 mediawiki-imagemap has
$ koji latest-pkg dist-f9-updates mediawiki-imagemap
Build                                     Tag                   Built by
----------------------------------------  --------------------  ----------------
mediawiki-imagemap-0-0.1.r37906.fc9       dist-f9-updates       olea
so F-8 mediawiki-imagemap must not have "0-0.2.xxxxx" as EVR
(0-0.2.rXXXX.fc8 > 0-0.1.rXXXX.fc9). In this case you can
use "0.1.r37906%{?dist}.1" for release number (see [4] ).

By the way as F-8 support is near end, while it is preferable that
you fix the spec file and build a new srpm for dist-f8-updates-candidate,
I don't think that pushing this (submitting push request on bodhi) for
F-8 branch only to fix dependency problem for ppc64 is needed.
(see: [5] )

[1] https://www.redhat.com/archives/fedora-devel-list/2008-October/msg02277.html
[2] http://koji.fedoraproject.org/koji/buildinfo?buildID=65333
[3] http://www.redhat.com/archives/rhl-devel-list/2007-October/msg00265.html
[4] https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Minor_release_bumps_for_old_branches
[5] https://www.redhat.com/archives/fedora-devel-list/2008-October/msg02280.html


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