Page MenuHomeFreeBSD

net/fping: update to 4.0
ClosedPublic

Authored by fernape on Jun 22 2018, 6:12 PM.

Details

Summary

Sent via PR 229172

Awaiting maintainer approval or timeout (on July 5th).

Test Plan
  • portlint -AC OK
  • poudriere builds for {10.4,11.1}{amd64,i386}, 12i386 OK
  • run test in 10.4 amd64 OK.

Note that I don't have an IPv6 address so I only tested IPv4
I tried freefall but it seems I wold need to be root to run it:

[fernape@freefall ~]$ ./fping ipv6.google.com
(null): can't create socket (must run as root?) : Protocol not supported
[fernape@freefall ~]$

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fernape created this revision.Jun 22 2018, 6:12 PM

This port is required by several others -- can you check that none of them expect fping6 to be present?

fernape updated this revision to Diff 44348.Jun 23 2018, 3:46 PM

Link fping to fping6 since a bunch of other ports (net-mgmt/librenms and others)
still expect that binary to exist.

Thanks for following up on this :)

net/fping/Makefile
33 ↗(On Diff #44348)

This should only be done if IPv6 support was enabled I assume.

fernape updated this revision to Diff 44350.Jun 23 2018, 4:28 PM

Create fping6 only when IPV6 option is selected

fernape marked an inline comment as done.Jun 23 2018, 4:34 PM

Thanks for following up on this :)

Thanks for pointing it out! :-)

A question about procedure:

What I did was find all the Makefiles which had net/fping in them and for every one of them, go and extract the distfile and see if they used fping6. I was lucky because I found a port at my third attempt :-) but... is there a better way to do something like this?

Thanks for following up on this :)

Thanks for pointing it out! :-)
A question about procedure:
What I did was find all the Makefiles which had net/fping in them and for every one of them, go and extract the distfile and see if they used fping6. I was lucky because I found a port at my third attempt :-) but... is there a better way to do something like this?

Yeah, also what I would have done :D -- you could have used freshports.org to get the list of the ports to look at -- I normally grep in the INDEX file to get a list of dependencies:

awk -F '|' '/fping-3/{print $2}' /usr/ports/INDEX-12

Maintainer approved the submitted patch.

mat added a comment.Jun 27 2018, 10:57 AM

Is there any reason to keep the options? Is the package so much bigger with both IPv4 and IPv6 defined?

It feels there should be no options and everything should be installed at once.

In D15970#339536, @mat wrote:

Is there any reason to keep the options? Is the package so much bigger with both IPv4 and IPv6 defined?

No, it's basically the same size.

It feels there should be no options and everything should be installed at once.

Makes sense since the same executable handles both protocols now. I'll update the patch in the PR and await maintainer approval.

fernape updated this revision to Diff 44529.Jun 27 2018, 3:26 PM

Remove OPTIONS.

The same executable now handles both IPv4 and IPv6.

mat added a comment.Jun 27 2018, 4:38 PM

You may need to add --enable-ipv6 --enable-ipv4 to CONFIGURE_ARGS.

net/fping/Makefile
27 ↗(On Diff #44529)

You could also symlink the man page, so that fping6 also get one.

fernape updated this revision to Diff 44548.Jun 27 2018, 5:36 PM

link manpage so we can man fping and man fping6

fernape marked an inline comment as done.Jun 27 2018, 5:39 PM
In D15970#339690, @mat wrote:

You may need to add --enable-ipv6 --enable-ipv4 to CONFIGURE_ARGS.

It seems there is not need. It compiles and works fine without passing them explicitly.

mat added a comment.Jun 27 2018, 7:42 PM
In D15970#339690, @mat wrote:

You may need to add --enable-ipv6 --enable-ipv4 to CONFIGURE_ARGS.

It seems there is not need. It compiles and works fine without passing them explicitly.

In the configure world "seem to work" is not really good enough, it may do things under the hood that you cannot guess
According to the configure.ac script, ipv4 is enabled by default and ipv6 is enabled if netinet/icmp6.h is present, so it could be safe to not use the arguments. On the other hand, keeping them will do no harm.

fernape updated this revision to Diff 44561.Jun 27 2018, 9:19 PM

Add --enable-ipv6 --enable-ipv4 to CONFIGURE_ARGS

In D15970#339804, @mat wrote:
In D15970#339690, @mat wrote:

You may need to add --enable-ipv6 --enable-ipv4 to CONFIGURE_ARGS.

It seems there is not need. It compiles and works fine without passing them explicitly.

In the configure world "seem to work" is not really good enough, it may do things under the hood that you cannot guess
According to the configure.ac script, ipv4 is enabled by default and ipv6 is enabled if netinet/icmp6.h is present, so it could be safe to not use the arguments. On the other hand, keeping them will do no harm.

Arguments added. Better safe than sorry :-)

tcberner accepted this revision.Jun 30 2018, 6:57 AM
This revision is now accepted and ready to land.Jun 30 2018, 6:57 AM
This revision was automatically updated to reflect the committed changes.