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

Add support for the ps command on FreeBSD, NetBSD, and OpenBSD #12892

Merged
merged 10 commits into from
May 22, 2024

Conversation

devyn
Copy link
Contributor

@devyn devyn commented May 17, 2024

Description

I feel like it's a little sad that BSDs get to enjoy almost everything other than the ps command, and there are some tests that rely on this command, so I figured it would be fun to patch that and make it work.

The different BSDs have diverged from each other somewhat, but generally have a similar enough API for reading process information via sysctl(), with some slightly different args.

This supports FreeBSD with the freebsd module, and NetBSD and OpenBSD with the netbsd module. OpenBSD is a fork of NetBSD and the interface has some minor differences but many things are the same.

I had wanted to try to support DragonFlyBSD too, but their Rust version in the latest release is only 1.72.0, which is too old for me to want to try to compile rustc up to 1.77.2... but I will revisit this whenever they do update it. Dragonfly is a fork of FreeBSD, so it's likely to be more or less the same - I just don't want to enable it without testing it.

Fixes #6862 (partially, we probably won't be adding zfs list)

User-Facing Changes

ps added for FreeBSD, NetBSD, and OpenBSD.

Tests + Formatting

The CI doesn't run tests for BSDs, so I'm not entirely sure if everything was already passing before. (Frankly, it's unlikely.) But nothing appears to be broken.

After Submitting

  • release notes?
  • DragonflyBSD, whenever they do update Rust to something close enough for me to try it

@fdncred
Copy link
Collaborator

fdncred commented May 17, 2024

I'm happy for this and it actually shows one-way nushell has evolved by people working on parts that they find fun in. I've changed the windows part a few times. Sophia has changed the linux parts a few times. We've both changed the macos parts a few times, all in the name of fun.

At some point, it would be nice to unite all these parts, so the readout is identically supported across os's. I haven't looked but as I recall, there are some things implemented around here that aren't the same in every os, like additional columns and such.

Good work! Keep it up!

@devyn
Copy link
Contributor Author

devyn commented May 17, 2024

I haven't looked but as I recall, there are some things implemented around here that aren't the same in every os, like additional columns and such.

Yeah, that's true. One big improvement I think would be to have the process information implement a trait rather than just being a convention. That would make it easier to be sure about what should be there, and errors about incorrect or missing functionality at compile time would be better.

As far as the extra columns go, I think it's a good thing that there are some more OS-specific things exposed - process management is very OS specific, and we have a nice set of universal columns that are pretty standardized for portable scripts to use. I would actually like to see some more things exposed on Unixes, like process group ID for example, in the long form.

@fdncred
Copy link
Collaborator

fdncred commented May 17, 2024

100%. A trait would probably make things easier, and I agree that having os-specific things are necessary/required and I always want more detailed under-the-hood information. IIRC, the last thing I added was a windows specific thing where you can see each running processes environment in the ps -l command. It's helped me understand what env vars are available in the running process by doing something like ps -l | where name == nu.exe | get environment.0. 🤔 I think that was the last thing... I twiddle in there from time to time too.

@devyn
Copy link
Contributor Author

devyn commented May 17, 2024

Environment is something we could probably support elsewhere. It's possible to configure the Linux kernel to not allow other processes to read environment variables, though, I think, and I'm not sure if macOS allows it or not

Still, this works for me on Linux, so I'm guessing it is usually allowed there:

open /proc/<pid>/environ | split row "\u{0}" | parse "{key}={value}"

@devyn devyn changed the title Add support for the ps command on BSDs Add support for the ps command on FreeBSD, NetBSD, and OpenBSD May 19, 2024
@devyn devyn marked this pull request as ready for review May 19, 2024 07:32
@devyn
Copy link
Contributor Author

devyn commented May 19, 2024

This is ready now.

@0323pin
Copy link
Contributor

0323pin commented May 19, 2024

@devyn 🚀
Awesome! Sure, I'll test this. Although, I won't get to my laptop before tomorrow.

@0323pin
Copy link
Contributor

0323pin commented May 20, 2024

Alright, so here we go 😄 ...

ps works flawlessly now, thank you!

2024-05-20-094028_1366x768_scrot

As for enabling mimalloc by default, I'm a bit more concerned.
I had to build using no default features, features = plugin which-support trash-support sqlite
NetBSD uses gcc as the default compiler and it does indeed fail to build when mimalloc is enabled.

@devyn
Copy link
Contributor Author

devyn commented May 20, 2024

Great! Yeah, unfortunately it doesn't seem to work with GCC. If it hasn't been reported already I will, but I'm not entirely sure if it's a GCC bug or a mimalloc bug

@0323pin
Copy link
Contributor

0323pin commented May 20, 2024

Thank you!

@devyn
Copy link
Contributor Author

devyn commented May 22, 2024

Going to merge this since it works in my VMs, and we'll just see if we get any feedback before the release

@devyn devyn merged commit 758c5d4 into nushell:main May 22, 2024
14 checks passed
@devyn devyn deleted the bsd-ps-support branch May 22, 2024 15:13
@0323pin
Copy link
Contributor

0323pin commented May 22, 2024

🥳

@hustcer hustcer added this to the v0.94.0 milestone May 23, 2024
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

Successfully merging this pull request may close these issues.

Freebsd, only "ls" shows a table.
4 participants