Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 470547 (emacs-semi) - Review Request: emacs-semi - MIME rendering library for Emacs
Summary: Review Request: emacs-semi - MIME rendering library for Emacs
Keywords:
Status: CLOSED NOTABUG
Alias: emacs-semi
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jonathan Underwood
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW emacs-wl
TreeView+ depends on / blocked
 
Reported: 2008-11-07 16:40 UTC by Vitaly Mayatskikh
Modified: 2013-03-15 09:04 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-03-15 09:04:56 UTC


Attachments (Terms of Use)

Description Vitaly Mayatskikh 2008-11-07 16:40:00 UTC
Spec URL: http://people.redhat.com/vmayatsk/wl/semi.spec
SRPM URL: http://people.redhat.com/vmayatsk/wl/semi-1.14.6-1.fc10.src.rpm
RPM URL: http://people.redhat.com/vmayatsk/wl/semi-1.14.6-1.fc10.noarch.rpm
Description:

SEMI is a library to provide MIME feature for GNU Emacs.  MIME is a
proposed internet standard for including content and headers other
than (ASCII) plain text in messages.

Wanderlust mail client requires this library to render messages.

SEMI + Wanderlust are my first packages for Fedora.

Comment 1 Jason Tibbitts 2008-11-07 19:24:38 UTC
Is there a dependency between this and Wanderlust?  If so, one of these tickets should block the other.

Comment 2 Vitaly Mayatskikh 2008-11-07 19:38:29 UTC
Yes, Wanderlust requires SEMI.

Comment 3 Alec Leamas 2008-11-16 08:15:50 UTC
Hi!

Unfortunately, I'm not a reviewer... But according to the instructions, I need to show some interest in reviewing other requests in order to get  a sponsor. So I'll do that. Please feel free to do the same for me, my request is bug 471575 :-)

For me, rpmlint gives the following

semi.src: E: no-buildroot-tag
semi.src: W: mixed-use-of-spaces-and-tabs (spaces: line 15, tab: line 2)
semi.src: W: non-standard-group Unspecified
semi.src: W: invalid-license GPL

My mock build bails out, complaining about the missing Group: field.

I think all of these issues should be closed. 

Copyright & license. Most (all?) files have a nice GPLv2 copyright notice. However, the I really miss the top-level file COPYING - the
notices refer to this. I think it should be part of the package.

See more below


> #%define _default_patch_fuzz 2
> %define	_semiver	1.14.6
> %define	_flimver	1.14.8
> %define	_emacsver	22.2
> 
> %define	_lispdir	%{_datadir}/emacs/site-lisp
> 
> Summary: Library to provide MIME feature for GNU Emacs
> Name: semi
> Version: %{_semiver}
> Release: 1%{?dist}
> License: GPL
> #Group: Applications/Internet

As lint says, there need to be a valid group  and license tag. As for license, see http://fedoraproject.org/wiki/Licensing - I think it boils down to GPLv2. For Group:, take a look at http://koti.welho.com/vskytta/packagers-handbook/packagers-handbook.html#guidelines-group-tag

> URL: ftp://ftp.m17n.org/pub/mule/semi/semi-1.14-for-flim-1.14
> Source0:        ftp://ftp.m17n.org/pub/mule/semi/semi-1.14-for-flim-1.14/semi-%{version}.tar.bz2

Unfortunately, these are password protected.

> BuildRequires: emacs >= %{_emacsver}, flim >= %{_flimver}
> BuildArch: noarch
> Requires: emacs >= %{_emacsver}, flim >= %{_flimver}
> 
> Patch1: semi-001-use-w3m-instead-of-w3.patch
> 
> %description
> SEMI is a library to provide MIME feature for GNU Emacs.  MIME is a
> proposed internet standard for including content and headers other than
> (ASCII) plain text in messages

[nit-picking] This was written some time ago... Isn't it fair these days to say that MIME is the way to handle content on Internet?

> 
> %prep
> 
> %setup -q -n semi-%{version}
> %patch1 -p1
> 
> # necessary to generate the auto-autoloads.el file:
> touch *.el
> 
> %build
> 
> %install
> 
> rm -rf %buildroot
> 
> %{__mkdir_p} %buildroot%{_lispdir}/semi
> 
> cd $RPM_BUILD_DIR/semi-%{version}
> 
> make LISPDIR=%buildroot%{_lispdir}
> make LISPDIR=%buildroot%{_lispdir} install
> 
> make clean

Why make clean here? If all goes well, %clean will take care of it. If not, I think we want everything. Or am I missing something?

