ogham / exa

A modern replacement for ‘ls’.
https://the.exa.website/
MIT License
23.59k stars 660 forks source link

`GIT_CEILING_DIRECTORIES` is not respected #1226

Closed mentalisttraceur closed 1 year ago

mentalisttraceur commented 1 year ago

Problem

When running a command with the --git option, it seems that GIT_CEILING_DIRECTORIES is not respected.

This can lead to huge delays when exa looks past a ceiling directory, finds a .git in some larger parent, and checks that entire larger tree for Git changes.

Reproducible Example

The easiest repro is probably to temporarily make your home directory into a git repo, and see what that does to your execution time, with or without GIT_CEILING_DIRECTORIES=~ in the environment:

mkdir ~/test &&
cd ~/test &&
time exa --all --long --git  # should be fast

cd ~ &&
git init &&
git commit --allow-empty -m 'test' &&
cd ~/test &&
time exa --all --long --git  # should be slow

export GIT_CEILING_DIRECTORIES=~ &&
time exa --all --long --git  # should be fast again

This is great for a test because your home directory is probably big enough to see the slowdown. (Just don't forget to rm -rf ~/.git when done, unless you're into that sort of thing... if you've just discovered you might be, I recommend git config status.showUntrackedFiles no and export GIT_CEILING_DIRECTORIES=~.)

Misc.

Output of exa --version (but I would guess this happens with any version):

exa - list files on the command-line
v0.10.1 [+git]
https://the.exa.website/

Observed on the following OS+hardware combinations (but I would guess it this happens on any OS and any hardware):

  1. Debian Linux on x86-64 CPU (Librem 14).
  2. Debian in WSL2 in Windows 11 on x86-64 CPU (different laptop).
  3. Termux in Android on Aarch64 CPU (two different phone models).
mentalisttraceur commented 1 year ago

Workaround?

Until this is fixed, we can only partially work around it as users. In the general case it's impractical since exa could for example get separate paths as arguments where some some are in Git repos leafwards of a Git ceiling dir while others are in Git repos rootwards of a Git ceiling dir.

[Click if you want more thoughts on developing a workaround, but I think it's not worth it - it's probably easier to just make 1-2 line change in Exa itself, even if you have to manually patch and build Exa from source.] Here's the smallest+simplest example workaround, which only works when correctly running `exa` on the current working directory (no paths specified): assuming you have Exa installed at `/use/bin/exa`, create a wrapper script like this, and put it somewhere else in your path: ```sh #!/bin/sh - if ! git rev-parse --git-common-dir 1>/dev/null 2>&1 then for argument in "$@" do shift if test "$argument" != '--git' then set -- "$@" "$argument" fi done fi exec /usr/bin/exa "$@" ``` Basically we first check if we're in a Git repo with the `git` CLI, and if that search fails (due to hitting a ceiling directory or otherwise), we remove any occurrences of the `--git` flag from Exa's arguments. (The simplest way to remove arguments in portable shell script is to cycle through the arguments, using the argument list like a FIFO queue: at each iteration, we pop each argument from the beginning of the list, and then only push it back to the end if we're keeping it; it's safe to mutate the argument list while looping on arguments because the `for argument in "$@"` is evaluated only once, before the iterations start.) One improvement might be to actually iterate on the arguments twice, and on the first loop, just check every non-option argument to see if it's in a Git repo accordingly to `git`. Then strip the `--git` option, depending on your preference, either if any of them aren't, or all of them aren't. An even more precise workaround would have to check if we're in the a Git repo *twice* - once with Git ceiling directories respected and a second time without, because we actually only need to disable `--git` if the former is false and the latter is true (if we're only working around for a single target path at once, this is needless extra work, but if we're trying to create a workaround which usually does the right thing when given multiple paths, and removes the `--git` option whenever ceiling directories would be violated, this would help us correctly detect only the cases where `--git` would do the wrong thing). Of course, the other angle to a complete workaround is to run a separate `exa` for every path argument (and automatically print the little separators/headers like `exa` would. To do that workaround as a shell script would require either a shell with arrays or some really convoluted argument list manipulation (or programmatically escaping the argument list so that you can safely `eval` it back later) - at that point it's probably better for most people to just use some other language to write the workaround wrapper. That would still not do the right thing in the case of a recursive listing starting outside of a ceiling directory and descending into it, I think. [Edit: it looks like Exa natively doesn't know how to switch the git column on or off as it recurses, but in principle it could one day, and this is a great example of something that an external workaround would really struggle to match.]
mentalisttraceur commented 1 year ago

Solution

Turns out the fix is just changing line 165 in git.rs from

        let repo = match git2::Repository::discover(&path) {

to

        let flags = git2::RepositoryOpenFlags::FROM_ENV;
        let repo = match git2::Repository::open_ext(&path, flags, &[] as &[&OsStr]) {
mentalisttraceur commented 1 year ago

The solution in my last comment builds fine, passes cargo test, worked correctly in all of my manual testing, and I'm now running a build of Exa with this patch on all of my devices.

The way I initially wrote it does cause a "trivial cast" warning (due to the cast of &[], since the open_ext template can't infer the type of an empty slice literal argument):

warning: trivial cast: `&[&OsStr; 0]` as `&[&OsStr]`
   --> src/fs/feature/git.rs:166:67
    |
166 |         let repo = match git2::Repository::open_ext(&path, flags, &[] as &[&OsStr]) {
    |                                                                   ^^^^^^^^^^^^^^^^
    |
    = help: cast can be replaced by coercion; this might require a temporary variable
note: the lint level is defined here
   --> src/main.rs:6:9
    |
6   | #![warn(trivial_casts, trivial_numeric_casts)]
    |         ^^^^^^^^^^^^^

Here's the best refactor I could quickly come up with to avoid that lint rule:

        let flags = git2::RepositoryOpenFlags::FROM_ENV;
        let unused: [&OsStr; 0] = [];
        let repo = match git2::Repository::open_ext(&path, flags, &unused) {

One objective advantage of writing it that way is that the code now explicitly hints that the third parameter of open_ext is ignored in this case.

cafkafk commented 1 year ago

@ariasuni also this!

ariasuni commented 1 year ago

Closing this since exa is unmaintained (see https://github.com/ogham/exa/issues/1243), and this has been done in the active fork eza.