Extensions to rmdir and mkdir utilities

* Rework GIT_DIRREMOVAL values to GIT_RMDIR flags, allowing
  combinations of flags
* Add GIT_RMDIR_EMPTY_PARENTS flag to remove parent dirs that
  are left empty after removal
* Add GIT_MKDIR_VERIFY_DIR to give an error if item is a file,
  not a dir (previously an EEXISTS error was ignored, even for
  files) and enable this flag for git_futils_mkpath2file call
* Improve accuracy of error messages from git_futils_mkdir

6 files changed, 134 insertions, 60 deletions

diff --git a/src/checkout.c b/src/checkout.c
index e068e4f5f..b9a5399cc 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -213,7 +213,7 @@ static int checkout_remove_the_old(
 		data->error = git_futils_rmdir_r(
 			delta->new_file.path,
 			git_repository_workdir(data->owner),
-			GIT_DIRREMOVAL_FILES_AND_DIRS);
+			GIT_RMDIR_REMOVE_FILES | GIT_RMDIR_EMPTY_PARENTS);
 
 		data->completed_steps++;
 		report_progress(data, delta->new_file.path);
diff --git a/src/clone.c b/src/clone.c
index 7352f5fb2..560d64c0d 100644
--- a/src/clone.c
+++ b/src/clone.c
@@ -338,7 +338,7 @@ static int clone_internal(
 						fetch_progress_cb, fetch_progress_payload)) < 0) {
 			/* Failed to fetch; clean up */
 			git_repository_free(repo);
-			git_futils_rmdir_r(path, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS);
+			git_futils_rmdir_r(path, NULL, GIT_RMDIR_REMOVE_FILES);
 		} else {
 			*out = repo;
 			retcode = 0;
diff --git a/src/fileops.c b/src/fileops.c
index 2aceb112a..d2cbd046d 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -14,7 +14,8 @@
 int git_futils_mkpath2file(const char *file_path, const mode_t mode)
 {
 	return git_futils_mkdir(
-		file_path, NULL, mode, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST);
+		file_path, NULL, mode,
+		GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST | GIT_MKDIR_VERIFY_DIR);
 }
 
 int git_futils_mktmp(git_buf *path_out, const char *filename)
@@ -250,6 +251,7 @@ int git_futils_mkdir(
 	mode_t mode,
 	uint32_t flags)
 {
+	int error = -1;
 	git_buf make_path = GIT_BUF_INIT;
 	ssize_t root = 0;
 	char lastch, *tail;
@@ -297,12 +299,28 @@ int git_futils_mkdir(
 		*tail = '\0';
 
 		/* make directory */
-		if (p_mkdir(make_path.ptr, mode) < 0 &&
-			(errno != EEXIST || (flags & GIT_MKDIR_EXCL) != 0))
-		{
-			giterr_set(GITERR_OS, "Failed to make directory '%s'",
-				make_path.ptr);
-			goto fail;
+		if (p_mkdir(make_path.ptr, mode) < 0) {
+			if (errno == EEXIST) {
+				if (!lastch && (flags & GIT_MKDIR_VERIFY_DIR) != 0) {
+					if (!git_path_isdir(make_path.ptr)) {
+						giterr_set(
+							GITERR_OS, "Existing path is not a directory '%s'",
+							make_path.ptr);
+						error = GIT_ENOTFOUND;
+						goto fail;
+					}
+				}
+				if ((flags & GIT_MKDIR_EXCL) != 0) {
+					giterr_set(GITERR_OS, "Directory already exists '%s'",
+						make_path.ptr);
+					error = GIT_EEXISTS;
+					goto fail;
+				}
+			} else {
+				giterr_set(GITERR_OS, "Failed to make directory '%s'",
+					make_path.ptr);
+				goto fail;
+			}
 		}
 
 		/* chmod if requested */
@@ -324,7 +342,7 @@ int git_futils_mkdir(
 
 fail:
 	git_buf_free(&make_path);
-	return -1;
+	return error;
 }
 
 int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode)
@@ -332,57 +350,103 @@ int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode)
 	return git_futils_mkdir(path, base, mode, GIT_MKDIR_PATH);
 }
 
