refactor

- move environment variable up to git-repository as plumbing crate can't
  deal with git-config style values and it's uncommon enough to not
  make that functionality available more easily in git-config.

Author: Byron

Cargo.lock

@@ -14,7 +14,6 @@ doctest = false
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-git-config = { version = "^0.4.0", path = "../git-config" }
 git-sec = { version = "^0.1.2", path = "../git-sec", features = ["thiserror"] }
 git-path = { version = "^0.1.3", path = "../git-path" }
 git-ref = { version = "^0.13.0", path = "../git-ref" }

git-discover/Cargo.toml

@@ -1,7 +1,5 @@
 use bstr::{ByteSlice, ByteVec};
-use git_config::values::Boolean as GitBoolean;
 use std::borrow::Cow;
-use std::convert::TryFrom;
 use std::env;
 use std::path::PathBuf;
 
@@ -68,23 +66,16 @@ impl Options {
     ///
     /// The environment variables are:
     /// - `GIT_CEILING_DIRECTORIES` for `ceiling_dirs`
-    /// - `GIT_DISCOVERY_ACROSS_FILESYSTEM` for `cross_fs`
+    ///
+    /// Note that `GIT_DISCOVERY_ACROSS_FILESYSTEM` for `cross_fs` is **not** read,
+    /// as it requires parsing of `git-config` style boolean values.
     // TODO: test
-    pub fn apply_environment(&mut self) {
-        /// Gets the values of an environment variable as bytes.
-        fn get_env_bytes(name: &str) -> Option<Vec<u8>> {
-            env::var_os(name).and_then(|c| Vec::from_os_string(c).ok())
-        }
-
-        if let Some(ceiling_dirs) = get_env_bytes("GIT_CEILING_DIRECTORIES") {
+    pub fn apply_environment(mut self) -> Self {
+        let name = "GIT_CEILING_DIRECTORIES";
+        if let Some(ceiling_dirs) = env::var_os(name).and_then(|c| Vec::from_os_string(c).ok()) {
             self.ceiling_dirs = parse_ceiling_dirs(&ceiling_dirs);
         }
-
-        if let Some(cross_fs) = get_env_bytes("GIT_DISCOVERY_ACROSS_FILESYSTEM") {
-            if let Ok(b) = GitBoolean::try_from(cross_fs) {
-                self.cross_fs = b.to_bool();
-            }
-        }
+        self
     }
 }
 

git-discover/src/upwards.rs

@@ -423,18 +423,37 @@ pub mod discover {
 
         /// Try to open a git repository directly from the environment, which reads `GIT_DIR`
         /// if it is set. If unset, discover upwards from `directory` until one is found,
-        /// while applying `options` with overrides from the environment.
+        /// while applying `options` with overrides from the environment which includes:
         ///
-        /// Then use the `trust_map` to determine which of our own repository options to use.
+        /// - `GIT_DISCOVERY_ACROSS_FILESYSTEM`
+        /// - `GIT_CEILING_DIRECTORIES`
+        ///
+        /// Finally, use the `trust_map` to determine which of our own repository options to use
+        /// based on the trust level of the effective repository directory.
         pub fn discover_with_environment_overrides_opts(
             directory: impl AsRef<Path>,
             mut options: upwards::Options,
             trust_map: git_sec::trust::Mapping<crate::open::Options>,
         ) -> Result<Self, Error> {
+            fn apply_additional_environment(mut opts: upwards::Options) -> upwards::Options {
+                use crate::bstr::ByteVec;
+                use std::convert::TryFrom;
+
+                if let Some(cross_fs) =
+                    std::env::var_os("GIT_DISCOVERY_ACROSS_FILESYSTEM").and_then(|v| Vec::from_os_string(v).ok())
+                {
+                    if let Ok(b) = git_config::values::Boolean::try_from(cross_fs) {
+                        opts.cross_fs = b.to_bool();
+                    }
+                }
+                opts
+            }
+
             if std::env::var_os("GIT_DIR").is_some() {
                 return Self::open_with_environment_overrides(directory.as_ref(), trust_map).map_err(Error::Open);
             }
-            options.apply_environment();
+
+            options = apply_additional_environment(options.apply_environment());
             Self::discover_opts(directory, options, trust_map)
         }
     }