Skip to content

Commit

Permalink
fix(init): Support dots in final path item (#1425)
Browse files Browse the repository at this point in the history
## Proposed Changes

Resolve any dots in paths when determining the environment name so that
`init -d` works when the final path item is a single or double dot.

We can't use `canonicalize` because it will return an error if the path
doesn't exits which breaks our existing behaviour for init against a
directory that doesn't yet exist:

-
https://doc.rust-lang.org/std/path/struct.Path.html#method.canonicalize

The following implementations all don't resolve a single `.` to a name:

- https://doc.rust-lang.org/std/path/fn.absolute.html
- https://docs.rs/path-absolutize/latest/path_absolutize/
- https://docs.rs/path-clean/latest/path_clean/
-
https://github.com/rust-lang/cargo/blob/0.79.0/crates/cargo-util/src/paths.rs#L76-L109

So the `path-dedot` crate seemed like the most appropriate choice.

I considered adding unit tests for this behaviour but the integration
tests are actually very fast for this functionality, make it simpler to
test changing current working directory in the context of running the
CLI, and give us some reassurance that we haven't affected the
downstream behaviour of creating an environment.

Output of the new tests before the implementation was added:

✗ c2.1: `flox init` with `--dir .` will create an environment in current
working directory. [37]
       tags: init
(from function `assert_success' in file
/nix/store/dhghqmkjk7fqpdbz71clvbh5zxp6p3lf-bats-with-libraries-1.10.0/share/bats/bats-assert/src/assert_success.bash,
line 42,
        in test file init.bats, line 115)
         `assert_success' failed
       ❌ ERROR: Can't init in root

       -- command failed --
       status : 1
       output : ❌ ERROR: Can't init in root
       --

       Last output:
       ❌ ERROR: Can't init in root
✗ c2.1: `flox init` with `--dir ..` will create an environment in parent
working directory. [39]
       tags: init
(from function `assert_success' in file
/nix/store/dhghqmkjk7fqpdbz71clvbh5zxp6p3lf-bats-with-libraries-1.10.0/share/bats/bats-assert/src/assert_success.bash,
line 42,
        in test file init.bats, line 125)
         `assert_success' failed

/var/folders/8q/spckhr654cv4xrcv0fxsrlvc0000gn/T/nix-shell.VLMq5i/bats-run-ZpmkUP/test/9/test/other
/var/folders/8q/spckhr654cv4xrcv0fxsrlvc0000gn/T/nix-shell.VLMq5i/bats-run-ZpmkUP/test/9/test
/var/folders/8q/spckhr654cv4xrcv0fxsrlvc0000gn/T/nix-shell.VLMq5i/flox-cli-tests-2WdbV0
       ❌ ERROR: Can't init in root

       -- command failed --
       status : 1
       output : ❌ ERROR: Can't init in root
       --

       Last output:
       ❌ ERROR: Can't init in root

## Release Notes

Support `flox init -d` where the final part of the path is a `.` or
`..`.
  • Loading branch information
dcarley authored and limeytexan committed May 14, 2024
1 parent 386be51 commit 261ac86
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 0 deletions.
10 changes: 10 additions & 0 deletions cli/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 cli/flox/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ tracing.workspace = true
url.workspace = true
uuid.workspace = true
xdg.workspace = true
path-dedot = "3.1.1"

[dev-dependencies]
pretty_assertions.workspace = true
Expand Down
2 changes: 2 additions & 0 deletions cli/flox/src/commands/init/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use flox_rust_sdk::models::pkgdb::scrape_input;
use flox_rust_sdk::models::search::{do_search, PathOrJson, Query, SearchParams, SearchResult};
use indoc::formatdoc;
use log::debug;
use path_dedot::ParseDot;
use toml_edit::{DocumentMut, Formatted, Item, Table, Value};
use tracing::instrument;

Expand Down Expand Up @@ -79,6 +80,7 @@ impl Init {
EnvironmentName::from_str(DEFAULT_NAME)?
} else {
let name = dir
.parse_dot()?
.file_name()
.map(|n| n.to_string_lossy().to_string())
.context("Can't init in root")?;
Expand Down
30 changes: 30 additions & 0 deletions cli/tests/init.bats
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,28 @@ function check_with_dir() {
assert_output ".flox"
}

@test "c2.1: \`flox init\` with \`--dir .\` will create an environment in current working directory." {
run "$FLOX_BIN" init -d .
assert_success
run ls -A
assert_output ".flox"
}

@test "c2.1: \`flox init\` with \`--dir ..\` will create an environment in parent working directory." {
mkdir -p "$PROJECT_DIR/other"

pushd other
run "$FLOX_BIN" init -d ..
assert_success
popd

run ls -A
assert_output - <<EOF
.flox
other
EOF
}

@test "c2.1: \`flox init\` with \`--dir <path>\` will create an environment in \`<path>\`. (relative)" {
mkdir -p "$PROJECT_DIR/other"

Expand All @@ -118,6 +140,14 @@ function check_with_dir() {
check_with_dir
}

@test "c2.1: \`flox init\` with \`--dir <path>\` will not create an environment where \`<path>\` is a file" {
touch "$PROJECT_DIR/other"

run "$FLOX_BIN" init -d ./other
assert_failure
assert_line --partial "Could not prepare a '.flox' directory: Not a directory"
}

@test "c2.1: \`flox init\` with \`--dir <path>\` will create an environment in \`<path>\`. (absolute)" {
mkdir -p "$PROJECT_DIR/other"

Expand Down

0 comments on commit 261ac86

Please sign in to comment.