Config get_xdg() and and get_global() weird state behaviour...

[bradwood]

Hi all,

I have this function:

fn get_user_config_type() -> Option<UserGitConfigLevel> {

    if GitConfig::find_xdg().is_ok() {
        Some(UserGitConfigLevel::XDG)
    } else {
        Some(UserGitConfigLevel::Global)
    }
}

And I have these 2 tests:

    #[test]
    fn test_get_user_config_type_xdg() {
        // set up XDG
        let home = assert_fs::TempDir::new().unwrap().into_persistent();
        env::set_var("HOME", home.path()); //fake home directory
        env::set_var("XDG_CONFIG_HOME", home.child(".config").path()); //fake XDG config directory
        std::fs::create_dir_all(home.child(".config/git").path()).unwrap(); // create the actual XDG config dir
        std::fs::write(home.child(".config/git/config").path(),"").unwrap(); // touch the xdg config file

        let config_type = get_user_config_type();

        assert_eq!(config_type.unwrap(), UserGitConfigLevel::XDG);

        home.close().unwrap()
    }

    #[test]
    fn test_get_user_config_type_global() {
        // set up home dir
        let home = assert_fs::TempDir::new().unwrap().into_persistent();
        env::set_var("HOME", home.path()); //fake home directory
        std::fs::write(home.child(".gitconfig").path(),"").unwrap(); // touch the xdg config file

        let config_type = get_user_config_type();

        assert_eq!(config_type.unwrap(), UserGitConfigLevel::Global);

        home.close().unwrap()
    }

When I call each test individually, like this, they both succeed:

git-lab-rust! ❯ cargo test get_user_config_type_xdg -- --test-threads=1 --show-output 
    Finished test [unoptimized + debuginfo] target(s) in 0.06s
     Running target/debug/deps/git_lab-088586bafc7dced7

running 1 test
test config::config_unit_tests::test_get_user_config_type_xdg ... ok

successes:

successes:
    config::config_unit_tests::test_get_user_config_type_xdg

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 35 filtered out

     Running target/debug/deps/cli_invocation_tests-a5abebe7e09b566f

running 0 tests

successes:

successes:

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out

git-lab-rust ❯ cargo test get_user_config_type_global -- --test-threads=1 --show-output 
    Finished test [unoptimized + debuginfo] target(s) in 0.06s
     Running target/debug/deps/git_lab-088586bafc7dced7

running 1 test
test config::config_unit_tests::test_get_user_config_type_global ... ok

successes:

successes:
    config::config_unit_tests::test_get_user_config_type_global

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 35 filtered out

     Running target/debug/deps/cli_invocation_tests-a5abebe7e09b566f

running 0 tests

successes:

successes:

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out

git-lab-rust ❯

However, when I run them both in one cargo test invocation, I always get one failure:

git-lab-rust! ❯ cargo test get_user_config_type -- --test-threads=1 --show-output 
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running target/debug/deps/git_lab-088586bafc7dced7

running 2 tests
test config::config_unit_tests::test_get_user_config_type_global ... ok
test config::config_unit_tests::test_get_user_config_type_xdg ... FAILED

successes:

successes:
    config::config_unit_tests::test_get_user_config_type_global

failures:

---- config::config_unit_tests::test_get_user_config_type_xdg stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `Global`,
 right: `XDG`', src/config.rs:493:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    config::config_unit_tests::test_get_user_config_type_xdg

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 34 filtered out

error: test failed, to rerun pass '--bin git-lab'
git-lab-rust! ❯ 

Can anyone explain why this is? There seems to be some state which is persisting when both tests are run in one invocation which seems completely weird and maybe is a bug...

Any thoughts or suggestions to make these tests work together would be much appreciated.

[bradwood]

I tried by unsetting the env vars, but this makes no difference:

    #[test]
    fn test_get_user_config_type_xdg() {
        // set up XDG
        let home = assert_fs::TempDir::new().unwrap().into_persistent();
        env::set_var("HOME", home.path()); //fake home directory
        env::set_var("XDG_CONFIG_HOME", home.child(".config").path()); //fake XDG config directory
        std::fs::create_dir_all(home.child(".config/git").path()).unwrap(); // create the actual XDG config dir
        std::fs::write(home.child(".config/git/config").path(),"").unwrap(); // touch the xdg config file

        let config_type = get_user_config_type();

        assert_eq!(config_type.unwrap(), UserGitConfigLevel::XDG);

        env::set_var("HOME", ""); //unset home directory
        env::set_var("XDG_CONFIG_HOME", ""); //unset XDG config directory
        home.close().unwrap()
    }

    #[test]
    fn test_get_user_config_type_global() {
        // set up home dir
        let home = assert_fs::TempDir::new().unwrap().into_persistent();
        env::set_var("HOME", home.path()); //fake home directory
        std::fs::write(home.child(".gitconfig").path(),"").unwrap(); // touch the global config file

        let config_type = get_user_config_type();

        assert_eq!(config_type.unwrap(), UserGitConfigLevel::Global);

        env::set_var("HOME", ""); //unset home directory
        env::set_var("XDG_CONFIG_HOME", ""); //unset XDG config directory
        home.close().unwrap()
    }

