Bug 428725 - Review Request: wiggle - help apply patches when 'patch' can't
Summary: Review Request: wiggle - help apply patches when 'patch' can't
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-01-14 20:39 UTC by Andy Gospodarek
Modified: 2014-06-29 22:59 UTC (History)
4 users (show)

Fixed In Version: rawhide
Clone Of:
Environment:
Last Closed: 2008-02-22 19:06:40 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Andy Gospodarek 2008-01-14 20:39:41 UTC
Spec URL: http://people.redhat.com/agospoda/wiggle/wiggle.spec
SRPM URL: http://people.redhat.com/agospoda/wiggle/wiggle-0.6-1.fc7.src.rpm
Description:
Wiggle is a program for applying patches that 'patch' cannot
apply due to conflicting changes in the original.

Wiggle will always apply all changes in the patch to the original.
If it cannot find a way to cleanly apply a patch, it inserts it
in the original in a manner similar to 'merge', and report an
unresolvable conflict.

This is the first package I've submitted for inclusion in Fedora.

Additional i686 versions for rhel4 and rhel5 are available here:
http://people.redhat.com/agospoda/#wiggle

Comment 1 Andy Gospodarek 2008-01-14 21:31:42 UTC
I got the licensing wrong, so I just updated wiggle.spec with this change:

--- wiggle.spec.orig    2008-01-14 16:18:54.000000000 -0500
+++ wiggle.spec 2008-01-14 16:19:00.000000000 -0500
@@ -4,7 +4,7 @@ Release:        1%{?dist}
 Summary:        A tool for applying patches with conflicts

 Group:          Development/Tools
-License:        GPL
+License:        GPLv2+
 URL:            http://www.cse.unsw.edu.au/~neilb/source/wiggle/
 Source0:       
http://www.cse.unsw.edu.au/~neilb/source/wiggle/%{name}-%{version}.tar.gz
 Patch0:         wiggle-various-changes.patch


Comment 2 manuel wolfshant 2008-01-14 22:24:36 UTC
Andy, are you already sponsored? If not, you have to follow
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored and also make
this review block bug 177841 ( FE-NEEDSPONSOR )

Comment 3 Andy Gospodarek 2008-01-14 23:52:58 UTC
Thanks, Manuel.  I forgot to add the blocker.

Comment 4 Jason Tibbitts 2008-01-15 00:45:38 UTC
A few comments:

The URL given in the spec is 403 (no permission) for me.

The package fails to build:

nroff -man wiggle.1 > wiggle.man
/bin/sh: nroff: command not found
make: *** [wiggle.man] Error 127
make: *** Waiting for unfinished jobs....
error: Bad exit status from /var/tmp/rpm-tmp.44906 (%build)

Looks like you need BuildRequires: groff

The package seems to have a test suite; it should be called in a %check section.
 Unfortunately it looks like the makefile calls it as part of the build process,
but you could just call the wiggle and wiggle.man targets manually.

The package doesn't build with the proper compiler flags, because the makefile
tkes the ill-advised step of overriding CFLAGS.  That will need to be patched
out or worked around so the compiler is called with $RPM_OPT_FLAGS.

Comment 5 Andy Gospodarek 2008-01-15 03:58:11 UTC
Thanks for the tips, Jason.  It's interesting that the link for the specfile
didn't work.  Here's a new one and a new srpm:

Spec URL: http://people.redhat.com/agospoda/wiggle/wiggle.spec
SRPM URL: http://people.redhat.com/agospoda/wiggle/wiggle-0.6-2.fc7.src.rpm

I made the suggested changes to the Makefile and included groff as a build
requirement in the specfile.

You should always be able to find the latest versions from my people page (at
least until wiggle makes it into rawhide):

http://people.redhat.com/agospoda/#wiggle


Comment 6 Jason Tibbitts 2008-01-15 18:07:09 UTC
OK, that gets further but also fails to build due to a missing dependency on the
"time" package.  Adding that gets things going OK.

I still didn't see the proper flags being passed to the compiler, but changing
the make line to:
  make CFLAGS="$RPM_OPT_FLAGS" %{?_smp_mflags}
gets that done and fixes the empty debuginfo package as well.

You might change "report" to "reports" in %description.

There's a COPYING file in the tarball; you need to include that as %doc in your
%files list.  You should also include ANNOUNCE and TODO; there's no point in
including INSTALL ad I'm not sure about DOC/diff.ps.

So still a few things to fix before I can approve this package.

Checklist:
* source files match upstream:
   639f8bd48c58b1fa4f24a65bc8aa3e53219e7d48726e63e7c40f0701d1d89b9c  
   wiggle-0.6.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK (one minor typo; no big deal)
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
X license text is in the tarball but not included in the package.
* latest version is being packaged.
X BuildRequires are proper (still needs 'time' to build; assuming it's there so 
   that I can complete this review)
X compiler flags are not proper.
* %clean is present.
* package builds in mock (rawhide, x86_64) (after adding BR: time)
* package installs properly
X debuginfo package is incomplete.
* rpmlint is silent.
* final provides and requires are sane:
   wiggle = 0.6-2.fc9
  =
   (nothing)
* %check is present and all tests pass (after adding BR: time):
   50 succeeded and 0 failed
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Comment 7 Andy Gospodarek 2008-01-17 02:58:16 UTC
OK, I think I took care of everything.  I don't think diff.ps is really needed
in the RPM -- it's an interesting paper, but doesn't add too much to the tool's
usage.  I made all the other suggested changes though and uploaded new packages
to the same spot:

http://people.redhat.com/agospoda/wiggle/wiggle.spec
http://people.redhat.com/agospoda/wiggle/wiggle-0.6-3.fc7.src.rpm
http://people.redhat.com/agospoda/wiggle/wiggle-0.6-3.fc7.i386.rpm
http://people.redhat.com/agospoda/wiggle/wiggle-debuginfo-0.6-3.fc7.i386.rpm



Comment 8 Jason Tibbitts 2008-01-17 05:52:23 UTC
Cool; builds fine and rpmlint is clean.  Everything looks good to me.

APPROVED

I'll sponsor you; just let me know if you need assistance with the remaining
bits of procedure.

Comment 9 Andy Gospodarek 2008-01-17 15:07:37 UTC
New Package CVS Request
=======================
Package Name: wiggle
Short Description: help apply patches when 'patch' cannot
Owners: gospo
Branches: F-9 EL-5
InitialCC: linville
Cvsextras Commits: no

Comment 10 Kevin Fenzi 2008-01-17 17:14:45 UTC
cvs done. 

Any particular reason for cvsextras commits: no? 
This will mean that a limited number of people could assist with your package or
fix things if you are unavailable. 

Comment 11 Andy Gospodarek 2008-01-21 15:00:03 UTC
(In reply to comment #10)
> cvs done. 
> 
> Any particular reason for cvsextras commits: no? 
> This will mean that a limited number of people could assist with your package or
> fix things if you are unavailable. 

not really, I'd be fine with changing that to 'yes.'

Comment 12 Kevin Fenzi 2008-01-21 16:26:46 UTC
Done... and thanks. 

Comment 13 Andy Gospodarek 2008-01-21 18:35:02 UTC
So I've been scanning the wiki and mailing-list archives and trying to figure
this out (and apparently missed the memo on this), but now that I've got and
EPEL-5 branch what does it take to actually make it out to the base
repo/mirrors/etc?  I read in the meeting notes that the next testing->stable
push will be in the next 2 weeks, but I'm not even sure how to query the status
of wiggle.

Comment 14 Xavier Lamien 2008-01-21 18:54:19 UTC
for epel, you can check from here :
http://buildsys.fedoraproject.org/build-status/index.psp


Comment 15 Jason Tibbitts 2008-02-21 21:34:54 UTC
Any reason why this shouldn't be closed now?

Comment 16 Andy Gospodarek 2008-02-22 19:06:40 UTC
Nope, no reason.  Thanks for reminding me.


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