Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

document and incorporate the perl ping module patch #801

Open
sarcasticadmin opened this issue Nov 25, 2024 · 6 comments · May be fixed by #804
Open

document and incorporate the perl ping module patch #801

sarcasticadmin opened this issue Nov 25, 2024 · 6 comments · May be fixed by #804

Comments

@sarcasticadmin
Copy link
Member

sarcasticadmin commented Nov 25, 2024

Description

per @owendelong message in signal there is a patch that should be incorporated into the ping perl module (https://metacpan.org/pod/Net::Ping). Lets document this for others and we can also incorporate this into the patch within our nix shell.

Patch:

% diff dist_[Ping.pm](http://ping.pm/) od_[Ping.pm](http://ping.pm/) 
230c230
<     croak("icmp ping requires root privilege") if !_isroot();
---
>     croak("icmp ping requires root privilege") if !_isroot() && ($^O ne "linux");
235,236c235,244
<     socket($self->{fh}, PF_INET, SOCK_RAW, $self->{proto_num}) ||
<       croak("icmp socket error - $!");
---
>     if ($^O eq "linux")
>     {
> 	    socket($self->{fh}, PF_INET, SOCK_DGRAM, $self->{proto_num}) ||
> 	      croak("icmp socket error - $!");
>     }
>     else
>     {
> 	    socket($self->{fh}, PF_INET, SOCK_RAW, $self->{proto_num}) ||
> 	      croak("icmp socket error - $!");
>     }
253,254c261,270
<     socket($self->{fh}, $AF_INET6, SOCK_RAW, $self->{proto_num}) ||
<       croak("icmp socket error - $!");
---
>     if ($^O eq "linux")
>     {
> 	    socket($self->{fh}, $AF_INET6, SOCK_DGRAM, $self->{proto_num}) ||
> 	      croak("icmp socket error - $!");
>     }
>     else
>     {
> 	    socket($self->{fh}, $AF_INET6, SOCK_RAW, $self->{proto_num}) ||
> 	      croak("icmp socket error - $!");
>     }

Acceptance Criteria

  • patch is more widely know and documented for the teams
  • patch incorporated into default nix shell
@djacu
Copy link
Contributor

djacu commented Nov 26, 2024

This should be trivial to add to the overlays once #800 is merged.

Need more info from @owendelong about which package needs patching. I'm not able to infer it from the patch above.

@sarcasticadmin
Copy link
Member Author

sarcasticadmin commented Nov 30, 2024

This should be trivial to add to the overlays once #800 is merged.

Need more info from @owendelong about which package needs patching. I'm not able to infer it from the patch above.

The modules https://metacpan.org/pod/Net::Ping and its in nixpkgs: https://github.com/NixOS/nixpkgs/blob/c71ad5c34d51dcbda4c15f44ea4e4aa6bb6ac1e9/pkgs/top-level/perl-packages.nix#L19105 I updated the original issue to include the link to the cpan module.

But there are a few cpan modules that arent. I can rebase my old branch with them now that we have the rework you did in #800

@sarcasticadmin
Copy link
Member Author

@owendelong where did you include your patch upstream to the Net::Ping? I see the issue raised here: https://rt.cpan.org/Public/Bug/Display.html?id=139820 but didn't see the patch anywhere upstream but Im not sure Im looking in the right place either.

@owendelong
Copy link
Collaborator

I may have only included it in the issue. I have now also sent an email to p5p (the current Net::Ping.pm package maintainer at CPAN).

@owendelong
Copy link
Collaborator

Actually, the RT Issue you referenced above is somewhat different and came from someone else. I think I emailed the package maintainer directly in my previous attempt as well. However, I have added my patch and some clarification to the RT issue you linked, so hopefully one of those two actions will gain some traction.

@sarcasticadmin
Copy link
Member Author

@owendelong there seems to be a github repo for Net::Ping https://github.com/rurban/Net-Ping It might be easiest to just create a PR there since it seems somewhat active.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants