Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Add formula for netcat6 #25916

Closed
wants to merge 1 commit into from
Closed

Add formula for netcat6 #25916

wants to merge 1 commit into from

Conversation

Roguelazer
Copy link
Contributor

Formula for netcat6, a fork of netcat with better IPv6 support than GNU netcat (which is already in homebrew), as well as some additional features (like file-transfer mode and half-close support).

Upstream's tarball is broken at the moment (for the first time I can remember in the last 7 years or so), so I put in the debian pool as a mirror and e-mailed upstream to get the tarballs back.

"--prefix=#{prefix}",
"--mandir=#{man}"
system "make install"
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a short test do block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, can you add one which actually tests it rather than just outputting the version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is IPv6 networking software. Any test that I write that does anything non-trivial with it is going to be fragile and depend on the target computers' state, which is probably undesirable. Does the homebrew project have test guidelines somewhere? I have pretty strong feelings about how tests should never go out over the network...

On Jan 17, 2014, at 2:22 AM, Mike McQuaid notifications@github.com wrote:

In Library/Formula/netcat6.rb:

@@ -0,0 +1,16 @@
+require 'formula'
+
+class Netcat6 < Formula

  • def install
  • system "./configure", "--disable-dependency-tracking",
  •                      "--prefix=#{prefix}",
    
  •                      "--mandir=#{man}"
    
  • system "make install"
  • end
    Sorry, can you add one which actually tests it rather than just outputting the version?


Reply to this email directly or view it on GitHub.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't but I'll try and write something. Just do something very simple with the software to make sure it doesn't crash when doing something semi-normal.

@Roguelazer
Copy link
Contributor Author

Okay. I feel pretty gross about hitting the network, but how does this look?

sha1 '50b1a3f7bfa610a2016727e5741791ad3a88bd07'

def patches
{ :p0 => 'https://gist.github.com/Roguelazer/8479401/raw/5c1743f2ca908ee0947faa130c6d85d011976adb/nc6_very_verbose.patch' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a patch applied by debian and red hat which removes a line output on every connection indicating whether the connection was tcp or udp

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please submit it upstream and see what they say. If it's unlikely to be applied upstream we don't want to apply it either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that they would if their CVS server were up. I'm still waiting for a response from upstream, though.

As far as things go, it's a very innocuous patch. Puts a debug statement behind the debug command line flag.

James Brown,
currently mobile

On Jan 18, 2014, at 1:28 AM, Mike McQuaid notifications@github.com wrote:

In Library/Formula/netcat6.rb:

@@ -0,0 +1,35 @@
+require 'formula'
+require 'test/unit/assertions'
+
+class Netcat6 < Formula


Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch looks OK to me, but it's small enough to inline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamv I'd rather we didn't pull patches like this that are desirable but not necessary for the tool to work.

@adamv
Copy link
Contributor

adamv commented Feb 22, 2014

@BrewTestBot test this please

@@ -0,0 +1,35 @@
require 'formula'
require 'test/unit/assertions'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line shouldn't be here.

@adamv adamv self-assigned this Feb 22, 2014
@adamv
Copy link
Contributor

adamv commented Mar 27, 2014

@BrewTestBot test this please

@adamv
Copy link
Contributor

adamv commented Mar 27, 2014

@MikeMcQuaid excepting some style issues, is this OK to pull?

@MikeMcQuaid
Copy link
Member

@adamv I'd rather we didn't pull the patch but not blocking.

@Roguelazer
Copy link
Contributor Author

bump?


class Netcat6 < Formula
homepage 'http://www.deepspace6.net/projects/netcat6.html'
# upstream is currently down, so we'll just, uh, borrow debian's
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these two commented lines; we can find the official tarball again from the homepage if we need ti.

@Roguelazer
Copy link
Contributor Author

I've rebased on master (which is required to use the new patch syntax requested by @adamv) and applied the requested style changes.

Let me just say that compared to just about every other open-source project I've contributed to (including Homebrew way back in 2011), this has been the longest and silliest pull request thread.

@adamv
Copy link
Contributor

adamv commented Apr 18, 2014

Please squash to a single commit (and take the GitHub handles out of the message to reduce messages being sent), thanks.

@adamv
Copy link
Contributor

adamv commented Apr 18, 2014

@MikeMcQuaid since Debian pulls that patch I don't have a problem with it, and will pull this tonight or over the weekend when I get a chance.

@MikeMcQuaid
Copy link
Member

@adamv I'm 👎 on carrying patches that aren't going to go upstream. Debian and I differ on this; they're happy to patch stuff indefinitely. Feel free to overrule here.

@Roguelazer We're doing our best and have simultaneous pressure from users to try and improve quality whilst actually keeping things up to date, secure and maintainable. New formulae always take longer to get included because we want them to be in a good standard when they go in. I'm sorry that it's not been a good process but it could have been improved if you had read some more of our documentation.

@Roguelazer
Copy link
Contributor Author

squashed as requested.

@adamv adamv closed this in 44e5b56 Apr 19, 2014
@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants