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

Support defining external justfile locations through env var #1849

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ semver = "1.0.20"
serde = { version = "1.0.130", features = ["derive", "rc"] }
serde_json = "1.0.68"
sha2 = "0.10"
shellexpand = "3.1.0"
similar = { version = "2.1.0", features = ["unicode"] }
snafu = "0.8.0"
strum = { version = "0.26.0", features = ["derive"] }
Expand Down
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2759,7 +2759,8 @@ A

The `import` path can be absolute or relative to the location of the justfile
containing it. A leading `~/` in the import path is replaced with the current
users home directory.
users home directory. Any variable used in the import path is replaced with
the value of an environment variable of the same name.

Justfiles are insensitive to order, so included files can reference variables
and recipes defined after the `import` statement.
Expand Down Expand Up @@ -2831,7 +2832,8 @@ mod foo 'PATH'

Which loads the module's source file from `PATH`, instead of from the usual
locations. A leading `~/` in `PATH` is replaced with the current user's home
directory.
directory. Any variable used in `PATH` is replaced with the value of an
environment variable of the same name.

Environment files are only loaded for the root justfile, and loaded environment
variables are available in submodules. Settings in submodules that affect
Expand Down
23 changes: 10 additions & 13 deletions src/compiler.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use super::*;
use {
super::*,
shellexpand::full,
};

pub(crate) struct Compiler;