-static int _rmdir_recurs_foreach(void *opaque, git_buf *path)
+typedef struct {
+	uint32_t flags;
+	int error;
+} futils__rmdir_data;
+
+static int futils__error_cannot_rmdir(const char *path, const char *filemsg)
 {
-	git_directory_removal_type removal_type = *(git_directory_removal_type *)opaque;
+	if (filemsg)
+		giterr_set(GITERR_OS, "Could not remove directory. File '%s' %s",
+				   path, filemsg);
+	else
+		giterr_set(GITERR_OS, "Could not remove directory '%s'", path);
 
-	if (git_path_isdir(path->ptr) == true) {
-		if (git_path_direach(path, _rmdir_recurs_foreach, opaque) < 0)
-			return -1;
+	return -1;
+}
 
-		if (p_rmdir(path->ptr) < 0) {
-			if (removal_type == GIT_DIRREMOVAL_ONLY_EMPTY_DIRS && (errno == ENOTEMPTY || errno == EEXIST))
-				return 0;
+static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
+{
+	futils__rmdir_data *data = opaque;
 
-			giterr_set(GITERR_OS, "Could not remove directory '%s'", path->ptr);
-			return -1;
+	if (git_path_isdir(path->ptr) == true) {
+		int error = git_path_direach(path, futils__rmdir_recurs_foreach, data);
+		if (error < 0)
+			return (error == GIT_EUSER) ? data->error : error;
+
+		data->error = p_rmdir(path->ptr);
+
+		if (data->error < 0) {
+			if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) != 0 &&
+				(errno == ENOTEMPTY || errno == EEXIST))
+				data->error = 0;
+			else
+				futils__error_cannot_rmdir(path->ptr, NULL);
 		}
-
-		return 0;
 	}
 
-	if (removal_type == GIT_DIRREMOVAL_FILES_AND_DIRS) {
-		if (p_unlink(path->ptr) < 0) {
-			giterr_set(GITERR_OS, "Could not remove directory.  File '%s' cannot be removed", path->ptr);
-			return -1;
-		}
+	else if ((data->flags & GIT_RMDIR_REMOVE_FILES) != 0) {
+		data->error = p_unlink(path->ptr);
 
-		return 0;
+		if (data->error < 0)
+			futils__error_cannot_rmdir(path->ptr, "cannot be removed");
 	}
 
-	if (removal_type == GIT_DIRREMOVAL_EMPTY_HIERARCHY) {
-		giterr_set(GITERR_OS, "Could not remove directory. File '%s' still present", path->ptr);
-		return -1;
+	else if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) == 0) {
+		data->error = futils__error_cannot_rmdir(path->ptr, "still present");
 	}
 
-	return 0;
+	return data->error;
+}
+
+static int futils__rmdir_empty_parent(void *opaque, git_buf *path)
+{
+	int error = p_rmdir(path->ptr);
+
+	GIT_UNUSED(opaque);
+
+	if (error) {
+		int en = errno;
+
+		if (en == ENOENT || en == ENOTDIR) {
+			giterr_clear();
+			error = 0;
+		} else if (en == ENOTEMPTY || en == EEXIST) {
+			giterr_clear();
+			error = GIT_ITEROVER;
+		} else {
+			futils__error_cannot_rmdir(path->ptr, NULL);
+		}
+	}
+
+	return error;
 }
 
 int git_futils_rmdir_r(
-	const char *path, const char *base, git_directory_removal_type removal_type)
+	const char *path, const char *base, uint32_t flags)
 {
 	int error;
 	git_buf fullpath = GIT_BUF_INIT;
-
-	assert(removal_type == GIT_DIRREMOVAL_EMPTY_HIERARCHY
-		|| removal_type == GIT_DIRREMOVAL_FILES_AND_DIRS
-		|| removal_type == GIT_DIRREMOVAL_ONLY_EMPTY_DIRS);
+	futils__rmdir_data data;
 
 	/* build path and find "root" where we should start calling mkdir */
 	if (git_path_join_unrooted(&fullpath, path, base, NULL) < 0)
 		return -1;
 
-	error = _rmdir_recurs_foreach(&removal_type, &fullpath);
+	data.flags = flags;
+	data.error = 0;
+
+	error = futils__rmdir_recurs_foreach(&data, &fullpath);
+
+	/* remove now-empty parents if requested */
+	if (!error && (flags & GIT_RMDIR_EMPTY_PARENTS) != 0) {
+		error = git_path_walk_up(
+			&fullpath, base, futils__rmdir_empty_parent, &data);
+
+		if (error == GIT_ITEROVER)
+			error = 0;
+	}
 
 	git_buf_free(&fullpath);
 
diff --git a/src/fileops.h b/src/fileops.h
index 25e62c504..6952c463c 100644
--- a/src/fileops.h
+++ b/src/fileops.h
@@ -65,6 +65,7 @@ extern int git_futils_mkdir_r(const char *path, const char *base, const mode_t m
  * * GIT_MKDIR_CHMOD says to chmod the final directory entry after creation
  * * GIT_MKDIR_CHMOD_PATH says to chmod each directory component in the path
  * * GIT_MKDIR_SKIP_LAST says to leave off the last element of the path
+ * * GIT_MKDIR_VERIFY_DIR says confirm final item is a dir, not just EEXIST
  *
  * Note that the chmod options will be executed even if the directory already
  * exists, unless GIT_MKDIR_EXCL is given.
@@ -74,7 +75,8 @@ typedef enum {
 	GIT_MKDIR_PATH = 2,
 	GIT_MKDIR_CHMOD = 4,
 	GIT_MKDIR_CHMOD_PATH = 8,
-	GIT_MKDIR_SKIP_LAST = 16
+	GIT_MKDIR_SKIP_LAST = 16,
+	GIT_MKDIR_VERIFY_DIR = 32,
 } git_futils_mkdir_flags;
 
 /**
@@ -98,27 +100,38 @@ extern int git_futils_mkdir(const char *path, const char *base, mode_t mode, uin
  */
 extern int git_futils_mkpath2file(const char *path, const mode_t mode);
 
+/**
+ * Flags to pass to `git_futils_rmdir_r`.
+ *
+ * * GIT_RMDIR_EMPTY_HIERARCHY - the default; remove hierarchy of empty
+ *       dirs and generate error if any files are found.
+ * * GIT_RMDIR_REMOVE_FILES    - attempt to remove files in the hierarchy.
+ * * GIT_RMDIR_SKIP_NONEMPTY   - skip non-empty directories with no error.
+ * * GIT_RMDIR_EMPTY_PARENTS   - remove containing directories up to base
+ *       if removing this item leaves them empty
+ *
+ * The old values translate into the new as follows:
+ *
+ * * GIT_DIRREMOVAL_EMPTY_HIERARCHY == GIT_RMDIR_EMPTY_HIERARCHY
+ * * GIT_DIRREMOVAL_FILES_AND_DIRS  ~= GIT_RMDIR_REMOVE_FILES
+ * * GIT_DIRREMOVAL_ONLY_EMPTY_DIRS == GIT_RMDIR_SKIP_NONEMPTY
+ */
 typedef enum {
-	GIT_DIRREMOVAL_EMPTY_HIERARCHY = 0,
-	GIT_DIRREMOVAL_FILES_AND_DIRS = 1,
-	GIT_DIRREMOVAL_ONLY_EMPTY_DIRS = 2,
-} git_directory_removal_type;
+	GIT_RMDIR_EMPTY_HIERARCHY = 0,
+	GIT_RMDIR_REMOVE_FILES    = (1 << 0),
+	GIT_RMDIR_SKIP_NONEMPTY   = (1 << 1),
+	GIT_RMDIR_EMPTY_PARENTS   = (1 << 2),
+} git_futils_rmdir_flags;
 
 /**
  * Remove path and any files and directories beneath it.
  *
  * @param path Path to to top level directory to process.
  * @param base Root for relative path.
- * @param removal_type GIT_DIRREMOVAL_EMPTY_HIERARCHY to remove a hierarchy
- *                     of empty directories (will fail if any file is found),
- *                     GIT_DIRREMOVAL_FILES_AND_DIRS to remove a hierarchy of
- *                     files and folders,
- *                     GIT_DIRREMOVAL_ONLY_EMPTY_DIRS to only remove empty
- *                     directories (no failure on file encounter).
- *
+ * @param flags Combination of git_futils_rmdir_flags values
  * @return 0 on success; -1 on error.
  */
-extern int git_futils_rmdir_r(const char *path, const char *base, git_directory_removal_type removal_type);
+extern int git_futils_rmdir_r(const char *path, const char *base, uint32_t flags);
 
 /**
  * Create and open a temporary file with a `_git2_` suffix.
diff --git a/src/reflog.c b/src/reflog.c
index 5d1465eca..0e333aa6f 100644
--- a/src/reflog.c
+++ b/src/reflog.c
@@ -378,7 +378,7 @@ int git_reflog_rename(git_reference *ref, const char *new_name)
 		goto cleanup;
 
 	if (git_path_isdir(git_buf_cstr(&new_path)) && 
-		(git_futils_rmdir_r(git_buf_cstr(&new_path), NULL, GIT_DIRREMOVAL_ONLY_EMPTY_DIRS) < 0))
+		(git_futils_rmdir_r(git_buf_cstr(&new_path), NULL, GIT_RMDIR_SKIP_NONEMPTY) < 0))
 		goto cleanup;
 
 	if (git_futils_mkpath2file(git_buf_cstr(&new_path), GIT_REFLOG_DIR_MODE) < 0)
diff --git a/src/refs.c b/src/refs.c
index bbf30ed9e..97c97563e 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -274,18 +274,15 @@ static int loose_write(git_reference *ref)
 	git_buf ref_path = GIT_BUF_INIT;
 	struct stat st;
 
-	if (git_buf_joinpath(&ref_path, ref->owner->path_repository, ref->name) < 0)
-		return -1;
-
 	/* Remove a possibly existing empty directory hierarchy
 	 * which name would collide with the reference name
 	 */
-	if (git_path_isdir(git_buf_cstr(&ref_path)) &&
-		git_futils_rmdir_r(git_buf_cstr(&ref_path), NULL,
-			GIT_DIRREMOVAL_ONLY_EMPTY_DIRS) < 0) {
-		git_buf_free(&ref_path);
+	if (git_futils_rmdir_r(ref->name, ref->owner->path_repository,
+		GIT_RMDIR_SKIP_NONEMPTY) < 0)
+		return -1;
+
+	if (git_buf_joinpath(&ref_path, ref->owner->path_repository, ref->name) < 0)
 		return -1;
-	}
 
 	if (git_filebuf_open(&file, ref_path.ptr, GIT_FILEBUF_FORCE) < 0) {
 		git_buf_free(&ref_path);