Skip to content

Conversation

@pedrobsaila
Copy link
Contributor

Fixes #83649

@ghost ghost added area-System.Net community-contribution Indicates that the PR has been added by a community member labels Apr 18, 2023
@ghost
Copy link

ghost commented Apr 18, 2023

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #83649

Author: pedrobsaila
Assignees: -
Labels:

area-System.Net

Milestone: -

// Ubuntu has ping under /bin, OSX under /sbin, ArchLinux under /usr/bin, Android under /system/bin.
private static readonly string[] s_binFolders = { "/bin/", "/sbin/", "/usr/bin/", "/system/bin" };
// Ubuntu has ping under /bin, OSX under /sbin, ArchLinux under /usr/bin, Android under /system/bin, NixOS under /run/current-system/sw/bin.
private static readonly string[] s_binFolders = { "/bin/", "/sbin/", "/usr/bin/", "/system/bin", "/run/current-system/sw/bin/" };
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private static readonly string[] s_binFolders = { "/bin/", "/sbin/", "/usr/bin/", "/system/bin", "/run/current-system/sw/bin/" };
private static readonly string[] s_binFolders = { "/bin", "/sbin", "/usr/bin", "/system/bin", "/run/current-system/sw/bin" };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Path.Combine handles perfectly the cases with / at the end and without

Copy link
Contributor

Choose a reason for hiding this comment

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

trailing slash isn't necessary, matching /system/bin, goes easy on eyes

@ManickaP
Copy link
Member

We don't have NixOS in our testing matrix. Did you test it manually? Or do we have a volunteer to do that?
Otherwise LGTM and thanks!

@pedrobsaila
Copy link
Contributor Author

pedrobsaila commented Apr 20, 2023

We don't have NixOS in our testing matrix. Did you test it manually? Or do we have a volunteer to do that?

Yes I did test it, thanks to this awesome github project https://github.com/nix-community/NixOS-WSL and thanks to the fact that the NixOS package manager contains already .NET 8.0 SDK preview 3 https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/dotnet/versions/8.0.nix

Here is the used code

Ping pingSender = new Ping();
            PingOptions options = new PingOptions();

            // Use the default Ttl value which is 128,
            // but change the fragmentation behavior.
            options.DontFragment = true;

            // Create a buffer of 32 bytes of data to be transmitted.
            string data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
            byte[] buffer = Encoding.ASCII.GetBytes(data);
            int timeout = 120;
            PingReply reply = pingSender.Send("www.google.com", timeout, buffer, options);
            if (reply.Status == IPStatus.Success)
            {
                Console.WriteLine("Address: {0}", reply.Address.ToString());
                Console.WriteLine("RoundTrip time: {0}", reply.RoundtripTime);
                Console.WriteLine("Time to live: {0}", reply.Options.Ttl);
                Console.WriteLine("Don't fragment: {0}", reply.Options.DontFragment);
                Console.WriteLine("Buffer size: {0}", reply.Buffer.Length);
            }

Result with code in master :
image

Result with code in this branch :
image

@ManickaP
Copy link
Member

Awesome, thank you and thanks for pointing out the NixOS on WSL project!

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

Thank you!

@ManickaP
Copy link
Member

Failures: #82894, #83551 and #85145

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@wfurt wfurt merged commit 47b4e0c into dotnet:main Apr 21, 2023
@pedrobsaila pedrobsaila deleted the 83649 branch April 21, 2023 19:28
@ghost ghost locked as resolved and limited conversation to collaborators May 22, 2023
@karelz karelz added this to the 8.0.0 milestone May 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[NixOS] ping utility not found

5 participants