Skip to content

add nixos ping executable path #84997

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

Merged
merged 2 commits into from
Apr 21, 2023
Merged

add nixos ping executable path #84997

merged 2 commits into from
Apr 21, 2023

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