Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
Summary: Review Request: connect-proxy - SSH Proxy command helper Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: bugzilla-sink@leemhuis.info ReportedBy: jwilson@redhat.com QAContact: fedora-package-review@redhat.com
Spec URL: http://wilsonet.com/packages/connect-proxy/connect-proxy.spec SRPM URL: http://wilsonet.com/packages/connect-proxy/connect-proxy-1.95-1.fc6.src.rpm
Description: -- connect-proxy is the simple relaying command to make network connection via SOCKS and https proxy. It is mainly intended to be used as proxy command of OpenSSH. You can make SSH session beyond the firewall with this command.
Features of connect-proxy are:
* Supports SOCKS (version 4/4a/5) and https CONNECT method. * Supports NO-AUTH and USERPASS authentication of SOCKS * Partially supports telnet proxy (experimental). * You can input password from tty, ssh-askpass or environment variable. * Simple and general program independent from OpenSSH. * You can also relay local socket stream instead of standard I/O. --
Note that the upstream name is simply "connect", but this has been packaged for Red Hat/Fedora (by me) and Debian (by someone else before me) already as connect-proxy, so maintaining this name is in line with the package naming guidelines.
I've pinged upstream to see about having a license file included, as well as the man page and Makefile I snarfed from the Debian package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
pertusus@free.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|bugzilla-sink@leemhuis.info |pertusus@free.fr OtherBugsDependingO|163776 |163778 nThis| |
------- Additional Comments From pertusus@free.fr 2006-07-05 17:03 EST ------- Needswork:
* rpm compilation flags are not used * wrong buildroot
Notes: * %{?_smp_mflags} is not usefull since there is nothing to parallelize * maybe it would be a good idea to have the html page packaged since it holds some documentation * since the source isn't versionned, all the source files will have the same name. A trick is to use the timestamp in the format YYYYMMDD obtained for example by using wget -N. Something along
Source0: connect.c-20060402 # real source is below, renamed to connect.c-YYYYMMDD based on latest # timestamp, obtained with wget -N #Source0: http://www.taiyo.co.jp/~gotoh/ssh/connect.c
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
------- Additional Comments From pertusus@free.fr 2006-07-05 17:09 EST ------- Alternatively you could also rename the file by appending the version number since there is a version number, maybe it would be cleaner in that case:
Source0: connect.c-1.95 # Real source is #Source0: http://www.taiyo.co.jp/~gotoh/ssh/connect.c
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
------- Additional Comments From jwilson@redhat.com 2006-07-05 17:24 EST ------- Blah, doing too many things at once. BuildRoot corrected, make now uses CFLAGS. Updated spec and srpm pushed.
I know the smp flags are useless in this case, but the guidelines suggest this is a required field unless there is something blocking its use. It doesn't help, but it also doesn't hurt, so nothing really blocks it from being in the spec. Who knows, maybe some day the author refactors the code and they become useful. :)
As for Source0... If I'm going to do anything to munge the name of the source, I just assume put it into a tarball and clean up the messy %setup line (and remove the line after it). Thoughts on that route versus just renaming the .c file?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
------- Additional Comments From pertusus@free.fr 2006-07-05 17:35 EST -------
I know the smp flags are useless in this case, but the guidelines suggest this is a required field unless there is something blocking its use. It doesn't help, but it also doesn't hurt, so nothing really blocks it from being in the spec. Who knows, maybe some day the author refactors the code and they become useful. :)
Ok, no problem here.
As for Source0... If I'm going to do anything to munge the name of the source, I just assume put it into a tarball and clean up the messy %setup line (and remove the line after it). Thoughts on that route versus just renaming the .c file?
I think it is better to only rename the file since it is easier to verify that it is the right file. However if you prefer doing a tarball, do it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
------- Additional Comments From jwilson@redhat.com 2006-07-05 17:46 EST ------- (In reply to comment #4)
I think it is better to only rename the file since it is easier to verify that it is the right file. However if you prefer doing a tarball, do it.
Nah, I'll go with renaming the source file, with a slight twist:
Source0: connect-%{version}.c
Build -3 pushed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
------- Additional Comments From pertusus@free.fr 2006-07-05 18:14 EST ------- The %setup may be simplified as:
%setup -q -T -c cp %{SOURCE0} connect.c %patch0 -p1
One rpmlint warning, not a big deal in my opinion (though I prefer spaces, since it looks the same in every editors):
W: connect-proxy mixed-use-of-spaces-and-tabs The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both.
There are many warning durings compilation, some may be worrying.
These are not blockers.
Full review:
* follow approximately naming guidelines, with a valid reason to deviate. * licence is GPL, not included since it is not included upstream * match upstream source b856937f1cdfca7a3ccfb2fac36ef726 ../SOURCES/connect-1.95.c * build on devel i386 * don't crash at start * remove $RPM_BUILD_ROOT in clean and install * right buildroot * %files right
APPROVED
I still think that having the html file located at the package URL would be a good idea. Not a blocker.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
pertusus@free.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|163778 |163779 nThis| |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
------- Additional Comments From jwilson@redhat.com 2006-07-05 23:14 EST ------- Made the %setup changes, those look a lot nicer, thank you. :)
Missed the spaces vs. tabs warning, didn't have the development rpmlint installed (just installed it), that's fixed too. Also added the html page as a %doc. I'll import a -4 build momentarily...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197716
jwilson@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
------- Additional Comments From jwilson@redhat.com 2006-07-10 11:21 EST ------- Packages all built and pushed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: connect-proxy - SSH Proxy command helper
https://bugzilla.redhat.com/show_bug.cgi?id=197716
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora Version|devel |rawhide
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla