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.

Sign up for GitHub

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

Jump to bottom

Set argv[0] to .roc file passed to 'roc run' #7172

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Set argv[0] to .roc file passed to 'roc run' #7172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 33 additions & 8 deletions crates/cli/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,15 @@ pub fn build(
// ManuallyDrop will leak the bytes because we don't drop manually
let bytes = &ManuallyDrop::new(std::fs::read(&binary_path).unwrap());

roc_run(&arena, opt_level, target, args, bytes, expect_metadata)
roc_run(
&arena,
path,
opt_level,
target,
args,
bytes,
expect_metadata,
)
}
BuildAndRunIfNoErrors => {
if problems.fatally_errored {
Expand Down Expand Up @@ -1021,7 +1029,15 @@ pub fn build(
// ManuallyDrop will leak the bytes because we don't drop manually
let bytes = &ManuallyDrop::new(std::fs::read(&binary_path).unwrap());

roc_run(&arena, opt_level, target, args, bytes, expect_metadata)
roc_run(
&arena,
path,
opt_level,
target,
args,
bytes,
expect_metadata,
)
}
}
}
Expand All @@ -1034,6 +1050,7 @@ pub fn build(

fn roc_run<'a, I: IntoIterator<Item = &'a OsStr>>(
arena: &Bump,
script_path: &Path,
opt_level: OptLevel,
target: Target,
args: I,
Expand Down Expand Up @@ -1073,7 +1090,14 @@ fn roc_run<'a, I: IntoIterator<Item = &'a OsStr>>(

Ok(0)
}
_ => roc_run_native(arena, opt_level, args, binary_bytes, expect_metadata),
_ => roc_run_native(
arena,
script_path,
opt_level,
args,
binary_bytes,
expect_metadata,
),
}
}

Expand All @@ -1090,16 +1114,15 @@ fn os_str_as_utf8_bytes(os_str: &OsStr) -> &[u8] {

fn make_argv_envp<'a, I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
arena: &'a Bump,
executable: &ExecutableFile,
script_path: &Path,
args: I,
) -> (
bumpalo::collections::Vec<'a, CString>,
bumpalo::collections::Vec<'a, CString>,
) {
use bumpalo::collections::CollectIn;

let path = executable.as_path();
let path_cstring = CString::new(os_str_as_utf8_bytes(path.as_os_str())).unwrap();
let path_cstring = CString::new(os_str_as_utf8_bytes(script_path.as_os_str())).unwrap();

// argv is an array of pointers to strings passed to the new program
// as its command-line arguments. By convention, the first of these
Expand Down Expand Up @@ -1137,6 +1160,7 @@ fn make_argv_envp<'a, I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
#[cfg(target_family = "unix")]
fn roc_run_native<I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
arena: &Bump,
script_path: &Path,
opt_level: OptLevel,
args: I,
binary_bytes: &[u8],
Expand All @@ -1145,7 +1169,7 @@ fn roc_run_native<I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
use bumpalo::collections::CollectIn;

let executable = roc_run_executable_file_path(binary_bytes)?;
let (argv_cstrings, envp_cstrings) = make_argv_envp(arena, &executable, args);
let (argv_cstrings, envp_cstrings) = make_argv_envp(arena, script_path, args);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the added tests, it looks like a script in /home/myuser/Documents/myscript.roc would have the relative path included along with the name, but I think we should prefer just the file_name (a.k.a. myscript.roc) since that's what will be wanted to display 99% of the time.

Copy link
Contributor Author

@jwoudenberg jwoudenberg Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the relative name is important if we want to use argv[0] to look up the script location in the application.

For instance, if I'm in /home/jasper and run: roc ./projects/foobar/main.roc, if argv[0] only tells me I'm running main.roc, then in the implementation of main.roc I can't know where the script is located, for instance to read some data.json file that's supposed to be next to the script file.

Bash works the same way btw! Try put this script somewhere:

#!/usr/bin/env bash
echo "$0"

And run it from a couple different places to observe the result.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, if we're doing what bash is doing, I'm happy.


let argv: bumpalo::collections::Vec<*const c_char> = argv_cstrings
.iter()
Expand Down Expand Up @@ -1400,6 +1424,7 @@ fn roc_run_executable_file_path(binary_bytes: &[u8]) -> std::io::Result<Executab
#[cfg(not(target_family = "unix"))]
fn roc_run_native<I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
arena: &Bump, // This should be passed an owned value, not a reference, so we can usefully mem::forget it!
script_path: &Path,
opt_level: OptLevel,
args: I,
binary_bytes: &[u8],
Expand All @@ -1411,7 +1436,7 @@ fn roc_run_native<I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
let executable = roc_run_executable_file_path(binary_bytes)?;

// TODO forward the arguments
let (argv_cstrings, envp_cstrings) = make_argv_envp(&arena, &executable, args);
let (argv_cstrings, envp_cstrings) = make_argv_envp(&arena, script_path, args);

let argv: bumpalo::collections::Vec<*const c_char> = argv_cstrings
.iter()
Expand Down
12 changes: 12 additions & 0 deletions crates/cli/tests/cli/argv0.roc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
app [main] {
pf: platform "https://github.com/roc-lang/basic-cli/releases/download/0.15.0/SlwdbJ-3GR7uBWQo6zlmYWNYOxnvo8r6YABXD-45UOw.tar.br",
}

import pf.Stdout
import pf.Arg

main =
args = Arg.list! {}
when List.first args is
Ok argv0 -> Stdout.line argv0
Err ListWasEmpty -> Stdout.line "Failed: argv was empty"
31 changes: 24 additions & 7 deletions crates/cli/tests/cli_run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ mod cli_run {
}
};