> 
> %clean
> rm -rf %buildroot
> 
> %files
> %defattr(-,root,root)
> %doc NEWS README* ChangeLog SEMI* TODO VERSION
> %{_lispdir}/semi
> 
> %changelog
> 
> * Fri Nov  7 2008 Vitaly Mayatskikh <vmayatsk@redhat.com> [1.14.6-1]
> - first build
> 

Cheers!

--alec

Comment 4 Alec Leamas 2008-11-16 08:35:58 UTC
Reading the Review Guidelines once more, I realize that my proposal just to add the COPYING file  wasn't that good. What needs to be done is to try to get upstream to do this. But I don't know if it's feasible, and anyway I think you need advice by someone more experienced than me about this. I have already been wrong once :-)

Comment 5 Alec Leamas 2008-11-17 05:37:37 UTC
#%define _default_patch_fuzz 2
> %define	_semiver	1.14.6
> %define	_flimver	1.14.8
> %define	_emacsver	22.2
> 
> %define	_lispdir	%{_datadir}/emacs/site-lisp

You should not use _* as a macro name - these are by convention reserved for "system" macros. Use semiver, flimver, emacsver and lispdir instead. Sorry I  missed that.

Comment 6 Vitaly Mayatskikh 2008-11-21 13:44:48 UTC
Thanks for your comments, Alec!

Comment 7 Jason Tibbitts 2009-07-14 18:32:38 UTC
Was an updated package ever released which addressed those comments?

Comment 8 Vitaly Mayatskikh 2009-07-14 19:13:39 UTC
Yes, half a year ago.

I've added one more patch to semi and updated packages.

Comment 9 Jason Tibbitts 2009-07-14 19:27:09 UTC
Where are the updated packages?  The spec URL is valid, but none of the package links exist.

Anyway, have you seen the emacs packaging guidelines?  From a quick inspection of the spec file, this package doesn't seem to follow them.  http://fedoraproject.org/wiki/Packaging:Emacs

Comment 11 Jens Petersen 2009-10-22 05:21:44 UTC
The spec file and package name should be the same.

I just note for the record that both semi and wl were
formerly in Fedora Core.

Comment 13 Jonathan Underwood 2010-04-06 00:22:26 UTC
Rebuild of packages in Comment #12 inside mock succeeds. rpmlint output on resulting rpms:

emacs-semi.noarch: W: incoherent-version-in-changelog [1.14.6-1] ['1.14.6-1.fc14', '1.14.6-1']

--> Needs fixing

emacs-semi.noarch: W: file-not-utf8 /usr/share/doc/emacs-semi-1.14.6/ChangeLog
emacs-semi.noarch: W: file-not-utf8 /usr/share/doc/emacs-semi-1.14.6/README.en
emacs-semi.noarch: W: file-not-utf8 /usr/share/doc/emacs-semi-1.14.6/VERSION

---> These need fixing using iconv in %prep

emacs-semi.noarch: W: empty-%post
emacs-semi.noarch: W: empty-%preun

---> Remove these sections


emacs-semi-el.noarch: W: spelling-error Summary(en_US) Elisp -> Lisp, Elise, Elisa
emacs-semi-el.noarch: W: spelling-error %description -l en_US elisp -> lisp, e lisp, Elise

---> False positives, safe to ignore


emacs-semi-el.noarch: W: no-documentation

---> False positive, ignore

3 packages and 0 specfiles checked; 0 errors, 9 warnings.

Comment 14 Jonathan Underwood 2010-04-06 00:29:14 UTC
Also, the spec file needs updating to comply with the latest Emacs add-on packaging guidelines. http://fedoraproject.org/wiki/Packaging:Emacs

Specifically, these changes need to be made to the spec file. 

1/ the pkgconfig stuff can be removed

2/the emacs specific macros need to be changed accordingly eg %{emacs_version} should now be %{_emacs_version} etc

3/ No need to buildrequire emacs-el

4/ Comments need adding to the spec file about the patches - have these been sent upstream? If so, supply a date, an email archive url or a bugzilla url.

5/ BuildRoot is no longer needed - remove.

6/ In install, remove the rm -rf $RPM_BUILD_ROOT

7/ Fix up the changelog entry to properly comply with the guidelines

Once these are done I'll finish the review.

Comment 16 Jason Tibbitts 2012-06-29 17:09:12 UTC
Can this just be closed out now?  It's been 26 months with no response.

Comment 17 Miroslav Suchý 2013-03-15 09:04:56 UTC
Stalled Review. Closing per #15, #16 and:
https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
If you ever want to continue with this review, please reopen or
submit new review.


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