[ehuss]

Environment variables are inherently global. You can maybe try with --test-threads=1, or write a test without a test harness to control concurrency yourself.

[bradwood]

I already use --test-threads=1 and am unsetting at the end of the test... I think something else is wrong here...

[ehuss]

I don't think env::set_var("XDG_CONFIG_HOME", ""); //unset XDG config directory unsets the variable, it just sets it to an empty string. remove_var is needed to unset it.

[bradwood]

RIght... On slack someone else pointed this out, so I did it... It was an oversight on my part...

Here it is now:

    #[test]
    fn test_get_user_config_type_xdg() {
        // set up XDG
        let home = assert_fs::TempDir::new().unwrap().into_persistent();
        env::set_var("HOME", home.path()); //fake home directory
        env::set_var("XDG_CONFIG_HOME", home.child(".config").path()); //fake XDG config directory
        std::fs::create_dir_all(home.child(".config/git").path()).unwrap(); // create the actual XDG config dir
        std::fs::write(home.child(".config/git/config").path(),"").unwrap(); // touch the xdg config file

        let config_type = get_user_config_type();

        assert_eq!(config_type.unwrap(), UserGitConfigLevel::XDG);

        env::remove_var("HOME"); //unset HOME directory
        env::remove_var("XDG_CONFIG_HOME"); //unset XDG config directory
        home.close().unwrap()
    }

    #[test]
    fn test_get_user_config_type_global() {
        // set up home dir
        let home = assert_fs::TempDir::new().unwrap().into_persistent();
        env::set_var("HOME", home.path()); //fake home directory
        std::fs::write(home.child(".gitconfig").path(),"").unwrap(); // touch the global config file

        let config_type = get_user_config_type();

        assert_eq!(config_type.unwrap(), UserGitConfigLevel::Global);

        env::remove_var("HOME"); //unset HOME directory
        home.close().unwrap()
    }

Still no change in behaviour though... This has me properly stumped... :-/

[ehuss]

Yea, it does look like libgit2 caches the system directories in a global (https://github.com/libgit2/libgit2/blob/918a7d19553a9a4181bf1561b8b828a5f82e60e0/src/sysdir.c#L184-L195). I'm not sure if it will be possible to swap HOME around like that.

[bradwood]

Ok, I also heard the caching thing mentioned on libgit2's slack channel. This is just for the test, not the actual code it's testing, but is there some way to perhaps restart, clear, or otherwise reset rust's binding to the underlying lib, to force this cache to reinitialise?

[ehuss]

I suspect the answer is "no".

[bradwood]

So with a bit of reworking I've made my test suite work without the need to change $HOME or $XDG_CONFIG_HOME midway through test execution. I think the documentation for the Config object should be updated to make this clearer... I'd be happy to submit a small PR to add a bit more detail here if anyone thinks that's worthwhile?

[ehuss]

Seems reasonable to mention!

[cruessler]

@bradwood At extrawurst/gitui#269, we were having the same issue, and it took us some digging to uncover that libgit2 does not respond to changes in environment variables, specifically $HOME. So I’d say it’d be a useful addition, too.

[extrawurst]

git_libgit2_shutdown is actually cleaning up that cache but depending on libgit2-sys directly is verboten. @alexcrichton would you be open to a PR adding such a shutdown method, that would allow us to cover more stuff in unittests that depend on environment variables?

[alexcrichton]

Sorry I'm confused, how is the env var changes related to git_libgit2_shutdown?

[extrawurst]

@alexcrichton it clears the caching of the env vars.

[extrawurst]

@alexcrichton any opinion?

[alexcrichton]

Sorry I haven't dug much into this issue. Calling shutdown is likely highly unsafe since it tampers with global state that may be used by other threads. Exposing a method for it seems possible as long as it's unsafe and tested to work.