Expand Down Expand Up @@ -48,7 +51,9 @@ impl Compiler {
let parent = current.path.parent().unwrap();

let import = if let Some(relative) = relative {
let path = parent.join(Self::expand_tilde(&relative.cooked)?);
let expanded_path = full(&relative.cooked).map_err(|error| Error::UnresolvedVariableInModule { variable: error.var_name, module: *name })?;

let path = parent.join(&*expanded_path);

if path.is_file() {
Some(path)
Expand Down Expand Up @@ -78,11 +83,13 @@ impl Compiler {
optional,
path,
} => {
let expanded_path = full(&relative.cooked).map_err(|error| Error::UnresolvedVariableInImport { variable: error.var_name, path: *path })?;

let import = current
.path
.parent()
.unwrap()
.join(Self::expand_tilde(&relative.cooked)?)
.join(&*expanded_path)
.lexiclean();
pprkut marked this conversation as resolved.
Show resolved Hide resolved

if import.is_file() {
Expand Down Expand Up @@ -155,16 +162,6 @@ impl Compiler {
}
}

fn expand_tilde(path: &str) -> RunResult<'static, PathBuf> {
Ok(if let Some(path) = path.strip_prefix("~/") {
dirs::home_dir()
.ok_or(Error::Homedir)?
.join(path.trim_start_matches('/'))
} else {
PathBuf::from(path)
})
}

#[cfg(test)]
pub(crate) fn test_compile(src: &str) -> CompileResult<Justfile> {
let tokens = Lexer::test_lex(src)?;
Expand Down
16 changes: 12 additions & 4 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ pub(crate) enum Error<'src> {
GetConfirmation {
io_error: io::Error,
},
Homedir,
InitExists {
justfile: PathBuf,
},
Expand Down Expand Up @@ -160,6 +159,14 @@ pub(crate) enum Error<'src> {
recipes: Vec<String>,
suggestion: Option<Suggestion<'src>>,
},
UnresolvedVariableInImport {
variable: String,
path: Token<'src>,
},
UnresolvedVariableInModule {
variable: String,
module: Name<'src>,
},
Unstable {
message: String,
},
Expand Down Expand Up @@ -191,6 +198,8 @@ impl<'src> Error<'src> {
Self::Compile { compile_error } => Some(compile_error.context()),
Self::FunctionCall { function, .. } => Some(function.token),
Self::MissingImportFile { path } => Some(*path),
Self::UnresolvedVariableInImport { variable: _, path } => Some(*path),
Self::UnresolvedVariableInModule { variable: _, module } => Some(module.token),
_ => None,
}
}
Expand Down Expand Up @@ -361,9 +370,6 @@ impl<'src> ColorDisplay for Error<'src> {
GetConfirmation { io_error } => {
write!(f, "Failed to read confirmation from stdin: {io_error}")?;
}
Homedir => {
write!(f, "Failed to get homedir")?;
}
InitExists { justfile } => {
write!(f, "Justfile `{}` already exists", justfile.display())?;
}
Expand Down Expand Up @@ -433,6 +439,8 @@ impl<'src> ColorDisplay for Error<'src> {
write!(f, "\n{suggestion}")?;
}
}
UnresolvedVariableInImport { variable, .. } => write!(f, "Path to file contains unresolved variable {variable}.")?,
UnresolvedVariableInModule { variable, module } => write!(f, "Source path for {module} contains unresolved variable {variable}.")?,
Unstable { message } => {
write!(f, "{message} Invoke `just` with the `--unstable` flag to enable unstable features.")?;
}
Expand Down
44 changes: 43 additions & 1 deletion tests/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ fn variables_in_import_are_overridden_by_variables_in_parent() {

#[cfg(not(windows))]
#[test]
fn import_paths_beginning_with_tilde_are_expanded_to_homdir() {
fn import_paths_beginning_with_tilde_are_expanded_to_homedir() {
Test::new()
.write("foobar/mod.just", "foo:\n @echo FOOBAR")
.justfile(
Expand All @@ -239,6 +239,48 @@ fn import_paths_beginning_with_tilde_are_expanded_to_homdir() {
.run();
}

#[test]
fn import_paths_with_variables_are_expanded() {
Test::new()
.write("foobar/mod.just", "foo:\n @echo FOOBAR")
.justfile(
"
import '$FOO/mod.just'
",
)
.test_round_trip(false)
.arg("foo")
.stdout("FOOBAR\n")
.env("FOO", "foobar")
.run();
}

#[test]
fn unresolved_variable_in_import_error() {
Test::new()
.justfile(
"
import '$FOO/import.justfile'

a:
@echo A
",
)
.test_round_trip(false)
.arg("a")
.status(EXIT_FAILURE)
.stderr(
"
error: Path to file contains unresolved variable FOO.
——▶ justfile:1:8
1 │ import '$FOO/import.justfile'
│ ^^^^^^^^^^^^^^^^^^^^^^
",
)
.run();
}

#[test]
fn imports_dump_correctly() {
Test::new()
Expand Down
43 changes: 42 additions & 1 deletion tests/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ fn submodule_shebang_recipes_run_in_submodule_directory() {

#[cfg(not(windows))]
#[test]
fn module_paths_beginning_with_tilde_are_expanded_to_homdir() {
fn module_paths_beginning_with_tilde_are_expanded_to_homedir() {
Test::new()
.write("foobar/mod.just", "foo:\n @echo FOOBAR")
.justfile(
Expand All @@ -695,6 +695,47 @@ fn module_paths_beginning_with_tilde_are_expanded_to_homdir() {
.run();
}

#[test]
fn module_paths_with_variables_are_expanded() {
Test::new()
.write("foobar/mod.just", "foo:\n @echo FOOBAR")
.justfile(
"
mod foo '$FOO/mod.just'
",
)
.test_round_trip(false)
.arg("--unstable")
.arg("foo")
.arg("foo")
.stdout("FOOBAR\n")
.env("FOO", "foobar")
.run();
}

#[test]
fn unresolved_variable_in_module_error() {
Test::new()
.justfile(
"
mod foo '$FOO/mod.just'
",
)
.test_round_trip(false)
.arg("--unstable")
.status(EXIT_FAILURE)
.stderr(
"
error: Source path for foo contains unresolved variable FOO.
——▶ justfile:1:5
1 │ mod foo '$FOO/mod.just'
│ ^^^
Comment on lines +732 to +733
Copy link
Author

Choose a reason for hiding this comment

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

The pointers here would be nicer on the path, but I couldn't figure out how to get to a token from relative

Copy link
Owner

Choose a reason for hiding this comment

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

Relative is an Option, but in the case of an error, you know there's a path, so you can unwrap it.

Copy link
Author

Choose a reason for hiding this comment

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

It's already unwrapped. The difficulty I face is that relative is a StringLiteral, and I need a Token in the error handling to position the pointers. I don't know how to get back from the literal string to the token.
I thought about perhaps using the module token and iterating to the next token after (something like module.token.next), but I don't see an easy way to do it this way either 😕

Copy link
Author

Choose a reason for hiding this comment

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

@casey Any remaining ideas on this? Otherwise I keep it as is 🙁

Copy link
Owner

Choose a reason for hiding this comment

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

The mod parser uses parse_string_literal, but there's also parse_string_literal token which returns both the string literal and the token, if you store the token in mod in addition to the string literal, you can use it in the error message.

",
)
.run();
}

#[test]
fn module_recipe_list_alignment_ignores_private_recipes() {
Test::new()
Expand Down
Loading