Can't use standard library module within the standard library (?)

[bobhy]

Describe the bug

Trying to add a call to std log info in crates/nu-std/lib/dirs.nu for, e.g, std dirs next, another module in the standard library.

It would be a grave restriction indeed if some commands in the standard library could not be used within other commands of the standard library.

How to reproduce

1. in crates/nu-std/lib/dirs.nu (for example), add use std log info in either of the locations indicated below (or both).
2. `〉cargo run -- -c 'use std *; dirs next'
3. or do a build and run the binary: > cargo build; target/debug/nu -c 'use std *; dirs next'

Expected behavior

Info log output from invoked dirs next command

Screenshots

10 repetitions of the 'module not found' error, all from the same source line.

~/src/rust/nushell〉cargo run -- -c 'use std *; dirs next'
    Finished dev [unoptimized + debuginfo] target(s) in 0.25s
     Running `target/debug/nu -c 'use std *; dirs next'`
Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

Error: nu::parser::module_not_found

  × Module not found.
    ╭─[dirs:33:1]
 33 │ ] {
 34 │     use std "log info"
    ·         ─┬─
    ·          ╰── module not found
 35 │ 
    ╰────
  help: module files and their paths must be available before your script is run as parsing occurs before anything
        is evaluated

thread 'main' panicked at 'Internal error: can't run a predeclaration without a body', crates/nu-protocol/src/signature.rs:686:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error: nu::shell::column_not_found

  

Configuration

key	value
version	0.78.1
branch	stdlib_contributing
commit_hash	f2b977b
build_os	linux-x86_64
build_target	x86_64-unknown-linux-gnu
rust_version	rustc 1.66.1 (90743e729 2023-01-10)
rust_channel	1.66.1-x86_64-unknown-linux-gnu
cargo_version	cargo 1.66.1 (ad779e08b 2023-01-10)
build_time	2023-04-07 21:17:57 -04:00
build_rust_channel	debug
features	default, zip
installed_plugins

Additional context

Here's the relevant chunk of dirs.nu:

# Maintain a list of working directories and navigates them

use std "log info"

# the directory stack
export-env {
    let-env DIRS_POSITION = 0
    let-env DIRS_LIST = [($env.PWD | path expand)]
}

# Add one or more directories to the list.
# PWD becomes first of the newly added directories.
export def-env "dirs add" [
    ...paths: string    # directory or directories to add to working list
    ] {
        mut abspaths = []
        for p in $paths {
            let exp = ($p | path expand)
            if ($exp | path type) != 'dir' {
                let span = (metadata $p).span
                error make {msg: "not a directory", label: {text: "not a directory", start: $span.start, end: $span.end } }
            }
        $abspaths = ($abspaths | append $exp)

        }
        let-env DIRS_LIST = ($env.DIRS_LIST | insert ($env.DIRS_POSITION + 1) $abspaths | flatten)
        let-env DIRS_POSITION = $env.DIRS_POSITION + 1

    _fetch 0
}

# Advance to the next directory in the list or wrap to beginning.
export def-env "dirs next" [
    N:int = 1   # number of positions to move.
] {
    use std "log info"

    _fetch $N
    log info $"advancing to position ($env.DIRS_POSITION)"
}

[amtoine]

i can reproduce this on latest main (0e23400)

here is a patch to apply to change the library in the way it breaks on latest main

diff --git a/crates/nu-std/lib/dirs.nu b/crates/nu-std/lib/dirs.nu
index d2e966446..5b2138eac 100644
--- a/crates/nu-std/lib/dirs.nu
+++ b/crates/nu-std/lib/dirs.nu
@@ -1,5 +1,7 @@
 # Maintain a list of working directories and navigates them
 
+use log 'log info'
+
 # the directory stack
 export-env {
     let-env DIRS_POSITION = 0
@@ -31,6 +33,8 @@ export def-env "dirs add" [
 export def-env "dirs next" [
     N:int = 1   # number of positions to move.
 ] {
+    log info "`dirs next` from global import"
+
     _fetch $N    
 }
 
@@ -56,6 +60,9 @@ export def-env "dirs drop" [] {
 
 # Display current working directories.
 export def-env "dirs show" [] {
+    use log 'log warning'
+    log warning "`dirs show` from local import"
+
     mut out = []
     for $p in ($env.DIRS_LIST | enumerate) {
         $out = ($out | append [

then

cargo run -- -c 'use std *; dirs next'

as said above 👍

[amtoine]

cargo run will already produce this error.
then a use std * won't actually import anything and dirs show gives

Error: nu::shell::external_command

  × External command failed
    ╭─[dirs:63:1]
 63 │ export def-env "dirs show" [] {
 64 │     log info "dirs show"
    ·     ─┬─
    ·      ╰── executable was not found
 65 │     mut out = []
    ╰────
  help: No such file or directory (os error 2)

and interestingly enough,

• dirs next will make nushell panic
• dirs show will only give an error without panicking

both with the same chain of nu::parser::module_not_found errors[

[amtoine]

📯 📯 i know what is happening here

in the loader, i.e. load_standard_library in nu-std, the modules are loaded in that very order:

• assert from lib/assert.nu
• dirs from lib/dirs.nu
• help from lib/help.nu
• log from lib/log.nu
• xml from lib/xml.nu
• and finally the main std module from lib/mod.nu (the std help commands of the prelude are loaded here)

❓ here you are trying to perform some kind of cyclic import right?
importing std inside of std itself 🤔

i think this will be tideous...

🔧 the first fix is to use log 'log info' instead of use std 'log info'

there is a second issue, it's that, as now, dirs is parsed BEFORE log and thus the log module is not available...

🔧 the second fix is to change the loading order with

diff --git a/crates/nu-std/src/lib.rs b/crates/nu-std/src/lib.rs
index 7c847cb49..6e6c20078 100644
--- a/crates/nu-std/src/lib.rs
+++ b/crates/nu-std/src/lib.rs
@@ -68,9 +68,10 @@ pub fn load_standard_library(
 
         let submodules = vec![
             ("assert", include_str!("../lib/assert.nu")),
+            ("log", include_str!("../lib/log.nu")),
+
             ("dirs", include_str!("../lib/dirs.nu")),
             ("help", include_str!("../lib/help.nu")),
-            ("log", include_str!("../lib/log.nu")),
             ("xml", include_str!("../lib/xml.nu")),
         ];

to make things like assert and log available in the other modules

then you can apply the patch above to the dirs module and it will work as expected!

Warning
these are just fixes for now, not a direction to follow in the future.
it's how the loading and parsing of std currently works 😌

Note
with the two patches

• cargo run -- --no-config-file -c 'use std *; dirs next' will give

INF|...|`dirs next` from global import

• cargo run -- --no-config-file -c 'use std *; dirs show' will give

WRN|...|`dirs show` from local import
╭───┬────────┬──────╮
│ # │ active │ path │
├───┼────────┼──────┤
│ 0 │ true   │ ...  │
╰───┴────────┴──────╯

[kubouch]

Instead of trying to use std log info, can you use log.nu inside dirs.nu and use just log info?

[amtoine]

Instead of trying to use std log info, can you use log.nu inside dirs.nu and use just log info?

no we can not because log.nu is not a runtime thing for nushell => there is no log.nu file in the final binary nor on the user's filesystem.

we can change the loading to name the module log.nu instead of log but that adds 3 unnecessary characters here and we still need to

• use log.nu * to have access to log info and log warning
• put the log.nu line at the top of the submodules in load_standard_library

🤔

[amtoine]

we have a namespace issue in the library, that's quite clear i think 😋

=> unticked the "structure" point of the roadmap and added a "namespace item" for now 😌

[kubouch]

Ah, I see. Seems like we should finally create the long-awaited file cache and make use etc. aware of it. Then, when calling use log.nu, the parser would look into the cache first, see that log.nu was already parsed before and give the parsed module without reparsing. That way you don't need the file to be on disk. Since the file is not in the file system, there is no chance it could change, so it should be safe to do so. We could` also write out the stdlib into real files but that brings issues with versioning etc.

