nu-0.59.1 build failure

[0323pin]

Describe the bug

nu-0.59.1 from commit 9db356e fails to build on NetBSD.

First attempt to build fails since NetBSD libc doesn't support trash-support

I've disabled this feature at build time and have the following features enabled, "plugin", "which", "zip-support".

But, I'm hitting the following build error,

Compiling nu-command v0.59.1 (/usr/pkgsrc/wip/nushell/work/nushell-9db356e174288e74f7b5374d679b72907731eb70/crates/nu-command)
error[E0425]: cannot find function `collect_proc` in crate `nu_system`
  --> crates/nu-command/src/system/ps.rs:74:28
   |
74 |     for proc in nu_system::collect_proc(Duration::from_millis(100), false) {
   |                            ^^^^^^^^^^^^ not found in `nu_system`

For more information about this error, try `rustc --explain E0425`.
error: could not compile `nu-command` due to previous error
*** Error code 101

Stop.

How to reproduce

Build nu from git-HEAD

Expected behavior

Build finishes successfully.

Screenshots

No response

Configuration

No response

Additional context

OS: NetBSD-current (9.99.94)
Rust: 1.59.0

[fdncred]

It looks like we may need to have a noop for systems other than macos, linux, and windows in nu-system maybe?

[sophiajt]

@0323pin - we can add build checks to ignore that call like @fdncred mentions. @0323pin - if you comment out that section, does it succeed in building?

[0323pin]

@jntrnr I can try tonight or, if I don't have time, early tomorrow.

[0323pin]

@jntrnr do you mean to remove the whole section from line 74 until the end of ps.rs? Sorry if this sounds weird but, I'm not a programmer 😞

In that case, I get this

error[E0425]: cannot find function `run_ps` in this scope
  --> crates/nu-command/src/system/ps.rs:40:9
   |
40 |         run_ps(engine_state, call)
   |         ^^^^^^ not found in this scope

warning: unused import: `std::time::Duration`
 --> crates/nu-command/src/system/ps.rs:1:5
  |
1 | use std::time::Duration;
  |     ^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused imports: `IntoInterruptiblePipelineData`, `ShellError`, `Value`
 --> crates/nu-command/src/system/ps.rs:6:24
  |
6 |     Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Value,
  |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                ^^^^^^^^^^             ^^^^^

For more information about this error, try `rustc --explain E0425`.
warning: `nu-command` (lib) generated 2 warnings
error: could not compile `nu-command` due to previous error; 2 warnings emitted
*** Error code 101

Stop.

Just for the record, ps and sys commands have never worked due to lack of support in sysinfo (GuillaumeGomez/sysinfo#631). I've been discussing the possibility of setting-up a CI internally but, I still have no concrete plan.

[fdncred]

I'm thinking you might need some conditional compilation maybe like this in lib.rs

#[cfg(target_os = "linux")]
mod linux;
#[cfg(target_os = "macos")]
mod macos;
#[cfg(not(target_os = "linux"))]
#[cfg(not(target_os = "macos"))]
#[cfg(not(target_os = "windows"))]
mod other;
#[cfg(target_os = "windows")]
mod windows;

#[cfg(target_os = "linux")]
pub use self::linux::*;
#[cfg(target_os = "macos")]
pub use self::macos::*;
#[cfg(not(target_os = "linux"))]
#[cfg(not(target_os = "macos"))]
#[cfg(not(target_os = "windows"))]
pub use self::other::*;
#[cfg(target_os = "windows")]
pub use self::windows::*;

and add an other.rs that doesn't really do anything but exposes the function. maybe like this.

use std::time::Duration;

pub struct ProcessInfo;

pub fn collect_proc(_interval: Duration, _with_thread: bool) -> Vec<ProcessInfo> {
    vec![]
}

this way, when ps calls collect_proc there is a function of "other" oses and it just returns an empty vec. I'm not sure how to test this since I don't have that os but it's an idea.

[0323pin]

@fdncred Thanks! I can test this, would it possible to get these in patch format?

Forgot to add that ps works if I shell-out of nu into ksh using ^ps
As for sys it outputs non-sense,

/home/pin()
2022-03-18 13:04 > sys | to md --pretty
| host         | mem                                   |
| ------------ | ------------------------------------- |
| [row uptime] | [row total free swap total swap free] |

EDIT: This is on nu-0.44.0.

[fdncred]

my changes are for 0.59.1 and higher

[0323pin]

@fdncred yes, I know this. I'm trying to build from the latest git-commit in the main branch. I was just pointing out the historical trouble with ps.

[fdncred]

oh, ok. i'm sorry, i misunderstood you. the 0.59.1 uses a combination of procs and sysinfo and other stuff we found. i don't know how to generate a patch.

[0323pin]

i don't know how to generate a patch.

😄 I've already patched lib.rs and added other.rs, the build is running.
other.rs should be inside /crates/nu-command correct?

Let's see if I also need to patch ps.rs.

[fdncred]

others.rs needs to be here /crates/nu-system/src/others.rs and the lib.rs that needs changing is /crates/nu-system/src/lib.rs

[0323pin]

I've tested your changes and appreciate your help.
Now, I get the following,

error[E0599]: no method named `pid` found for struct `ProcessInfo` in the current scope
  --> crates/nu-command/src/system/ps.rs:80:23
   |
80 |             val: proc.pid() as i64,
   |                       ^^^ method not found in `ProcessInfo`

error[E0599]: no method named `name` found for struct `ProcessInfo` in the current scope
  --> crates/nu-command/src/system/ps.rs:86:23
   |
86 |             val: proc.name(),
   |                       ^^^^ method not found in `ProcessInfo`
   |
   = help: items from traits can only be used if the trait is implemented and in scope
note: `HashDigest` defines an item `name`, perhaps you need to implement it
  --> crates/nu-command/src/hash/generic_digest.rs:7:1
   |
7  | pub trait HashDigest: digest::Digest + Clone {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0599]: no method named `status` found for struct `ProcessInfo` in the current scope
  --> crates/nu-command/src/system/ps.rs:95:27
   |
95 |                 val: proc.status(),
   |                           ^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `cpu_usage` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:102:23
    |
102 |             val: proc.cpu_usage(),
    |                       ^^^^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `mem_size` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:108:23
    |
108 |             val: proc.mem_size() as i64,
    |                       ^^^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `virtual_size` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:114:23
    |
114 |             val: proc.virtual_size() as i64,
    |                       ^^^^^^^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `command` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:121:27
    |
121 |                 val: proc.command(),
    |                           ^^^^^^^ method not found in `ProcessInfo`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `nu-command` due to 7 previous errors
*** Error code 101

Stop.

ps.rs still needs something else 😢

[fdncred]

It looks like the easiest solution from here may be to change run_ps() to something like this below, where it checks to see if the process_info is empty, which it should be if it's coming from other.rs. At least it's something to play around with.

fn run_ps(engine_state: &EngineState, call: &Call) -> Result<PipelineData, ShellError> {
    let mut output = vec![];
    let span = call.head;
    let long = call.has_flag("long");

    let process_info = nu_system::collect_proc(Duration::from_millis(100), false);
    if !process_info.is_empty() {
        for proc in process_info {
            let mut cols = vec![];
            let mut vals = vec![];

            cols.push("pid".to_string());
            vals.push(Value::Int {
                val: proc.pid() as i64,
                span,
            });

            cols.push("name".to_string());
            vals.push(Value::String {
                val: proc.name(),
                span,
            });

            #[cfg(not(windows))]
            {
                // Hide status on Windows until we can find a good way to support it
                cols.push("status".to_string());
                vals.push(Value::String {
                    val: proc.status(),
                    span,
                });
            }

            cols.push("cpu".to_string());
            vals.push(Value::Float {
                val: proc.cpu_usage(),
                span,
            });

            cols.push("mem".to_string());
            vals.push(Value::Filesize {
                val: proc.mem_size() as i64,
                span,
            });

            cols.push("virtual".to_string());
            vals.push(Value::Filesize {
                val: proc.virtual_size() as i64,
                span,
            });

            if long {
                cols.push("command".to_string());
                vals.push(Value::String {
                    val: proc.command(),
                    span,
                });
                #[cfg(windows)]
                {
                    cols.push("cwd".to_string());
                    vals.push(Value::String {
                        val: proc.cwd(),
                        span,
                    });
                    cols.push("environment".to_string());
                    vals.push(Value::List {
                        vals: proc
                            .environ()
                            .iter()
                            .map(|x| Value::string(x.to_string(), span))
                            .collect(),
                        span,
                    });
                }
            }

            output.push(Value::Record { cols, vals, span });
        }
    }

    Ok(output
        .into_iter()
        .into_pipeline_data(engine_state.ctrlc.clone()))
}

[0323pin]

@fdncred started another build, will report back.

[0323pin]

I will double check all the patches and re-try. The last build failed in the same way as the previous.
No more time today, though.

[fdncred]

that's odd - i would think that vec![] would evaluate to .is_empty()

[0323pin]

@fdncred I'm building from a fork I've created to make patching more flexibel. You can check the diffs and see if you spot anything obvious, https://github.com/0323pin/nushell

[fdncred]

a quick glance at your changes and i think they look right. i'm not sure what is going on.

[0323pin]

Maybe @jntrnr can spot something. Thanks for looking.
I can start another build tomorrow on a clean directory, just to make sure.

[sophiajt]

Just from looking at the error, it seems like there isn't any *bsd support for ProcessInfo? Not sure why else all those methods would be missing. My hunch is that nu-system might need some bsd-specific fixes it doesn't have just yet

[0323pin]

it seems like there isn't any *bsd support for ProcessInfo

Are you using an external crate for these or, internal code?
I guess, the question is what is needed to add required support?

I've imported nushell-0.34.0 into NetBSD and have been using it since. It would be sad if we can't keep updating the package.
Anything I can do?

[0323pin]

I already knew this but for the record the build fails on a clean build environment with the following error,

   Compiling nu-command v0.59.1 (/usr/pkgsrc/wip/nushell/work/nushell-ae46d2e46ee4369020f4a835816448a0f0feab98/crates/nu-command)
error[E0599]: no method named `pid` found for struct `ProcessInfo` in the current scope
  --> crates/nu-command/src/system/ps.rs:82:27
   |
82 |                 val: proc.pid() as i64,
   |                           ^^^ method not found in `ProcessInfo`

error[E0599]: no method named `name` found for struct `ProcessInfo` in the current scope
  --> crates/nu-command/src/system/ps.rs:88:27
   |
88 |                 val: proc.name(),
   |                           ^^^^ method not found in `ProcessInfo`
   |
   = help: items from traits can only be used if the trait is implemented and in scope
note: `HashDigest` defines an item `name`, perhaps you need to implement it
  --> crates/nu-command/src/hash/generic_digest.rs:7:1
   |
7  | pub trait HashDigest: digest::Digest + Clone {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0599]: no method named `status` found for struct `ProcessInfo` in the current scope
  --> crates/nu-command/src/system/ps.rs:97:31
   |
97 |                     val: proc.status(),
   |                               ^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `cpu_usage` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:104:27
    |
104 |                 val: proc.cpu_usage(),
    |                           ^^^^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `mem_size` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:110:27
    |
110 |                 val: proc.mem_size() as i64,
    |                           ^^^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `virtual_size` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:116:27
    |
116 |                 val: proc.virtual_size() as i64,
    |                           ^^^^^^^^^^^^ method not found in `ProcessInfo`

error[E0599]: no method named `command` found for struct `ProcessInfo` in the current scope
   --> crates/nu-command/src/system/ps.rs:123:31
    |
123 |                     val: proc.command(),
    |                               ^^^^^^^ method not found in `ProcessInfo`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `nu-command` due to 7 previous errors
*** Error code 101

Stop.

[grtcdr]

nu-system appears to be only available for Windows, macOS and Linux only at the moment. Which is why you're seeing all those no method named errors.

(Like @jntrnr previously mentioned)

[0323pin]

@grtcdr Hi my friend, thanks for jumping in, appreciated.

The failing build has all the above patches included though. lib.rs in my build is as follows, as suggested by @fdncred

#[cfg(target_os = "linux")]
mod linux;
#[cfg(target_os = "macos")]
mod macos;
#[cfg(not(target_os = "linux"))]
#[cfg(not(target_os = "macos"))]
#[cfg(not(target_os = "windows"))]
mod other;
#[cfg(target_os = "windows")]
mod windows;

#[cfg(target_os = "linux")]
pub use self::linux::*;
#[cfg(target_os = "macos")]
pub use self::macos::*;
#[cfg(not(target_os = "linux"))]
#[cfg(not(target_os = "macos"))]
#[cfg(not(target_os = "windows"))]
pub use self::other::*;
#[cfg(target_os = "windows")]
pub use self::windows::*;

[grtcdr]

Has the patch helped in getting a successful build?

[0323pin]

@grtcdr nope. The error above is with all the 3 patches provided earlier by @fdncred

[grtcdr]

The patch needs to implement all the methods that are reported as not having been defined in the crate.

[grtcdr]

At this point, the patch should just be added to a netbsd.rs module in nu-system by the nu team.

[0323pin]

@grtcdr I'd have to agree with this

[sophiajt]

We'd be happy to take a PR to nu-system by someone familiar with netbsd. What @grtcdr suggests creating a netbsd.rs definitely sounds like a good way to do it.

[grtcdr]

I could help you achieve that, but I'll have to setup a new NetBSD environment from scratch, which could take me a while due to my terrible internet connection. Would you like me to do that?

[0323pin]

@grtcdr that would be awesome.
Although, I would like to help and learn a bit more, so maybe next time I get at least a start.

[grtcdr]

The first thing that comes to mind for me is to lay out a barebones implementation of ProcessInfo, where we can make use of a handy macro that Rust provides called todo!(). This satisfies the compiler's needs to a certain degree, so that it doesn't scream back saying the method doesn't exist. We'll do this for every function, until we can begin the real work and actually implement the functionalities that ProcessInfo exposes.

I'll begin by setting up my NetBSD environment, after which I'll fork the repository and make a draft PR.

[0323pin]

Closing this now as a potential solution is more complex than what I can handle. @grtcdr thank you for looking and for your patches, the build now fails to compile the procfs crate, as this is Linux only.

nushell-0.44.0 will be available as a legacy package for sometime.

[sophiajt]

@0323pin - you could leave this open and see if someone else with bsd experience can help.

[0323pin]

@jntrnr Appreciate your thought, I'll re-open this.
I'll share it with my fellow NetBSD developers, the amount of work required is rather large, though.

[0323pin]

I'm actually thinking that a NetBSD implementation might be closer to the existing implementation for MacOS, rather than the Linux one. I might kick a build after the weekend using,

[target.'cfg(target_os = "macos", target_os="netbsd"))'.dependencies]
libproc = "0.10"
errno = "0.2"
users = "0.11"
which = "4"
libc = "0.2"

[0323pin]

@jntrnr Ok, the above won't really work at the moment, neither procfs or, libproc have support for any of the *BSDs.

Lets take one step at the time, though. I found someone willing to help :)
Could you please list all stuff (pid, env, full-cmd, thread-list?, ...) that's needed to implement netbsd.rs?
Thanks!

[fdncred]

@0323pin if you look at this function, you can see what each platform (Mac, linux, windows) is returning by looking at the cols.push() lines.

nushell/crates/nu-command/src/system/ps.rs

Lines 69 to 149 in 8a9cc33

	fn run_ps(engine_state: &EngineState, call: &Call) -> Result<PipelineData, ShellError> {
	let mut output = vec![];
	let span = call.head;
	let long = call.has_flag("long");
	for proc in nu_system::collect_proc(Duration::from_millis(100), false) {
	let mut cols = vec![];
	let mut vals = vec![];
	cols.push("pid".to_string());
	vals.push(Value::Int {
	val: proc.pid() as i64,
	span,
	});
	cols.push("name".to_string());
	vals.push(Value::String {
	val: proc.name(),
	span,
	});
	#[cfg(not(windows))]
	{
	// Hide status on Windows until we can find a good way to support it
	cols.push("status".to_string());
	vals.push(Value::String {
	val: proc.status(),
	span,
	});
	}
	cols.push("cpu".to_string());
	vals.push(Value::Float {
	val: proc.cpu_usage(),
	span,
	});
	cols.push("mem".to_string());
	vals.push(Value::Filesize {
	val: proc.mem_size() as i64,
	span,
	});
	cols.push("virtual".to_string());
	vals.push(Value::Filesize {
	val: proc.virtual_size() as i64,
	span,
	});
	if long {
	cols.push("command".to_string());
	vals.push(Value::String {
	val: proc.command(),
	span,
	});
	#[cfg(windows)]
	{
	cols.push("cwd".to_string());
	vals.push(Value::String {
	val: proc.cwd(),
	span,
	});
	cols.push("environment".to_string());
	vals.push(Value::List {
	vals: proc
	.environ()
	.iter()
	.map(|x| Value::string(x.to_string(), span))
	.collect(),
	span,
	});
	}
	}
	output.push(Value::Record { cols, vals, span });
	}
	Ok(output
	.into_iter()
	.into_pipeline_data(engine_state.ctrlc.clone()))
	}

[0323pin]

@fdncred Thanks.

[hustcer]

@0323pin Guess this issue should be fixed by #6456, Can you have a try on the latest main branch?

[0323pin]

This was unexpected ... yes, looking at the changes, it should have fixed it.
I'll try a build when I find the time and report back. Let's keep this open until then.
Thanks for reaching out.

[0323pin]

@hustcer So, I couldn't wait that long 😄

I've just built from git-HEAD with the following features enabled, plugin and which-support, trash-support has always failed. But, I should try again as it seams the trash crate has evolved recently.

I will try also enabling dataframe and database features later.

EDIT: Forgot to mention it, the resulting binary works fine. Man, I have a lot to get used to after 6 months on elvish and big changes from 0.44.0

[0323pin]

@jntrnr Any hint on when 0.67.1 will be released? We are still providing pre-compiled binaries for 0.44.0 and it would be nice to bump this to the 0.6x.x series.

[0323pin]

Alright, I'll close this issue now.
All the following features can be enabled and build fine, plugin, which-support, dataframe and database.

The trash-support feature still causes the build to fail, so I'll leave it out for now. dataframe and database are now set as optional build features that can be enabled by the user. Default build is set with plugin and which-support enabled.
Everything is ready to update the package, as soon as a new release is out.

Thanks!

[fdncred]

@0323pin We plan on releasing 0.68.0 on Sept 6

[0323pin]

Updated, http://mail-index.netbsd.org/pkgsrc-changes/2022/09/06/msg260136.html