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

fix(init): Support dots in final path item #1425

Merged
merged 2 commits into from
May 9, 2024
Merged

fix(init): Support dots in final path item #1425

merged 2 commits into from
May 9, 2024

Commits on May 9, 2024

  1. fix(init): Add integration test for file path 

    Test the existing behaviour of running `init` against a path that is a
file because I don't want to affect the user facing error message with
an intermediate error when implementing support for `-d .`
    @dcarley
    dcarley committed May 9, 2024
    Configuration menu
    Copy the full SHA
    6f1ee6f View commit details
    Browse the repository at this point in the history

  2. fix(init): Support dots in final path item 

    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
    @dcarley
    dcarley committed May 9, 2024
    Configuration menu
    Copy the full SHA
    9e361c2 View commit details
    Browse the repository at this point in the history