Bug 226187 - Merge Review: nc
Summary: Merge Review: nc
Keywords:
Status: CLOSED NOTABUG
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 Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:15 UTC by Nobody's working on this, feel free to take it
Modified: 2012-10-01 20:06 UTC (History)
8 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2012-10-01 20:06:33 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Rename nmap-ncat to ncat (1.57 KB, patch)
2012-07-30 11:34 UTC, Kalev Lember
no flags Details | Diff

Description Nobody's working on this, feel free to take it 2007-01-31 20:15:18 UTC
Fedora Merge Review: nc

http://cvs.fedora.redhat.com/viewcvs/devel/nc/
Initial Owner: rvokal

Comment 1 Jason Tibbitts 2007-02-03 23:09:19 UTC
Where does the tarball come from?  Upstream doesn't seem to have any actual
tarball abailable for download.  If it's from a CVS checkout, can you detail
in the spec (or in a script) how you do the checkout, and name the tarball and
choose the release appropriately based on the checkout date as detailed in the
naming guidelines at http://fedoraproject.org/wiki/Packaging/NamingGuidelines

Basically, the version-release pair should be something like
1.84-11.20070203cvs.  If you're checking out a tag, I'm not sure what the best
way to name it is.

Or perhaps we could consider whether or not one of the other netcat variants
is a better choice.

Rpmlint has a few complaints:

W: nc summary-ended-with-dot Reads and writes data across network connections using TCP or UDP.
W: nc summary-ended-with-dot Reads and writes data across network connections using TCP or UDP.
  Easy to fix these up.

W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/alta /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/bsh /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/dist.sh /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/irc /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/iscan /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/ncp /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/probe /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/web /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/webproxy /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/webrelay /bin/sh
W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/websearch /bin/sh
  Documentation shouldn't be executable.

Review:
X I can't check whether the source files match upstream.
X package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
X build root is not correct; should be
  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
X license field matches the actual license.
   I can find no mention of the GPL; this looks to me to be more like the X11
   license.
* license is open source-compatible.  License text not included upstream.
? latest version is being packaged.
* BuildRequires are proper.  The requirement for pkgconfig is unnecessary,
  though, as glib2-devel requires it.
* compiler flags are appropriate.
* %clean is present.
* %makeinstall is not used.
* package builds in mock.
* debuginfo package looks complete.
X rpmlint is silent.
* final provides and requires are sane:
   nc = 1.84-10.fc7
  =
   /bin/sh
   glib2
* %check is not present; no test suite upstream.
* 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 libtool .la droppings.


Comment 2 Radek Vokál 2007-02-04 11:45:31 UTC
The source comes from OpenBSD cvs. No tarball available. I will look at these
hopefully next week or the week after. Patches are welcome.

Comment 3 Radek Vokál 2007-02-13 14:29:46 UTC
Add upstream source. I have no idea when the checkout was made either which tag.
I would like to keep the source file as it is. 


W: nc doc-file-dependency /usr/share/doc/nc-1.84/scripts/alta /bin/sh
- these are only warnings and I'm not going to change it. The directory contains
test scripts, examples which are obviously part of documentation and it's
correct that they are executable.

See changes in 1.84-11

Comment 4 Jason Tibbitts 2007-02-23 16:32:10 UTC
Another ticket that disappeared from my list due to assignment pingponging. 
Fortunately we don't need to do that any longer.  I'll get back to this today.

Comment 5 Jason Tibbitts 2007-02-24 17:55:54 UTC
Unfortunately the source thing is really problematic for me and I suspect many
others; it's pretty much completely antithetical to the goal of Fedora to track
upstream as much as possible.  Currently we have an unreproducible tarball and
17 patches that taken together are larger than the source itself.

I understand that it would be truly difficult to rebase all of these patches, so
I don't really know what should happen here.  Perhaps we need a few volunteers
to work on the problem.  Or perhaps switching to one of the forked upstream
projects would be better, especially if they support ipv6.  But I would be
really uncomfortable about putting my "approved" stamp on this package without
having some public discussion on the issue.

