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

Zoxide stores the symlink targets #80

Closed
aruhier opened this issue Jun 1, 2020 · 3 comments
Closed

Zoxide stores the symlink targets #80

aruhier opened this issue Jun 1, 2020 · 3 comments

Comments

@aruhier
Copy link
Contributor

aruhier commented Jun 1, 2020

Hi,
Giving this example, with one symlink to another directory:

$ ls ~/tmp
  foo -> foo-target
  foo-target

If I cd into ~/tmp/foo, zoxide will follow the symlink and stores this target (foo-target) in its database. So if I do:

$ zq foo
/home/anthony/tmp/foo-target

Which is a bit non-intuitive for me. Do you see a possibility of adding an option to ignore symlinks, and consider them as normal folders?

@aruhier
Copy link
Contributor Author

aruhier commented Jun 7, 2020

@ajeetdsouza: FYI, I'm working on this feature and will do a PR soon.

I was just a bit disappointed that std::fs doesn't have a function that gives the absolute path of a target without resolving the symlinks, so I had to quickly workaround it.

@ajeetdsouza
Copy link
Owner

ajeetdsouza commented Jun 8, 2020

@aruhier I've done some work on this too. It turns out that finding normalized paths including symlinks is a non-trivial task - there was even a PR to add it into Rust at one point, but it was closed because it could not pass all the test cases due to Path::components not being 100% accurate.

The most obvious approach would be to manually parse and write your own path normalizer. This is easy on Linux; on Windows, not so much. See this to know how messy it can get. On the bright side, this is slightly simplified by the fact that we don't have to handle UNC paths.

However, there are easier ways of doing this (I think), especially since we do want to interactive with the filesystem to validate the existence of the resultant path.

I came up with the following (admittedly hacky) ways of solving this:

  • On Windows, env::set_current_dir() followed by env::current_dir() seems to return a normalized path without resolving symlinks, and fails if the path isn't valid - perfect for our use case.

  • However, env::current_dir() uses getcwd() under the hood, which on POSIX, is required to resolve symlinks. The upside here is that most shells also set the PWD environment variable, which does not resolve symlinks. So, we can validate PWD as an absolute path and fallback to env::current_dir() if the path is invalid. This is exactly what the pwd utility does.

  • Alternatively, we can solve this problem on POSIX systems by imitating realpath -ms, adding a validation check to see if the resultant path currently exists.

Do let me know your thoughts.

@aruhier
Copy link
Contributor Author

aruhier commented Jun 8, 2020

I came up with the following (admittedly hacky) ways of solving this:

* On Windows, `env::set_current_dir()` followed by `env::current_dir()` seems to return a normalized path without resolving symlinks, and fails if the path isn't valid - perfect for our use case.

* However, `env::current_dir()` uses `getcwd()` under the hood, which on POSIX, is required to resolve symlinks. The upside here is that most shells also set the `PWD` environment variable, which does not resolve symlinks. So, we can validate `PWD` as an absolute path and fallback to `env::current_dir()` if the path is invalid. This is exactly what the [`pwd`](https://github.com/wertarbyte/coreutils/blob/master/src/pwd.c) utility does.

That's exactly what I wanted to do on my side. Here is what I wrote for the moment to get the absolute_path from a base_dir:

pub fn absolute_path<P: AsRef<Path>>(base_dir: &P, path: &P) -> Result<PathBuf> {
    let path = path.as_ref();
    let mut abs_path : PathBuf;

    // If path is an absolute path, base_dir is not needed.
    // "absolute" does not mean the path is clean (can be "/foo/../bar"), cleaning the path is still needed.
    if path.is_absolute() {
        abs_path = PathBuf::from(Component::RootDir.as_os_str())
    } else {
        abs_path = PathBuf::from(base_dir.as_ref());
    }

    for c in path.components() {
        match c {
            Component::CurDir | Component::RootDir => {},
            Component::ParentDir => {
                if abs_path.parent() != None {
                    abs_path.pop();
                }
            }
            _ => {
                abs_path.push(PathBuf::from(c.as_os_str()));
            }
        }
    }

    Ok(abs_path)
}

I planned to send env::current_dir() for the base_dir, and clean the path for windows with https://docs.rs/dunce/1.0.1/dunce/fn.simplified.html.

Edit: words.

ajeetdsouza pushed a commit that referenced this issue Jul 3, 2020
Fixes #80.

Disable by default the symlinks resolution, making a symlink and its
target 2 different entries in the database. Adds the
_ZO_RESOLVE_SYMLINKS env variable to re-enable it.

Example:
  /tmp/foo-target is a directory
  /tmp/foo symlinks to /tmp/foo-target

With _ZO_RESOLVE_SYMLINKS=1, `z add /tmp/foo` adds `/tmp/foo-target` in the database.

With _ZO_RESOLVE_SYMLINKS=0 or unset, `z add /tmp/foo` adds `/tmp/foo` in the database.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants