{"thread":{"id":"41516","subject":"[PATCH v2 00/20] Delete directories left empty after ref deletion","startedAt":"2016-02-25T13:15:59Z","lastAt":"2016-02-25T13:16:19Z","messageCount":21,"participants":["Michael Haggerty"],"isPatch":true,"patchVersion":2,"patchTotal":20},"messages":[{"id":"279314","messageId":"cover.1456405698.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":null,"subject":"[PATCH v2 00/20] Delete directories left empty after ref deletion","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:15:59Z","receivedAt":"2016-02-25T13:15:59Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This re-roll addresses a few minor points that were brought up about v1 [1]:\n\n* \"safe_create_leading_directories(): set errno on SCLD_EXISTS\":\n  * Set errno to ENOTDIR rather than EEXIST.\n\n* \"raceproof_create_file(): new function\":\n  * Improve comments.\n\n* \"rename_tmp_log(): use raceproof_create_file()\":\n  * Fix whitespace.\n\n* \"rename_tmp_log(): improve error reporting\":\n  * Fix whitespace.\n\nThis patch series is also available from my GitHub account [2] as\nbranch delete-empty-refs-dirs.\n\nThanks to Junio and Peff for their feedback about v1.\n\nMichael\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/286370\n[2] http://github.com/mhagger/git\n\nMichael Haggerty (20):\n  safe_create_leading_directories_const(): preserve errno\n  safe_create_leading_directories(): set errno on SCLD_EXISTS\n  raceproof_create_file(): new function\n  lock_ref_sha1_basic(): use raceproof_create_file()\n  rename_tmp_log(): use raceproof_create_file()\n  rename_tmp_log(): improve error reporting\n  log_ref_setup(): separate code for create vs non-create\n  log_ref_setup(): improve robustness against races\n  log_ref_setup(): pass the open file descriptor back to the caller\n  log_ref_write_1(): don't depend on logfile\n  log_ref_setup(): manage the name of the reflog file internally\n  log_ref_write_1(): inline function\n  try_remove_empty_parents(): rename parameter \"name\" -> \"refname\"\n  try_remove_empty_parents(): don't trash argument contents\n  try_remove_empty_parents(): don't accommodate consecutive slashes\n  t5505: use \"for-each-ref\" to test for the non-existence of references\n  delete_ref_loose(): derive loose reference path from lock\n  delete_ref_loose(): inline function\n  try_remove_empty_parents(): teach to remove parents of reflogs, too\n  ref_transaction_commit(): clean up empty directories\n\n cache.h               |  52 ++++++-\n refs/files-backend.c  | 370 ++++++++++++++++++++++++++------------------------\n refs/refs-internal.h  |   9 +-\n sha1_file.c           |  77 ++++++++++-\n t/t1400-update-ref.sh |  27 ++++\n t/t5505-remote.sh     |   2 +-\n 6 files changed, 351 insertions(+), 186 deletions(-)\n\n-- \n2.7.0\n"},{"id":"279333","messageId":"1ff7b215e37593e935d8db81ea43c3c864161f71.1456405698.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 01/20] safe_create_leading_directories_const(): preserve errno","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:00Z","receivedAt":"2016-02-25T13:16:00Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Theoretically, free() is allowed to change errno. So preserve the errno\nfrom safe_create_leading_directories() across the call to free().\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n sha1_file.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex aab1872..568120e 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -164,10 +164,14 @@ enum scld_error safe_create_leading_directories(char *path)\n \n enum scld_error safe_create_leading_directories_const(const char *path)\n {\n+\tint save_errno;\n \t/* path points to cache entries, so xstrdup before messing with it */\n \tchar *buf = xstrdup(path);\n \tenum scld_error result = safe_create_leading_directories(buf);\n+\n+\tsave_errno = errno;\n \tfree(buf);\n+\terrno = save_errno;\n \treturn result;\n }\n \n-- \n2.7.0\n"},{"id":"279328","messageId":"bf790258cb69fc63fd6ebaddd249face1ff2bdf6.1456405698.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 02/20] safe_create_leading_directories(): set errno on SCLD_EXISTS","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:01Z","receivedAt":"2016-02-25T13:16:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The exit path for SCLD_EXISTS wasn't setting errno, which some callers\nuse to generate error messages for the user. Fix the problem and\ndocument that the function sets errno correctly to help avoid similar\nregressions in the future.\n\nWhile we're at it, document the difference between\nsafe_create_leading_directories() and\nsafe_create_leading_directories_const().\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n cache.h     | 10 ++++++++--\n sha1_file.c |  4 +++-\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 26640b4..7d3f80c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -950,8 +950,9 @@ int adjust_shared_perm(const char *path);\n \n /*\n  * Create the directory containing the named path, using care to be\n- * somewhat safe against races.  Return one of the scld_error values\n- * to indicate success/failure.\n+ * somewhat safe against races. Return one of the scld_error values to\n+ * indicate success/failure. On error, set errno to describe the\n+ * problem.\n  *\n  * SCLD_VANISHED indicates that one of the ancestor directories of the\n  * path existed at one point during the function call and then\n@@ -959,6 +960,11 @@ int adjust_shared_perm(const char *path);\n  * directory while we were working.  To be robust against this kind of\n  * race, callers might want to try invoking the function again when it\n  * returns SCLD_VANISHED.\n+ *\n+ * The non _const version of this function temporarily modifies its\n+ * path parameter while it is working but restores it before exiting.\n+ * The _const version does not modify path (at the cost of having to\n+ * make a temporary scratch copy).\n  */\n enum scld_error {\n \tSCLD_OK = 0,\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 568120e..94c6779 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -135,8 +135,10 @@ enum scld_error safe_create_leading_directories(char *path)\n \t\t*slash = '\\0';\n \t\tif (!stat(path, &st)) {\n \t\t\t/* path exists */\n-\t\t\tif (!S_ISDIR(st.st_mode))\n+\t\t\tif (!S_ISDIR(st.st_mode)) {\n+\t\t\t\terrno = ENOTDIR;\n \t\t\t\tret = SCLD_EXISTS;\n+\t\t\t}\n \t\t} else if (mkdir(path, 0777)) {\n \t\t\tif (errno == EEXIST &&\n \t\t\t    !stat(path, &st) && S_ISDIR(st.st_mode))\n-- \n2.7.0\n"},{"id":"279329","messageId":"70ffbbd27e47b65af5d93d3cc6a2ac38d5e78984.1456405698.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 03/20] raceproof_create_file(): new function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:02Z","receivedAt":"2016-02-25T13:16:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Add a function that tries to create a file and any containing\ndirectories in a way that is robust against races with other processes\nthat might be cleaning up empty directories at the same time.\n\nThe actual file creation is done by a callback function, which, if it\nfails, should set errno to EISDIR or ENOENT according to the convention\nof open(). raceproof_create_file() detects such failures, and\nrespectively either tries to delete empty directories that might be in\nthe way of the file or tries to create the containing directories. Then\nit retries the callback function.\n\nThis function is not yet used.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n cache.h     | 42 +++++++++++++++++++++++++++++++++++++\n sha1_file.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 111 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 7d3f80c..36b9c8b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -976,6 +976,48 @@ enum scld_error {\n enum scld_error safe_create_leading_directories(char *path);\n enum scld_error safe_create_leading_directories_const(const char *path);\n \n+/*\n+ * Callback function for raceproof_create_file(). This function is\n+ * expected to do something that makes dirname(path) permanent despite\n+ * the fact that other processes might be cleaning up empty\n+ * directories at the same time. Usually it will create a file named\n+ * path, but alternatively it could create another file in that\n+ * directory, or even chdir() into that directory. The function should\n+ * return 0 if the action was completed successfully. On error, it\n+ * should return a nonzero result and set errno.\n+ * raceproof_create_file() treats two errno values specially:\n+ *\n+ * - ENOENT -- dirname(path) does not exist. In this case,\n+ *             raceproof_create_file() tries creating dirname(path)\n+ *             (and any parent directories, if necessary) and calls\n+ *             the function again.\n+ *\n+ * - EISDIR -- the file already exists and is a directory. In this\n+ *             case, raceproof_create_file() deletes the directory\n+ *             (recursively) if it is empty and calls the function\n+ *             again.\n+ *\n+ * Any other errno causes raceproof_create_file() to fail with the\n+ * same return value and errno.\n+ *\n+ * Obviously, this function should be OK with being called again if it\n+ * fails with ENOENT or EISDIR. In other scenarios it will not be\n+ * called again.\n+ */\n+typedef int create_file_fn(const char *path, void *cb);\n+\n+/*\n+ * Create a file in dirname(path) by calling fn, creating leading\n+ * directories if necessary. Retry a few times in case we are racing\n+ * with another process that is trying to clean up the directory that\n+ * contains path. See the documentation for create_file_fn for more\n+ * details.\n+ *\n+ * Return the value and set the errno that resulted from the most\n+ * recent call of fn.\n+ */\n+int raceproof_create_file(const char *path, create_file_fn fn, void *cb);\n+\n int mkdir_in_gitdir(const char *path);\n extern char *expand_user_path(const char *path);\n const char *enter_repo(const char *path, int strict);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 94c6779..344aeeb 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -177,6 +177,75 @@ enum scld_error safe_create_leading_directories_const(const char *path)\n \treturn result;\n }\n \n+int raceproof_create_file(const char *path, create_file_fn fn, void *cb)\n+{\n+\t/*\n+\t * The number of times we will try to remove empty directories\n+\t * in the way of path. This is only 1 because if another\n+\t * process is racily creating directories that conflict with\n+\t * us, we don't want to fight against them.\n+\t */\n+\tint remove_directories_remaining = 1;\n+\n+\t/*\n+\t * The number of times that we will try to create the\n+\t * directories containing path. We are willing to attempt this\n+\t * more than once, because another process could be trying to\n+\t * clean up empty directories at the same time as we are\n+\t * trying to create them.\n+\t */\n+\tint create_directories_remaining = 3;\n+\n+\t/* A scratch copy of path, filled lazily if we need it: */\n+\tstruct strbuf path_copy = STRBUF_INIT;\n+\n+\tint save_errno;\n+\tint ret;\n+\n+retry_fn:\n+\tret = fn(path, cb);\n+\tsave_errno = errno;\n+\tif (!ret)\n+\t\tgoto out;\n+\n+\tif (errno == EISDIR && remove_directories_remaining > 0) {\n+\t\t/*\n+\t\t * A directory is in the way. Maybe it is empty; try\n+\t\t * to remove it:\n+\t\t */\n+\t\tif (!path_copy.len)\n+\t\t\tstrbuf_addstr(&path_copy, path);\n+\n+\t\tif (!remove_dir_recursively(&path_copy, REMOVE_DIR_EMPTY_ONLY)) {\n+\t\t\tremove_directories_remaining--;\n+\t\t\tgoto retry_fn;\n+\t\t}\n+\t} else if (errno == ENOENT && create_directories_remaining > 0) {\n+\t\t/*\n+\t\t * Maybe the containing directory didn't exist, or\n+\t\t * maybe it was just deleted by a process that is\n+\t\t * racing with us to clean up empty directories. Try\n+\t\t * to create it:\n+\t\t */\n+\t\tenum scld_error scld_result;\n+\n+\t\tif (!path_copy.len)\n+\t\t\tstrbuf_addstr(&path_copy, path);\n+\n+\t\tdo {\n+\t\t\tcreate_directories_remaining--;\n+\t\t\tscld_result = safe_create_leading_directories(path_copy.buf);\n+\t\t\tif (scld_result == SCLD_OK)\n+\t\t\t\tgoto retry_fn;\n+\t\t} while (scld_result == SCLD_VANISHED && create_directories_remaining > 0);\n+\t}\n+\n+out:\n+\tstrbuf_release(&path_copy);\n+\terrno = save_errno;\n+\treturn ret;\n+}\n+\n static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)\n {\n \tint i;\n-- \n2.7.0\n"},{"id":"279315","messageId":"1843ade32d9eb444205778a28e65fb93f69d3444.1456405698.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 04/20] lock_ref_sha1_basic(): use raceproof_create_file()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:03Z","receivedAt":"2016-02-25T13:16:03Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Instead of coding the retry loop inline, use raceproof_create_file() to\nmake lock acquisition safe against directory creation/deletion races.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 47 +++++++++++++++++++----------------------------\n 1 file changed, 19 insertions(+), 28 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex b569762..a549942 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1889,6 +1889,19 @@ static int remove_empty_directories(struct strbuf *path)\n \treturn remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);\n }\n \n+struct create_reflock_data {\n+\tstruct lock_file *lk;\n+\tint lflags;\n+};\n+\n+static int create_reflock(const char *path, void *cb)\n+{\n+\tstruct create_reflock_data *data = cb;\n+\n+\treturn hold_lock_file_for_update(data->lk, path, data->lflags) < 0\n+\t\t? -1 : 0;\n+}\n+\n /*\n  * Locks a ref returning the lock on success and NULL on failure.\n  * On failure errno is set to something meaningful.\n@@ -1906,10 +1919,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tstruct ref_lock *lock;\n \tint last_errno = 0;\n \tint type;\n-\tint lflags = 0;\n \tint mustexist = (old_sha1 && !is_null_sha1(old_sha1));\n \tint resolve_flags = 0;\n-\tint attempts_remaining = 3;\n+\tstruct create_reflock_data create_reflock_data = {NULL, 0};\n \n \tassert(err);\n \n@@ -1921,7 +1933,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\tresolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;\n \tif (flags & REF_NODEREF) {\n \t\tresolve_flags |= RESOLVE_REF_NO_RECURSE;\n-\t\tlflags |= LOCK_NO_DEREF;\n+\t\tcreate_reflock_data.lflags |= LOCK_NO_DEREF;\n \t}\n \n \trefname = resolve_ref_unsafe(refname, resolve_flags,\n@@ -1980,35 +1992,14 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tlock->orig_ref_name = xstrdup(orig_refname);\n \tstrbuf_git_path(&ref_file, \"%s\", refname);\n \n- retry:\n-\tswitch (safe_create_leading_directories_const(ref_file.buf)) {\n-\tcase SCLD_OK:\n-\t\tbreak; /* success */\n-\tcase SCLD_VANISHED:\n-\t\tif (--attempts_remaining > 0)\n-\t\t\tgoto retry;\n-\t\t/* fall through */\n-\tdefault:\n+\tcreate_reflock_data.lk = lock->lk;\n+\n+\tif (raceproof_create_file(ref_file.buf, create_reflock, &create_reflock_data)) {\n \t\tlast_errno = errno;\n-\t\tstrbuf_addf(err, \"unable to create directory for %s\",\n-\t\t\t    ref_file.buf);\n+\t\tunable_to_lock_message(ref_file.buf, errno, err);\n \t\tgoto error_return;\n \t}\n \n-\tif (hold_lock_file_for_update(lock->lk, ref_file.buf, lflags) < 0) {\n-\t\tlast_errno = errno;\n-\t\tif (errno == ENOENT && --attempts_remaining > 0)\n-\t\t\t/*\n-\t\t\t * Maybe somebody just deleted one of the\n-\t\t\t * directories leading to ref_file.  Try\n-\t\t\t * again:\n-\t\t\t */\n-\t\t\tgoto retry;\n-\t\telse {\n-\t\t\tunable_to_lock_message(ref_file.buf, errno, err);\n-\t\t\tgoto error_return;\n-\t\t}\n-\t}\n \tif (verify_lock(lock, old_sha1, mustexist, err)) {\n \t\tlast_errno = errno;\n \t\tgoto error_return;\n-- \n2.7.0\n"},{"id":"279317","messageId":"df9cf93f627da4df7060886552b3220a86586767.1456405698.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 05/20] rename_tmp_log(): use raceproof_create_file()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:04Z","receivedAt":"2016-02-25T13:16:04Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Besides shortening the code, this saves an unnecessary call to\nsafe_create_leading_directories_const() in almost all cases.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 76 ++++++++++++++++++++++------------------------------\n 1 file changed, 32 insertions(+), 44 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a549942..2cb402d 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2400,55 +2400,43 @@ out:\n  */\n #define TMP_RENAMED_LOG  \"logs/refs/.tmp-renamed-log\"\n \n+static int rename_tmp_log_callback(const char *path, void *cb)\n+{\n+\tint *true_errno = cb;\n+\n+\tif (rename(git_path(TMP_RENAMED_LOG), path)) {\n+\t\t/*\n+\t\t * rename(a, b) when b is an existing directory ought\n+\t\t * to result in ISDIR, but Solaris 5.8 gives ENOTDIR.\n+\t\t * Sheesh. Record the true errno for error reporting,\n+\t\t * but report EISDIR to raceproof_create_file() so\n+\t\t * that it knows to retry.\n+\t\t */\n+\t\t*true_errno = errno;\n+\t\tif (errno == ENOTDIR)\n+\t\t\terrno = EISDIR;\n+\t\treturn -1;\n+\t} else {\n+\t\treturn 0;\n+\t}\n+}\n+\n static int rename_tmp_log(const char *newrefname)\n {\n-\tint attempts_remaining = 4;\n-\tstruct strbuf path = STRBUF_INIT;\n-\tint ret = -1;\n+\tchar *path = git_pathdup(\"logs/%s\", newrefname);\n+\tint true_errno;\n+\tint ret;\n \n- retry:\n-\tstrbuf_reset(&path);\n-\tstrbuf_git_path(&path, \"logs/%s\", newrefname);\n-\tswitch (safe_create_leading_directories_const(path.buf)) {\n-\tcase SCLD_OK:\n-\t\tbreak; /* success */\n-\tcase SCLD_VANISHED:\n-\t\tif (--attempts_remaining > 0)\n-\t\t\tgoto retry;\n-\t\t/* fall through */\n-\tdefault:\n-\t\terror(\"unable to create directory for %s\", newrefname);\n-\t\tgoto out;\n-\t}\n-\n-\tif (rename(git_path(TMP_RENAMED_LOG), path.buf)) {\n-\t\tif ((errno==EISDIR || errno==ENOTDIR) && --attempts_remaining > 0) {\n-\t\t\t/*\n-\t\t\t * rename(a, b) when b is an existing\n-\t\t\t * directory ought to result in ISDIR, but\n-\t\t\t * Solaris 5.8 gives ENOTDIR.  Sheesh.\n-\t\t\t */\n-\t\t\tif (remove_empty_directories(&path)) {\n-\t\t\t\terror(\"Directory not empty: logs/%s\", newrefname);\n-\t\t\t\tgoto out;\n-\t\t\t}\n-\t\t\tgoto retry;\n-\t\t} else if (errno == ENOENT && --attempts_remaining > 0) {\n-\t\t\t/*\n-\t\t\t * Maybe another process just deleted one of\n-\t\t\t * the directories in the path to newrefname.\n-\t\t\t * Try again from the beginning.\n-\t\t\t */\n-\t\t\tgoto retry;\n-\t\t} else {\n+\tret = raceproof_create_file(path, rename_tmp_log_callback, &true_errno);\n+\tif (ret) {\n+\t\tif (true_errno == EISDIR)\n+\t\t\terror(\"Directory not empty: %s\", path);\n+\t\telse\n \t\t\terror(\"unable to move logfile \"TMP_RENAMED_LOG\" to logs/%s: %s\",\n-\t\t\t\tnewrefname, strerror(errno));\n-\t\t\tgoto out;\n-\t\t}\n+\t\t\t\tnewrefname, strerror(true_errno));\n \t}\n-\tret = 0;\n-out:\n-\tstrbuf_release(&path);\n+\n+\tfree(path);\n \treturn ret;\n }\n \n-- \n2.7.0\n"},{"id":"279316","messageId":"c0614de75e2d6e23695f09cd8c043ef71bb49ddd.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 06/20] rename_tmp_log(): improve error reporting","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:05Z","receivedAt":"2016-02-25T13:16:05Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"* Don't capitalize error strings\n* Report true paths of affected files\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 2cb402d..ed29b3b 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2430,10 +2430,11 @@ static int rename_tmp_log(const char *newrefname)\n \tret = raceproof_create_file(path, rename_tmp_log_callback, &true_errno);\n \tif (ret) {\n \t\tif (true_errno == EISDIR)\n-\t\t\terror(\"Directory not empty: %s\", path);\n+\t\t\terror(\"directory not empty: %s\", path);\n \t\telse\n-\t\t\terror(\"unable to move logfile \"TMP_RENAMED_LOG\" to logs/%s: %s\",\n-\t\t\t\tnewrefname, strerror(true_errno));\n+\t\t\terror(\"unable to move logfile %s to %s: %s\",\n+\t\t\t      git_path(TMP_RENAMED_LOG), path,\n+\t\t\t      strerror(true_errno));\n \t}\n \n \tfree(path);\n-- \n2.7.0\n"},{"id":"279330","messageId":"bc674e78b4d3f528d5e0bfe9d8dbff421b180538.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 07/20] log_ref_setup(): separate code for create vs non-create","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:06Z","receivedAt":"2016-02-25T13:16:06Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The behavior of this function (especially how it handles errors) is\nquite different depending on whether we are willing to create the reflog\nvs. whether we are only trying to open an existing reflog. So separate\nthe code paths.\n\nThis also simplifies the next steps.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 56 ++++++++++++++++++++++++++++++++++------------------\n 1 file changed, 37 insertions(+), 19 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex ed29b3b..8fff1a8 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2590,7 +2590,7 @@ static int commit_ref(struct ref_lock *lock)\n  */\n static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)\n {\n-\tint logfd, oflags = O_APPEND | O_WRONLY;\n+\tint logfd;\n \n \tstrbuf_git_path(logfile, \"logs/%s\", refname);\n \tif (force_create || should_autocreate_reflog(refname)) {\n@@ -2599,36 +2599,54 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str\n \t\t\t\t    \"%s\", logfile->buf, strerror(errno));\n \t\t\treturn -1;\n \t\t}\n-\t\toflags |= O_CREAT;\n-\t}\n-\n-\tlogfd = open(logfile->buf, oflags, 0666);\n-\tif (logfd < 0) {\n-\t\tif (!(oflags & O_CREAT) && (errno == ENOENT || errno == EISDIR))\n-\t\t\treturn 0;\n+\t\tlogfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);\n+\t\tif (logfd < 0) {\n+\t\t\tif (errno == EISDIR) {\n+\t\t\t\t/*\n+\t\t\t\t * The directory that is in the way might be\n+\t\t\t\t * empty. Try to remove it.\n+\t\t\t\t */\n+\t\t\t\tif (remove_empty_directories(logfile)) {\n+\t\t\t\t\tstrbuf_addf(err, \"There are still logs under \"\n+\t\t\t\t\t\t    \"'%s'\", logfile->buf);\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n+\t\t\t\tlogfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);\n+\t\t\t}\n \n-\t\tif (errno == EISDIR) {\n-\t\t\tif (remove_empty_directories(logfile)) {\n-\t\t\t\tstrbuf_addf(err, \"There are still logs under \"\n-\t\t\t\t\t    \"'%s'\", logfile->buf);\n+\t\t\tif (logfd < 0) {\n+\t\t\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n+\t\t\t\t\t    logfile->buf, strerror(errno));\n \t\t\t\treturn -1;\n \t\t\t}\n-\t\t\tlogfd = open(logfile->buf, oflags, 0666);\n \t\t}\n \n+\t\tadjust_shared_perm(logfile->buf);\n+\t\tclose(logfd);\n+\t} else {\n+\t\tlogfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);\n \t\tif (logfd < 0) {\n-\t\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n-\t\t\t\t    logfile->buf, strerror(errno));\n-\t\t\treturn -1;\n+\t\t\tif (errno == ENOENT || errno == EISDIR) {\n+\t\t\t\t/*\n+\t\t\t\t * The logfile doesn't already exist,\n+\t\t\t\t * but that is not an error; it only\n+\t\t\t\t * means that we won't write log\n+\t\t\t\t * entries to it.\n+\t\t\t\t */\n+\t\t\t} else {\n+\t\t\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n+\t\t\t\t\t    logfile->buf, strerror(errno));\n+\t\t\t\treturn -1;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tadjust_shared_perm(logfile->buf);\n+\t\t\tclose(logfd);\n \t\t}\n \t}\n \n-\tadjust_shared_perm(logfile->buf);\n-\tclose(logfd);\n \treturn 0;\n }\n \n-\n int safe_create_reflog(const char *refname, int force_create, struct strbuf *err)\n {\n \tint ret;\n-- \n2.7.0\n"},{"id":"279332","messageId":"2554cdb15206319537c694299f7c78238e7706b3.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 08/20] log_ref_setup(): improve robustness against races","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:07Z","receivedAt":"2016-02-25T13:16:07Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change log_ref_setup() to use raceproof_create_file() to create the new\nlogfile. This makes it more robust against a race against another\nprocess that might be trying to clean up empty directories while we are\ntrying to create a new logfile.\n\nThis also means that it will only call create_leading_directories() if\nopen() fails, which should be a net win. Even in the cases where we are\nwilling to create a new logfile, it will usually be the case that the\nlogfile already exists, or if not then that the directory containing the\nlogfile already exists. In such cases, we will save some work that was\npreviously done unconditionally.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 46 +++++++++++++++++++++-------------------------\n 1 file changed, 21 insertions(+), 25 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 8fff1a8..2cc9489 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2582,6 +2582,14 @@ static int commit_ref(struct ref_lock *lock)\n \treturn 0;\n }\n \n+static int open_or_create_logfile(const char *path, void *cb)\n+{\n+\tint *fd = cb;\n+\n+\t*fd = open(path, O_APPEND | O_WRONLY | O_CREAT, 0666);\n+\treturn (*fd < 0) ? -1 : 0;\n+}\n+\n /*\n  * Create a reflog for a ref.  If force_create = 0, the reflog will\n  * only be created for certain refs (those for which\n@@ -2593,36 +2601,24 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str\n \tint logfd;\n \n \tstrbuf_git_path(logfile, \"logs/%s\", refname);\n+\n \tif (force_create || should_autocreate_reflog(refname)) {\n-\t\tif (safe_create_leading_directories(logfile->buf) < 0) {\n-\t\t\tstrbuf_addf(err, \"unable to create directory for %s: \"\n-\t\t\t\t    \"%s\", logfile->buf, strerror(errno));\n-\t\t\treturn -1;\n-\t\t}\n-\t\tlogfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);\n-\t\tif (logfd < 0) {\n-\t\t\tif (errno == EISDIR) {\n-\t\t\t\t/*\n-\t\t\t\t * The directory that is in the way might be\n-\t\t\t\t * empty. Try to remove it.\n-\t\t\t\t */\n-\t\t\t\tif (remove_empty_directories(logfile)) {\n-\t\t\t\t\tstrbuf_addf(err, \"There are still logs under \"\n-\t\t\t\t\t\t    \"'%s'\", logfile->buf);\n-\t\t\t\t\treturn -1;\n-\t\t\t\t}\n-\t\t\t\tlogfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);\n-\t\t\t}\n-\n-\t\t\tif (logfd < 0) {\n+\t\tif (raceproof_create_file(logfile->buf, open_or_create_logfile, &logfd) < 0) {\n+\t\t\tif (errno == ENOENT) {\n+\t\t\t\tstrbuf_addf(err, \"unable to create directory for %s: \"\n+\t\t\t\t\t    \"%s\", logfile->buf, strerror(errno));\n+\t\t\t} else if (errno == EISDIR) {\n+\t\t\t\tstrbuf_addf(err, \"there are still logs under %s\",\n+\t\t\t\t\t    logfile->buf);\n+\t\t\t} else {\n \t\t\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n \t\t\t\t\t    logfile->buf, strerror(errno));\n-\t\t\t\treturn -1;\n \t\t\t}\n+\t\t\treturn -1;\n+\t\t} else {\n+\t\t\tadjust_shared_perm(logfile->buf);\n+\t\t\tclose(logfd);\n \t\t}\n-\n-\t\tadjust_shared_perm(logfile->buf);\n-\t\tclose(logfd);\n \t} else {\n \t\tlogfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);\n \t\tif (logfd < 0) {\n-- \n2.7.0\n"},{"id":"279334","messageId":"19b83a0ba835dc711a78e56be0b287d5340f2816.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 09/20] log_ref_setup(): pass the open file descriptor back to the caller","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:08Z","receivedAt":"2016-02-25T13:16:08Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This function will most often be called by log_ref_write_1(), which\nwants to append to the reflog file. In that case, it is silly to close\nthe file only for the caller to reopen it immediately. So, in the case\nthat the file was opened, pass the open file descriptor back to the\ncaller.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 37 +++++++++++++++++++++----------------\n 1 file changed, 21 insertions(+), 16 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 2cc9489..5937099 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2591,19 +2591,23 @@ static int open_or_create_logfile(const char *path, void *cb)\n }\n \n /*\n- * Create a reflog for a ref.  If force_create = 0, the reflog will\n- * only be created for certain refs (those for which\n- * should_autocreate_reflog returns non-zero.  Otherwise, create it\n- * regardless of the ref name.  Fill in *err and return -1 on failure.\n+ * Create a reflog for a ref. Store its path to *logfile. If\n+ * force_create = 0, only create the reflog for certain refs (those\n+ * for which should_autocreate_reflog returns non-zero). Otherwise,\n+ * create it regardless of the reference name. If the logfile already\n+ * existed or was created, return 0 and set *logfd to the file\n+ * descriptor opened for appending to the file. If no logfile exists\n+ * and we decided not to create one, return 0 and set *logfd to -1. On\n+ * failure, fill in *err and return -1.\n  */\n-static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)\n+static int log_ref_setup(const char *refname,\n+\t\t\t struct strbuf *logfile, int *logfd,\n+\t\t\t struct strbuf *err, int force_create)\n {\n-\tint logfd;\n-\n \tstrbuf_git_path(logfile, \"logs/%s\", refname);\n \n \tif (force_create || should_autocreate_reflog(refname)) {\n-\t\tif (raceproof_create_file(logfile->buf, open_or_create_logfile, &logfd) < 0) {\n+\t\tif (raceproof_create_file(logfile->buf, open_or_create_logfile, logfd) < 0) {\n \t\t\tif (errno == ENOENT) {\n \t\t\t\tstrbuf_addf(err, \"unable to create directory for %s: \"\n \t\t\t\t\t    \"%s\", logfile->buf, strerror(errno));\n@@ -2617,11 +2621,10 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str\n \t\t\treturn -1;\n \t\t} else {\n \t\t\tadjust_shared_perm(logfile->buf);\n-\t\t\tclose(logfd);\n \t\t}\n \t} else {\n-\t\tlogfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);\n-\t\tif (logfd < 0) {\n+\t\t*logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);\n+\t\tif (*logfd < 0) {\n \t\t\tif (errno == ENOENT || errno == EISDIR) {\n \t\t\t\t/*\n \t\t\t\t * The logfile doesn't already exist,\n@@ -2636,7 +2639,6 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str\n \t\t\t}\n \t\t} else {\n \t\t\tadjust_shared_perm(logfile->buf);\n-\t\t\tclose(logfd);\n \t\t}\n \t}\n \n@@ -2647,8 +2649,11 @@ int safe_create_reflog(const char *refname, int force_create, struct strbuf *err\n {\n \tint ret;\n \tstruct strbuf sb = STRBUF_INIT;\n+\tint fd;\n \n-\tret = log_ref_setup(refname, &sb, err, force_create);\n+\tret = log_ref_setup(refname, &sb, &fd, err, force_create);\n+\tif (!ret && fd >= 0)\n+\t\tclose(fd);\n \tstrbuf_release(&sb);\n \treturn ret;\n }\n@@ -2684,17 +2689,17 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,\n \t\t\t   struct strbuf *logfile, int flags,\n \t\t\t   struct strbuf *err)\n {\n-\tint logfd, result, oflags = O_APPEND | O_WRONLY;\n+\tint logfd, result;\n \n \tif (log_all_ref_updates < 0)\n \t\tlog_all_ref_updates = !is_bare_repository();\n \n-\tresult = log_ref_setup(refname, logfile, err, flags & REF_FORCE_CREATE_REFLOG);\n+\tresult = log_ref_setup(refname, logfile, &logfd, err,\n+\t\t\t       flags & REF_FORCE_CREATE_REFLOG);\n \n \tif (result)\n \t\treturn result;\n \n-\tlogfd = open(logfile->buf, oflags);\n \tif (logfd < 0)\n \t\treturn 0;\n \tresult = log_ref_write_fd(logfd, old_sha1, new_sha1,\n-- \n2.7.0\n"},{"id":"279331","messageId":"0f2ae0f9766d36b697291cea21c517a3c53c9c69.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 10/20] log_ref_write_1(): don't depend on logfile","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:09Z","receivedAt":"2016-02-25T13:16:09Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It's unnecessary to pass a strbuf holding the reflog path up and down\nthe call stack now that it is hardly needed by the callers. Remove the\nplaces where log_ref_write_1() uses it, in preparation for making it\ninternal to log_ref_setup().\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 5937099..4f25932 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2705,14 +2705,14 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,\n \tresult = log_ref_write_fd(logfd, old_sha1, new_sha1,\n \t\t\t\t  git_committer_info(0), msg);\n \tif (result) {\n-\t\tstrbuf_addf(err, \"unable to append to %s: %s\", logfile->buf,\n-\t\t\t    strerror(errno));\n+\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n+\t\t\t    git_path(\"logs/%s\", refname), strerror(errno));\n \t\tclose(logfd);\n \t\treturn -1;\n \t}\n \tif (close(logfd)) {\n-\t\tstrbuf_addf(err, \"unable to append to %s: %s\", logfile->buf,\n-\t\t\t    strerror(errno));\n+\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n+\t\t\t    git_path(\"logs/%s\", refname), strerror(errno));\n \t\treturn -1;\n \t}\n \treturn 0;\n-- \n2.7.0\n"},{"id":"279318","messageId":"9597542b805c7d1e1a1b0a89560d539e5ac02cb9.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 11/20] log_ref_setup(): manage the name of the reflog file internally","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:10Z","receivedAt":"2016-02-25T13:16:10Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Instead of writing the name of the reflog file into a strbuf that is\nsupplied by the caller but not needed there, write it into a local\ntemporary buffer and remove the strbuf parameter entirely.\n\nAnd while we're adjusting the function signature, reorder the arguments\nto move the input parameters before the output parameters.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 66 +++++++++++++++++++++++-----------------------------\n 1 file changed, 29 insertions(+), 37 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4f25932..49713b3 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2591,39 +2591,38 @@ static int open_or_create_logfile(const char *path, void *cb)\n }\n \n /*\n- * Create a reflog for a ref. Store its path to *logfile. If\n- * force_create = 0, only create the reflog for certain refs (those\n- * for which should_autocreate_reflog returns non-zero). Otherwise,\n- * create it regardless of the reference name. If the logfile already\n- * existed or was created, return 0 and set *logfd to the file\n- * descriptor opened for appending to the file. If no logfile exists\n- * and we decided not to create one, return 0 and set *logfd to -1. On\n- * failure, fill in *err and return -1.\n+ * Create a reflog for a ref. If force_create = 0, only create the\n+ * reflog for certain refs (those for which should_autocreate_reflog\n+ * returns non-zero). Otherwise, create it regardless of the reference\n+ * name. If the logfile already existed or was created, return 0 and\n+ * set *logfd to the file descriptor opened for appending to the file.\n+ * If no logfile exists and we decided not to create one, return 0 and\n+ * set *logfd to -1. On failure, fill in *err and return -1.\n  */\n-static int log_ref_setup(const char *refname,\n-\t\t\t struct strbuf *logfile, int *logfd,\n-\t\t\t struct strbuf *err, int force_create)\n+static int log_ref_setup(const char *refname, int force_create,\n+\t\t\t int *logfd, struct strbuf *err)\n {\n-\tstrbuf_git_path(logfile, \"logs/%s\", refname);\n+\tchar *logfile = git_pathdup(\"logs/%s\", refname);\n+\tint ret = 0;\n \n \tif (force_create || should_autocreate_reflog(refname)) {\n-\t\tif (raceproof_create_file(logfile->buf, open_or_create_logfile, logfd) < 0) {\n+\t\tif (raceproof_create_file(logfile, open_or_create_logfile, logfd) < 0) {\n \t\t\tif (errno == ENOENT) {\n \t\t\t\tstrbuf_addf(err, \"unable to create directory for %s: \"\n-\t\t\t\t\t    \"%s\", logfile->buf, strerror(errno));\n+\t\t\t\t\t    \"%s\", logfile, strerror(errno));\n \t\t\t} else if (errno == EISDIR) {\n \t\t\t\tstrbuf_addf(err, \"there are still logs under %s\",\n-\t\t\t\t\t    logfile->buf);\n+\t\t\t\t\t    logfile);\n \t\t\t} else {\n \t\t\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n-\t\t\t\t\t    logfile->buf, strerror(errno));\n+\t\t\t\t\t    logfile, strerror(errno));\n \t\t\t}\n-\t\t\treturn -1;\n+\t\t\tret = -1;\n \t\t} else {\n-\t\t\tadjust_shared_perm(logfile->buf);\n+\t\t\tadjust_shared_perm(logfile);\n \t\t}\n \t} else {\n-\t\t*logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);\n+\t\t*logfd = open(logfile, O_APPEND | O_WRONLY, 0666);\n \t\tif (*logfd < 0) {\n \t\t\tif (errno == ENOENT || errno == EISDIR) {\n \t\t\t\t/*\n@@ -2634,27 +2633,25 @@ static int log_ref_setup(const char *refname,\n \t\t\t\t */\n \t\t\t} else {\n \t\t\t\tstrbuf_addf(err, \"unable to append to %s: %s\",\n-\t\t\t\t\t    logfile->buf, strerror(errno));\n-\t\t\t\treturn -1;\n+\t\t\t\t\t    logfile, strerror(errno));\n+\t\t\t\tret = -1;\n \t\t\t}\n \t\t} else {\n-\t\t\tadjust_shared_perm(logfile->buf);\n+\t\t\tadjust_shared_perm(logfile);\n \t\t}\n \t}\n \n-\treturn 0;\n+\tfree(logfile);\n+\treturn ret;\n }\n \n int safe_create_reflog(const char *refname, int force_create, struct strbuf *err)\n {\n-\tint ret;\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tint fd;\n+\tint ret, fd;\n \n-\tret = log_ref_setup(refname, &sb, &fd, err, force_create);\n+\tret = log_ref_setup(refname, force_create, &fd, err);\n \tif (!ret && fd >= 0)\n \t\tclose(fd);\n-\tstrbuf_release(&sb);\n \treturn ret;\n }\n \n@@ -2686,16 +2683,15 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\n \n static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,\n \t\t\t   const unsigned char *new_sha1, const char *msg,\n-\t\t\t   struct strbuf *logfile, int flags,\n-\t\t\t   struct strbuf *err)\n+\t\t\t   int flags, struct strbuf *err)\n {\n \tint logfd, result;\n \n \tif (log_all_ref_updates < 0)\n \t\tlog_all_ref_updates = !is_bare_repository();\n \n-\tresult = log_ref_setup(refname, logfile, &logfd, err,\n-\t\t\t       flags & REF_FORCE_CREATE_REFLOG);\n+\tresult = log_ref_setup(refname, flags & REF_FORCE_CREATE_REFLOG,\n+\t\t\t       &logfd, err);\n \n \tif (result)\n \t\treturn result;\n@@ -2730,11 +2726,7 @@ int files_log_ref_write(const char *refname, const unsigned char *old_sha1,\n \t\t\tconst unsigned char *new_sha1, const char *msg,\n \t\t\tint flags, struct strbuf *err)\n {\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tint ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb, flags,\n-\t\t\t\t  err);\n-\tstrbuf_release(&sb);\n-\treturn ret;\n+\treturn log_ref_write_1(refname, old_sha1, new_sha1, msg, flags, err);\n }\n \n /*\n-- \n2.7.0\n"},{"id":"279319","messageId":"cc99e0c3c2f939abc9d7c3822c181e54213e15a3.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 12/20] log_ref_write_1(): inline function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:11Z","receivedAt":"2016-02-25T13:16:11Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Now log_ref_write_1() doesn't do anything, so inline it.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 29 +++++++++++------------------\n 1 file changed, 11 insertions(+), 18 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 49713b3..bd25ae2 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2681,9 +2681,17 @@ static int log_ref_write_fd(int fd, const unsigned char *old_sha1,\n \treturn 0;\n }\n \n-static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,\n-\t\t\t   const unsigned char *new_sha1, const char *msg,\n-\t\t\t   int flags, struct strbuf *err)\n+static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n+\t\t\t const unsigned char *new_sha1, const char *msg,\n+\t\t\t int flags, struct strbuf *err)\n+{\n+\treturn files_log_ref_write(refname, old_sha1, new_sha1, msg, flags,\n+\t\t\t\t   err);\n+}\n+\n+int files_log_ref_write(const char *refname, const unsigned char *old_sha1,\n+\t\t\tconst unsigned char *new_sha1, const char *msg,\n+\t\t\tint flags, struct strbuf *err)\n {\n \tint logfd, result;\n \n@@ -2714,21 +2722,6 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,\n \treturn 0;\n }\n \n-static int log_ref_write(const char *refname, const unsigned char *old_sha1,\n-\t\t\t const unsigned char *new_sha1, const char *msg,\n-\t\t\t int flags, struct strbuf *err)\n-{\n-\treturn files_log_ref_write(refname, old_sha1, new_sha1, msg, flags,\n-\t\t\t\t   err);\n-}\n-\n-int files_log_ref_write(const char *refname, const unsigned char *old_sha1,\n-\t\t\tconst unsigned char *new_sha1, const char *msg,\n-\t\t\tint flags, struct strbuf *err)\n-{\n-\treturn log_ref_write_1(refname, old_sha1, new_sha1, msg, flags, err);\n-}\n-\n /*\n  * Write sha1 into the open lockfile, then close the lockfile. On\n  * errors, rollback the lockfile, fill in *err and\n-- \n2.7.0\n"},{"id":"279320","messageId":"d6ab397f974f9b97e9ff99f3fd4fb67ee5e81ea7.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 13/20] try_remove_empty_parents(): rename parameter \"name\" -> \"refname\"","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:12Z","receivedAt":"2016-02-25T13:16:12Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is the standard nomenclature.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex bd25ae2..543fd8e 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2200,13 +2200,13 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \n /*\n  * Remove empty parents, but spare refs/ and immediate subdirs.\n- * Note: munges *name.\n+ * Note: munges *refname.\n  */\n-static void try_remove_empty_parents(char *name)\n+static void try_remove_empty_parents(char *refname)\n {\n \tchar *p, *q;\n \tint i;\n-\tp = name;\n+\tp = refname;\n \tfor (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */\n \t\twhile (*p && *p != '/')\n \t\t\tp++;\n@@ -2224,7 +2224,7 @@ static void try_remove_empty_parents(char *name)\n \t\tif (q == p)\n \t\t\tbreak;\n \t\t*q = '\\0';\n-\t\tif (rmdir(git_path(\"%s\", name)))\n+\t\tif (rmdir(git_path(\"%s\", refname)))\n \t\t\tbreak;\n \t}\n }\n-- \n2.7.0\n"},{"id":"279321","messageId":"1cdb052a14cd1fe0615697527b5e84aabff5774c.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 14/20] try_remove_empty_parents(): don't trash argument contents","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:13Z","receivedAt":"2016-02-25T13:16:13Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 543fd8e..1fcbb6f 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2200,13 +2200,15 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \n /*\n  * Remove empty parents, but spare refs/ and immediate subdirs.\n- * Note: munges *refname.\n  */\n-static void try_remove_empty_parents(char *refname)\n+static void try_remove_empty_parents(const char *refname)\n {\n+\tstruct strbuf buf = STRBUF_INIT;\n \tchar *p, *q;\n \tint i;\n-\tp = refname;\n+\n+\tstrbuf_addstr(&buf, refname);\n+\tp = buf.buf;\n \tfor (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */\n \t\twhile (*p && *p != '/')\n \t\t\tp++;\n@@ -2214,8 +2216,7 @@ static void try_remove_empty_parents(char *refname)\n \t\twhile (*p == '/')\n \t\t\tp++;\n \t}\n-\tfor (q = p; *q; q++)\n-\t\t;\n+\tq = buf.buf + buf.len;\n \twhile (1) {\n \t\twhile (q > p && *q != '/')\n \t\t\tq--;\n@@ -2223,10 +2224,11 @@ static void try_remove_empty_parents(char *refname)\n \t\t\tq--;\n \t\tif (q == p)\n \t\t\tbreak;\n-\t\t*q = '\\0';\n-\t\tif (rmdir(git_path(\"%s\", refname)))\n+\t\tstrbuf_setlen(&buf, q - buf.buf);\n+\t\tif (rmdir(git_path(\"%s\", buf.buf)))\n \t\t\tbreak;\n \t}\n+\tstrbuf_release(&buf);\n }\n \n /* make sure nobody touched the ref, and unlink */\n-- \n2.7.0\n"},{"id":"279327","messageId":"c9ae141d685e6ab835838252b726c5da519cb0fd.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 15/20] try_remove_empty_parents(): don't accommodate consecutive slashes","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:14Z","receivedAt":"2016-02-25T13:16:14Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\"refname\" has already been checked by check_refname_format(), so it\ncannot have consecutive slashes.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 1fcbb6f..46dcc23 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2212,15 +2212,14 @@ static void try_remove_empty_parents(const char *refname)\n \tfor (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */\n \t\twhile (*p && *p != '/')\n \t\t\tp++;\n-\t\t/* tolerate duplicate slashes; see check_refname_format() */\n-\t\twhile (*p == '/')\n+\t\tif (*p == '/')\n \t\t\tp++;\n \t}\n \tq = buf.buf + buf.len;\n \twhile (1) {\n \t\twhile (q > p && *q != '/')\n \t\t\tq--;\n-\t\twhile (q > p && *(q-1) == '/')\n+\t\tif (q > p && *(q-1) == '/')\n \t\t\tq--;\n \t\tif (q == p)\n \t\t\tbreak;\n-- \n2.7.0\n"},{"id":"279322","messageId":"6f79ef9e9ac452019567f150553fe8635bf22bc3.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 16/20] t5505: use \"for-each-ref\" to test for the non-existence of references","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:15Z","receivedAt":"2016-02-25T13:16:15Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Instead of looking on the filesystem inside \".git/refs/remotes/origin\",\nuse \"git for-each-ref\" to check for leftover references under the\nremote's old name.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n t/t5505-remote.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 1a8e3b8..d2ac346 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -687,7 +687,7 @@ test_expect_success 'rename a remote' '\n \t(\n \t\tcd four &&\n \t\tgit remote rename origin upstream &&\n-\t\trmdir .git/refs/remotes/origin &&\n+\t\ttest -z \"$(git for-each-ref refs/remotes/origin)\" &&\n \t\ttest \"$(git symbolic-ref refs/remotes/upstream/HEAD)\" = \"refs/remotes/upstream/master\" &&\n \t\ttest \"$(git rev-parse upstream/master)\" = \"$(git rev-parse master)\" &&\n \t\ttest \"$(git config remote.upstream.fetch)\" = \"+refs/heads/*:refs/remotes/upstream/*\" &&\n-- \n2.7.0\n"},{"id":"279323","messageId":"592eaa8e9ccebb38df8fd60e35b23892bdc92648.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 17/20] delete_ref_loose(): derive loose reference path from lock","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:16Z","receivedAt":"2016-02-25T13:16:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It is simpler to derive the path to the file that must be deleted from\n\"lock->ref_name\" than from the lock_file object.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 46dcc23..7e825bd 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2345,10 +2345,7 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n \t\t * loose.  The loose file name is the same as the\n \t\t * lockfile name, minus \".lock\":\n \t\t */\n-\t\tchar *loose_filename = get_locked_file_path(lock->lk);\n-\t\tint res = unlink_or_msg(loose_filename, err);\n-\t\tfree(loose_filename);\n-\t\tif (res)\n+\t\tif (unlink_or_msg(git_path(\"%s\", lock->ref_name), err))\n \t\t\treturn 1;\n \t}\n \treturn 0;\n-- \n2.7.0\n"},{"id":"279325","messageId":"663b630a54a4d497ee2b49a24c3a783a80761065.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 18/20] delete_ref_loose(): inline function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:17Z","receivedAt":"2016-02-25T13:16:17Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It was hardly doing anything anymore.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 25 +++++++------------------\n 1 file changed, 7 insertions(+), 18 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 7e825bd..b137171 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2336,21 +2336,6 @@ static int repack_without_refs(struct string_list *refnames, struct strbuf *err)\n \treturn ret;\n }\n \n-static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n-{\n-\tassert(err);\n-\n-\tif (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {\n-\t\t/*\n-\t\t * loose.  The loose file name is the same as the\n-\t\t * lockfile name, minus \".lock\":\n-\t\t */\n-\t\tif (unlink_or_msg(git_path(\"%s\", lock->ref_name), err))\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n int delete_refs(struct string_list *refnames)\n {\n \tstruct strbuf err = STRBUF_INIT;\n@@ -3257,9 +3242,13 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (update->flags & REF_DELETING) {\n-\t\t\tif (delete_ref_loose(update->lock, update->type, err)) {\n-\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n-\t\t\t\tgoto cleanup;\n+\t\t\tif (!(update->type & REF_ISPACKED) ||\n+\t\t\t    update->type & REF_ISSYMREF) {\n+\t\t\t\t/* It is a loose reference. */\n+\t\t\t\tif (unlink_or_msg(git_path(\"%s\", update->lock->ref_name), err)) {\n+\t\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n+\t\t\t\t\tgoto cleanup;\n+\t\t\t\t}\n \t\t\t}\n \n \t\t\tif (!(update->flags & REF_ISPRUNING))\n-- \n2.7.0\n"},{"id":"279326","messageId":"2b81d46035f578f0144843065b178ed428c54acb.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 19/20] try_remove_empty_parents(): teach to remove parents of reflogs, too","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:18Z","receivedAt":"2016-02-25T13:16:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Add a new \"flags\" parameter that tells the function whether to remove\nempty parent directories of the loose reference file, of the reflog\nfile, or both. The new functionality is not yet used.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c | 24 ++++++++++++++++++------\n 1 file changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex b137171..9ebb188 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2198,10 +2198,18 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)\n \treturn 0;\n }\n \n+enum {\n+\tREMOVE_EMPTY_PARENTS_REF = 0x01,\n+\tREMOVE_EMPTY_PARENTS_REFLOG = 0x02\n+};\n+\n /*\n- * Remove empty parents, but spare refs/ and immediate subdirs.\n+ * Remove empty parent directories associated with the specified\n+ * reference and/or its reflog, but spare [logs/]refs/ and immediate\n+ * subdirs. flags is a combination of REMOVE_EMPTY_PARENTS_REF and/or\n+ * REMOVE_EMPTY_PARENTS_REFLOG.\n  */\n-static void try_remove_empty_parents(const char *refname)\n+static void try_remove_empty_parents(const char *refname, unsigned int flags)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tchar *p, *q;\n@@ -2216,7 +2224,7 @@ static void try_remove_empty_parents(const char *refname)\n \t\t\tp++;\n \t}\n \tq = buf.buf + buf.len;\n-\twhile (1) {\n+\twhile (flags & (REMOVE_EMPTY_PARENTS_REF | REMOVE_EMPTY_PARENTS_REFLOG)) {\n \t\twhile (q > p && *q != '/')\n \t\t\tq--;\n \t\tif (q > p && *(q-1) == '/')\n@@ -2224,8 +2232,12 @@ static void try_remove_empty_parents(const char *refname)\n \t\tif (q == p)\n \t\t\tbreak;\n \t\tstrbuf_setlen(&buf, q - buf.buf);\n-\t\tif (rmdir(git_path(\"%s\", buf.buf)))\n-\t\t\tbreak;\n+\t\tif ((flags & REMOVE_EMPTY_PARENTS_REF) &&\n+\t\t    rmdir(git_path(\"%s\", buf.buf)))\n+\t\t\tflags &= ~REMOVE_EMPTY_PARENTS_REF;\n+\t\tif ((flags & REMOVE_EMPTY_PARENTS_REFLOG) &&\n+\t\t    rmdir(git_path(\"logs/%s\", buf.buf)))\n+\t\t\tflags &= ~REMOVE_EMPTY_PARENTS_REFLOG;\n \t}\n \tstrbuf_release(&buf);\n }\n@@ -2251,7 +2263,7 @@ static void prune_ref(struct ref_to_prune *r)\n \t}\n \tref_transaction_free(transaction);\n \tstrbuf_release(&err);\n-\ttry_remove_empty_parents(r->name);\n+\ttry_remove_empty_parents(r->name, REMOVE_EMPTY_PARENTS_REF);\n }\n \n static void prune_refs(struct ref_to_prune *r)\n-- \n2.7.0\n"},{"id":"279324","messageId":"1c2176ceec2f28709ca9f16adf291c34abb00cf3.1456405699.git.mhagger@alum.mit.edu","threadId":"41516","inReplyTo":"cover.1456405698.git.mhagger@alum.mit.edu","subject":"[PATCH v2 20/20] ref_transaction_commit(): clean up empty directories","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-02-25T13:16:19Z","receivedAt":"2016-02-25T13:16:19Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"When deleting/pruning references, remove any directories that are made\nempty by the deletion of loose references or of reflogs. Otherwise such\nempty directories can survive forever and accumulate over time. (Even\n'pack-refs', which is smart enough to remove the parent directories of\nloose references that it prunes, leaves directories that were already\nempty.)\n\nAnd now that ref_transaction_commit() takes care of deleting the parent\ndirectories of loose references that it prunes, we don't have to do that\nin prune_ref() anymore.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n refs/files-backend.c  | 37 +++++++++++++++++++++++++++++++------\n refs/refs-internal.h  |  9 ++++++++-\n t/t1400-update-ref.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 66 insertions(+), 7 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 9ebb188..9973958 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2263,7 +2263,6 @@ static void prune_ref(struct ref_to_prune *r)\n \t}\n \tref_transaction_free(transaction);\n \tstrbuf_release(&err);\n-\ttry_remove_empty_parents(r->name, REMOVE_EMPTY_PARENTS_REF);\n }\n \n static void prune_refs(struct ref_to_prune *r)\n@@ -3261,6 +3260,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n \t\t\t\t\tgoto cleanup;\n \t\t\t\t}\n+\t\t\t\tupdate->flags |= REF_DELETED_LOOSE;\n \t\t\t}\n \n \t\t\tif (!(update->flags & REF_ISPRUNING))\n@@ -3273,16 +3273,41 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tret = TRANSACTION_GENERIC_ERROR;\n \t\tgoto cleanup;\n \t}\n-\tfor_each_string_list_item(ref_to_delete, &refs_to_delete)\n-\t\tunlink_or_warn(git_path(\"logs/%s\", ref_to_delete->string));\n+\n+\t/* Delete the reflogs of any references that were deleted: */\n+\tfor_each_string_list_item(ref_to_delete, &refs_to_delete) {\n+\t\tif (!unlink_or_warn(git_path(\"logs/%s\", ref_to_delete->string)))\n+\t\t\ttry_remove_empty_parents(ref_to_delete->string,\n+\t\t\t\t\t\t REMOVE_EMPTY_PARENTS_REFLOG);\n+\t}\n+\n \tclear_loose_ref_cache(&ref_cache);\n \n cleanup:\n \ttransaction->state = REF_TRANSACTION_CLOSED;\n \n-\tfor (i = 0; i < n; i++)\n-\t\tif (updates[i]->lock)\n-\t\t\tunlock_ref(updates[i]->lock);\n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (!update->lock)\n+\t\t\tcontinue;\n+\n+\t\tif (update->flags & REF_DELETED_LOOSE) {\n+\t\t\t/*\n+\t\t\t * The loose reference was deleted. We want to\n+\t\t\t * delete any empty parent directories, but\n+\t\t\t * that can only work after we have removed\n+\t\t\t * the lockfile:\n+\t\t\t */\n+\t\t\tchar *path = xstrdup(update->lock->ref_name);\n+\t\t\tunlock_ref(update->lock);\n+\t\t\ttry_remove_empty_parents(path, REMOVE_EMPTY_PARENTS_REF);\n+\t\t\tfree(path);\n+\t\t} else {\n+\t\t\tunlock_ref(update->lock);\n+\t\t}\n+\t}\n+\n \tstring_list_clear(&refs_to_delete, 0);\n \tstring_list_clear(&affected_refnames, 0);\n \treturn ret;\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex c7dded3..dd09be1 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -43,6 +43,12 @@\n  */\n \n /*\n+ * Used as a flag in ref_update::flags when the loose reference has\n+ * been deleted.\n+ */\n+#define REF_DELETED_LOOSE 0x100\n+\n+/*\n  * Return true iff refname is minimally safe. \"Safe\" here means that\n  * deleting a loose reference by this name will not do any damage, for\n  * example by causing a file that is not a reference to be deleted.\n@@ -141,7 +147,8 @@ struct ref_update {\n \tunsigned char old_sha1[20];\n \t/*\n \t * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,\n-\t * REF_DELETING, and REF_ISPRUNING:\n+\t * REF_DELETING, REF_ISPRUNING, REF_NEEDS_COMMIT, and\n+\t * REF_DELETED_LOOSE:\n \t */\n \tunsigned int flags;\n \tstruct ref_lock *lock;\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex af1b20d..00284eb 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -191,6 +191,33 @@ test_expect_success \\\n \t git update-ref HEAD'\" $A &&\n \t test $A\"' = $(cat .git/'\"$m\"')'\n \n+test_expect_success \"empty directory removal\" '\n+\tgit branch d1/d2/r1 HEAD &&\n+\tgit branch d1/r2 HEAD &&\n+\ttest -f .git/refs/heads/d1/d2/r1 &&\n+\ttest -f .git/logs/refs/heads/d1/d2/r1 &&\n+\tgit branch -d d1/d2/r1 &&\n+\t! test -e .git/refs/heads/d1/d2 &&\n+\t! test -e .git/logs/refs/heads/d1/d2 &&\n+\ttest -f .git/refs/heads/d1/r2 &&\n+\ttest -f .git/logs/refs/heads/d1/r2\n+'\n+\n+test_expect_success \"symref empty directory removal\" '\n+\tgit branch e1/e2/r1 HEAD &&\n+\tgit branch e1/r2 HEAD &&\n+\tgit checkout e1/e2/r1 &&\n+\ttest_when_finished \"git checkout master\" &&\n+\ttest -f .git/refs/heads/e1/e2/r1 &&\n+\ttest -f .git/logs/refs/heads/e1/e2/r1 &&\n+\tgit update-ref -d HEAD &&\n+\t! test -e .git/refs/heads/e1/e2 &&\n+\t! test -e .git/logs/refs/heads/e1/e2 &&\n+\ttest -f .git/refs/heads/e1/r2 &&\n+\ttest -f .git/logs/refs/heads/e1/r2 &&\n+\ttest -f .git/logs/HEAD\n+'\n+\n cat >expect <<EOF\n $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000\tInitial Creation\n $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000\tSwitch\n-- \n2.7.0\n"}]}