Merge branch 'adjustments-for-cargo'

Byron · Byron · commit 94750e158319 · 2022-12-11T19:43:08.000+01:00
diff --git a/git-discover/src/is.rs b/git-discover/src/is.rs
@@ -7,7 +7,7 @@ use crate::{DOT_GIT_DIR, MODULES};
 /// Please note that repositories without an index generally _look_ bare, even though they might also be uninitialized.
 pub fn bare(git_dir_candidate: impl AsRef<Path>) -> bool {
     let git_dir = git_dir_candidate.as_ref();
-    !(git_dir.join("index").exists() || (git_dir.file_name() == Some(OsStr::new(DOT_GIT_DIR)) && git_dir.is_file()))
+    !(git_dir.join("index").exists() || (git_dir.file_name() == Some(OsStr::new(DOT_GIT_DIR))))
 }
 
 /// Returns true if `git_dir` is is located within a `.git/modules` directory, indicating it's a submodule clone.
@@ -130,7 +130,6 @@ pub fn git(git_dir: impl AsRef<Path>) -> Result<crate::repository::Kind, crate::
             return Err(crate::is_git::Error::MissingRefsDirectory { missing: refs_path });
         }
     }
-
     Ok(match kind {
         Kind::LinkedWorkTreeDir => crate::repository::Kind::WorkTree {
             linked_git_dir: Some(dot_git.into_owned()),
diff --git a/git-discover/tests/fixtures/make_basic_repo.sh b/git-discover/tests/fixtures/make_basic_repo.sh
@@ -26,3 +26,24 @@ rm -R worktrees/c-worktree-deleted
   git worktree add ../worktrees/from-bare/d-private-dir-deleted
   rm -R -v ./worktrees/d-private-dir-deleted
 )
+
+git clone --bare --shared . bare-no-config.git
+(cd bare-no-config.git
+  rm config
+)
+
+git init --bare bare-no-config-after-init.git
+(cd bare-no-config-after-init.git
+  rm config
+)
+
+git clone --shared . worktree-no-config
+(cd worktree-no-config
+  rm .git/config
+)
+
+git init worktree-no-config-after-init
+(cd worktree-no-config-after-init
+  rm .git/config
+)
+
diff --git a/git-discover/tests/is_git/mod.rs b/git-discover/tests/is_git/mod.rs
@@ -1,3 +1,5 @@
+use crate::upwards::repo_path;
+
 #[cfg(target_os = "macos")]
 #[test]
 fn verify_on_exfat() -> crate::Result<()> {
@@ -37,3 +39,23 @@ fn verify_on_exfat() -> crate::Result<()> {
     );
     Ok(())
 }
+
+#[test]
+fn missing_configuration_file_is_not_a_dealbreaker_in_bare_repo() -> crate::Result {
+    for name in ["bare-no-config-after-init.git", "bare-no-config.git"] {
+        let repo = repo_path()?.join(name);
+        let kind = git_discover::is_git(repo)?;
+        assert_eq!(kind, git_discover::repository::Kind::Bare);
+    }
+    Ok(())
+}
+
+#[test]
+fn missing_configuration_file_is_not_a_dealbreaker_in_nonbare_repo() -> crate::Result {
+    for name in ["worktree-no-config-after-init/.git", "worktree-no-config/.git"] {
+        let repo = repo_path()?.join(name);
+        let kind = git_discover::is_git(&repo)?;
+        assert_eq!(kind, git_discover::repository::Kind::WorkTree { linked_git_dir: None });
+    }
+    Ok(())
+}
diff --git a/git-discover/tests/upwards/mod.rs b/git-discover/tests/upwards/mod.rs
@@ -22,6 +22,18 @@ fn from_bare_git_dir() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn from_bare_git_dir_without_config_file() -> crate::Result {
+    for name in ["bare-no-config.git", "bare-no-config-after-init.git"] {
+        let dir = repo_path()?.join(name);
+        let (path, trust) = git_discover::upwards(&dir)?;
+        assert_eq!(path.as_ref(), dir, "the bare .git dir is directly returned");
+        assert_eq!(path.kind(), Kind::Bare);
+        assert_eq!(trust, expected_trust());
+    }
+    Ok(())
+}
+
 #[test]
 fn from_inside_bare_git_dir() -> crate::Result {
     let git_dir = repo_path()?.join("bare.git");
@@ -61,6 +73,18 @@ fn from_working_dir() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn from_working_dir_no_config() -> crate::Result {
+    for name in ["worktree-no-config-after-init", "worktree-no-config"] {
+        let dir = repo_path()?.join(name);
+        let (path, trust) = git_discover::upwards(&dir)?;
+        assert_eq!(path.kind(), Kind::WorkTree { linked_git_dir: None },);
+        assert_eq!(path.as_ref(), dir, "a working tree dir yields the git dir");
+        assert_eq!(trust, expected_trust());
+    }
+    Ok(())
+}
+
 #[test]
 fn from_nested_dir() -> crate::Result {
     let working_dir = repo_path()?;
@@ -328,6 +352,6 @@ mod submodules {
     }
 }
 
-fn repo_path() -> crate::Result<PathBuf> {
+pub(crate) fn repo_path() -> crate::Result<PathBuf> {
     git_testtools::scripted_fixture_repo_read_only("make_basic_repo.sh")
 }
diff --git a/git-protocol/src/handshake/function.rs b/git-protocol/src/handshake/function.rs
@@ -42,10 +42,10 @@ where
             Ok(v) => Ok(v),
             Err(client::Error::Io(ref err)) if err.kind() == std::io::ErrorKind::PermissionDenied => {
                 drop(result); // needed to workaround this: https://github.com/rust-lang/rust/issues/76149
-                let url = transport.to_url();
+                let url = transport.to_url().into_owned();
                 progress.set_name("authentication");
                 let credentials::protocol::Outcome { identity, next } =
-                    authenticate(credentials::helper::Action::get_for_url(url.into_owned()))?
+                    authenticate(credentials::helper::Action::get_for_url(url.clone()))?
                         .expect("FILL provides an identity or errors");
                 transport.set_identity(identity)?;
                 progress.step();
@@ -58,7 +58,7 @@ where
                     // Still no permission? Reject the credentials.
                     Err(client::Error::Io(err)) if err.kind() == std::io::ErrorKind::PermissionDenied => {
                         authenticate(next.erase())?;
-                        Err(client::Error::Io(err))
+                        return Err(Error::InvalidCredentials { url });
                     }
                     // Otherwise, do nothing, as we don't know if it actually got to try the credentials.
                     // If they were previously stored, they remain. In the worst case, the user has to enter them again
diff --git a/git-protocol/src/handshake/mod.rs b/git-protocol/src/handshake/mod.rs
@@ -57,6 +57,7 @@ pub struct Outcome {
 }
 
 mod error {
+    use bstr::BString;
     use git_transport::client;
 
     use crate::{credentials, handshake::refs};
@@ -65,8 +66,10 @@ mod error {
     #[derive(Debug, thiserror::Error)]
     #[allow(missing_docs)]
     pub enum Error {
-        #[error(transparent)]
+        #[error("Failed to obtain credentials")]
         Credentials(#[from] credentials::protocol::Error),
+        #[error("Credentials provided for \"{url}\" were not accepted by the remote")]
+        InvalidCredentials { url: BString },
         #[error(transparent)]
         Transport(#[from] client::Error),
         #[error("The transport didn't accept the advertised server version {actual_version:?} and closed the connection client side")]
diff --git a/git-repository/src/config/cache/incubate.rs b/git-repository/src/config/cache/incubate.rs
@@ -30,7 +30,9 @@ impl StageOne {
             lossy,
         )?;
 
-        let is_bare = util::config_bool(&config, "core.bare", false, lenient)?;
+        // Note that we assume the repo is bare by default unless we are told otherwise. This is relevant if
+        // the repo doesn't have a configuration file.
+        let is_bare = util::config_bool(&config, "core.bare", true, lenient)?;
         let repo_format_version = config
             .value::<git_config::Integer>("core", None, "repositoryFormatVersion")
             .map_or(0, |v| v.to_decimal().unwrap_or_default());
@@ -82,7 +84,12 @@ fn load_config(
     lossy: Option<bool>,
 ) -> Result<git_config::File<'static>, Error> {
     buf.clear();
-    std::io::copy(&mut std::fs::File::open(&config_path)?, buf)?;
+    let mut file = match std::fs::File::open(&config_path) {
+        Ok(f) => f,
+        Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(git_config::File::default()),
+        Err(err) => return Err(err.into()),
+    };
+    std::io::copy(&mut file, buf)?;
 
     let config = git_config::File::from_bytes_owned(
         buf,
diff --git a/git-repository/src/config/cache/init.rs b/git-repository/src/config/cache/init.rs
@@ -90,7 +90,7 @@ impl Cache {
                             "HOME" => Some(home_env),
                             _ => None,
                         }
-                        .and_then(|perm| std::env::var_os(name).and_then(|val| perm.check_opt(val)))
+                        .and_then(|perm| perm.check_opt(name).and_then(std::env::var_os))
                     })
                     .map(|p| (source, p.into_owned()))
             })
diff --git a/git-repository/src/open/mod.rs b/git-repository/src/open/mod.rs
@@ -33,8 +33,11 @@ pub struct Options {
 pub enum Error {
     #[error("Failed to load the git configuration")]
     Config(#[from] config::Error),
-    #[error(transparent)]
-    NotARepository(#[from] git_discover::is_git::Error),
+    #[error("\"{path}\" does not appear to be a git repository")]
+    NotARepository {
+        source: git_discover::is_git::Error,
+        path: PathBuf,
+    },
     #[error(transparent)]
     Io(#[from] std::io::Error),
     #[error("The git directory at '{}' is considered unsafe as it's not owned by the current user.", .path.display())]
diff --git a/git-repository/src/open/repository.rs b/git-repository/src/open/repository.rs
@@ -52,7 +52,9 @@ impl ThreadSafeRepository {
                 Ok(kind) => (path, kind),
                 Err(_err) => {
                     let git_dir = path.join(git_discover::DOT_GIT_DIR);
-                    git_discover::is_git(&git_dir).map(|kind| (git_dir, kind))?
+                    git_discover::is_git(&git_dir)
+                        .map(|kind| (git_dir, kind))
+                        .map_err(|err| Error::NotARepository { source: err, path })?
                 }
             }
         };
@@ -85,10 +87,20 @@ impl ThreadSafeRepository {
     ) -> Result<Self, Error> {
         let overrides = EnvironmentOverrides::from_env()?;
         let (path, path_kind): (PathBuf, _) = match overrides.git_dir {
-            Some(git_dir) => git_discover::is_git(&git_dir).map(|kind| (git_dir, kind))?,
+            Some(git_dir) => git_discover::is_git(&git_dir)
+                .map_err(|err| Error::NotARepository {
+                    source: err,
+                    path: git_dir.clone(),
+                })
+                .map(|kind| (git_dir, kind))?,
             None => {
                 let fallback_directory = fallback_directory.into();
-                git_discover::is_git(&fallback_directory).map(|kind| (fallback_directory, kind))?
+                git_discover::is_git(&fallback_directory)
+                    .map_err(|err| Error::NotARepository {
+                        source: err,
+                        path: fallback_directory.clone(),
+                    })
+                    .map(|kind| (fallback_directory, kind))?
             }
         };
 
diff --git a/git-repository/src/remote/connection/access.rs b/git-repository/src/remote/connection/access.rs
@@ -12,6 +12,10 @@ impl<'a, 'repo, T, P> Connection<'a, 'repo, T, P> {
     /// user.
     ///
     /// A custom function may also be used to prevent accessing resources with authentication.
+    ///
+    /// Use the [configured_credentials()][Connection::configured_credentials()] method to obtain the implementation
+    /// that would otherwise be used, which can be useful to proxy the default configuration and obtain information about the
+    /// URLs to authenticate with.
     pub fn with_credentials(
         mut self,
         helper: impl FnMut(git_credentials::helper::Action) -> git_credentials::protocol::Result + 'a,
diff --git a/git-repository/src/repository/worktree.rs b/git-repository/src/repository/worktree.rs
@@ -50,7 +50,7 @@ impl crate::Repository {
     /// This is not to be confused with the [`worktree()`][crate::Repository::worktree()] worktree, which may exists if this instance
     /// was opened in a worktree that was created separately.
     pub fn is_bare(&self) -> bool {
-        self.config.is_bare
+        self.config.is_bare && self.work_dir().is_none()
     }
 
     /// Open a new copy of the index file and decode it entirely.
diff --git a/git-repository/tests/fixtures/generated-archives/make_config_repos.tar.xz b/git-repository/tests/fixtures/generated-archives/make_config_repos.tar.xz
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:3d893d90b1870819d92b25f84214d10e7565f6cab71032959659f284642dc5a4
-size 13808
+oid sha256:f90e312e56b67e6da1596b444bb4a266238b28b7ca6744b59f650ab86ec195fa
+size 14404
diff --git a/git-repository/tests/fixtures/make_config_repos.sh b/git-repository/tests/fixtures/make_config_repos.sh
@@ -111,3 +111,21 @@ git init disabled-object-caches
   git config core.deltaBaseCacheLimit 0
   git config gitoxide.objects.cacheLimit 0
 )
+
+git init --bare bare-no-config
+(cd bare-no-config
+  rm config
+  git config -l >/dev/null
+)
+
+git init worktree-no-config
+(cd worktree-no-config
+  rm .git/config
+  git config -l >/dev/null
+)
+
+mkdir not-a-repo-empty-dir;
+mkdir not-a-repo-with-files;
+(cd not-a-repo-with-files
+  touch this that
+)
diff --git a/git-repository/tests/repository/open.rs b/git-repository/tests/repository/open.rs
@@ -1,3 +1,51 @@
+mod missing_config_file {
+    use crate::util::named_subrepo_opts;
+    use git_repository as git;
+
+    #[test]
+    fn bare() -> crate::Result {
+        let repo = named_subrepo_opts("make_config_repos.sh", "bare-no-config", git::open::Options::isolated())?;
+        assert!(
+            repo.is_bare(),
+            "without config, we can't really know what the repo is actually but can guess by not having a worktree"
+        );
+        assert_eq!(repo.work_dir(), None);
+        assert!(repo.worktree().is_none());
+        Ok(())
+    }
+
+    #[test]
+    fn non_bare() -> crate::Result {
+        let repo = named_subrepo_opts(
+            "make_config_repos.sh",
+            "worktree-no-config",
+            git::open::Options::isolated(),
+        )?;
+        assert!(repo.work_dir().is_some());
+        assert!(repo.worktree().is_some());
+        assert!(
+            !repo.is_bare(),
+            "without config, we can't really know what the repo is actually but can guess as there is a worktree"
+        );
+        Ok(())
+    }
+}
+
+mod not_a_repository {
+    use git_repository as git;
+
+    #[test]
+    fn shows_proper_error() -> crate::Result {
+        for name in ["empty-dir", "with-files"] {
+            let name = format!("not-a-repo-{name}");
+            let repo_path = git_testtools::scripted_fixture_repo_read_only("make_config_repos.sh")?.join(name);
+            let err = git::open_opts(&repo_path, git::open::Options::isolated()).unwrap_err();
+            assert!(matches!(err, git::open::Error::NotARepository { path, .. } if path == repo_path));
+        }
+        Ok(())
+    }
+}
+
 mod submodules {
     use std::path::Path;
 
diff --git a/git-transport/src/client/blocking_io/http/curl/mod.rs b/git-transport/src/client/blocking_io/http/curl/mod.rs
@@ -22,23 +22,25 @@ pub enum Error {
 impl crate::IsSpuriousError for Error {
     fn is_spurious(&self) -> bool {
         match self {
-            Error::Curl(err) => {
-                err.is_couldnt_connect()
-                    || err.is_couldnt_resolve_proxy()
-                    || err.is_couldnt_resolve_host()
-                    || err.is_operation_timedout()
-                    || err.is_recv_error()
-                    || err.is_send_error()
-                    || err.is_http2_error()
-                    || err.is_http2_stream_error()
-                    || err.is_ssl_connect_error()
-                    || err.is_partial_file()
-            }
+            Error::Curl(err) => curl_is_spurious(err),
             _ => false,
         }
     }
 }
 
+pub fn curl_is_spurious(err: &curl::Error) -> bool {
+    err.is_couldnt_connect()
+        || err.is_couldnt_resolve_proxy()
+        || err.is_couldnt_resolve_host()
+        || err.is_operation_timedout()
+        || err.is_recv_error()
+        || err.is_send_error()
+        || err.is_http2_error()
+        || err.is_http2_stream_error()
+        || err.is_ssl_connect_error()
+        || err.is_partial_file()
+}
+
 pub struct Curl {
     req: SyncSender<remote::Request>,
     res: Receiver<remote::Response>,
diff --git a/git-transport/src/client/blocking_io/http/curl/remote.rs b/git-transport/src/client/blocking_io/http/curl/remote.rs
diff --git a/git-transport/src/client/blocking_io/http/mod.rs b/git-transport/src/client/blocking_io/http/mod.rs
diff --git a/git-transport/src/client/blocking_io/http/reqwest/remote.rs b/git-transport/src/client/blocking_io/http/reqwest/remote.rs
diff --git a/git-transport/tests/blocking-transport-http.rs b/git-transport/tests/blocking-transport-http.rs
diff --git a/git-transport/tests/client/blocking_io/http/mod.rs b/git-transport/tests/client/blocking_io/http/mod.rs

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ use crate::{DOT_GIT_DIR, MODULES};`
`7`	`7`	`/// Please note that repositories without an index generally _look_ bare, even though they might also be uninitialized.`
`8`	`8`	`pub fn bare(git_dir_candidate: impl AsRef<Path>) -> bool {`
`9`	`9`	`let git_dir = git_dir_candidate.as_ref();`
`10`		`- !(git_dir.join("index").exists() \|\| (git_dir.file_name() == Some(OsStr::new(DOT_GIT_DIR)) && git_dir.is_file()))`
	`10`	`+ !(git_dir.join("index").exists() \|\| (git_dir.file_name() == Some(OsStr::new(DOT_GIT_DIR))))`
`11`	`11`	`}`
`12`	`12`
`13`	`13`	/// Returns true if `git_dir` is is located within a `.git/modules` directory, indicating it's a submodule clone.
`@@ -130,7 +130,6 @@ pub fn git(git_dir: impl AsRef<Path>) -> Result<crate::repository::Kind, crate::`
`130`	`130`	`return Err(crate::is_git::Error::MissingRefsDirectory { missing: refs_path });`
`131`	`131`	`}`
`132`	`132`	`}`
`133`		`-`
`134`	`133`	`Ok(match kind {`
`135`	`134`	`Kind::LinkedWorkTreeDir => crate::repository::Kind::WorkTree {`
`136`	`135`	`linked_git_dir: Some(dot_git.into_owned()),`
Original file line number	Diff line number	Diff line change
`@@ -90,7 +90,7 @@ impl Cache {`
`90`	`90`	`"HOME" => Some(home_env),`
`91`	`91`	`_ => None,`
`92`	`92`	`}`
`93`		`- .and_then(\|perm\| std::env::var_os(name).and_then(\|val\| perm.check_opt(val)))`
	`93`	`+ .and_then(\|perm\| perm.check_opt(name).and_then(std::env::var_os))`
`94`	`94`	`})`
`95`	`95`	`.map(\|p\| (source, p.into_owned()))`
`96`	`96`	`})`