Bug 459540 - (mediawiki-imagemap) Review Request: mediawiki-imagemap
Review Request: mediawiki-imagemap
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-19 17:05 EDT by Ismael Olea
Modified: 2008-10-24 08:00 EDT (History)
5 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Ismael Olea 2008-08-19 17:05:13 EDT
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 17:41:55 EDT
What is the xhtml2fo-style-xsl srpm for?
Comment 2 Ismael Olea 2008-08-19 17:52:40 EDT
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 04:18:10 EDT
I'll review it
Comment 4 Peter Lemenkov 2008-08-23 07:57:14 EDT
Notes:

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

# cvs -d:pserver:anonymous@cvs.sippy.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 07:35:26 EDT
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 08:08:41 EDT
> 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 13:46:01 EDT
(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 13:56:20 EDT
(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 02:15:36 EDT
(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 02:48:34 EDT
OK. All issues now resolved.
Don't forget to raise fedora-cvs flag to "?" :)
Comment 11 Ismael Olea 2008-09-17 16:34:38 EDT
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 00:05:29 EDT
cvs done
Comment 13 Peter Lemenkov 2008-09-27 13:53:32 EDT
Ismael, any news since mid September? Are you already sponsored by someone or not?
Comment 14 Mamoru TASAKA 2008-09-27 13:58:52 EDT
(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-05 21:36:26 EDT
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 00:24:33 EDT
Exactly.

https://admin.fedoraproject.org/updates
Comment 17 Ismael Olea 2008-10-12 16:10:14 EDT
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 03:43:53 EDT
I think is fine to close now with the «NEXTRELEASE» tag.
Comment 19 Mamoru TASAKA 2008-10-24 08:00:35 EDT
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.