If possible, I'd prefer not having submodules aware of being a part of some top module because it adds a lot of complexity and hinders code reuse. So far, we used a simple file-based module system that I think works quite well and is very straightforward. The stdlib is special because it doesn't have real files at runtime, but I think that's the only case where you'd have a Nushell module without a corresponding .nu file. The only other one I can think of are plugins in the future. If we make the file cache aware of this, we should be able to get around it.

[fdncred]

@amtoine, as a quick fix, it seems reasonable to me to load helpers first so they can be used by all, like log and assert.

[amtoine]

@kubouch
agree with the fact that std is a special case of a "runtime module" without any .nu files 👍

@fdncred
cool, i can write a patch for this right now 👍

[bobhy]

The key thing about the standard library is that we want all script modules to be able to use the tools defined in it. Ideally, the rules for importing stdlib tools would be literally the same for user written and stdlib modules (beyond a few, really core, stdlib modules if we can't avoid that).

If I'm understanding correctly, the file cache idea would allow us to say the rule is use use std 'log xxx' to import from stdlib in all module, full stop. Specifically, it would be use std log xxxinstd dirs, std clip, etc. @kubouch, is that where we'd be heading? It sounds like you're more concerned about whether the used module is on disk or not, which sound like an internal implementation detail that should not be exposed in an external API (the use` command and its usage).

For now, it sounds the patch to declare the helpers (my "core") first is good enough to document the long-term rule, which will be used by stdlib developers who are not working on the helpers (log, assert, etc.). I'll do that as soon as I can verify it works. Then we can drop in the more robust solution later on and not rewrite the interface contract.

External interfaces are sticky things, and I think the boundary between interpreter and standard library is one of those, even if they are (currently) produced by the same team. So, I'm using the need to document things in CONTRIBUTING as a forcing function to stabilize (and rationalize) the interface. Maybe I'm being too pedantic, or I'm off on a tangent? If you don't agree, don't hesitate to let me know!

[amtoine]

i think the goal is to have

• std for the top-level library to import all
• std log for the log commands
• std assert for the assertion commands
• ...

two things that might change (which i do not think is a problem as this is at a very early stage) though

• std help ..., from the prelude, might be renamed to help ...` if and only if they replace the built-in
• std assert might change a bit with stdlib: complete the test framework #8819

that can sound like a lot, but we do not have that much in the library for now anyway 😋
apart from the prelude, use std ... will probably say i'd say? 😌

[amtoine]

i hope that answers a bit what you said above 😮

we do not have to plan too much, let's keep it growing and fix and define things as we get more contributions 😉

[bobhy]

With the fix from #8841, I'm finding that this does not work:

dirs.nu
export def-env "dirs next" [
    N:int = 1   # number of positions to move.
] {
    use std 'log debug'

    log debug $"position was ($env.DIRS_POSITION)"
    _fetch $N    
    log debug $"position is now ($env.DIRS_POSITION)"
    
}

I get the 'module not found' error.

But if I change use std 'log debug' to use log 'log debug', it does work.

Can't let the perfect be the enemy of the good. I think this is good enough for now.

I'll document this as one rule for stdlib developers (use log 'log ...', in CONTRIBUTING) and a second rule for ordinary script module developers (use std 'log ...', README). If that's still wrong, you'll correct me in the PR review.

[amtoine]

yeahh... this is the namespace issue mentionned in the roadmap 👀

[kubouch]

@bobhy The file cache would allow you to use use log.nu from inside std.nu loaded inside Nushell without the files being on the disk. It would need to be marked as a "virtual file" or something like that which would bypass the otherwise mandatory check whether the file contents have changed (they cannot change because it's read-only). At least that's my idea how to solve this problem that doesn't require redesigning the current module system.