let self_path = file.display().to_string();
let self_path = file.parent().unwrap().display().to_string();

let actual_cmd_stdout = ignore_test_timings(&strip_colors(&cmd_output.stdout))
.replace(&self_path, "<ignored for tests>");
Expand Down Expand Up @@ -573,12 +573,12 @@ mod cli_run {
words : List Str
words = ["this", "will", "for", "sure", "be", "a", "large", "string", "so", "when", "we", "split", "it", "it", "will", "use", "seamless", "slices", "which", "affect", "printing"]

[<ignored for tests>:31] x = 42
[<ignored for tests>:33] "Fjoer en ferdjer frieten oan dyn geve lea" = "Fjoer en ferdjer frieten oan dyn geve lea"
[<ignored for tests>:35] "this is line 24" = "this is line 24"
[<ignored for tests>:21] x = "abc"
[<ignored for tests>:21] x = 10
[<ignored for tests>:21] x = (A (B C))
[<ignored for tests>/expects.roc:31] x = 42
[<ignored for tests>/expects.roc:33] "Fjoer en ferdjer frieten oan dyn geve lea" = "Fjoer en ferdjer frieten oan dyn geve lea"
[<ignored for tests>/expects.roc:35] "this is line 24" = "this is line 24"
[<ignored for tests>/expects.roc:21] x = "abc"
[<ignored for tests>/expects.roc:21] x = 10
[<ignored for tests>/expects.roc:21] x = (A (B C))
Program finished!
"#
),
Expand Down Expand Up @@ -1203,6 +1203,23 @@ mod cli_run {
)
}

#[test]
#[serial(cli_platform)]
#[cfg_attr(windows, ignore)]
#[ignore = "broken because of a bug in basic-cli: https://github.com/roc-lang/basic-cli/issues/82"]
fn argv0() {
test_roc_app(
"crates/cli/tests/cli",
"argv0.roc",
&[],
&[],
&[],
"<ignored for tests>/argv0.roc\n",
UseValgrind::No,
TestCliCommands::Run,
)
}

#[test]
#[serial(cli_platform)]
#[cfg_attr(windows, ignore)]
Expand Down