[ref] handle existng empty directories more gracefully…

…and push the cost for checking if it's a directory or not to the error case, instead of doing it beforehand like in `find_one()`.

Sebastian Thiel committed 5 years ago

Commit 0849c701 parent d967e302 files · +40 −25Browse files

Unified · Split

2 files

git-ref/src/store/file/loose/find_one.rs +7 −7
git-ref/src/store/file/loose/reflog.rs +33 −18

git-ref/src/store/file/loose/find_one.rs+7−7

         let mut buf = Vec::new();
         let ref_path = self.ref_path(&relative_path);
-        if ref_path.is_dir() {
+        match std::fs::File::open(&ref_path) {
-            return Ok(None);
-        }
-        match std::fs::File::open(ref_path) {
             Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
-            Err(err) => return Err(err),
+            Err(err) => return if ref_path.is_dir() { Ok(None) } else { Err(err) },
-            Ok(mut file) => file.read_to_end(&mut buf)?,
+            Ok(mut file) => {
+                if let Err(err) = file.read_to_end(&mut buf) {
+                    return if ref_path.is_dir() { Ok(None) } else { Err(err) };
+                }
+            }
         };
         Ok(Some(buf))
     }

git-ref/src/store/file/loose/reflog.rs+33−18

         crate::name::Error: From<E>,
     {
         let name: FullName<'_> = name.try_into().map_err(|err| Error::RefnameValidation(err.into()))?;
-        let file = match std::fs::File::open(self.reflog_path(name)) {
+        let path = self.reflog_path(name);
+        let file = match std::fs::File::open(&path) {
             Ok(file) => file,
             Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None),
-            Err(err) => return Err(err.into()),
+            Err(err) => return if path.is_dir() { Ok(None) } else { Err(err.into()) }, // TODO: a test to validate this
         };
         Ok(Some(log::iter::reverse(file, buf)?))
     }
         crate::name::Error: From<E>,
     {
         let name: FullName<'_> = name.try_into().map_err(|err| Error::RefnameValidation(err.into()))?;
-        match std::fs::File::open(self.reflog_path(name)) {
+        let path = self.reflog_path(name);
+        match std::fs::File::open(&path) {
             Ok(mut file) => {
                 buf.clear();
-                file.read_to_end(buf)?;
+                if let Err(err) = file.read_to_end(buf) {
+                    return if path.is_dir() { Ok(None) } else { Err(err.into()) };
+                }
             }
             Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None),
-            Err(err) => return Err(err.into()),
+            Err(err) => return if path.is_dir() { Ok(None) } else { Err(err.into()) },
         };
         Ok(Some(log::iter::forward(buf)))
     }
                         options.create(true);
                     };
-                    let file_for_appending: Option<std::fs::File> = options
+                    let file_for_appending: Option<std::fs::File> = match options.open(&log_path) {
-                        .open(&log_path)
+                        Ok(f) => Some(f),
-                        .map(Some)
+                        Err(err) if err.kind() == std::io::ErrorKind::NotFound => None,
-                        .or_else(|err| {
+                        Err(err) => {
-                            if err.kind() == std::io::ErrorKind::NotFound {
+                            #[cfg(target_os = "windows")]
-                                Ok(None)
+                            let special_kind = std::io::ErrorKind::PermissionDenied;
+                            #[cfg(not(target_os = "windows"))]
+                            let special_kind = std::io::ErrorKind::Other;
+                            if err.kind() == special_kind {
+                                std::fs::remove_dir(&log_path)
+                                    .and_then(|_| options.open(&log_path))
+                                    .map(Some)
+                                    .map_err(|_| Error::Append {
+                                        err,
+                                        reflog_path: self.reflock_resource_to_log_path(lock),
+                                    })?
                             } else {
-                                Err(err)
+                                return Err(Error::Append {
+                                    err,
+                                    reflog_path: log_path,
+                                });
                             }
-                        })
+                        }
-                        .map_err(|err| Error::Append {
+                    };
-                            err,
-                            reflog_path: log_path,
-                        })?;
                     if let Some(mut file) = file_for_appending {
                         write!(
                 let full_name = "refs/heads/other";
                 let lock = reflock(&store, full_name).unwrap();
                 let reflog_path = store.reflog_path_inner(Path::new(full_name));
-                std::fs::create_dir(&reflog_path).unwrap();
+                std::fs::create_dir_all(&reflog_path).unwrap();
                 store
                     .reflog_create_or_append(