Comment 6 Radek Vokál 2007-02-26 09:21:28 UTC
The issue I have with OpenBSD source is that it's not compileable with our
gcc/kernel. Basically I guess that BSD sockets have some new features we don't
support yet. Current OpenBSD source cannot be compiled on Linux without patching
it. I would rather make nc obsolete in favor of nc6 which implements also better
IPv6 support. At one point the nc6 upstream was quite active, but right now they
seem to be rather silent.

Comment 7 Jason Tibbitts 2007-03-08 22:08:00 UTC
OK, let's take a pragmatic view.  We can't just pull nc from the distro and what
we have is working.  According to the spec there are only nine patches instead
if the 17 patches that are in CVS.  So let's do this:

Document in the spec that the tarball was extracted from whatever CVS repository
it came from, but indicate that the pull date or upstream tag is unknown.

Clean up the unapplied patches from the CVS repo so the situation doesn't look
so bad.

Make a plan for what's going to happen post-F7, be that to rebase to current
upstream (though I doubt they'll take our patches) or to switch to something else.

Comment 8 Jason Tibbitts 2007-03-08 22:28:23 UTC
Not sure how the component got changed. 

BTW, it took me just a few minutes to figure out that you can get the upstream
source used in the tarball with:

CVSROOT=anoncvs.openbsd.org:/cvs cvs co -D 2005-10-26 src/usr.bin/nc

However, the scripts directory that appears in the tarball present in the SRPM
has a scripts directory that does not come from upstream CVS.

Comment 9 Ben Weir 2008-11-27 00:53:26 UTC
The version of netcat that is in fedora now has a bug where it refuses to connect to proxies that return "HTTP/1.1 200" messages (it only accepts ("HTTP/1.0 200" as valid).  I discovered this bug when attempting to ssh out through a web proxy using openssh's ProxyCommand directive like this:

ProxyCommand = /usr/bin/nc -X connect -x proxy:8080 %h %p

Looking at the OpenBSD repository it looks like socks.c was patched to fix this over 2 years ago.  Would be nice if this change got pulled down into the Fedora package.

http://www.openbsd.org/cgi-bin/cvsweb/src/usr.bin/nc/socks.c.diff?r1=1.16;r2=1.17;f=h

Comment 10 Jason Tibbitts 2008-11-27 04:18:15 UTC
Ben, you should open a ticket for your issue as it is not relevant to this review.

Comment 11 Jan Zeleny 2010-01-06 15:55:30 UTC
Hi, I'm current maintainer of nc and I'd like to finish this review. Could you please check latest build in Koji? I think everything is ok now.

Comment 12 Jason Tibbitts 2010-01-06 21:49:16 UTC
OK, I checked out the current CVS and indeed it builds fine and rpmlint is silent.

However, there are still a couple of issues:

There's still no documentation on actually recreating the CVS checkout.  I don't know if the information I posted nearly three years ago in comment #8 is still valid, but we really so need to get some information on precisely where the source comes from.  http://fedoraproject.org/wiki/Packaging:SourceURL

When a package accumulates so many patches, it is really important to keep track of their upstream status.  http://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
At this point, given the number of patches with no information about their upstream status and the lack of information on the upstream source, it begins to seem as if Fedora is maintaining its own fork of nc, which I doubt is something that we want to be doing.

Nothing seems to own /usr/share/nc; you should probably just add "%{_datadir}/nc" to %files and remove the two lines referencing %{_datadir}/nc/scripts.  But then there are a couple of other points:

If you expect that people will actually want to run those scripts, why not give them sensible non-conflicting names and actually install them in the usual path for executables?

If they aren't supposed to be run, why not keep them in %doc?

Comment 13 Jan Zeleny 2010-01-07 15:18:06 UTC
I have prepared another commit, but first I need to resolve the patch tracking thing. The thing is that the situation is very similar to what you described. Fedora took sources from OpenBSD nc and some patches are there as a result of porting process, so in fact, we kinda have Linux fork of nc.

My point here is that describing patches and making tracking bugs for them is IMO an unnecessary work. First reason I already wrote. Then there is the fact, that nc upstream seems to be almost dead, there are only small changes in CVS and they appear like twice a year. There have also been almost no big changes in Fedora version of netcat (rebase didn't appear in last 4 years). And finally - I don't intent to make some major changes in nc. In fact, I'd like to get ncat to Fedora and continue major development there. For now, I want to keep nc and maybe in time it will be appropriate to retire nc entirely.

So what do you think - should I really try to track all the patches that haven't been tracked for years or is it ok to skip this step considering my arguments?

Comment 14 Jason Tibbitts 2010-01-07 21:30:51 UTC
Honestly I'm not sure what the best course of action would be.  The current situation is something we want to get away from; we don't want to be maintaining the source directly inside the Fedora package.  We really don't like pointless forking and all the "Fedora doesn't work with upstream" grousing that comes with it, and we've worked very hard to avoid that kind of thing.

Stopping short of filing a bunch of upstream tickets for each of our patches, how hard would it be to let upstream know that regardless of what's happened in the past, Fedora would like to reduce or eliminate the current divergence and show them the current set of patches we're carrying?  If they reject or ignore the attempt at contact then we can at least get a quick fedorahosted project set up so that there's some public source for what's in this package.

Alternately, I'm happy to open a FESCo ticket and ask them for guidance.  They could grant an exception, but more likely they'd recommend something not much different than I detail above.

Comment 15 Jan Zeleny 2010-01-12 15:13:24 UTC
I re-wrote all patches and emailed them to upstream (email to their mailing list is the only way they handle bug reports and similar stuff).

Is it enough to comment those patches which I've sent them (not all are convenient for upstream, they are Linux and GCC specific) that I sent them to upstream? So far I cannot find usable link to their mailing list archives. The rest of patches will be commented that they are Fedora specific.

Comment 16 Jason Tibbitts 2012-05-10 19:26:06 UTC
I had typed up several comments and then lost them all because I added the wrong @redhat.com address for the current maintainer.  Let me try again.

Comment 17 Jason Tibbitts 2012-05-10 20:24:22 UTC
OK, that worked.

Looking at the package again after all this time, I see that it's considerably cleaned up.  Really there are two issues:

The line for doing the CVS checkout will get whatever's at the head of the tree at the point when you do the checkout.  It's not generally useful to get what's in the tarball (and indeed, I get something different if I check out now).  If upstream doesn't actually provide tarballs and doesn't actually tag their releases then the best you can do is provide a checkout date.

The bile b64_ntop.c appeared without any providence.  It appears to be pulled from another project.  While I find it hard to believe there's no small library around with base64-decoding function, I guess that's the case.  I'll run this by the FPC to see if there's agreement that this is an issue.

Comment 18 Petr Šabata 2012-05-11 09:26:32 UTC
(In reply to comment #17)
> OK, that worked.
> 
> Looking at the package again after all this time, I see that it's considerably
> cleaned up.  Really there are two issues:
> 
> The line for doing the CVS checkout will get whatever's at the head of the tree
> at the point when you do the checkout.  It's not generally useful to get what's
> in the tarball (and indeed, I get something different if I check out now).  If
> upstream doesn't actually provide tarballs and doesn't actually tag their
> releases then the best you can do is provide a checkout date.

That's a good point.  I've just been following the versioning tradition used before I took this package.  It would be fine if the whole project was just netcat.c :)

> The bile b64_ntop.c appeared without any providence.  It appears to be pulled
> from another project.  While I find it hard to believe there's no small library
> around with base64-decoding function, I guess that's the case.  I'll run this
> by the FPC to see if there's agreement that this is an issue.

This is somehow available on BSDs.  One option was to add this to libbsd but I saw it was quite common to "bundle" this implementation with other projects.  I don't think this case, although ugly, is against our guidelines.  However,  checking with FPC is a good idea.

Comment 19 Jason Tibbitts 2012-05-16 02:15:01 UTC
I filed https://fedorahosted.org/fpc/ticket/172 to track the bundling issue, but Toshio commented that the function in question is part of glibc already so there appears to be no reason to bundle it already.  You just need to include resolv.h and link with libresolv.

You obviously know more about this than I do, so I'm sure there's something I'm missing.  Is the glibc implementation not useful somehow?

Comment 20 Jason Tibbitts 2012-05-19 16:02:26 UTC
FPC approved a bundling exception for this code.  So it's up to you; use what's already in glibc, or continue to bundle and add Provides: bundled(base64-isc) to the spec.

Comment 21 Petr Šabata 2012-05-22 15:40:45 UTC
I've tried using the glibc implementation and it seems to work flawlessly.  I'll happily unbundle what we have now.

About the version -- I guess I should switch to <netcat.c version>.<checkout date> to be more clear what's actually in the package.

In the long run, I'd prefer killing nc and nc6 in Fedora in favor of nmap ncat which I think is the best netcat around now.  The only problem is the nmap package is rather large and not all nc users probably need or want it.  I wonder if some of its tools could be subpackaged? CC'ing nmap maintainer for insight.

Comment 22 Michal Hlavinka 2012-05-23 14:58:40 UTC
> In the long run, I'd prefer killing nc and nc6 in Fedora in favor of nmap
> ncat which I think is the best netcat around now.  The only problem is the
> nmap package is rather large and not all nc users probably need or want it. 
> I wonder if some of its tools could be subpackaged? CC'ing nmap maintainer
> for insight.

It's possible to create nmap sub-package, but I'm going to invest that effort only if nc is really going to die.

Comment 23 Petr Šabata 2012-05-31 14:23:23 UTC
(In reply to comment #22)
> > In the long run, I'd prefer killing nc and nc6 in Fedora in favor of nmap
> > ncat which I think is the best netcat around now.  The only problem is the
> > nmap package is rather large and not all nc users probably need or want it. 
> > I wonder if some of its tools could be subpackaged? CC'ing nmap maintainer
> > for insight.
> 
> It's possible to create nmap sub-package, but I'm going to invest that
> effort only if nc is really going to die.

Sounds like a tough task when you put it like this :)

I've just pushed the suggested changes to rawhide and f17 (https://admin.fedoraproject.org/updates/nc-1.107.20120403-1.fc17).

Comment 24 Enrico Scholz 2012-07-19 11:42:47 UTC
ncat does not support talking over unix socket which might break existing use cases.

Comment 25 Kalev Lember 2012-07-27 12:21:33 UTC
Have you considered calling the subpackage "ncat" instead of "nmap-ncat"? The package name is prominently visible when searching/installing for packages, so it might make sense to keep it simple.

Comment 26 Petr Šabata 2012-07-30 06:59:12 UTC
(In reply to comment #24)
> ncat does not support talking over unix socket which might break existing
> use cases.

A workaround script calling socat is now in place.

(In reply to comment #25)
> Have you considered calling the subpackage "ncat" instead of "nmap-ncat"?
> The package name is prominently visible when searching/installing for
> packages, so it might make sense to keep it simple.

Since it's an nmap subpackage, such a name is not possible.
Whether something is visible or not is a matter of package management tools; I believe searching for 'ncat' should return this package too -- or that's something I expect from any of those tools out there :)

Anyhow, the previous name 'nc' is still provided so if you search for that, you get nmap-ncat too.

Comment 27 Kalev Lember 2012-07-30 11:33:00 UTC
(In reply to comment #26)
> Since it's an nmap subpackage, such a name is not possible.

It actually is possible, with the -n switch that %package, %description and %files take. It makes it possible to use any name for binary packages, not only those that are prefixed with the source package name.

I'm attaching a patch that demonstrates this.

Comment 28 Kalev Lember 2012-07-30 11:34:00 UTC
Created attachment 601219 [details]
Rename nmap-ncat to ncat

Comment 29 Petr Šabata 2012-07-30 12:24:49 UTC
That's actually nice.  I didn't know about this; thank you.

Comment 30 Michal Hlavinka 2012-07-30 12:40:30 UTC
(In reply to comment #25)
> Have you considered calling the subpackage "ncat" instead of "nmap-ncat"?
> The package name is prominently visible when searching/installing for
> packages, so it might make sense to keep it simple.

Yes, I considered it, but I don't want to change its name. First, I don't think there is any trouble with the name as it is right now. I don't find it confusing and it's short and simple. Second, it'll be easier for users to find out what is a correct component in bugzilla when filing new bug reports.

Comment 31 Jason Tibbitts 2012-10-01 20:06:33 UTC
Given that nc is retired and not branched for f18, I'm going to go ahead and close this out.  Thanks for making things happen.


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