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

pinger: refactor "OS" detection? #186

Closed
dm9pZCAq opened this issue Jun 20, 2022 · 3 comments
Closed

pinger: refactor "OS" detection? #186

dm9pZCAq opened this issue Jun 20, 2022 · 3 comments

Comments

@dm9pZCAq
Copy link
Contributor

dm9pZCAq commented Jun 20, 2022

i use Gentoo and i got Error: Unsupported OS Gentoo Linux, also in Gentoo ping may be from both iputils and busybox (related to #140)

for me this error appears in gping-1.3.2, in gping-1.3.1 everything works fine

here is code which choose what ping executable to use, but there are may be endless number of linux distros and they can have different ping implementation

gping/pinger/src/lib.rs

Lines 142 to 184 in 9697f84

match os_type {
#[cfg(windows)]
Type::Windows => {
let mut p = windows::WindowsPinger::default();
p.set_interval(interval);
p.start::<windows::WindowsParser>(addr)
}
Type::Amazon
| Type::Arch
| Type::CentOS
| Type::Debian
| Type::EndeavourOS
| Type::Fedora
| Type::Linux
| Type::NixOS
| Type::Manjaro
| Type::Mint
| Type::openSUSE
| Type::OracleLinux
| Type::Redhat
| Type::RedHatEnterprise
| Type::SUSE
| Type::Ubuntu
| Type::Pop
| Type::Solus
| Type::Raspbian
| Type::Android => {
let mut p = linux::LinuxPinger::default();
p.set_interval(interval);
p.start::<linux::LinuxParser>(addr)
}
Type::Alpine => {
let mut p = linux::AlpinePinger::default();
p.set_interval(interval);
p.start::<linux::LinuxParser>(addr)
}
Type::Macos => {
let mut p = macos::MacOSPinger::default();
p.set_interval(interval);
p.start::<macos::MacOSParser>(addr)
}
_ => Err(PingError::UnsupportedOS(os_type.to_string()).into()),
}

so my suggestion is:

  1. detect most popular ping version (from iputils or from busybox)
  2. don't match all supported OS by os_info (Type::), just match Type::Alpine, Type::Macos and othervice use "default" ping

here is dirty example of how first suggestion can be implemented:

pub fn ping_with_interval(addr: String, interval: Duration) -> Result<mpsc::Receiver<PingResult>> {
    #[cfg(windows)]
    {
        let mut p = windows::WindowsPinger::default();
        p.set_interval(interval);
        return p.start::<windows::WindowsParser>(addr);
    }
    #[cfg(unix)]
    {
	return if cfg!(target_os = "macos") {
            let mut p = macos::MacOSPinger::default();
            p.set_interval(interval);
            p.start::<macos::MacOSParser>(addr)
	} else {
            if busybox_ping() {
                let mut p = linux::LinuxPinger::default();
                p.set_interval(interval);
                p.start::<linux::LinuxParser>(addr)
	    } else if iptools_ping() {
                let mut p =linux::AlpinePinger::default();
                p.set_interval(interval);
                p.start::<linux::LinuxParser>(addr)
	    } else {
                Err(anyhow!("unsupported ping"))?
	    }
	}
    }

    Err(anyhow!("some error msg about unsupported os"))?
}
@orf
Copy link
Owner

orf commented Sep 19, 2022

Yeah, I think this is the best way forward. Urgh. I really wanted to avoid trying to do ping version detection, but you're right. I'll try and implement this now.

@orf
Copy link
Owner

orf commented Sep 19, 2022

Thanks for the suggestion @dm9pZCAq. I've implemented this here: https://github.com/orf/gping/blob/master/pinger/src/linux.rs#L24-L48

Let's see if the tests pass...

@dm9pZCAq
Copy link
Contributor Author

yes, it works for me

thank you very much!

@orf orf closed this as completed Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants