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

Gitoxide fails to load binary configuration on windows 11 with git 2.22 #790

Closed
1 task done
pascalkuthe opened this issue Mar 25, 2023 · 1 comment
Closed
1 task done

Comments

@pascalkuthe
Copy link
Contributor

Duplicates

  • I have searched the existing issues

Current behavior 😯

First reported in helix-editor/helix#6358

Opening a repository fails if the system git version is 2.22 on windows 11 (probably other windows versions too) when gitoxide trys to read configuration from the system git executable fails with the following error:

0: Failed to load the git configuration
1: Could not read configuration file
2: The filename, directory name, or volume label syntax is incorrect. (os error 123)'

Installing git 2.40 fixes the problem

Expected behavior 🤔

  • Properly load the configuration with git 2.22
  • don't fail to open the repo because shelling out to git fails, simply log an error instead. Implementing a retry downstream would be cumbersome and necessarily slow (repo discovery would run twice). If falling is still desired by default maybe a config option could be added?

Steps to reproduce 🕹

  1. on windows install git 2.22
  2. try to open a git repository with helix (log failures in the git integration helix-editor/helix#6441) and observe the error in the cli, git integration is not working.

Failing function using gitoxide:

fn open_repo(path: &Path, ceiling_dir: Option<&Path>) -> Result<ThreadSafeRepository> {
        let mut git_open_opts_map = gix::sec::trust::Mapping::<gix::open::Options>::default();
        let config = gix::permissions::Config {
            system: true,
            git: true,
            user: true,
            env: true,
            includes: true,
            git_binary: cfg!(windows),
        };
        git_open_opts_map.reduced = git_open_opts_map.reduced.permissions(gix::Permissions {
            config,
            ..gix::Permissions::default_for_level(gix::sec::Trust::Reduced)
        });
        git_open_opts_map.full = git_open_opts_map.full.permissions(gix::Permissions {
            config,
            ..gix::Permissions::default_for_level(gix::sec::Trust::Full)
        });
        let mut open_options = gix::discover::upwards::Options::default();
        if let Some(ceiling_dir) = ceiling_dir {
            open_options.ceiling_dirs = vec![ceiling_dir.to_owned()];
        }
        let res = ThreadSafeRepository::discover_with_environment_overrides_opts(
            path,
            open_options,
            git_open_opts_map,
        )?;
        Ok(res)
    }
Byron added a commit that referenced this issue Mar 26, 2023
)

Sometimes, IO errors can occour consistently when reading from certain
paths (on Windows, really), and it should be possible to not only see
which path that is but also to ignore them entirely.

Now IO errors contain the path of the failed configuration file.
There is also a new option flag to ignore errors (off by default).
Byron added a commit that referenced this issue Mar 26, 2023
…s. (#790)

These will instead be logged, but won't make it impossible to open an
otherwise fine repository.
Byron added a commit that referenced this issue Mar 26, 2023
#790)

Previously we would return paths that contained quotes.
Note that we don't properly unquote C-style strings
(which this is: https://github.com/git/git/blob/d9d677b2d8cc5f70499db04e633ba7a400f64cbf/builtin/config.c#L197)
thinking that the git-binary configuration paths are well-known and don't need the complete decoding.
If so, this is already implemented in `gix_quote::ansi_c::undo()`.
@Byron
Copy link
Member

Byron commented Mar 26, 2023

Thanks a lot for letting me know!

This issue has been fixed in gix 0.43 so no change should be required. But for extra caution, it will now also ignore IO errors by default and log them, unless strict mode is enabled.

@Byron Byron closed this as completed Mar 26, 2023
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