Bug 239901 - Review Request: corkscrew - Tool for tunneling SSH through HTTP proxies.
Review Request: corkscrew - Tool for tunneling SSH through HTTP proxies.
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-12 02:11 EDT by Debarshi Ray
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-12 11:54:39 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Debarshi Ray 2007-05-12 02:11:53 EDT
Spec URL: http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew.spec

SRPM URL: http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew-2.0-1.fc7.src.rpm


Description:

Corkscrew is a tool for tunneling SSH through HTTP proxies.

It has been tested with the following HTTP proxies :
 * Gauntlet
 * CacheFlow
 * JunkBuster
 * Apache mod_proxy
Comment 1 Jason Tibbitts 2007-05-15 17:59:51 EDT
The package builds fine; rpmlint says:
  W: corkscrew summary-ended-with-dot Tool for tunneling SSH through HTTP proxies.
Should be a trivial fix.

Note that Source0: should be a URL, so that spectool, for example, can download
the source.

You must include the license text (COPYING) and really should include the rest
of the included documentation (AUTHORS, ChangeLog, TODO) as %doc.  INSTALL and
NEWS are generic and content-free so there's no need to include them.

Have you investigated how other distros package this program?  Debian includes a
manpage and a couple of bugfix patches.  When packaging software like this that
hasn't been touched in years, it's always worth looking at other distros to see
how they're handling bugfixes and such.

Finally, are you sponsored?  A quick search didn't turn you up as the owner of
any other packages.  If you don't have a sponsor, you'll need to add
FE-NEEDSPONSOR to the "Bug 239901 blocks" field and point out reviews that
you've done or other packages you've submitted so that the sponsors will have
enough information to decide whether they would like to sponsor you.

Here's a review:
* source files match upstream:
   0d0fcbb41cba4a81c4ab494459472086f377f9edb78a2e2238ed19b58956b0be  
   corkscrew-2.0.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
X license text included in source but not in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint has a valid complaint
* final provides and requires are sane.
O %check is not present; no test suite upstream.  Reviewer has no means to test 
  this software.
* 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 droppings.
* not a GUI app.
Comment 2 Debarshi Ray 2007-05-19 01:51:43 EDT
(In reply to comment #1)
> The package builds fine; rpmlint says:
>   W: corkscrew summary-ended-with-dot Tool for tunneling SSH through HTTP proxies.
> Should be a trivial fix.

Fixed.

> Note that Source0: should be a URL, so that spectool, for example, can download
> the source.

Fixed.

> You must include the license text (COPYING) and really should include the rest
> of the included documentation (AUTHORS, ChangeLog, TODO) as %doc.

Done.

> Have you investigated how other distros package this program?  Debian includes a
> manpage and a couple of bugfix patches.

I have imported the online manual page, and some modifications to README and
corkscrew.c from Debian. I have also put in a patch for a typo in README.


> Finally, are you sponsored? A quick search didn't turn you up as the owner of
> any other packages.  If you don't have a sponsor, you'll need to add
> FE-NEEDSPONSOR to the "Bug 239901 blocks" field and point out reviews that
> you've done or other packages you've submitted so that the sponsors will have
> enough information to decide whether they would like to sponsor you.

No. I am not sponsored and have not yet reviewed any packages for Fedora since I
am very new to packaging myself. My involvement with Fedora till now has been in
the form of bug reports and a successful Summer of Code proposal
(http://fedoraproject.org/wiki/SummerOfCode/2007/DebarshiRay). Apart from Fedora
I contribute to GNU too.

NB: The modified SRPM and SPEC file are available at the links mentioned in the
initial review request.
Comment 3 manuel wolfshant 2007-05-19 05:47:33 EDT
Please increment sa release field (and create a new src.rpm) each time you
modify the spec file. It makes easier for reviewers (and potential sponsors) to
track the evolution of the package. Of course, a change log entry should also be
added with each release bump.
Comment 4 Debarshi Ray 2007-05-19 23:59:37 EDT
(In reply to comment #3)
> Please increment sa release field (and create a new src.rpm) each time you
> modify the spec file.

I thought that was only necessary after the package actually got into the
repositories. I was wrong.

Here is the new and renamed SPEC:
http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew-2.spec

Here is the new and renamed SRPM:
http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew-2.0-2.fc7.src.rpm

Please note that I have renamed the SPEC file just to avoid replacing the older one.
Comment 5 manuel wolfshant 2007-06-06 09:43:59 EDT
Just glanced over the package and there are a couple of issues:
- please use one of the BuildRoot values listed at
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473
- please ditch the [ -d $RPM_BUILD_ROOT ] part in %install, it is not required
any more

The rest of the stuff seems fine. Please correct the above and I will do a
formal review. Note that I will not be allowed to approve the package because
you need a sponsor.
Comment 6 Debarshi Ray 2007-06-06 14:37:20 EDT
(In reply to comment #5)
> - please use one of the BuildRoot values listed at
>
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473

Done.

> - please ditch the [ -d $RPM_BUILD_ROOT ] part in %install, it is not required
> any more

Done.

Here is the new and renamed SPEC file:
http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew-3.spec

Here is the new and renamed SRPM:
http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew-2.0-3.fc7.src.rpm

Please note that I have renamed the SPEC file just to avoid replacing the older one.
Comment 7 Mamoru TASAKA 2007-06-08 12:32:26 EDT
Jason, are you reviewing this? For me the newest spec file
seems almost okay (though I don't know if "export CC=gcc" is needed
and I don't have checked some details yet).
Comment 8 Debarshi Ray 2007-06-08 12:37:59 EDT
(In reply to comment #7)
> I don't know if "export CC=gcc" is needed
> and I don't have checked some details yet).

For what it is worth, I stole that "export CC=gcc" line from the Python
package's SPEC file. :-)

Comment 9 Jason Tibbitts 2007-06-08 13:09:44 EDT
I promise that I'll assign the bug to myself and set fedora-review if I am doing
to do a full review and offer sponsorship.  I had intended to look at this
ticket again after my initial comments but never got around to it.  So many
tickets in the queue, you know.

Certainly if someone else wants to do the review and offer sponsorship, I'll be
quite happy to see that happen.
Comment 10 Mamoru TASAKA 2007-06-08 13:21:27 EDT
Then assigning to me.
Comment 11 Mamoru TASAKA 2007-06-08 13:45:04 EDT
Acutally the line "export CC=gcc" was not needed.

Well, this package is okay, but as this is needsponsor ticket....

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------
Comment 12 Debarshi Ray 2007-06-08 14:08:24 EDT
In case you are interested, I had made a compat-python24 package for Fedora
also. I did not submit it for package review since the main Python maintainer
opposed the idea of such a package. However the Zope package maintainer is
looking to use this for a Zope package for Fedora 7, which would most likely be
 put on a 3rd party repository.

Here is the SPEC file:
http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=compat-python.spec

Here is the SRPM file:
http://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=compat-python-2.4.4-1.fc7.src.rpm

This almost a copy of the Python 2.4 package for FC6. I have just omitted out
some patches which cause build failures on Rawhide, plus a few renames to
prevent collision with the system's Python 2.5 files.
Comment 13 Mamoru TASAKA 2007-06-08 14:20:28 EDT
Well, I want to look at another review request you really
submitted (if any), or want to check your pre-review.
Comment 14 Debarshi Ray 2007-06-10 13:23:28 EDT
I have removed the 'export CC=gcc' part, and here are the new files:

Here is the SPEC file:
https://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew-4.spec

Here is the SRPM file:
https://fedoraproject.org/wiki/DebarshiRay?action=AttachFile&do=get&target=corkscrew-2.0-4.fc7.src.rpm

I have also submitted a review request for GNU Gengetopt here:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=243607
Comment 15 Mamoru TASAKA 2007-06-11 10:27:45 EDT
Well,

* This package is okay.
* The package gengetopt is almost okay (for gengetopt, I have not
  checked in detail).

Okay.
---------------------------------------------------------
  This package (corkscrew) is APPROVED by me
---------------------------------------------------------

Please follow the procedure written on
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account"
When you requested someone to sponsor you (in the procedure
above), please make a note on this bug that you did so.

If you want to push this package also on F-7, you
also have to check:
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
after the URL above.

!! Well, recenctly Fedora package system changed a lot !!
   If you have some questions, please let me know.
Comment 16 Mamoru TASAKA 2007-06-11 13:55:30 EDT
I am now sponsoring
Comment 17 Debarshi Ray 2007-06-11 14:05:09 EDT
New Package CVS Request
=======================
Package Name: corkscrew
Short Description: Tool for tunneling SSH through HTTP proxies.
Owners: debarshi.ray@gmail.com
Branches: FC-6, F-7
InitialCC:
Comment 18 Debarshi Ray 2007-06-11 14:12:30 EDT
I am unable to set the 'fedora-cvs' flag to '?'. It  gives an error message
saying I am unauthorized to set the flag.
Comment 19 Mamoru TASAKA 2007-06-11 14:17:02 EDT
(In reply to comment #18)
> I am unable to set the 'fedora-cvs' flag to '?'. It  gives an error message
> saying I am unauthorized to set the flag.

Perhaps fedora account system is on the way of syncing something.
You must be able to set the flag within a few hours(?), however
for now I set the flag.

Also, as now you are sponsored, FE-NEEDSPONSOR blocker is not needed.
Comment 20 Kevin Fenzi 2007-06-11 19:16:28 EDT
cvs done.
Comment 21 Debarshi Ray 2007-10-04 08:16:56 EDT
Package Change Request
======================
Package Name: corkscrew
New Branches: F-8
Comment 22 Kevin Fenzi 2007-10-05 12:44:31 EDT
cvs done.

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