{"thread":{"id":"37729","subject":"[PATCH v23 0/25] rs/ref-transaction (\"Use ref transactions\", part 3)","startedAt":"2014-10-15T00:45:23Z","lastAt":"2014-10-16T23:27:19Z","messageCount":28,"participants":["Jonathan Nieder","Junio C Hamano","Michael Haggerty"],"isPatch":true,"patchVersion":23,"patchTotal":25},"messages":[{"id":"250632","messageId":"20141015004522.GD32245@google.com","threadId":"37729","inReplyTo":null,"subject":"[PATCH v23 0/25] rs/ref-transaction (\"Use ref transactions\", part 3)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:45:23Z","receivedAt":"2014-10-15T00:45:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This series by Ronnie was last seen on-list at [1]. It includes some\nimprovements to the ref-transaction API, improves handling of poorly\nnamed refs (which should make it easier to tighten the refname format\nchecks in the future), and is a stepping stone toward later series\nthat use the ref-transaction API more and make it support alternate\nbackends (yay!).\n\nThe changes since (a merge of 'master' and) v22 are very minor and can\nbe seen below[2]. The more important change is that it's rebased\nagainst current 'master'.\n\nReview comments from Michael and Junio were very helpful in getting\nthis in shape. Thanks much to both.\n\nThe series can also be found at\n\n git://repo.or.cz/git/jrn.git tags/rs/ref-transaction\n\nIt is based against current 'master' (670a3c1d, 2014-10-14) and\nintended for 'next'.\n\nThoughts welcome, as always. Improvements preferred in the form of\npatches on top of the series.\n\nJonathan Nieder (6):\n mv test: recreate mod/ directory instead of relying on stale copy\n branch -d: avoid repeated symref resolution\n packed-ref cache: forbid dot-components in refnames\n refs.c: do not permit err == NULL\n lockfile: remove unable_to_lock_error\n ref_transaction_commit: bail out on failure to remove a ref\n\nJunio C Hamano (1):\n reflog test: test interaction with detached HEAD\n\nRonnie Sahlberg (18):\n wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success\n refs.c: lock_ref_sha1_basic is used for all refs\n wrapper.c: add a new function unlink_or_msg\n refs.c: add an err argument to delete_ref_loose\n refs.c: pass the ref log message to _create/delete/update instead of\n _commit\n rename_ref: don't ask read_ref_full where the ref came from\n refs.c: refuse to lock badly named refs in lock_ref_sha1_basic\n refs.c: call lock_ref_sha1_basic directly from commit\n refs.c: pass a list of names to skip to is_refname_available\n refs.c: ref_transaction_commit: distinguish name conflicts from other\n errors\n fetch.c: change s_update_ref to use a ref transaction\n refs.c: make write_ref_sha1 static\n refs.c: change resolve_ref_unsafe reading argument to be a flags field\n branch -d: simplify by using RESOLVE_REF_READING\n test: put tests for handling of bad ref names in one place\n refs.c: allow listing and deleting badly named refs\n for-each-ref: skip and warn about broken ref names\n remote rm/prune: print a message when writing packed-refs fails\n\n branch.c | 6 +-\n builtin/blame.c | 2 +-\n builtin/branch.c | 22 ++-\n builtin/checkout.c | 6 +-\n builtin/clone.c | 2 +-\n builtin/commit.c | 6 +-\n builtin/fetch.c | 34 ++--\n builtin/fmt-merge-msg.c | 2 +-\n builtin/for-each-ref.c | 11 +-\n builtin/fsck.c | 2 +-\n builtin/log.c | 3 +-\n builtin/merge.c | 2 +-\n builtin/notes.c | 2 +-\n builtin/receive-pack.c | 9 +-\n builtin/remote.c | 20 ++-\n builtin/replace.c | 5 +-\n builtin/show-branch.c | 7 +-\n builtin/symbolic-ref.c | 2 +-\n builtin/tag.c | 4 +-\n builtin/update-ref.c | 13 +-\n bundle.c | 2 +-\n cache.h | 44 +++--\n fast-import.c | 8 +-\n git-compat-util.h | 16 +-\n http-backend.c | 4 +-\n lockfile.c | 10 --\n lockfile.h | 1 -\n notes-merge.c | 2 +-\n reflog-walk.c | 5 +-\n refs.c | 446 ++++++++++++++++++++++++++++++++---------------\n refs.h | 44 +++--\n remote.c | 11 +-\n sequencer.c | 8 +-\n t/t1400-update-ref.sh | 62 +++----\n t/t1413-reflog-detach.sh | 70 ++++++++\n t/t1430-bad-ref-name.sh | 207 ++++++++++++++++++++++\n t/t3200-branch.sh | 9 +\n t/t7001-mv.sh | 15 +-\n t/t9300-fast-import.sh | 30 ----\n transport-helper.c | 5 +-\n transport.c | 5 +-\n upload-pack.c | 2 +-\n walker.c | 5 +-\n wrapper.c | 28 ++-\n wt-status.c | 2 +-\n 45 files changed, 850 insertions(+), 351 deletions(-)\n create mode 100755 t/t1413-reflog-detach.sh\n create mode 100755 t/t1430-bad-ref-name.sh\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/254501/focus=257771\n[2]\n cache.h | 11 ++++---\n git-compat-util.h | 4 +--\n refs.c | 96 +++++++++++++++++++++++++++++--------------------------\n refs.h | 6 +++-\n 4 files changed, 64 insertions(+), 53 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 209e8ba..0501f7d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -983,7 +983,8 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n * packed references), REF_ISSYMREF (if the initial reference was a\n * symbolic reference), REF_BAD_NAME (if the reference name is ill\n * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN\n- * (if the ref is malformed).\n+ * (if the ref is malformed or has a bad name). See refs.h for more detail\n+ * on each flag.\n *\n * If ref is not a properly-formatted, normalized reference, return\n * NULL. If more than MAXDEPTH recursive symbolic lookups are needed,\n@@ -991,12 +992,14 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n *\n * RESOLVE_REF_ALLOW_BAD_NAME allows resolving refs even when their\n * name is invalid according to git-check-ref-format(1). If the name\n- * is bad then the value stored in sha1 will be null_sha1 and the\n- * REF_ISBROKEN and REF_BAD_NAME flags will be set.\n+ * is bad then the value stored in sha1 will be null_sha1 and the two\n+ * flags REF_ISBROKEN and REF_BAD_NAME will be set.\n *\n * Even with RESOLVE_REF_ALLOW_BAD_NAME, names that escape the refs/\n * directory and do not consist of all caps and underscores cannot be\n- * resolved. The function returns NULL for such ref names.\n+ * resolved. The function returns NULL for such ref names.\n+ * Caps and underscores refers to the special refs, such as HEAD,\n+ * FETCH_HEAD and friends, that all live outside of the refs/ directory.\n */\n #define RESOLVE_REF_READING 0x01\n #define RESOLVE_REF_NO_RECURSE 0x02\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 8f805bf..59ecf21 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -779,7 +779,7 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n \n /*\n * Preserves errno, prints a message, but gives no warning for ENOENT.\n- * Returns 0 on success which includes trying to unlink an object that does\n+ * Returns 0 on success, which includes trying to unlink an object that does\n * not exist.\n */\n int unlink_or_warn(const char *path);\n@@ -792,7 +792,7 @@ int unlink_or_warn(const char *path);\n int unlink_or_msg(const char *file, struct strbuf *err);\n /*\n * Preserves errno, prints a message, but gives no warning for ENOENT.\n- * Returns 0 on success which includes trying to remove a directory that does\n+ * Returns 0 on success, which includes trying to remove a directory that does\n * not exist.\n */\n int rmdir_or_warn(const char *path);\ndiff --git a/refs.c b/refs.c\nindex bee0e39..0368ed4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -274,29 +274,31 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \treturn dir;\n }\n \n-static int escapes_cwd(const char *path) {\n-\tchar *buf;\n-\tint result;\n-\n-\tif (is_absolute_path(path))\n-\t\treturn 1;\n-\tbuf = xmalloc(strlen(path) + 1);\n-\tresult = !!normalize_path_copy(buf, path);\n-\tfree(buf);\n-\treturn result;\n-}\n-\n /*\n * Check if a refname is safe.\n- * For refs that start with \"refs/\" we consider it safe as long as the rest\n- * of the path components does not allow it to escape from this directory.\n+ * For refs that start with \"refs/\" we consider it safe as long they do\n+ * not try to resolve to outside of refs/.\n+ *\n * For all other refs we only consider them safe iff they only contain\n- * upper case characters and '_'.\n+ * upper case characters and '_' (like \"HEAD\" AND \"MERGE_HEAD\", and not like\n+ * \"config\").\n */\n static int refname_is_safe(const char *refname)\n {\n-\tif (starts_with(refname, \"refs/\"))\n-\t\treturn !escapes_cwd(refname + strlen(\"refs/\"));\n+\tif (starts_with(refname, \"refs/\")) {\n+\t\tchar *buf;\n+\t\tint result;\n+\n+\t\tbuf = xmalloc(strlen(refname) + 1);\n+\t\t/*\n+\t\t * Does the refname try to escape refs/?\n+\t\t * For example: refs/foo/../bar is safe but refs/foo/../../bar\n+\t\t * is not.\n+\t\t */\n+\t\tresult = !normalize_path_copy(buf, refname + strlen(\"refs/\"));\n+\t\tfree(buf);\n+\t\treturn result;\n+\t}\n \twhile (*refname) {\n \t\tif (!isupper(*refname) && *refname != '_')\n \t\t\treturn 0;\n@@ -812,13 +814,13 @@ static void prime_ref_dir(struct ref_dir *dir)\n \t}\n }\n \n-static int entry_matches(struct ref_entry *entry, struct string_list *list)\n+static int entry_matches(struct ref_entry *entry, const struct string_list *list)\n {\n \treturn list && string_list_has_string(list, entry->name);\n }\n \n struct nonmatching_ref_data {\n-\tstruct string_list *skip;\n+\tconst struct string_list *skip;\n \tstruct ref_entry *found;\n };\n \n@@ -842,18 +844,20 @@ static void report_refname_conflict(struct ref_entry *entry,\n /*\n * Return true iff a reference named refname could be created without\n * conflicting with the name of an existing reference in dir. If\n- * skiplist is non-NULL, ignore potential conflicts with names in\n- * skiplist (e.g., because those refs are scheduled for deletion in\n- * the same operation). skiplist must be sorted.\n+ * skip is non-NULL, ignore potential conflicts with refs in skip\n+ * (e.g., because they are scheduled for deletion in the same\n+ * operation).\n *\n * Two reference names conflict if one of them exactly matches the\n * leading components of the other; e.g., \"foo/bar\" conflicts with\n * both \"foo\" and with \"foo/bar/baz\" but not with \"foo/bar\" or\n * \"foo/barbados\".\n+ *\n+ * skip must be sorted.\n */\n static int is_refname_available(const char *refname,\n-\t\t\t\tstruct ref_dir *dir,\n-\t\t\t\tstruct string_list *skiplist)\n+\t\t\t\tconst struct string_list *skip,\n+\t\t\t\tstruct ref_dir *dir)\n {\n \tconst char *slash;\n \tsize_t len;\n@@ -866,12 +870,12 @@ static int is_refname_available(const char *refname,\n \t\t * looking for a conflict with a leaf entry.\n \t\t *\n \t\t * If we find one, we still must make sure it is\n-\t\t * not in \"skiplist\".\n+\t\t * not in \"skip\".\n \t\t */\n \t\tpos = search_ref_dir(dir, refname, slash - refname);\n \t\tif (pos >= 0) {\n \t\t\tstruct ref_entry *entry = dir->entries[pos];\n-\t\t\tif (entry_matches(entry, skiplist))\n+\t\t\tif (entry_matches(entry, skip))\n \t\t\t\treturn 1;\n \t\t\treport_refname_conflict(entry, refname);\n \t\t\treturn 0;\n@@ -903,14 +907,14 @@ static int is_refname_available(const char *refname,\n \tif (pos >= 0) {\n \t\t/*\n \t\t * We found a directory named \"refname\". It is a\n-\t\t * problem iff it contains any ref that is not in\n-\t\t * \"skiplist\".\n+\t\t * problem iff it contains any ref that is not\n+\t\t * in \"skip\".\n \t\t */\n \t\tstruct ref_entry *entry = dir->entries[pos];\n \t\tstruct ref_dir *dir = get_ref_dir(entry);\n \t\tstruct nonmatching_ref_data data;\n \n-\t\tdata.skip = skiplist;\n+\t\tdata.skip = skip;\n \t\tsort_ref_dir(dir);\n \t\tif (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data))\n \t\t\treturn 1;\n@@ -1596,7 +1600,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \t\t}\n \t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (flags)\n-\t\t\t\t*flags |= REF_BAD_NAME | REF_ISBROKEN;\n+\t\t\t\t*flags |= REF_ISBROKEN;\n \n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n \t\t\t !refname_is_safe(buf)) {\n@@ -2217,12 +2221,12 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)\n }\n \n /*\n- * Locks a \"refs/\" ref returning the lock on success and NULL on failure.\n+ * Locks a ref returning the lock on success and NULL on failure.\n * On failure errno is set to something meaningful.\n */\n static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\t\t\t\t const unsigned char *old_sha1,\n-\t\t\t\t\t struct string_list *skiplist,\n+\t\t\t\t\t const struct string_list *skip,\n \t\t\t\t\t int flags, int *type_p)\n {\n \tchar *ref_file;\n@@ -2278,8 +2282,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t * name is a proper prefix of our refname.\n \t */\n \tif (missing &&\n-\t !is_refname_available(refname, get_packed_refs(&ref_cache),\n-\t\t\t\t skiplist)) {\n+\t !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {\n \t\tlast_errno = ENOTDIR;\n \t\tgoto error_return;\n \t}\n@@ -2780,6 +2783,18 @@ static int rename_tmp_log(const char *newrefname)\n \treturn 0;\n }\n \n+static int rename_ref_available(const char *oldname, const char *newname)\n+{\n+\tstruct string_list skip = STRING_LIST_INIT_NODUP;\n+\tint ret;\n+\n+\tstring_list_insert(&skip, oldname);\n+\tret = is_refname_available(newname, &skip, get_packed_refs(&ref_cache))\n+\t && is_refname_available(newname, &skip, get_loose_refs(&ref_cache));\n+\tstring_list_clear(&skip, 0);\n+\treturn ret;\n+}\n+\n static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,\n \t\t\t const char *logmsg);\n \n@@ -2791,7 +2806,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tstruct stat loginfo;\n \tint log = !lstat(git_path(\"logs/%s\", oldrefname), &loginfo);\n \tconst char *symref = NULL;\n-\tstruct string_list skiplist = STRING_LIST_INIT_NODUP;\n \n \tif (log && S_ISLNK(loginfo.st_mode))\n \t\treturn error(\"reflog for %s is a symlink\", oldrefname);\n@@ -2804,18 +2818,8 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tif (!symref)\n \t\treturn error(\"refname %s not found\", oldrefname);\n \n-\tstring_list_insert(&skiplist, oldrefname);\n-\tif (!is_refname_available(newrefname, get_packed_refs(&ref_cache),\n-\t\t\t\t &skiplist)) {\n-\t\tstring_list_clear(&skiplist, 0);\n+\tif (!rename_ref_available(oldrefname, newrefname))\n \t\treturn 1;\n-\t}\n-\tif (!is_refname_available(newrefname, get_loose_refs(&ref_cache),\n-\t\t\t\t &skiplist)) {\n-\t\tstring_list_clear(&skiplist, 0);\n-\t\treturn 1;\n-\t}\n-\tstring_list_clear(&skiplist, 0);\n \n \tif (log && rename(git_path(\"logs/%s\", oldrefname), git_path(TMP_RENAMED_LOG)))\n \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\ndiff --git a/refs.h b/refs.h\nindex e1c43f9..2bc3556 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -62,7 +62,11 @@ struct ref_transaction;\n */\n #define REF_ISBROKEN 0x04\n \n-/* Reference name is not well formed (see git-check-ref-format(1)). */\n+/*\n+ * Reference name is not well formed.\n+ *\n+ * See git-check-ref-format(1) for the definition of well formed ref names.\n+ */\n #define REF_BAD_NAME 0x08\n \n /*\n"},{"id":"250633","messageId":"20141015004628.GE32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 01/25] mv test: recreate mod/ directory instead of relying on stale copy","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:46:28Z","receivedAt":"2014-10-15T00:46:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Jonathan Nieder \nDate: Wed, 10 Sep 2014 14:01:46 -0700\n\nThe tests for 'git mv moves a submodule' functionality often run\ncommands like\n\n\tgit mv sub mod/sub\n\nto move a submodule into a subdirectory. Just like plain /bin/mv,\nthis is supposed to succeed if the mod/ parent directory exists\nand fail if it doesn't exist.\n\nUsually these tests mkdir the parent directory beforehand, but some\ninstead rely on it being left behind by previous tests.\n\nMore precisely, when 'git reset --hard' tries to move to a state where\nmod/sub is not present any more, it would perform the following\noperations:\n\n\trmdir(\"mod/sub\")\n\trmdir(\"mod\")\n\nThe first fails with ENOENT because the test script removed mod/sub\nwith \"rm -rf\" already, so 'reset --hard' doesn't bother to move on to\nthe second, and the mod/ directory is kept around.\n\nBetter to explicitly remove and re-create the mod/ directory so later\ntests don't have to depend on the directory left behind by the earlier\nones at all (making it easier to rearrange or skip some tests in the\nfile or to tweak 'reset --hard' behavior without breaking unrelated\ntests).\n\nNoticed while testing a patch that fixes the reset --hard behavior\ndescribed above.\n\nSigned-off-by: Jonathan Nieder \nReviewed-by: Ronnie Sahlberg \n---\n t/t7001-mv.sh | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 54d7807..69f11bd 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -350,10 +350,11 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu\n '\n \n test_expect_success 'git mv moves a submodule with gitfile' '\n-\trm -rf mod/sub &&\n+\trm -rf mod &&\n \tgit reset --hard &&\n \tgit submodule update &&\n \tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tmkdir mod &&\n \t(\n \t\tcd mod &&\n \t\tgit mv ../sub/ .\n@@ -372,11 +373,12 @@ test_expect_success 'git mv moves a submodule with gitfile' '\n '\n \n test_expect_success 'mv does not complain when no .gitmodules file is found' '\n-\trm -rf mod/sub &&\n+\trm -rf mod &&\n \tgit reset --hard &&\n \tgit submodule update &&\n \tgit rm .gitmodules &&\n \tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tmkdir mod &&\n \tgit mv sub mod/sub 2>actual.err &&\n \t! test -s actual.err &&\n \t! test -e sub &&\n@@ -390,11 +392,12 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' '\n '\n \n test_expect_success 'mv will error out on a modified .gitmodules file unless staged' '\n-\trm -rf mod/sub &&\n+\trm -rf mod &&\n \tgit reset --hard &&\n \tgit submodule update &&\n \tgit config -f .gitmodules foo.bar true &&\n \tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tmkdir mod &&\n \ttest_must_fail git mv sub mod/sub 2>actual.err &&\n \ttest -s actual.err &&\n \ttest -e sub &&\n@@ -413,13 +416,14 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta\n '\n \n test_expect_success 'mv issues a warning when section is not found in .gitmodules' '\n-\trm -rf mod/sub &&\n+\trm -rf mod &&\n \tgit reset --hard &&\n \tgit submodule update &&\n \tgit config -f .gitmodules --remove-section submodule.sub &&\n \tgit add .gitmodules &&\n \tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n \techo \"warning: Could not find section in .gitmodules where path=sub\" >expect.err &&\n+\tmkdir mod &&\n \tgit mv sub mod/sub 2>actual.err &&\n \ttest_i18ncmp expect.err actual.err &&\n \t! test -e sub &&\n@@ -433,9 +437,10 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule\n '\n \n test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' '\n-\trm -rf mod/sub &&\n+\trm -rf mod &&\n \tgit reset --hard &&\n \tgit submodule update &&\n+\tmkdir mod &&\n \tgit mv -n sub mod/sub 2>actual.err &&\n \ttest -f sub/.git &&\n \tgit diff-index --exit-code HEAD &&\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250634","messageId":"20141015004653.GF32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 02/25] wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:46:53Z","receivedAt":"2014-10-15T00:46:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Wed, 16 Jul 2014 11:01:18 -0700\n\nSimplify the function warn_if_unremovable slightly. Additionally, change\nbehaviour slightly. If we failed to remove the object because the object\ndoes not exist, we can still return success back to the caller since none of\nthe callers depend on \"fail if the file did not exist\".\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n git-compat-util.h | 7 +++++--\n refs.c | 2 +-\n wrapper.c | 14 ++++++--------\n 3 files changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex fb41118..d67592f 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -777,11 +777,14 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n \n /*\n * Preserves errno, prints a message, but gives no warning for ENOENT.\n- * Always returns the return value of unlink(2).\n+ * Returns 0 on success, which includes trying to unlink an object that does\n+ * not exist.\n */\n int unlink_or_warn(const char *path);\n /*\n- * Likewise for rmdir(2).\n+ * Preserves errno, prints a message, but gives no warning for ENOENT.\n+ * Returns 0 on success, which includes trying to remove a directory that does\n+ * not exist.\n */\n int rmdir_or_warn(const char *path);\n /*\ndiff --git a/refs.c b/refs.c\nindex a77458f..2dcf6c6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2607,7 +2607,7 @@ static int delete_ref_loose(struct ref_lock *lock, int flag)\n \t\tchar *loose_filename = get_locked_file_path(lock->lk);\n \t\tint err = unlink_or_warn(loose_filename);\n \t\tfree(loose_filename);\n-\t\tif (err && errno != ENOENT)\n+\t\tif (err)\n \t\t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/wrapper.c b/wrapper.c\nindex 5b77d2a..8d4be66 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -466,14 +466,12 @@ int xmkstemp_mode(char *template, int mode)\n \n static int warn_if_unremovable(const char *op, const char *file, int rc)\n {\n-\tif (rc < 0) {\n-\t\tint err = errno;\n-\t\tif (ENOENT != err) {\n-\t\t\twarning(\"unable to %s %s: %s\",\n-\t\t\t\top, file, strerror(errno));\n-\t\t\terrno = err;\n-\t\t}\n-\t}\n+\tint err;\n+\tif (!rc || errno == ENOENT)\n+\t\treturn 0;\n+\terr = errno;\n+\twarning(\"unable to %s %s: %s\", op, file, strerror(errno));\n+\terrno = err;\n \treturn rc;\n }\n \n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250635","messageId":"20141015004713.GG32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 03/25] refs.c: lock_ref_sha1_basic is used for all refs","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:47:13Z","receivedAt":"2014-10-15T00:47:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 2 Oct 2014 07:59:02 -0700\n\nlock_ref_sha1_basic is used to lock refs that sit directly in the .git\ndir such as HEAD and MERGE_HEAD in addition to the more ordinary refs\nunder \"refs/\". Remove the note claiming otherwise.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 2dcf6c6..4f2564d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2134,7 +2134,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)\n }\n \n /*\n- * Locks a \"refs/\" ref returning the lock on success and NULL on failure.\n+ * Locks a ref returning the lock on success and NULL on failure.\n * On failure errno is set to something meaningful.\n */\n static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250636","messageId":"20141015004737.GH32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 04/25] wrapper.c: add a new function unlink_or_msg","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:47:37Z","receivedAt":"2014-10-15T00:47:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Wed, 16 Jul 2014 11:20:36 -0700\n\nThis behaves like unlink_or_warn except that on failure it writes the message\nto its 'err' argument, which the caller can display in an appropriate way or\nignore.\n\nSigned-off-by: Ronnie Sahlberg \nReviewed-by: Michael Haggerty \nSigned-off-by: Jonathan Nieder \n---\n git-compat-util.h | 9 +++++++++\n wrapper.c | 14 ++++++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex d67592f..59ecf21 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -326,6 +326,8 @@ static inline char *git_find_last_dir_sep(const char *path)\n \n #include \"wildmatch.h\"\n \n+struct strbuf;\n+\n /* General helper functions */\n extern void vreportf(const char *prefix, const char *err, va_list params);\n extern void vwritef(int fd, const char *prefix, const char *err, va_list params);\n@@ -781,6 +783,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n * not exist.\n */\n int unlink_or_warn(const char *path);\n+ /*\n+ * Tries to unlink file. Returns 0 if unlink succeeded\n+ * or the file already didn't exist. Returns -1 and\n+ * appends a message to err suitable for\n+ * 'error(\"%s\", err->buf)' on error.\n+ */\n+int unlink_or_msg(const char *file, struct strbuf *err);\n /*\n * Preserves errno, prints a message, but gives no warning for ENOENT.\n * Returns 0 on success, which includes trying to remove a directory that does\ndiff --git a/wrapper.c b/wrapper.c\nindex 8d4be66..007ec0d 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -475,6 +475,20 @@ static int warn_if_unremovable(const char *op, const char *file, int rc)\n \treturn rc;\n }\n \n+int unlink_or_msg(const char *file, struct strbuf *err)\n+{\n+\tint rc = unlink(file);\n+\n+\tassert(err);\n+\n+\tif (!rc || errno == ENOENT)\n+\t\treturn 0;\n+\n+\tstrbuf_addf(err, \"unable to unlink %s: %s\",\n+\t\t file, strerror(errno));\n+\treturn -1;\n+}\n+\n int unlink_or_warn(const char *file)\n {\n \treturn warn_if_unremovable(\"unlink\", file, unlink(file));\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250637","messageId":"20141015004756.GI32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 05/25] refs.c: add an err argument to delete_ref_loose","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:47:56Z","receivedAt":"2014-10-15T00:47:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 15 May 2014 08:25:23 -0700\n\nAdd an err argument to delete_ref_loose so that we can pass a descriptive\nerror string back to the caller. Pass the err argument from transaction\ncommit to this function so that transaction users will have a nice error\nstring if the transaction failed due to delete_ref_loose.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4f2564d..430857b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2597,7 +2597,7 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \treturn ret;\n }\n \n-static int delete_ref_loose(struct ref_lock *lock, int flag)\n+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)\n {\n \tif (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {\n \t\t/*\n@@ -2605,9 +2605,9 @@ static int delete_ref_loose(struct ref_lock *lock, int flag)\n \t\t * lockfile name, minus \".lock\":\n \t\t */\n \t\tchar *loose_filename = get_locked_file_path(lock->lk);\n-\t\tint err = unlink_or_warn(loose_filename);\n+\t\tint res = unlink_or_msg(loose_filename, err);\n \t\tfree(loose_filename);\n-\t\tif (err)\n+\t\tif (res)\n \t\t\treturn 1;\n \t}\n \treturn 0;\n@@ -3658,7 +3658,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (update->lock) {\n-\t\t\tret |= delete_ref_loose(update->lock, update->type);\n+\t\t\tret |= delete_ref_loose(update->lock, update->type,\n+\t\t\t\t\t\terr);\n \t\t\tif (!(update->flags & REF_ISPRUNING))\n \t\t\t\tdelnames[delnum++] = update->lock->ref_name;\n \t\t}\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250638","messageId":"20141015004822.GJ32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 06/25] refs.c: pass the ref log message to _create/delete/update instead of _commit","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:48:22Z","receivedAt":"2014-10-15T00:48:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Wed, 30 Apr 2014 12:22:42 -0700\n\nChange the ref transaction API so that we pass the reflog message to the\ncreate/delete/update functions instead of to ref_transaction_commit.\nThis allows different reflog messages for each ref update in a multi-ref\ntransaction.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n branch.c | 4 ++--\n builtin/commit.c | 4 ++--\n builtin/receive-pack.c | 5 +++--\n builtin/replace.c | 5 +++--\n builtin/tag.c | 4 ++--\n builtin/update-ref.c | 13 +++++++------\n fast-import.c | 8 ++++----\n refs.c | 34 +++++++++++++++++++++-------------\n refs.h | 12 ++++++------\n sequencer.c | 4 ++--\n walker.c | 5 ++---\n 11 files changed, 54 insertions(+), 44 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 9a2228e..884c62c 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -285,8 +285,8 @@ void create_branch(const char *head,\n \t\ttransaction = ref_transaction_begin(&err);\n \t\tif (!transaction ||\n \t\t ref_transaction_update(transaction, ref.buf, sha1,\n-\t\t\t\t\t null_sha1, 0, !forcing, &err) ||\n-\t\t ref_transaction_commit(transaction, msg, &err))\n+\t\t\t\t\t null_sha1, 0, !forcing, msg, &err) ||\n+\t\t ref_transaction_commit(transaction, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n \t\tstrbuf_release(&err);\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 81dc622..a7857ab 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1811,8 +1811,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t ref_transaction_update(transaction, \"HEAD\", sha1,\n \t\t\t\t current_head\n \t\t\t\t ? current_head->object.sha1 : NULL,\n-\t\t\t\t 0, !!current_head, &err) ||\n-\t ref_transaction_commit(transaction, sb.buf, &err)) {\n+\t\t\t\t 0, !!current_head, sb.buf, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\trollback_index_files();\n \t\tdie(\"%s\", err.buf);\n \t}\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex f2f6c67..df6c337 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -842,8 +842,9 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\ttransaction = ref_transaction_begin(&err);\n \t\tif (!transaction ||\n \t\t ref_transaction_update(transaction, namespaced_name,\n-\t\t\t\t\t new_sha1, old_sha1, 0, 1, &err) ||\n-\t\t ref_transaction_commit(transaction, \"push\", &err)) {\n+\t\t\t\t\t new_sha1, old_sha1, 0, 1, \"push\",\n+\t\t\t\t\t &err) ||\n+\t\t ref_transaction_commit(transaction, &err)) {\n \t\t\tref_transaction_free(transaction);\n \n \t\t\trp_error(\"%s\", err.buf);\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex 8020db8..85d39b5 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -171,8 +171,9 @@ static int replace_object_sha1(const char *object_ref,\n \n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n-\t ref_transaction_update(transaction, ref, repl, prev, 0, 1, &err) ||\n-\t ref_transaction_commit(transaction, NULL, &err))\n+\t ref_transaction_update(transaction, ref, repl, prev,\n+\t\t\t\t 0, 1, NULL, &err) ||\n+\t ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tref_transaction_free(transaction);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex a81b9e4..e633f4e 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -733,8 +733,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_update(transaction, ref.buf, object, prev,\n-\t\t\t\t 0, 1, &err) ||\n-\t ref_transaction_commit(transaction, NULL, &err))\n+\t\t\t\t 0, 1, NULL, &err) ||\n+\t ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n \tif (force && !is_null_sha1(prev) && hashcmp(prev, object))\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 54a48c0..6c9be05 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = {\n \n static char line_termination = '\\n';\n static int update_flags;\n+static const char *msg;\n \n /*\n * Parse one whitespace- or NUL-terminated, possibly C-quoted argument\n@@ -198,7 +199,7 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,\n \t\tdie(\"update %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, &err))\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n@@ -229,7 +230,7 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,\n \t\tdie(\"create %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_create(transaction, refname, new_sha1,\n-\t\t\t\t update_flags, &err))\n+\t\t\t\t update_flags, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n@@ -264,7 +265,7 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,\n \t\tdie(\"delete %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_delete(transaction, refname, old_sha1,\n-\t\t\t\t update_flags, have_old, &err))\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n@@ -300,7 +301,7 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,\n \t\tdie(\"verify %s: extra input: %s\", refname, next);\n \n \tif (ref_transaction_update(transaction, refname, new_sha1, old_sha1,\n-\t\t\t\t update_flags, have_old, &err))\n+\t\t\t\t update_flags, have_old, msg, &err))\n \t\tdie(\"%s\", err.buf);\n \n \tupdate_flags = 0;\n@@ -354,7 +355,7 @@ static void update_refs_stdin(struct ref_transaction *transaction)\n \n int cmd_update_ref(int argc, const char **argv, const char *prefix)\n {\n-\tconst char *refname, *oldval, *msg = NULL;\n+\tconst char *refname, *oldval;\n \tunsigned char sha1[20], oldsha1[20];\n \tint delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;\n \tstruct option options[] = {\n@@ -385,7 +386,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin(transaction);\n-\t\tif (ref_transaction_commit(transaction, msg, &err))\n+\t\tif (ref_transaction_commit(transaction, &err))\n \t\t\tdie(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n \t\tstrbuf_release(&err);\ndiff --git a/fast-import.c b/fast-import.c\nindex fee7906..d0bd285 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1716,8 +1716,8 @@ static int update_branch(struct branch *b)\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_update(transaction, b->name, b->sha1, old_sha1,\n-\t\t\t\t 0, 1, &err) ||\n-\t ref_transaction_commit(transaction, msg, &err)) {\n+\t\t\t\t 0, 1, msg, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_free(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&err);\n@@ -1757,12 +1757,12 @@ static void dump_tags(void)\n \t\tstrbuf_addf(&ref_name, \"refs/tags/%s\", t->name);\n \n \t\tif (ref_transaction_update(transaction, ref_name.buf, t->sha1,\n-\t\t\t\t\t NULL, 0, 0, &err)) {\n+\t\t\t\t\t NULL, 0, 0, msg, &err)) {\n \t\t\tfailure |= error(\"%s\", err.buf);\n \t\t\tgoto cleanup;\n \t\t}\n \t}\n-\tif (ref_transaction_commit(transaction, msg, &err))\n+\tif (ref_transaction_commit(transaction, &err))\n \t\tfailure |= error(\"%s\", err.buf);\n \n cleanup:\ndiff --git a/refs.c b/refs.c\nindex 430857b..f5d7bd7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2445,8 +2445,8 @@ static void prune_ref(struct ref_to_prune *r)\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, r->name, r->sha1,\n-\t\t\t\t REF_ISPRUNING, 1, &err) ||\n-\t ref_transaction_commit(transaction, NULL, &err)) {\n+\t\t\t\t REF_ISPRUNING, 1, NULL, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_free(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&err);\n@@ -2621,8 +2621,8 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, sha1, delopt,\n-\t\t\t\t sha1 && !is_null_sha1(sha1), &err) ||\n-\t ref_transaction_commit(transaction, NULL, &err)) {\n+\t\t\t\t sha1 && !is_null_sha1(sha1), NULL, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\n \t\tstrbuf_release(&err);\n@@ -3404,6 +3404,7 @@ struct ref_update {\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n \tstruct ref_lock *lock;\n \tint type;\n+\tchar *msg;\n \tconst char refname[FLEX_ARRAY];\n };\n \n@@ -3446,9 +3447,10 @@ void ref_transaction_free(struct ref_transaction *transaction)\n \tif (!transaction)\n \t\treturn;\n \n-\tfor (i = 0; i < transaction->nr; i++)\n+\tfor (i = 0; i < transaction->nr; i++) {\n+\t\tfree(transaction->updates[i]->msg);\n \t\tfree(transaction->updates[i]);\n-\n+\t}\n \tfree(transaction->updates);\n \tfree(transaction);\n }\n@@ -3469,7 +3471,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const unsigned char *new_sha1,\n \t\t\t const unsigned char *old_sha1,\n-\t\t\t int flags, int have_old,\n+\t\t\t int flags, int have_old, const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n@@ -3486,13 +3488,15 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \tupdate->have_old = have_old;\n \tif (have_old)\n \t\thashcpy(update->old_sha1, old_sha1);\n+\tif (msg)\n+\t\tupdate->msg = xstrdup(msg);\n \treturn 0;\n }\n \n int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const unsigned char *new_sha1,\n-\t\t\t int flags,\n+\t\t\t int flags, const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n@@ -3509,13 +3513,15 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \thashclr(update->old_sha1);\n \tupdate->flags = flags;\n \tupdate->have_old = 1;\n+\tif (msg)\n+\t\tupdate->msg = xstrdup(msg);\n \treturn 0;\n }\n \n int ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const unsigned char *old_sha1,\n-\t\t\t int flags, int have_old,\n+\t\t\t int flags, int have_old, const char *msg,\n \t\t\t struct strbuf *err)\n {\n \tstruct ref_update *update;\n@@ -3533,6 +3539,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n \t\tassert(!is_null_sha1(old_sha1));\n \t\thashcpy(update->old_sha1, old_sha1);\n \t}\n+\tif (msg)\n+\t\tupdate->msg = xstrdup(msg);\n \treturn 0;\n }\n \n@@ -3546,8 +3554,8 @@ int update_ref(const char *action, const char *refname,\n \tt = ref_transaction_begin(&err);\n \tif (!t ||\n \t ref_transaction_update(t, refname, sha1, oldval, flags,\n-\t\t\t\t !!oldval, &err) ||\n-\t ref_transaction_commit(t, action, &err)) {\n+\t\t\t\t !!oldval, action, &err) ||\n+\t ref_transaction_commit(t, &err)) {\n \t\tconst char *str = \"update_ref failed for ref '%s': %s\";\n \n \t\tref_transaction_free(t);\n@@ -3593,7 +3601,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n }\n \n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t const char *msg, struct strbuf *err)\n+\t\t\t struct strbuf *err)\n {\n \tint ret = 0, delnum = 0, i;\n \tconst char **delnames;\n@@ -3642,7 +3650,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \t\tif (!is_null_sha1(update->new_sha1)) {\n \t\t\tret = write_ref_sha1(update->lock, update->new_sha1,\n-\t\t\t\t\t msg);\n+\t\t\t\t\t update->msg);\n \t\t\tupdate->lock = NULL; /* freed by write_ref_sha1 */\n \t\t\tif (ret) {\n \t\t\t\tif (err)\ndiff --git a/refs.h b/refs.h\nindex 2328f06..1a55cab 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -274,8 +274,8 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);\n * The following functions add a reference check or update to a\n * ref_transaction. In all of them, refname is the name of the\n * reference to be affected. The functions make internal copies of\n- * refname, so the caller retains ownership of the parameter. flags\n- * can be REF_NODEREF; it is passed to update_ref_lock().\n+ * refname and msg, so the caller retains ownership of these parameters.\n+ * flags can be REF_NODEREF; it is passed to update_ref_lock().\n */\n \n /*\n@@ -292,7 +292,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const unsigned char *new_sha1,\n \t\t\t const unsigned char *old_sha1,\n-\t\t\t int flags, int have_old,\n+\t\t\t int flags, int have_old, const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\n@@ -307,7 +307,7 @@ int ref_transaction_update(struct ref_transaction *transaction,\n int ref_transaction_create(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const unsigned char *new_sha1,\n-\t\t\t int flags,\n+\t\t\t int flags, const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\n@@ -321,7 +321,7 @@ int ref_transaction_create(struct ref_transaction *transaction,\n int ref_transaction_delete(struct ref_transaction *transaction,\n \t\t\t const char *refname,\n \t\t\t const unsigned char *old_sha1,\n-\t\t\t int flags, int have_old,\n+\t\t\t int flags, int have_old, const char *msg,\n \t\t\t struct strbuf *err);\n \n /*\n@@ -330,7 +330,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n * problem.\n */\n int ref_transaction_commit(struct ref_transaction *transaction,\n-\t\t\t const char *msg, struct strbuf *err);\n+\t\t\t struct strbuf *err);\n \n /*\n * Free an existing transaction and all associated data.\ndiff --git a/sequencer.c b/sequencer.c\nindex 1b9a35e..47aac75 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -252,8 +252,8 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,\n \tif (!transaction ||\n \t ref_transaction_update(transaction, \"HEAD\",\n \t\t\t\t to, unborn ? null_sha1 : from,\n-\t\t\t\t 0, 1, &err) ||\n-\t ref_transaction_commit(transaction, sb.buf, &err)) {\n+\t\t\t\t 0, 1, sb.buf, &err) ||\n+\t ref_transaction_commit(transaction, &err)) {\n \t\tref_transaction_free(transaction);\n \t\terror(\"%s\", err.buf);\n \t\tstrbuf_release(&sb);\ndiff --git a/walker.c b/walker.c\nindex 18a67d3..f149371 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -300,14 +300,13 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\tstrbuf_addf(&refname, \"refs/%s\", write_ref[i]);\n \t\tif (ref_transaction_update(transaction, refname.buf,\n \t\t\t\t\t &sha1[20 * i], NULL, 0, 0,\n+\t\t\t\t\t msg ? msg : \"fetch (unknown)\",\n \t\t\t\t\t &err)) {\n \t\t\terror(\"%s\", err.buf);\n \t\t\tgoto done;\n \t\t}\n \t}\n-\tif (ref_transaction_commit(transaction,\n-\t\t\t\t msg ? msg : \"fetch (unknown)\",\n-\t\t\t\t &err)) {\n+\tif (ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tgoto done;\n \t}\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250639","messageId":"20141015004846.GK32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 07/25] rename_ref: don't ask read_ref_full where the ref came from","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:48:46Z","receivedAt":"2014-10-15T00:48:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Wed, 30 Apr 2014 12:41:04 -0700\n\nWe call read_ref_full with a pointer to flags from rename_ref but since\nwe never actually use the returned flags we can just pass NULL here instead.\n\nSigned-off-by: Ronnie Sahlberg \nReviewed-by: Michael Haggerty \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex f5d7bd7..3c45615 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2721,7 +2721,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \t\tgoto rollback;\n \t}\n \n-\tif (!read_ref_full(newrefname, sha1, 1, &flag) &&\n+\tif (!read_ref_full(newrefname, sha1, 1, NULL) &&\n \t delete_ref(newrefname, sha1, REF_NODEREF)) {\n \t\tif (errno==EISDIR) {\n \t\t\tif (remove_empty_directories(git_path(\"%s\", newrefname))) {\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250640","messageId":"20141015004911.GL32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 08/25] refs.c: refuse to lock badly named refs in lock_ref_sha1_basic","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:49:11Z","receivedAt":"2014-10-15T00:49:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 1 May 2014 10:40:10 -0700\n\nMove the check for check_refname_format from lock_any_ref_for_update to\nlock_ref_sha1_basic. At some later stage we will get rid of\nlock_any_ref_for_update completely. This has no visible impact to callers\nexcept for the inability to lock badly named refs, which is not possible\ntoday already for other reasons.(*)\n\nKeep lock_any_ref_for_update as a no-op wrapper. It is the public facing\nversion of this interface and keeping it as a separate function will make\nit easier to experiment with the internal lock_ref_sha1_basic signature.\n\n(*) For example, if lock_ref_sha1_basic checks the refname format and\nrefuses to lock badly named refs, it will not be possible to delete\nsuch refs because the first step of deletion is to lock the ref. We\ncurrently already fail in that case because these refs are not recognized\nto exist:\n\n $ cp .git/refs/heads/master .git/refs/heads/echo...\\*\\*\n $ git branch -D .git/refs/heads/echo...\\*\\*\n error: branch '.git/refs/heads/echo...**' not found.\n\nThis has been broken for a while. Later patches in the series will start\nrepairing the handling of badly named refs.\n\nSigned-off-by: Ronnie Sahlberg \nReviewed-by: Michael Haggerty \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 3c45615..9c01623 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2150,6 +2150,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tint missing = 0;\n \tint attempts_remaining = 3;\n \n+\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n+\t\terrno = EINVAL;\n+\t\treturn NULL;\n+\t}\n+\n \tlock = xcalloc(1, sizeof(struct ref_lock));\n \tlock->lock_fd = -1;\n \n@@ -2241,8 +2246,6 @@ struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t const unsigned char *old_sha1,\n \t\t\t\t\t int flags, int *type_p)\n {\n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n-\t\treturn NULL;\n \treturn lock_ref_sha1_basic(refname, old_sha1, flags, type_p);\n }\n \n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250641","messageId":"20141015004927.GM32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 09/25] refs.c: call lock_ref_sha1_basic directly from commit","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:49:27Z","receivedAt":"2014-10-15T00:49:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 1 May 2014 10:43:39 -0700\n\nSkip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic\ndirectly from the commit function.\n\nSigned-off-by: Ronnie Sahlberg \nReviewed-by: Michael Haggerty \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9c01623..b591b9c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3632,12 +3632,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n-\t\tupdate->lock = lock_any_ref_for_update(update->refname,\n-\t\t\t\t\t\t (update->have_old ?\n-\t\t\t\t\t\t\tupdate->old_sha1 :\n-\t\t\t\t\t\t\tNULL),\n-\t\t\t\t\t\t update->flags,\n-\t\t\t\t\t\t &update->type);\n+\t\tupdate->lock = lock_ref_sha1_basic(update->refname,\n+\t\t\t\t\t\t (update->have_old ?\n+\t\t\t\t\t\t update->old_sha1 :\n+\t\t\t\t\t\t NULL),\n+\t\t\t\t\t\t update->flags,\n+\t\t\t\t\t\t &update->type);\n \t\tif (!update->lock) {\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250642","messageId":"20141015005009.GN32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 10/25] refs.c: pass a list of names to skip to is_refname_available","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:50:09Z","receivedAt":"2014-10-15T00:50:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 1 May 2014 11:16:07 -0700\n\nChange is_refname_available to take a list of strings to exclude when\nchecking for conflicts instead of just one single name. We can already\nexclude a single name for the sake of renames. This generalizes that support.\n\nref_transaction_commit already tracks a set of refs that are being deleted\nin an array. This array is then used to exclude refs from being written to\nthe packed-refs file. At some stage we will want to change this array to a\nstruct string_list and then we can pass it to is_refname_available via the\ncall to lock_ref_sha1_basic. That will allow us to perform transactions\nthat perform multiple renames as long as there are no conflicts within the\nstarting or ending state.\n\nFor example, that would allow a single transaction that contains two\nrenames that are both individually conflicting:\n\n m -> n/n\n n -> m/m\n\nNo functional change intended yet.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 50 ++++++++++++++++++++++++++++++++------------------\n 1 file changed, 32 insertions(+), 18 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex b591b9c..a007cf3 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -787,13 +787,13 @@ static void prime_ref_dir(struct ref_dir *dir)\n \t}\n }\n \n-static int entry_matches(struct ref_entry *entry, const char *refname)\n+static int entry_matches(struct ref_entry *entry, const struct string_list *list)\n {\n-\treturn refname && !strcmp(entry->name, refname);\n+\treturn list && string_list_has_string(list, entry->name);\n }\n \n struct nonmatching_ref_data {\n-\tconst char *skip;\n+\tconst struct string_list *skip;\n \tstruct ref_entry *found;\n };\n \n@@ -817,16 +817,19 @@ static void report_refname_conflict(struct ref_entry *entry,\n /*\n * Return true iff a reference named refname could be created without\n * conflicting with the name of an existing reference in dir. If\n- * oldrefname is non-NULL, ignore potential conflicts with oldrefname\n- * (e.g., because oldrefname is scheduled for deletion in the same\n+ * skip is non-NULL, ignore potential conflicts with refs in skip\n+ * (e.g., because they are scheduled for deletion in the same\n * operation).\n *\n * Two reference names conflict if one of them exactly matches the\n * leading components of the other; e.g., \"foo/bar\" conflicts with\n * both \"foo\" and with \"foo/bar/baz\" but not with \"foo/bar\" or\n * \"foo/barbados\".\n+ *\n+ * skip must be sorted.\n */\n-static int is_refname_available(const char *refname, const char *oldrefname,\n+static int is_refname_available(const char *refname,\n+\t\t\t\tconst struct string_list *skip,\n \t\t\t\tstruct ref_dir *dir)\n {\n \tconst char *slash;\n@@ -840,12 +843,12 @@ static int is_refname_available(const char *refname, const char *oldrefname,\n \t\t * looking for a conflict with a leaf entry.\n \t\t *\n \t\t * If we find one, we still must make sure it is\n-\t\t * not \"oldrefname\".\n+\t\t * not in \"skip\".\n \t\t */\n \t\tpos = search_ref_dir(dir, refname, slash - refname);\n \t\tif (pos >= 0) {\n \t\t\tstruct ref_entry *entry = dir->entries[pos];\n-\t\t\tif (entry_matches(entry, oldrefname))\n+\t\t\tif (entry_matches(entry, skip))\n \t\t\t\treturn 1;\n \t\t\treport_refname_conflict(entry, refname);\n \t\t\treturn 0;\n@@ -878,13 +881,13 @@ static int is_refname_available(const char *refname, const char *oldrefname,\n \t\t/*\n \t\t * We found a directory named \"refname\". It is a\n \t\t * problem iff it contains any ref that is not\n-\t\t * \"oldrefname\".\n+\t\t * in \"skip\".\n \t\t */\n \t\tstruct ref_entry *entry = dir->entries[pos];\n \t\tstruct ref_dir *dir = get_ref_dir(entry);\n \t\tstruct nonmatching_ref_data data;\n \n-\t\tdata.skip = oldrefname;\n+\t\tdata.skip = skip;\n \t\tsort_ref_dir(dir);\n \t\tif (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data))\n \t\t\treturn 1;\n@@ -2139,6 +2142,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)\n */\n static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\t\t\t\t const unsigned char *old_sha1,\n+\t\t\t\t\t const struct string_list *skip,\n \t\t\t\t\t int flags, int *type_p)\n {\n \tchar *ref_file;\n@@ -2188,7 +2192,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t * name is a proper prefix of our refname.\n \t */\n \tif (missing &&\n-\t !is_refname_available(refname, NULL, get_packed_refs(&ref_cache))) {\n+\t !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {\n \t\tlast_errno = ENOTDIR;\n \t\tgoto error_return;\n \t}\n@@ -2246,7 +2250,7 @@ struct ref_lock *lock_any_ref_for_update(const char *refname,\n \t\t\t\t\t const unsigned char *old_sha1,\n \t\t\t\t\t int flags, int *type_p)\n {\n-\treturn lock_ref_sha1_basic(refname, old_sha1, flags, type_p);\n+\treturn lock_ref_sha1_basic(refname, old_sha1, NULL, flags, type_p);\n }\n \n /*\n@@ -2690,6 +2694,18 @@ static int rename_tmp_log(const char *newrefname)\n \treturn 0;\n }\n \n+static int rename_ref_available(const char *oldname, const char *newname)\n+{\n+\tstruct string_list skip = STRING_LIST_INIT_NODUP;\n+\tint ret;\n+\n+\tstring_list_insert(&skip, oldname);\n+\tret = is_refname_available(newname, &skip, get_packed_refs(&ref_cache))\n+\t && is_refname_available(newname, &skip, get_loose_refs(&ref_cache));\n+\tstring_list_clear(&skip, 0);\n+\treturn ret;\n+}\n+\n int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)\n {\n \tunsigned char sha1[20], orig_sha1[20];\n@@ -2709,10 +2725,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tif (!symref)\n \t\treturn error(\"refname %s not found\", oldrefname);\n \n-\tif (!is_refname_available(newrefname, oldrefname, get_packed_refs(&ref_cache)))\n-\t\treturn 1;\n-\n-\tif (!is_refname_available(newrefname, oldrefname, get_loose_refs(&ref_cache)))\n+\tif (!rename_ref_available(oldrefname, newrefname))\n \t\treturn 1;\n \n \tif (log && rename(git_path(\"logs/%s\", oldrefname), git_path(TMP_RENAMED_LOG)))\n@@ -2742,7 +2755,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \n \tlogmoved = log;\n \n-\tlock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL);\n+\tlock = lock_ref_sha1_basic(newrefname, NULL, NULL, 0, NULL);\n \tif (!lock) {\n \t\terror(\"unable to lock %s for update\", newrefname);\n \t\tgoto rollback;\n@@ -2757,7 +2770,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \treturn 0;\n \n rollback:\n-\tlock = lock_ref_sha1_basic(oldrefname, NULL, 0, NULL);\n+\tlock = lock_ref_sha1_basic(oldrefname, NULL, NULL, 0, NULL);\n \tif (!lock) {\n \t\terror(\"unable to lock %s for rollback\", oldrefname);\n \t\tgoto rollbacklog;\n@@ -3636,6 +3649,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\t\t (update->have_old ?\n \t\t\t\t\t\t update->old_sha1 :\n \t\t\t\t\t\t NULL),\n+\t\t\t\t\t\t NULL,\n \t\t\t\t\t\t update->flags,\n \t\t\t\t\t\t &update->type);\n \t\tif (!update->lock) {\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250643","messageId":"20141015005045.GO32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 11/25] refs.c: ref_transaction_commit: distinguish name conflicts from other errors","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:50:46Z","receivedAt":"2014-10-15T00:50:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Fri, 16 May 2014 14:14:38 -0700\n\nIn _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either\nwhen we lstat the new refname or if the name checking function reports that\nthe same type of conflict happened. In both cases, it means that we can not\ncreate the new ref due to a name conflict.\n\nStart defining specific return codes for _commit. TRANSACTION_NAME_CONFLICT\nrefers to a failure to create a ref due to a name conflict with another ref.\nTRANSACTION_GENERIC_ERROR is for all other errors.\n\nWhen \"git fetch\" is creating refs, name conflicts differ from other errors in\nthat they are likely to be resolved by running \"git remote prune \".\n\"git fetch\" currently inspects errno to decide whether to give that advice.\nOnce it switches to the transaction API, it can check for\nTRANSACTION_NAME_CONFLICT instead.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 26 ++++++++++++++++----------\n refs.h | 9 +++++++--\n 2 files changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a007cf3..9d9bbeb 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3637,9 +3637,10 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \n \t/* Copy, sort, and reject duplicate refs */\n \tqsort(updates, n, sizeof(*updates), ref_update_compare);\n-\tret = ref_update_reject_duplicates(updates, n, err);\n-\tif (ret)\n+\tif (ref_update_reject_duplicates(updates, n, err)) {\n+\t\tret = TRANSACTION_GENERIC_ERROR;\n \t\tgoto cleanup;\n+\t}\n \n \t/* Acquire all locks while verifying old values */\n \tfor (i = 0; i < n; i++) {\n@@ -3653,10 +3654,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t\t\t\t update->flags,\n \t\t\t\t\t\t &update->type);\n \t\tif (!update->lock) {\n+\t\t\tret = (errno == ENOTDIR)\n+\t\t\t\t? TRANSACTION_NAME_CONFLICT\n+\t\t\t\t: TRANSACTION_GENERIC_ERROR;\n \t\t\tif (err)\n \t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n \t\t\t\t\t update->refname);\n-\t\t\tret = 1;\n \t\t\tgoto cleanup;\n \t\t}\n \t}\n@@ -3666,15 +3669,16 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (!is_null_sha1(update->new_sha1)) {\n-\t\t\tret = write_ref_sha1(update->lock, update->new_sha1,\n-\t\t\t\t\t update->msg);\n-\t\t\tupdate->lock = NULL; /* freed by write_ref_sha1 */\n-\t\t\tif (ret) {\n+\t\t\tif (write_ref_sha1(update->lock, update->new_sha1,\n+\t\t\t\t\t update->msg)) {\n+\t\t\t\tupdate->lock = NULL; /* freed by write_ref_sha1 */\n \t\t\t\tif (err)\n \t\t\t\t\tstrbuf_addf(err, \"Cannot update the ref '%s'.\",\n \t\t\t\t\t\t update->refname);\n+\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n \t\t\t\tgoto cleanup;\n \t\t\t}\n+\t\t\tupdate->lock = NULL; /* freed by write_ref_sha1 */\n \t\t}\n \t}\n \n@@ -3683,14 +3687,16 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (update->lock) {\n-\t\t\tret |= delete_ref_loose(update->lock, update->type,\n-\t\t\t\t\t\terr);\n+\t\t\tif (delete_ref_loose(update->lock, update->type, err))\n+\t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n+\n \t\t\tif (!(update->flags & REF_ISPRUNING))\n \t\t\t\tdelnames[delnum++] = update->lock->ref_name;\n \t\t}\n \t}\n \n-\tret |= repack_without_refs(delnames, delnum, err);\n+\tif (repack_without_refs(delnames, delnum, err))\n+\t\tret = TRANSACTION_GENERIC_ERROR;\n \tfor (i = 0; i < delnum; i++)\n \t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\n \tclear_loose_ref_cache(&ref_cache);\ndiff --git a/refs.h b/refs.h\nindex 1a55cab..50b115a 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -326,9 +326,14 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n \n /*\n * Commit all of the changes that have been queued in transaction, as\n- * atomically as possible. Return a nonzero value if there is a\n- * problem.\n+ * atomically as possible.\n+ *\n+ * Returns 0 for success, or one of the below error codes for errors.\n */\n+/* Naming conflict (for example, the ref names A and A/B conflict). */\n+#define TRANSACTION_NAME_CONFLICT -1\n+/* All other errors. */\n+#define TRANSACTION_GENERIC_ERROR -2\n int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\t struct strbuf *err);\n \n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250644","messageId":"20141015005104.GP32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 12/25] fetch.c: change s_update_ref to use a ref transaction","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:51:04Z","receivedAt":"2014-10-15T00:51:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Mon, 28 Apr 2014 13:49:07 -0700\n\nChange s_update_ref to use a ref transaction for the ref update.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n builtin/fetch.c | 34 ++++++++++++++++++++++++----------\n 1 file changed, 24 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 159fb7e..6ffd023 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -404,23 +404,37 @@ static int s_update_ref(const char *action,\n {\n \tchar msg[1024];\n \tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n-\tstatic struct ref_lock *lock;\n+\tstruct ref_transaction *transaction;\n+\tstruct strbuf err = STRBUF_INIT;\n+\tint ret, df_conflict = 0;\n \n \tif (dry_run)\n \t\treturn 0;\n \tif (!rla)\n \t\trla = default_rla.buf;\n \tsnprintf(msg, sizeof(msg), \"%s: %s\", rla, action);\n-\tlock = lock_any_ref_for_update(ref->name,\n-\t\t\t\t check_old ? ref->old_sha1 : NULL,\n-\t\t\t\t 0, NULL);\n-\tif (!lock)\n-\t\treturn errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n-\t\t\t\t\t STORE_REF_ERROR_OTHER;\n-\tif (write_ref_sha1(lock, ref->new_sha1, msg) < 0)\n-\t\treturn errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n-\t\t\t\t\t STORE_REF_ERROR_OTHER;\n+\n+\ttransaction = ref_transaction_begin(&err);\n+\tif (!transaction ||\n+\t ref_transaction_update(transaction, ref->name, ref->new_sha1,\n+\t\t\t\t ref->old_sha1, 0, check_old, msg, &err))\n+\t\tgoto fail;\n+\n+\tret = ref_transaction_commit(transaction, &err);\n+\tif (ret) {\n+\t\tdf_conflict = (ret == TRANSACTION_NAME_CONFLICT);\n+\t\tgoto fail;\n+\t}\n+\n+\tref_transaction_free(transaction);\n+\tstrbuf_release(&err);\n \treturn 0;\n+fail:\n+\tref_transaction_free(transaction);\n+\terror(\"%s\", err.buf);\n+\tstrbuf_release(&err);\n+\treturn df_conflict ? STORE_REF_ERROR_DF_CONFLICT\n+\t\t\t : STORE_REF_ERROR_OTHER;\n }\n \n #define REFCOL_WIDTH 10\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250645","messageId":"20141015005125.GQ32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 13/25] refs.c: make write_ref_sha1 static","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:51:25Z","receivedAt":"2014-10-15T00:51:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Mon, 28 Apr 2014 15:36:58 -0700\n\nNo external users call write_ref_sha1 any more so let's declare it static.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n refs.c | 10 ++++++++--\n refs.h | 3 ---\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 9d9bbeb..6b4236a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2706,6 +2706,9 @@ static int rename_ref_available(const char *oldname, const char *newname)\n \treturn ret;\n }\n \n+static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,\n+\t\t\t const char *logmsg);\n+\n int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)\n {\n \tunsigned char sha1[20], orig_sha1[20];\n@@ -2950,8 +2953,11 @@ int is_branch(const char *refname)\n \treturn !strcmp(refname, \"HEAD\") || starts_with(refname, \"refs/heads/\");\n }\n \n-/* This function must return a meaningful errno */\n-int write_ref_sha1(struct ref_lock *lock,\n+/*\n+ * Write sha1 into the ref specified by the lock. Make sure that errno\n+ * is sane on error.\n+ */\n+static int write_ref_sha1(struct ref_lock *lock,\n \tconst unsigned char *sha1, const char *logmsg)\n {\n \tstatic char term = '\\n';\ndiff --git a/refs.h b/refs.h\nindex 50b115a..eea1044 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -197,9 +197,6 @@ extern int commit_ref(struct ref_lock *lock);\n /** Release any lock taken but not written. **/\n extern void unlock_ref(struct ref_lock *lock);\n \n-/** Writes sha1 into the ref specified by the lock. **/\n-extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);\n-\n /*\n * Setup reflog before using. Set errno to something meaningful on failure.\n */\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250646","messageId":"20141015005147.GR32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 14/25] refs.c: change resolve_ref_unsafe reading argument to be a flags field","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:51:47Z","receivedAt":"2014-10-15T00:51:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Tue, 15 Jul 2014 12:59:36 -0700\n\nresolve_ref_unsafe takes a boolean argument for reading (a nonexistent ref\nresolves successfully for writing but not for reading). Change this to be\na flags field instead, and pass the new constant RESOLVE_REF_READING when\nwe want this behaviour.\n\nWhile at it, swap two of the arguments in the function to put output\narguments at the end. As a nice side effect, this ensures that we can\ncatch callers that were unaware of the new API so they can be audited.\n\nGive the wrapper functions resolve_refdup and read_ref_full the same\ntreatment for consistency.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n branch.c | 2 +-\n builtin/blame.c | 2 +-\n builtin/branch.c | 9 ++---\n builtin/checkout.c | 6 ++--\n builtin/clone.c | 2 +-\n builtin/commit.c | 2 +-\n builtin/fmt-merge-msg.c | 2 +-\n builtin/for-each-ref.c | 6 ++--\n builtin/fsck.c | 2 +-\n builtin/log.c | 3 +-\n builtin/merge.c | 2 +-\n builtin/notes.c | 2 +-\n builtin/receive-pack.c | 4 +--\n builtin/remote.c | 5 +--\n builtin/show-branch.c | 7 ++--\n builtin/symbolic-ref.c | 2 +-\n bundle.c | 2 +-\n cache.h | 23 ++++++------\n http-backend.c | 4 ++-\n notes-merge.c | 2 +-\n reflog-walk.c | 5 +--\n refs.c | 93 ++++++++++++++++++++++++++++---------------------\n remote.c | 11 +++---\n sequencer.c | 4 +--\n transport-helper.c | 5 ++-\n transport.c | 5 +--\n upload-pack.c | 2 +-\n wt-status.c | 2 +-\n 28 files changed, 124 insertions(+), 92 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 884c62c..4bab55a 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -170,7 +170,7 @@ int validate_new_branchname(const char *name, struct strbuf *ref,\n \t\tconst char *head;\n \t\tunsigned char sha1[20];\n \n-\t\thead = resolve_ref_unsafe(\"HEAD\", sha1, 0, NULL);\n+\t\thead = resolve_ref_unsafe(\"HEAD\", 0, sha1, NULL);\n \t\tif (!is_bare_repository() && head && !strcmp(head, ref->buf))\n \t\t\tdie(_(\"Cannot force update the current branch.\"));\n \t}\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 3838be2..303e217 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2286,7 +2286,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,\n \tcommit->date = now;\n \tparent_tail = &commit->parents;\n \n-\tif (!resolve_ref_unsafe(\"HEAD\", head_sha1, 1, NULL))\n+\tif (!resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, head_sha1, NULL))\n \t\tdie(\"no such ref: HEAD\");\n \n \tparent_tail = append_parent(parent_tail, head_sha1);\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 6785097..6491bac 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -131,7 +131,8 @@ static int branch_merged(int kind, const char *name,\n \t\t branch->merge[0] &&\n \t\t branch->merge[0]->dst &&\n \t\t (reference_name = reference_name_to_free =\n-\t\t resolve_refdup(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)\n+\t\t resolve_refdup(branch->merge[0]->dst, RESOLVE_REF_READING,\n+\t\t\t\t sha1, NULL)) != NULL)\n \t\t\treference_rev = lookup_commit_reference(sha1);\n \t}\n \tif (!reference_rev)\n@@ -235,7 +236,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tfree(name);\n \n \t\tname = mkpathdup(fmt, bname.buf);\n-\t\ttarget = resolve_ref_unsafe(name, sha1, 0, &flags);\n+\t\ttarget = resolve_ref_unsafe(name, 0, sha1, &flags);\n \t\tif (!target ||\n \t\t (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {\n \t\t\terror(remote_branch\n@@ -299,7 +300,7 @@ static char *resolve_symref(const char *src, const char *prefix)\n \tint flag;\n \tconst char *dst;\n \n-\tdst = resolve_ref_unsafe(src, sha1, 0, &flag);\n+\tdst = resolve_ref_unsafe(src, 0, sha1, &flag);\n \tif (!(dst && (flag & REF_ISSYMREF)))\n \t\treturn NULL;\n \tif (prefix)\n@@ -869,7 +870,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \n \ttrack = git_branch_track;\n \n-\thead = resolve_refdup(\"HEAD\", head_sha1, 0, NULL);\n+\thead = resolve_refdup(\"HEAD\", 0, head_sha1, NULL);\n \tif (!head)\n \t\tdie(_(\"Failed to resolve HEAD as a valid ref.\"));\n \tif (!strcmp(head, \"HEAD\"))\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b4decd5..5410dac 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -355,7 +355,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n \tif (write_locked_index(&the_index, lock_file, COMMIT_LOCK))\n \t\tdie(_(\"unable to write new index file\"));\n \n-\tread_ref_full(\"HEAD\", rev, 0, &flag);\n+\tread_ref_full(\"HEAD\", 0, rev, &flag);\n \thead = lookup_commit_reference_gently(rev, 1);\n \n \terrs |= post_checkout_hook(head, head, 0);\n@@ -775,7 +775,7 @@ static int switch_branches(const struct checkout_opts *opts,\n \tunsigned char rev[20];\n \tint flag, writeout_error = 0;\n \tmemset(&old, 0, sizeof(old));\n-\told.path = path_to_free = resolve_refdup(\"HEAD\", rev, 0, &flag);\n+\told.path = path_to_free = resolve_refdup(\"HEAD\", 0, rev, &flag);\n \told.commit = lookup_commit_reference_gently(rev, 1);\n \tif (!(flag & REF_ISSYMREF))\n \t\told.path = NULL;\n@@ -1072,7 +1072,7 @@ static int checkout_branch(struct checkout_opts *opts,\n \t\tunsigned char rev[20];\n \t\tint flag;\n \n-\t\tif (!read_ref_full(\"HEAD\", rev, 0, &flag) &&\n+\t\tif (!read_ref_full(\"HEAD\", 0, rev, &flag) &&\n \t\t (flag & REF_ISSYMREF) && is_null_sha1(rev))\n \t\t\treturn switch_unborn_to_new_branch(opts);\n \t}\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex d3bf953..7f509d0 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -623,7 +623,7 @@ static int checkout(void)\n \tif (option_no_checkout)\n \t\treturn 0;\n \n-\thead = resolve_refdup(\"HEAD\", sha1, 1, NULL);\n+\thead = resolve_refdup(\"HEAD\", RESOLVE_REF_READING, sha1, NULL);\n \tif (!head) {\n \t\twarning(_(\"remote HEAD refers to nonexistent ref, \"\n \t\t\t \"unable to checkout.\\n\"));\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a7857ab..aa0940e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1515,7 +1515,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,\n \trev.diffopt.break_opt = 0;\n \tdiff_setup_done(&rev.diffopt);\n \n-\thead = resolve_ref_unsafe(\"HEAD\", junk_sha1, 0, NULL);\n+\thead = resolve_ref_unsafe(\"HEAD\", 0, junk_sha1, NULL);\n \tif (!strcmp(head, \"HEAD\"))\n \t\thead = _(\"detached HEAD\");\n \telse\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 79df05e..37177c6 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -602,7 +602,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n \n \t/* get current branch */\n \tcurrent_branch = current_branch_to_free =\n-\t\tresolve_refdup(\"HEAD\", head_sha1, 1, NULL);\n+\t\tresolve_refdup(\"HEAD\", RESOLVE_REF_READING, head_sha1, NULL);\n \tif (!current_branch)\n \t\tdie(\"No current branch\");\n \tif (starts_with(current_branch, \"refs/heads/\"))\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex fda0f04..492265d 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -635,7 +635,8 @@ static void populate_value(struct refinfo *ref)\n \n \tif (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {\n \t\tunsigned char unused1[20];\n-\t\tref->symref = resolve_refdup(ref->refname, unused1, 1, NULL);\n+\t\tref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,\n+\t\t\t\t\t unused1, NULL);\n \t\tif (!ref->symref)\n \t\t\tref->symref = \"\";\n \t}\n@@ -693,7 +694,8 @@ static void populate_value(struct refinfo *ref)\n \t\t\tconst char *head;\n \t\t\tunsigned char sha1[20];\n \n-\t\t\thead = resolve_ref_unsafe(\"HEAD\", sha1, 1, NULL);\n+\t\t\thead = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n+\t\t\t\t\t\t sha1, NULL);\n \t\t\tif (!strcmp(ref->refname, head))\n \t\t\t\tv->s = \"*\";\n \t\t\telse\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex e9ba576..a27515a 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -556,7 +556,7 @@ static int fsck_head_link(void)\n \tif (verbose)\n \t\tfprintf(stderr, \"Checking HEAD link\\n\");\n \n-\thead_points_at = resolve_ref_unsafe(\"HEAD\", head_sha1, 0, &flag);\n+\thead_points_at = resolve_ref_unsafe(\"HEAD\", 0, head_sha1, &flag);\n \tif (!head_points_at)\n \t\treturn error(\"Invalid HEAD\");\n \tif (!strcmp(head_points_at, \"HEAD\"))\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1202eba..83c0b92 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1400,7 +1400,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (check_head) {\n \t\t\tunsigned char sha1[20];\n \t\t\tconst char *ref, *v;\n-\t\t\tref = resolve_ref_unsafe(\"HEAD\", sha1, 1, NULL);\n+\t\t\tref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n+\t\t\t\t\t\t sha1, NULL);\n \t\t\tif (ref && skip_prefix(ref, \"refs/heads/\", &v))\n \t\t\t\tbranch_name = xstrdup(v);\n \t\t\telse\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4513fad..bebbe5b 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1101,7 +1101,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\n \t * current branch.\n \t */\n-\tbranch = branch_to_free = resolve_refdup(\"HEAD\", head_sha1, 0, &flag);\n+\tbranch = branch_to_free = resolve_refdup(\"HEAD\", 0, head_sha1, &flag);\n \tif (branch && starts_with(branch, \"refs/heads/\"))\n \t\tbranch += 11;\n \tif (!branch || is_null_sha1(head_sha1))\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 67d0bb1..68b6cd8 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -702,7 +702,7 @@ static int merge_commit(struct notes_merge_options *o)\n \tinit_notes(t, \"NOTES_MERGE_PARTIAL\", combine_notes_overwrite, 0);\n \n \to->local_ref = local_ref_to_free =\n-\t\tresolve_refdup(\"NOTES_MERGE_REF\", sha1, 0, NULL);\n+\t\tresolve_refdup(\"NOTES_MERGE_REF\", 0, sha1, NULL);\n \tif (!o->local_ref)\n \t\tdie(\"Failed to resolve NOTES_MERGE_REF\");\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex df6c337..27e9dc2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -909,7 +909,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)\n \tint flag;\n \n \tstrbuf_addf(&buf, \"%s%s\", get_git_namespace(), cmd->ref_name);\n-\tdst_name = resolve_ref_unsafe(buf.buf, sha1, 0, &flag);\n+\tdst_name = resolve_ref_unsafe(buf.buf, 0, sha1, &flag);\n \tstrbuf_release(&buf);\n \n \tif (!(flag & REF_ISSYMREF))\n@@ -1070,7 +1070,7 @@ static void execute_commands(struct command *commands,\n \tcheck_aliased_updates(commands);\n \n \tfree(head_name_to_free);\n-\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n+\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", 0, sha1, NULL);\n \n \tchecked_connectivity = 1;\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 9a4640d..42a533a 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -567,7 +567,8 @@ static int read_remote_branches(const char *refname,\n \tstrbuf_addf(&buf, \"refs/remotes/%s/\", rename->old);\n \tif (starts_with(refname, buf.buf)) {\n \t\titem = string_list_append(rename->remote_branches, xstrdup(refname));\n-\t\tsymref = resolve_ref_unsafe(refname, orig_sha1, 1, &flag);\n+\t\tsymref = resolve_ref_unsafe(refname, RESOLVE_REF_READING,\n+\t\t\t\t\t orig_sha1, &flag);\n \t\tif (flag & REF_ISSYMREF)\n \t\t\titem->util = xstrdup(symref);\n \t\telse\n@@ -703,7 +704,7 @@ static int mv(int argc, const char **argv)\n \t\tint flag = 0;\n \t\tunsigned char sha1[20];\n \n-\t\tread_ref_full(item->string, sha1, 1, &flag);\n+\t\tread_ref_full(item->string, RESOLVE_REF_READING, sha1, &flag);\n \t\tif (!(flag & REF_ISSYMREF))\n \t\t\tcontinue;\n \t\tif (delete_ref(item->string, NULL, REF_NODEREF))\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex 199b081..270e39c 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -728,7 +728,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\tif (ac == 0) {\n \t\t\tstatic const char *fake_av[2];\n \n-\t\t\tfake_av[0] = resolve_refdup(\"HEAD\", sha1, 1, NULL);\n+\t\t\tfake_av[0] = resolve_refdup(\"HEAD\",\n+\t\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t\t sha1, NULL);\n \t\t\tfake_av[1] = NULL;\n \t\t\tav = fake_av;\n \t\t\tac = 1;\n@@ -789,7 +791,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t}\n \t}\n \n-\thead_p = resolve_ref_unsafe(\"HEAD\", head_sha1, 1, NULL);\n+\thead_p = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n+\t\t\t\t head_sha1, NULL);\n \tif (head_p) {\n \t\thead_len = strlen(head_p);\n \t\tmemcpy(head, head_p, head_len + 1);\ndiff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c\nindex b6a711d..29fb3f1 100644\n--- a/builtin/symbolic-ref.c\n+++ b/builtin/symbolic-ref.c\n@@ -13,7 +13,7 @@ static int check_symref(const char *HEAD, int quiet, int shorten, int print)\n {\n \tunsigned char sha1[20];\n \tint flag;\n-\tconst char *refname = resolve_ref_unsafe(HEAD, sha1, 0, &flag);\n+\tconst char *refname = resolve_ref_unsafe(HEAD, 0, sha1, &flag);\n \n \tif (!refname)\n \t\tdie(\"No such ref: %s\", HEAD);\ndiff --git a/bundle.c b/bundle.c\nindex 9a22ab5..fa67057 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -310,7 +310,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\t\tcontinue;\n \t\tif (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)\n \t\t\tcontinue;\n-\t\tif (read_ref_full(e->name, sha1, 1, &flag))\n+\t\tif (read_ref_full(e->name, RESOLVE_REF_READING, sha1, &flag))\n \t\t\tflag = 0;\n \t\tdisplay_ref = (flag & REF_ISSYMREF) ? e->name : ref;\n \ndiff --git a/cache.h b/cache.h\nindex 5b86065..7200639 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -950,8 +950,8 @@ extern int for_each_abbrev(const char *prefix, each_abbrev_fn, void *);\n extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n \n extern char *sha1_to_hex(const unsigned char *sha1);\t/* static buffer result! */\n-extern int read_ref_full(const char *refname, unsigned char *sha1,\n-\t\t\t int reading, int *flags);\n+extern int read_ref_full(const char *refname, int resolve_flags,\n+\t\t\t unsigned char *sha1, int *flags);\n extern int read_ref(const char *refname, unsigned char *sha1);\n \n /*\n@@ -963,20 +963,20 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n * or the input ref.\n *\n * If the reference cannot be resolved to an object, the behavior\n- * depends on the \"reading\" argument:\n+ * depends on the RESOLVE_REF_READING flag:\n *\n- * - If reading is set, return NULL.\n+ * - If RESOLVE_REF_READING is set, return NULL.\n *\n- * - If reading is not set, clear sha1 and return the name of the last\n- * reference name in the chain, which will either be a non-symbolic\n+ * - If RESOLVE_REF_READING is not set, clear sha1 and return the name of\n+ * the last reference name in the chain, which will either be a non-symbolic\n * reference or an undefined reference. If this is a prelude to\n * \"writing\" to the ref, the return value is the name of the ref\n * that will actually be created or changed.\n *\n- * If flag is non-NULL, set the value that it points to the\n+ * If flags is non-NULL, set the value that it points to the\n * combination of REF_ISPACKED (if the reference was found among the\n- * packed references) and REF_ISSYMREF (if the initial reference was a\n- * symbolic reference).\n+ * packed references), REF_ISSYMREF (if the initial reference was a\n+ * symbolic reference) and REF_ISBROKEN (if the ref is malformed).\n *\n * If ref is not a properly-formatted, normalized reference, return\n * NULL. If more than MAXDEPTH recursive symbolic lookups are needed,\n@@ -984,8 +984,9 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n *\n * errno is set to something meaningful on error.\n */\n-extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1, int reading, int *flag);\n-extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);\n+#define RESOLVE_REF_READING 0x01\n+extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n+extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n \n extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);\n extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);\ndiff --git a/http-backend.c b/http-backend.c\nindex 404e682..9977c5d 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -412,7 +412,9 @@ static int show_head_ref(const char *refname, const unsigned char *sha1,\n \n \tif (flag & REF_ISSYMREF) {\n \t\tunsigned char unused[20];\n-\t\tconst char *target = resolve_ref_unsafe(refname, unused, 1, NULL);\n+\t\tconst char *target = resolve_ref_unsafe(refname,\n+\t\t\t\t\t\t\tRESOLVE_REF_READING,\n+\t\t\t\t\t\t\tunused, NULL);\n \t\tconst char *target_nons = strip_namespace(target);\n \n \t\tstrbuf_addf(buf, \"ref: %s\\n\", target_nons);\ndiff --git a/notes-merge.c b/notes-merge.c\nindex fd5fae2..7eb9d7a 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -549,7 +549,7 @@ int notes_merge(struct notes_merge_options *o,\n \t o->local_ref, o->remote_ref);\n \n \t/* Dereference o->local_ref into local_sha1 */\n-\tif (read_ref_full(o->local_ref, local_sha1, 0, NULL))\n+\tif (read_ref_full(o->local_ref, 0, local_sha1, NULL))\n \t\tdie(\"Failed to resolve local notes ref '%s'\", o->local_ref);\n \telse if (!check_refname_format(o->local_ref, 0) &&\n \t\tis_null_sha1(local_sha1))\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 0e5174b..222de76 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -48,7 +48,8 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)\n \t\tunsigned char sha1[20];\n \t\tconst char *name;\n \t\tvoid *name_to_free;\n-\t\tname = name_to_free = resolve_refdup(ref, sha1, 1, NULL);\n+\t\tname = name_to_free = resolve_refdup(ref, RESOLVE_REF_READING,\n+\t\t\t\t\t\t sha1, NULL);\n \t\tif (name) {\n \t\t\tfor_each_reflog_ent(name, read_one_reflog, reflogs);\n \t\t\tfree(name_to_free);\n@@ -174,7 +175,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \t\tif (*branch == '\\0') {\n \t\t\tunsigned char sha1[20];\n \t\t\tfree(branch);\n-\t\t\tbranch = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n+\t\t\tbranch = resolve_refdup(\"HEAD\", 0, sha1, NULL);\n \t\t\tif (!branch)\n \t\t\t\tdie (\"No current branch\");\n \ndiff --git a/refs.c b/refs.c\nindex 6b4236a..f8d59ab 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1251,7 +1251,9 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n \t\t\t\t\thashclr(sha1);\n \t\t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t\t}\n-\t\t\t} else if (read_ref_full(refname.buf, sha1, 1, &flag)) {\n+\t\t\t} else if (read_ref_full(refname.buf,\n+\t\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t\t sha1, &flag)) {\n \t\t\t\thashclr(sha1);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t}\n@@ -1376,9 +1378,9 @@ static struct ref_entry *get_packed_ref(const char *refname)\n * options are forwarded from resolve_safe_unsafe().\n */\n static const char *handle_missing_loose_ref(const char *refname,\n+\t\t\t\t\t int resolve_flags,\n \t\t\t\t\t unsigned char *sha1,\n-\t\t\t\t\t int reading,\n-\t\t\t\t\t int *flag)\n+\t\t\t\t\t int *flags)\n {\n \tstruct ref_entry *entry;\n \n@@ -1389,12 +1391,12 @@ static const char *handle_missing_loose_ref(const char *refname,\n \tentry = get_packed_ref(refname);\n \tif (entry) {\n \t\thashcpy(sha1, entry->u.value.sha1);\n-\t\tif (flag)\n-\t\t\t*flag |= REF_ISPACKED;\n+\t\tif (flags)\n+\t\t\t*flags |= REF_ISPACKED;\n \t\treturn refname;\n \t}\n \t/* The reference is not a packed reference, either. */\n-\tif (reading) {\n+\tif (resolve_flags & RESOLVE_REF_READING) {\n \t\treturn NULL;\n \t} else {\n \t\thashclr(sha1);\n@@ -1403,21 +1405,20 @@ static const char *handle_missing_loose_ref(const char *refname,\n }\n \n /* This function needs to return a meaningful errno on failure */\n-const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int reading, int *flag)\n+const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)\n {\n \tint depth = MAXDEPTH;\n \tssize_t len;\n \tchar buffer[256];\n \tstatic char refname_buffer[256];\n \n-\tif (flag)\n-\t\t*flag = 0;\n+\tif (flags)\n+\t\t*flags = 0;\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\terrno = EINVAL;\n \t\treturn NULL;\n \t}\n-\n \tfor (;;) {\n \t\tchar path[PATH_MAX];\n \t\tstruct stat st;\n@@ -1443,8 +1444,8 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \tstat_ref:\n \t\tif (lstat(path, &st) < 0) {\n \t\t\tif (errno == ENOENT)\n-\t\t\t\treturn handle_missing_loose_ref(refname, sha1,\n-\t\t\t\t\t\t\t\treading, flag);\n+\t\t\t\treturn handle_missing_loose_ref(refname,\n+\t\t\t\t\t\tresolve_flags, sha1, flags);\n \t\t\telse\n \t\t\t\treturn NULL;\n \t\t}\n@@ -1464,8 +1465,8 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\t\t\t\t!check_refname_format(buffer, 0)) {\n \t\t\t\tstrcpy(refname_buffer, buffer);\n \t\t\t\trefname = refname_buffer;\n-\t\t\t\tif (flag)\n-\t\t\t\t\t*flag |= REF_ISSYMREF;\n+\t\t\t\tif (flags)\n+\t\t\t\t\t*flags |= REF_ISSYMREF;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t}\n@@ -1510,21 +1511,21 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\t\t */\n \t\t\tif (get_sha1_hex(buffer, sha1) ||\n \t\t\t (buffer[40] != '\\0' && !isspace(buffer[40]))) {\n-\t\t\t\tif (flag)\n-\t\t\t\t\t*flag |= REF_ISBROKEN;\n+\t\t\t\tif (flags)\n+\t\t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n \t\t\treturn refname;\n \t\t}\n-\t\tif (flag)\n-\t\t\t*flag |= REF_ISSYMREF;\n+\t\tif (flags)\n+\t\t\t*flags |= REF_ISSYMREF;\n \t\tbuf = buffer + 4;\n \t\twhile (isspace(*buf))\n \t\t\tbuf++;\n \t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n-\t\t\tif (flag)\n-\t\t\t\t*flag |= REF_ISBROKEN;\n+\t\t\tif (flags)\n+\t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\terrno = EINVAL;\n \t\t\treturn NULL;\n \t\t}\n@@ -1532,9 +1533,9 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t}\n }\n \n-char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag)\n+char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags)\n {\n-\tconst char *ret = resolve_ref_unsafe(ref, sha1, reading, flag);\n+\tconst char *ret = resolve_ref_unsafe(ref, resolve_flags, sha1, flags);\n \treturn ret ? xstrdup(ret) : NULL;\n }\n \n@@ -1545,22 +1546,22 @@ struct ref_filter {\n \tvoid *cb_data;\n };\n \n-int read_ref_full(const char *refname, unsigned char *sha1, int reading, int *flags)\n+int read_ref_full(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)\n {\n-\tif (resolve_ref_unsafe(refname, sha1, reading, flags))\n+\tif (resolve_ref_unsafe(refname, resolve_flags, sha1, flags))\n \t\treturn 0;\n \treturn -1;\n }\n \n int read_ref(const char *refname, unsigned char *sha1)\n {\n-\treturn read_ref_full(refname, sha1, 1, NULL);\n+\treturn read_ref_full(refname, RESOLVE_REF_READING, sha1, NULL);\n }\n \n int ref_exists(const char *refname)\n {\n \tunsigned char sha1[20];\n-\treturn !!resolve_ref_unsafe(refname, sha1, 1, NULL);\n+\treturn !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, sha1, NULL);\n }\n \n static int filter_refs(const char *refname, const unsigned char *sha1, int flags,\n@@ -1673,7 +1674,7 @@ int peel_ref(const char *refname, unsigned char *sha1)\n \t\treturn 0;\n \t}\n \n-\tif (read_ref_full(refname, base, 1, &flag))\n+\tif (read_ref_full(refname, RESOLVE_REF_READING, base, &flag))\n \t\treturn -1;\n \n \t/*\n@@ -1714,7 +1715,7 @@ static int warn_if_dangling_symref(const char *refname, const unsigned char *sha\n \tif (!(flags & REF_ISSYMREF))\n \t\treturn 0;\n \n-\tresolves_to = resolve_ref_unsafe(refname, junk, 0, NULL);\n+\tresolves_to = resolve_ref_unsafe(refname, 0, junk, NULL);\n \tif (!resolves_to\n \t || (d->refname\n \t\t? strcmp(resolves_to, d->refname)\n@@ -1839,7 +1840,7 @@ static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n \t\treturn 0;\n \t}\n \n-\tif (!read_ref_full(\"HEAD\", sha1, 1, &flag))\n+\tif (!read_ref_full(\"HEAD\", RESOLVE_REF_READING, sha1, &flag))\n \t\treturn fn(\"HEAD\", sha1, flag, cb_data);\n \n \treturn 0;\n@@ -1919,7 +1920,7 @@ int head_ref_namespaced(each_ref_fn fn, void *cb_data)\n \tint flag;\n \n \tstrbuf_addf(&buf, \"%sHEAD\", get_git_namespace());\n-\tif (!read_ref_full(buf.buf, sha1, 1, &flag))\n+\tif (!read_ref_full(buf.buf, RESOLVE_REF_READING, sha1, &flag))\n \t\tret = fn(buf.buf, sha1, flag, cb_data);\n \tstrbuf_release(&buf);\n \n@@ -2014,7 +2015,9 @@ int refname_match(const char *abbrev_name, const char *full_name)\n static struct ref_lock *verify_lock(struct ref_lock *lock,\n \tconst unsigned char *old_sha1, int mustexist)\n {\n-\tif (read_ref_full(lock->ref_name, lock->old_sha1, mustexist, NULL)) {\n+\tif (read_ref_full(lock->ref_name,\n+\t\t\t mustexist ? RESOLVE_REF_READING : 0,\n+\t\t\t lock->old_sha1, NULL)) {\n \t\tint save_errno = errno;\n \t\terror(\"Can't verify ref %s\", lock->ref_name);\n \t\tunlock_ref(lock);\n@@ -2087,7 +2090,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)\n \n \t\tthis_result = refs_found ? sha1_from_ref : sha1;\n \t\tmksnpath(fullref, sizeof(fullref), *p, len, str);\n-\t\tr = resolve_ref_unsafe(fullref, this_result, 1, &flag);\n+\t\tr = resolve_ref_unsafe(fullref, RESOLVE_REF_READING,\n+\t\t\t\t this_result, &flag);\n \t\tif (r) {\n \t\t\tif (!refs_found++)\n \t\t\t\t*ref = xstrdup(r);\n@@ -2116,7 +2120,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)\n \t\tconst char *ref, *it;\n \n \t\tmksnpath(path, sizeof(path), *p, len, str);\n-\t\tref = resolve_ref_unsafe(path, hash, 1, NULL);\n+\t\tref = resolve_ref_unsafe(path, RESOLVE_REF_READING,\n+\t\t\t\t\t hash, NULL);\n \t\tif (!ref)\n \t\t\tcontinue;\n \t\tif (reflog_exists(path))\n@@ -2151,6 +2156,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tint last_errno = 0;\n \tint type, lflags;\n \tint mustexist = (old_sha1 && !is_null_sha1(old_sha1));\n+\tint resolve_flags = 0;\n \tint missing = 0;\n \tint attempts_remaining = 3;\n \n@@ -2162,7 +2168,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tlock = xcalloc(1, sizeof(struct ref_lock));\n \tlock->lock_fd = -1;\n \n-\trefname = resolve_ref_unsafe(refname, lock->old_sha1, mustexist, &type);\n+\tif (mustexist)\n+\t\tresolve_flags |= RESOLVE_REF_READING;\n+\n+\trefname = resolve_ref_unsafe(refname, resolve_flags,\n+\t\t\t\t lock->old_sha1, &type);\n \tif (!refname && errno == EISDIR) {\n \t\t/* we are trying to lock foo but we used to\n \t\t * have foo/bar which now does not exist;\n@@ -2175,7 +2185,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \t\t\terror(\"there are still refs under '%s'\", orig_refname);\n \t\t\tgoto error_return;\n \t\t}\n-\t\trefname = resolve_ref_unsafe(orig_refname, lock->old_sha1, mustexist, &type);\n+\t\trefname = resolve_ref_unsafe(orig_refname, resolve_flags,\n+\t\t\t\t\t lock->old_sha1, &type);\n \t}\n \tif (type_p)\n \t *type_p = type;\n@@ -2517,7 +2528,7 @@ static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)\n \t\tunsigned char sha1[20];\n \t\tint flags;\n \n-\t\tif (read_ref_full(entry->name, sha1, 0, &flags))\n+\t\tif (read_ref_full(entry->name, 0, sha1, &flags))\n \t\t\t/* We should at least have found the packed ref. */\n \t\t\tdie(\"Internal error\");\n \t\tif ((flags & REF_ISSYMREF) || !(flags & REF_ISPACKED)) {\n@@ -2721,7 +2732,8 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \tif (log && S_ISLNK(loginfo.st_mode))\n \t\treturn error(\"reflog for %s is a symlink\", oldrefname);\n \n-\tsymref = resolve_ref_unsafe(oldrefname, orig_sha1, 1, &flag);\n+\tsymref = resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING,\n+\t\t\t\t orig_sha1, &flag);\n \tif (flag & REF_ISSYMREF)\n \t\treturn error(\"refname %s is a symbolic ref, renaming it is not supported\",\n \t\t\toldrefname);\n@@ -2740,7 +2752,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms\n \t\tgoto rollback;\n \t}\n \n-\tif (!read_ref_full(newrefname, sha1, 1, NULL) &&\n+\tif (!read_ref_full(newrefname, RESOLVE_REF_READING, sha1, NULL) &&\n \t delete_ref(newrefname, sha1, REF_NODEREF)) {\n \t\tif (errno==EISDIR) {\n \t\t\tif (remove_empty_directories(git_path(\"%s\", newrefname))) {\n@@ -3018,7 +3030,8 @@ static int write_ref_sha1(struct ref_lock *lock,\n \t\tunsigned char head_sha1[20];\n \t\tint head_flag;\n \t\tconst char *head_ref;\n-\t\thead_ref = resolve_ref_unsafe(\"HEAD\", head_sha1, 1, &head_flag);\n+\t\thead_ref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n+\t\t\t\t\t head_sha1, &head_flag);\n \t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n \t\t !strcmp(head_ref, lock->ref_name))\n \t\t\tlog_ref_write(\"HEAD\", lock->old_sha1, sha1, logmsg);\n@@ -3389,7 +3402,7 @@ static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data\n \t\t\t\tretval = do_for_each_reflog(name, fn, cb_data);\n \t\t\t} else {\n \t\t\t\tunsigned char sha1[20];\n-\t\t\t\tif (read_ref_full(name->buf, sha1, 0, NULL))\n+\t\t\t\tif (read_ref_full(name->buf, 0, sha1, NULL))\n \t\t\t\t\tretval = error(\"bad ref for %s\", name->buf);\n \t\t\t\telse\n \t\t\t\t\tretval = fn(name->buf, sha1, 0, cb_data);\ndiff --git a/remote.c b/remote.c\nindex ce785f8..f624217 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -508,7 +508,7 @@ static void read_config(void)\n \t\treturn;\n \tdefault_remote_name = \"origin\";\n \tcurrent_branch = NULL;\n-\thead_ref = resolve_ref_unsafe(\"HEAD\", sha1, 0, &flag);\n+\thead_ref = resolve_ref_unsafe(\"HEAD\", 0, sha1, &flag);\n \tif (head_ref && (flag & REF_ISSYMREF) &&\n \t skip_prefix(head_ref, \"refs/heads/\", &head_ref)) {\n \t\tcurrent_branch = make_branch(head_ref, 0);\n@@ -1138,7 +1138,8 @@ static char *guess_ref(const char *name, struct ref *peer)\n \tstruct strbuf buf = STRBUF_INIT;\n \tunsigned char sha1[20];\n \n-\tconst char *r = resolve_ref_unsafe(peer->name, sha1, 1, NULL);\n+\tconst char *r = resolve_ref_unsafe(peer->name, RESOLVE_REF_READING,\n+\t\t\t\t\t sha1, NULL);\n \tif (!r)\n \t\treturn NULL;\n \n@@ -1199,7 +1200,9 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\tunsigned char sha1[20];\n \t\tint flag;\n \n-\t\tdst_value = resolve_ref_unsafe(matched_src->name, sha1, 1, &flag);\n+\t\tdst_value = resolve_ref_unsafe(matched_src->name,\n+\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t sha1, &flag);\n \t\tif (!dst_value ||\n \t\t ((flag & REF_ISSYMREF) &&\n \t\t !starts_with(dst_value, \"refs/heads/\")))\n@@ -1673,7 +1676,7 @@ static int ignore_symref_update(const char *refname)\n \tunsigned char sha1[20];\n \tint flag;\n \n-\tif (!resolve_ref_unsafe(refname, sha1, 0, &flag))\n+\tif (!resolve_ref_unsafe(refname, 0, sha1, &flag))\n \t\treturn 0; /* non-existing refs are OK */\n \treturn (flag & REF_ISSYMREF);\n }\ndiff --git a/sequencer.c b/sequencer.c\nindex 47aac75..a03d4fa 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -331,7 +331,7 @@ static int is_index_unchanged(void)\n \tunsigned char head_sha1[20];\n \tstruct commit *head_commit;\n \n-\tif (!resolve_ref_unsafe(\"HEAD\", head_sha1, 1, NULL))\n+\tif (!resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, head_sha1, NULL))\n \t\treturn error(_(\"Could not resolve HEAD commit\\n\"));\n \n \thead_commit = lookup_commit(head_sha1);\n@@ -871,7 +871,7 @@ static int rollback_single_pick(void)\n \tif (!file_exists(git_path(\"CHERRY_PICK_HEAD\")) &&\n \t !file_exists(git_path(\"REVERT_HEAD\")))\n \t\treturn error(_(\"no cherry-pick or revert in progress\"));\n-\tif (read_ref_full(\"HEAD\", head_sha1, 0, NULL))\n+\tif (read_ref_full(\"HEAD\", 0, head_sha1, NULL))\n \t\treturn error(_(\"cannot resolve HEAD\"));\n \tif (is_null_sha1(head_sha1))\n \t\treturn error(_(\"cannot abort from a branch yet to be born\"));\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 2b24d51..9b29620 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -897,7 +897,10 @@ static int push_refs_with_export(struct transport *transport,\n \t\t\t\t\tint flag;\n \n \t\t\t\t\t/* Follow symbolic refs (mainly for HEAD). */\n-\t\t\t\t\tname = resolve_ref_unsafe(ref->peer_ref->name, sha1, 1, &flag);\n+\t\t\t\t\tname = resolve_ref_unsafe(\n+\t\t\t\t\t\t ref->peer_ref->name,\n+\t\t\t\t\t\t RESOLVE_REF_READING,\n+\t\t\t\t\t\t sha1, &flag);\n \t\t\t\t\tif (!name || !(flag & REF_ISSYMREF))\n \t\t\t\t\t\tname = ref->peer_ref->name;\n \ndiff --git a/transport.c b/transport.c\nindex 055d2a2..b56620e 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -168,7 +168,8 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n \t\t/* Follow symbolic refs (mainly for HEAD). */\n \t\tlocalname = ref->peer_ref->name;\n \t\tremotename = ref->name;\n-\t\ttmp = resolve_ref_unsafe(localname, sha, 1, &flag);\n+\t\ttmp = resolve_ref_unsafe(localname, RESOLVE_REF_READING,\n+\t\t\t\t\t sha, &flag);\n \t\tif (tmp && flag & REF_ISSYMREF &&\n \t\t\tstarts_with(tmp, \"refs/heads/\"))\n \t\t\tlocalname = tmp;\n@@ -743,7 +744,7 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n \tunsigned char head_sha1[20];\n \tchar *head;\n \n-\thead = resolve_refdup(\"HEAD\", head_sha1, 1, NULL);\n+\thead = resolve_refdup(\"HEAD\", RESOLVE_REF_READING, head_sha1, NULL);\n \n \tif (verbose) {\n \t\tfor (ref = refs; ref; ref = ref->next)\ndiff --git a/upload-pack.c b/upload-pack.c\nindex c789ec0..ac9ac15 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -744,7 +744,7 @@ static int find_symref(const char *refname, const unsigned char *sha1, int flag,\n \n \tif ((flag & REF_ISSYMREF) == 0)\n \t\treturn 0;\n-\tsymref_target = resolve_ref_unsafe(refname, unused, 0, &flag);\n+\tsymref_target = resolve_ref_unsafe(refname, 0, unused, &flag);\n \tif (!symref_target || (flag & REF_ISSYMREF) == 0)\n \t\tdie(\"'%s' is a symref but it is not?\", refname);\n \titem = string_list_append(cb_data, refname);\ndiff --git a/wt-status.c b/wt-status.c\nindex 1bf5d72..f367066 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -128,7 +128,7 @@ void wt_status_prepare(struct wt_status *s)\n \ts->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;\n \ts->use_color = -1;\n \ts->relative_paths = 1;\n-\ts->branch = resolve_refdup(\"HEAD\", sha1, 0, NULL);\n+\ts->branch = resolve_refdup(\"HEAD\", 0, sha1, NULL);\n \ts->reference = \"HEAD\";\n \ts->fp = stdout;\n \ts->index_file = get_index_file();\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250647","messageId":"20141015005213.GS32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 15/25] reflog test: test interaction with detached HEAD","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:52:13Z","receivedAt":"2014-10-15T00:52:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Junio C Hamano \nDate: Sat, 13 Sep 2014 10:52:25 -0700\n\nA proposed patch produced broken HEAD reflog entries when checking out\nanything other than a branch. The testsuite still passed, so it took\na few days for the bug to be noticed.\n\nAdd tests checking the content of the reflog after detaching and\nreattaching HEAD so we don't have to rely on manual testing to catch\nsuch problems in the future.\n\n[jn: using 'log -g --format=%H' instead of parsing --oneline output,\n resetting state in each test so they can be safely reordered or\n skipped]\n\nSigned-off-by: Jonathan Nieder \nReviewed-by: Ronnie Sahlberg \n---\n t/t1413-reflog-detach.sh | 70 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 70 insertions(+)\n create mode 100755 t/t1413-reflog-detach.sh\n\ndiff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh\nnew file mode 100755\nindex 0000000..c730600\n--- /dev/null\n+++ b/t/t1413-reflog-detach.sh\n@@ -0,0 +1,70 @@\n+#!/bin/sh\n+\n+test_description='Test reflog interaction with detached HEAD'\n+. ./test-lib.sh\n+\n+reset_state () {\n+\tgit checkout master &&\n+\tcp saved_reflog .git/logs/HEAD\n+}\n+\n+test_expect_success setup '\n+\ttest_tick &&\n+\tgit commit --allow-empty -m initial &&\n+\tgit branch side &&\n+\ttest_tick &&\n+\tgit commit --allow-empty -m second &&\n+\tcat .git/logs/HEAD >saved_reflog\n+'\n+\n+test_expect_success baseline '\n+\treset_state &&\n+\tgit rev-parse master master^ >expect &&\n+\tgit log -g --format=%H >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'switch to branch' '\n+\treset_state &&\n+\tgit rev-parse side master master^ >expect &&\n+\tgit checkout side &&\n+\tgit log -g --format=%H >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'detach to other' '\n+\treset_state &&\n+\tgit rev-parse master side master master^ >expect &&\n+\tgit checkout side &&\n+\tgit checkout master^0 &&\n+\tgit log -g --format=%H >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'detach to self' '\n+\treset_state &&\n+\tgit rev-parse master master master^ >expect &&\n+\tgit checkout master^0 &&\n+\tgit log -g --format=%H >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'attach to self' '\n+\treset_state &&\n+\tgit rev-parse master master master master^ >expect &&\n+\tgit checkout master^0 &&\n+\tgit checkout master &&\n+\tgit log -g --format=%H >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'attach to other' '\n+\treset_state &&\n+\tgit rev-parse side master master master^ >expect &&\n+\tgit checkout master^0 &&\n+\tgit checkout side &&\n+\tgit log -g --format=%H >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250648","messageId":"20141015005234.GT32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 16/25] branch -d: avoid repeated symref resolution","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:52:34Z","receivedAt":"2014-10-15T00:52:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Wed, 10 Sep 2014 18:22:48 -0700\n\nIf a repository gets in a broken state with too much symref nesting,\nit cannot be repaired with \"git branch -d\":\n\n $ git symbolic-ref refs/heads/nonsense refs/heads/nonsense\n $ git branch -d nonsense\n error: branch 'nonsense' not found.\n\nWorse, \"git update-ref --no-deref -d\" doesn't work for such repairs\neither:\n\n $ git update-ref -d refs/heads/nonsense\n error: unable to resolve reference refs/heads/nonsense: Too many levels of symbolic links\n\nFix both by teaching resolve_ref_unsafe a new RESOLVE_REF_NO_RECURSE\nflag and passing it when appropriate.\n\nCallers can still read the value of a symref (for example to print a\nmessage about it) with that flag set --- resolve_ref_unsafe will\nresolve one level of symrefs and stop there.\n\nSigned-off-by: Jonathan Nieder \nReviewed-by: Ronnie Sahlberg \n---\n builtin/branch.c | 3 ++-\n cache.h | 6 ++++++\n refs.c | 17 +++++++++++++++--\n refs.h | 2 ++\n t/t1400-update-ref.sh | 34 ++++++++++++++++++++++++++++++++++\n t/t3200-branch.sh | 9 +++++++++\n 6 files changed, 68 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 6491bac..2ad2d0b 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -236,7 +236,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tfree(name);\n \n \t\tname = mkpathdup(fmt, bname.buf);\n-\t\ttarget = resolve_ref_unsafe(name, 0, sha1, &flags);\n+\t\ttarget = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t\t sha1, &flags);\n \t\tif (!target ||\n \t\t (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {\n \t\t\terror(remote_branch\ndiff --git a/cache.h b/cache.h\nindex 7200639..b735f3f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -973,6 +973,11 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n * \"writing\" to the ref, the return value is the name of the ref\n * that will actually be created or changed.\n *\n+ * If the RESOLVE_REF_NO_RECURSE flag is passed, only resolves one\n+ * level of symbolic reference. The value stored in sha1 for a symbolic\n+ * reference will always be null_sha1 in this case, and the return\n+ * value is the reference that the symref refers to directly.\n+ *\n * If flags is non-NULL, set the value that it points to the\n * combination of REF_ISPACKED (if the reference was found among the\n * packed references), REF_ISSYMREF (if the initial reference was a\n@@ -985,6 +990,7 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n * errno is set to something meaningful on error.\n */\n #define RESOLVE_REF_READING 0x01\n+#define RESOLVE_REF_NO_RECURSE 0x02\n extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n \ndiff --git a/refs.c b/refs.c\nindex f8d59ab..4fe263e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1467,6 +1467,10 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \t\t\t\trefname = refname_buffer;\n \t\t\t\tif (flags)\n \t\t\t\t\t*flags |= REF_ISSYMREF;\n+\t\t\t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n+\t\t\t\t\thashclr(sha1);\n+\t\t\t\t\treturn refname;\n+\t\t\t\t}\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t}\n@@ -1523,13 +1527,17 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \t\tbuf = buffer + 4;\n \t\twhile (isspace(*buf))\n \t\t\tbuf++;\n+\t\trefname = strcpy(refname_buffer, buf);\n+\t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n+\t\t\thashclr(sha1);\n+\t\t\treturn refname;\n+\t\t}\n \t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (flags)\n \t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\terrno = EINVAL;\n \t\t\treturn NULL;\n \t\t}\n-\t\trefname = strcpy(refname_buffer, buf);\n \t}\n }\n \n@@ -2170,6 +2178,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \n \tif (mustexist)\n \t\tresolve_flags |= RESOLVE_REF_READING;\n+\tif (flags & REF_NODEREF && flags & REF_DELETING)\n+\t\tresolve_flags |= RESOLVE_REF_NO_RECURSE;\n \n \trefname = resolve_ref_unsafe(refname, resolve_flags,\n \t\t\t\t lock->old_sha1, &type);\n@@ -3664,13 +3674,16 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t/* Acquire all locks while verifying old values */\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n+\t\tint flags = update->flags;\n \n+\t\tif (is_null_sha1(update->new_sha1))\n+\t\t\tflags |= REF_DELETING;\n \t\tupdate->lock = lock_ref_sha1_basic(update->refname,\n \t\t\t\t\t\t (update->have_old ?\n \t\t\t\t\t\t update->old_sha1 :\n \t\t\t\t\t\t NULL),\n \t\t\t\t\t\t NULL,\n-\t\t\t\t\t\t update->flags,\n+\t\t\t\t\t\t flags,\n \t\t\t\t\t\t &update->type);\n \t\tif (!update->lock) {\n \t\t\tret = (errno == ENOTDIR)\ndiff --git a/refs.h b/refs.h\nindex eea1044..79802d7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -177,10 +177,12 @@ extern int peel_ref(const char *refname, unsigned char *sha1);\n * ref_transaction_create(), etc.\n * REF_NODEREF: act on the ref directly, instead of dereferencing\n * symbolic references.\n+ * REF_DELETING: tolerate broken refs\n *\n * Flags >= 0x100 are reserved for internal use.\n */\n #define REF_NODEREF\t0x01\n+#define REF_DELETING\t0x02\n /*\n * This function sets errno to something meaningful on failure.\n */\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 0218e96..7c8c41a 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -110,6 +110,40 @@ test_expect_success \"delete symref without dereference when the referred ref is\n cp -f .git/HEAD.orig .git/HEAD\n git update-ref -d $m\n \n+test_expect_success 'update-ref -d is not confused by self-reference' '\n+\tgit symbolic-ref refs/heads/self refs/heads/self &&\n+\ttest_when_finished \"rm -f .git/refs/heads/self\" &&\n+\ttest_path_is_file .git/refs/heads/self &&\n+\ttest_must_fail git update-ref -d refs/heads/self &&\n+\ttest_path_is_file .git/refs/heads/self\n+'\n+\n+test_expect_success 'update-ref --no-deref -d can delete self-reference' '\n+\tgit symbolic-ref refs/heads/self refs/heads/self &&\n+\ttest_when_finished \"rm -f .git/refs/heads/self\" &&\n+\ttest_path_is_file .git/refs/heads/self &&\n+\tgit update-ref --no-deref -d refs/heads/self &&\n+\ttest_path_is_missing .git/refs/heads/self\n+'\n+\n+test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '\n+\t>.git/refs/heads/bad &&\n+\ttest_when_finished \"rm -f .git/refs/heads/bad\" &&\n+\tgit symbolic-ref refs/heads/ref-to-bad refs/heads/bad &&\n+\ttest_when_finished \"rm -f .git/refs/heads/ref-to-bad\" &&\n+\ttest_path_is_file .git/refs/heads/ref-to-bad &&\n+\tgit update-ref --no-deref -d refs/heads/ref-to-bad &&\n+\ttest_path_is_missing .git/refs/heads/ref-to-bad\n+'\n+\n+test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '\n+\tgit symbolic-ref refs/heads/badname refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n+\ttest_path_is_file .git/refs/heads/badname &&\n+\tgit update-ref --no-deref -d refs/heads/badname &&\n+\ttest_path_is_missing .git/refs/heads/badname\n+'\n+\n test_expect_success '(not) create HEAD with old sha1' \"\n \ttest_must_fail git update-ref HEAD $A $B\n \"\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex ac31b71..432921b 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -285,6 +285,15 @@ test_expect_success 'deleting a dangling symref' '\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_success 'deleting a self-referential symref' '\n+\tgit symbolic-ref refs/heads/self-reference refs/heads/self-reference &&\n+\ttest_path_is_file .git/refs/heads/self-reference &&\n+\techo \"Deleted branch self-reference (was refs/heads/self-reference).\" >expect &&\n+\tgit branch -d self-reference >actual &&\n+\ttest_path_is_missing .git/refs/heads/self-reference &&\n+\ttest_i18ncmp expect actual\n+'\n+\n test_expect_success 'renaming a symref is not allowed' '\n \tgit symbolic-ref refs/heads/master2 refs/heads/master &&\n \ttest_must_fail git branch -m master2 master3 &&\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250649","messageId":"20141015005255.GU32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 17/25] branch -d: simplify by using RESOLVE_REF_READING","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:52:55Z","receivedAt":"2014-10-15T00:52:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 11 Sep 2014 10:34:36 -0700\n\nWhen \"git branch -d\" reads the branch it is about to delete, it used\nto avoid passing the RESOLVE_REF_READING ('treat missing ref as\nerror') flag because a symref pointing to a nonexistent ref would show\nup as missing instead of as something that could be deleted. To check\nif a ref is actually missing, we then check\n\n - is it a symref?\n - if not, did it resolve to null_sha1?\n\nNow we pass RESOLVE_REF_NO_RECURSE and the correct information is\nreturned for a symref even when it points to a missing ref. Simplify\nby relying on RESOLVE_REF_READING.\n\nNo functional change intended.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n builtin/branch.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 2ad2d0b..0c7aac0 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -236,10 +236,11 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tfree(name);\n \n \t\tname = mkpathdup(fmt, bname.buf);\n-\t\ttarget = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE,\n+\t\ttarget = resolve_ref_unsafe(name,\n+\t\t\t\t\t RESOLVE_REF_READING\n+\t\t\t\t\t | RESOLVE_REF_NO_RECURSE,\n \t\t\t\t\t sha1, &flags);\n-\t\tif (!target ||\n-\t\t (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {\n+\t\tif (!target) {\n \t\t\terror(remote_branch\n \t\t\t ? _(\"remote branch '%s' not found.\")\n \t\t\t : _(\"branch '%s' not found.\"), bname.buf);\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250650","messageId":"20141015005323.GV32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 18/25] packed-ref cache: forbid dot-components in refnames","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:53:23Z","receivedAt":"2014-10-15T00:53:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Fri, 26 Sep 2014 12:22:22 -0700\n\nSince v1.7.9-rc1~10^2 (write_head_info(): handle \"extra refs\" locally,\n2012-01-06), this trick to keep track of \".have\" refs that are only\nvalid on the wire and not on the filesystem is not needed any more.\n\nSimplify by removing support for the REFNAME_DOT_COMPONENT flag.\n\nThis means we'll be slightly stricter with invalid refs found in a\npacked-refs file or during clone. read_loose_refs() already checks\nfor and skips refnames with .components so it is not affected.\n\nSigned-off-by: Jonathan Nieder \nReviewed-by: Ronnie Sahlberg \n---\n refs.c | 14 +++-----------\n refs.h | 6 +-----\n 2 files changed, 4 insertions(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4fe263e..fe1352a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -70,16 +70,8 @@ static int check_refname_component(const char *refname, int flags)\n out:\n \tif (cp == refname)\n \t\treturn 0; /* Component has zero length. */\n-\tif (refname[0] == '.') {\n-\t\tif (!(flags & REFNAME_DOT_COMPONENT))\n-\t\t\treturn -1; /* Component starts with '.'. */\n-\t\t/*\n-\t\t * Even if leading dots are allowed, don't allow \".\"\n-\t\t * as a component (\"..\" is prevented by a rule above).\n-\t\t */\n-\t\tif (refname[1] == '\\0')\n-\t\t\treturn -1; /* Component equals \".\". */\n-\t}\n+\tif (refname[0] == '.')\n+\t\treturn -1; /* Component starts with '.'. */\n \tif (cp - refname >= LOCK_SUFFIX_LEN &&\n \t !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN))\n \t\treturn -1; /* Refname ends with \".lock\". */\n@@ -290,7 +282,7 @@ static struct ref_entry *create_ref_entry(const char *refname,\n \tstruct ref_entry *ref;\n \n \tif (check_name &&\n-\t check_refname_format(refname, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))\n+\t check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n \t\tdie(\"Reference has invalid format: '%s'\", refname);\n \tlen = strlen(refname) + 1;\n \tref = xmalloc(sizeof(struct ref_entry) + len);\ndiff --git a/refs.h b/refs.h\nindex 79802d7..c61b8f4 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -229,7 +229,6 @@ extern int for_each_reflog(each_ref_fn, void *);\n \n #define REFNAME_ALLOW_ONELEVEL 1\n #define REFNAME_REFSPEC_PATTERN 2\n-#define REFNAME_DOT_COMPONENT 4\n \n /*\n * Return 0 iff refname has the correct format for a refname according\n@@ -237,10 +236,7 @@ extern int for_each_reflog(each_ref_fn, void *);\n * If REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level\n * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then\n * allow a \"*\" wildcard character in place of one of the name\n- * components. No leading or repeated slashes are accepted. If\n- * REFNAME_DOT_COMPONENT is set in flags, then allow refname\n- * components to start with \".\" (but not a whole component equal to\n- * \".\" or \"..\").\n+ * components. No leading or repeated slashes are accepted.\n */\n extern int check_refname_format(const char *refname, int flags);\n \n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250651","messageId":"20141015005348.GW32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 19/25] test: put tests for handling of bad ref names in one place","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:53:48Z","receivedAt":"2014-10-15T00:53:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 25 Sep 2014 15:02:39 -0700\n\nThere's no straightforward way to grep for all tests dealing with\ninvalid refs. Put them in a single test script so it is easy to see\nwhat functionality has not been exercised with bad ref names yet.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n t/t1400-update-ref.sh | 44 --------------------------\n t/t1430-bad-ref-name.sh | 84 +++++++++++++++++++++++++++++++++++++++++++++++++\n t/t9300-fast-import.sh | 30 ------------------\n 3 files changed, 84 insertions(+), 74 deletions(-)\n create mode 100755 t/t1430-bad-ref-name.sh\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7c8c41a..7b4707b 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -136,14 +136,6 @@ test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' '\n \ttest_path_is_missing .git/refs/heads/ref-to-bad\n '\n \n-test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '\n-\tgit symbolic-ref refs/heads/badname refs/heads/broken...ref &&\n-\ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n-\ttest_path_is_file .git/refs/heads/badname &&\n-\tgit update-ref --no-deref -d refs/heads/badname &&\n-\ttest_path_is_missing .git/refs/heads/badname\n-'\n-\n test_expect_success '(not) create HEAD with old sha1' \"\n \ttest_must_fail git update-ref HEAD $A $B\n \"\n@@ -408,12 +400,6 @@ test_expect_success 'stdin fails create with no ref' '\n \tgrep \"fatal: create: missing \" err\n '\n \n-test_expect_success 'stdin fails create with bad ref name' '\n-\techo \"create ~a $m\" >stdin &&\n-\ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: invalid ref format: ~a\" err\n-'\n-\n test_expect_success 'stdin fails create with no new value' '\n \techo \"create $a\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n@@ -432,12 +418,6 @@ test_expect_success 'stdin fails update with no ref' '\n \tgrep \"fatal: update: missing \" err\n '\n \n-test_expect_success 'stdin fails update with bad ref name' '\n-\techo \"update ~a $m\" >stdin &&\n-\ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: invalid ref format: ~a\" err\n-'\n-\n test_expect_success 'stdin fails update with no new value' '\n \techo \"update $a\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n@@ -456,12 +436,6 @@ test_expect_success 'stdin fails delete with no ref' '\n \tgrep \"fatal: delete: missing \" err\n '\n \n-test_expect_success 'stdin fails delete with bad ref name' '\n-\techo \"delete ~a $m\" >stdin &&\n-\ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: invalid ref format: ~a\" err\n-'\n-\n test_expect_success 'stdin fails delete with too many arguments' '\n \techo \"delete $a $m $m\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n@@ -734,12 +708,6 @@ test_expect_success 'stdin -z fails create with no ref' '\n \tgrep \"fatal: create: missing \" err\n '\n \n-test_expect_success 'stdin -z fails create with bad ref name' '\n-\tprintf $F \"create ~a \" \"$m\" >stdin &&\n-\ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: invalid ref format: ~a \" err\n-'\n-\n test_expect_success 'stdin -z fails create with no new value' '\n \tprintf $F \"create $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n@@ -764,12 +732,6 @@ test_expect_success 'stdin -z fails update with too few args' '\n \tgrep \"fatal: update $a: unexpected end of input when reading \" err\n '\n \n-test_expect_success 'stdin -z fails update with bad ref name' '\n-\tprintf $F \"update ~a\" \"$m\" \"\" >stdin &&\n-\ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: invalid ref format: ~a\" err\n-'\n-\n test_expect_success 'stdin -z emits warning with empty new value' '\n \tgit update-ref $a $m &&\n \tprintf $F \"update $a\" \"\" \"\" >stdin &&\n@@ -802,12 +764,6 @@ test_expect_success 'stdin -z fails delete with no ref' '\n \tgrep \"fatal: delete: missing \" err\n '\n \n-test_expect_success 'stdin -z fails delete with bad ref name' '\n-\tprintf $F \"delete ~a\" \"$m\" >stdin &&\n-\ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: invalid ref format: ~a\" err\n-'\n-\n test_expect_success 'stdin -z fails delete with no old value' '\n \tprintf $F \"delete $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\ndiff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh\nnew file mode 100755\nindex 0000000..c7b19b0\n--- /dev/null\n+++ b/t/t1430-bad-ref-name.sh\n@@ -0,0 +1,84 @@\n+#!/bin/sh\n+\n+test_description='Test handling of ref names that check-ref-format rejects'\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\ttest_commit one\n+'\n+\n+test_expect_success 'fast-import: fail on invalid branch name \".badbranchname\"' '\n+\ttest_when_finished \"rm -f .git/objects/pack_* .git/objects/index_*\" &&\n+\tcat >input <<-INPUT_END &&\n+\t\tcommit .badbranchname\n+\t\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\t\tdata <input <<-INPUT_END &&\n+\t\tcommit bad[branch]name\n+\t\tcommitter $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n+\t\tdata <stdin &&\n+\ttest_must_fail git update-ref --stdin err &&\n+\tgrep \"fatal: invalid ref format: ~a\" err\n+'\n+\n+test_expect_success 'update-ref --stdin fails update with bad ref name' '\n+\techo \"update ~a refs/heads/master\" >stdin &&\n+\ttest_must_fail git update-ref --stdin err &&\n+\tgrep \"fatal: invalid ref format: ~a\" err\n+'\n+\n+test_expect_success 'update-ref --stdin fails delete with bad ref name' '\n+\techo \"delete ~a refs/heads/master\" >stdin &&\n+\ttest_must_fail git update-ref --stdin err &&\n+\tgrep \"fatal: invalid ref format: ~a\" err\n+'\n+\n+test_expect_success 'update-ref --stdin -z fails create with bad ref name' '\n+\tprintf \"%s\\0\" \"create ~a \" refs/heads/master >stdin &&\n+\ttest_must_fail git update-ref -z --stdin err &&\n+\tgrep \"fatal: invalid ref format: ~a \" err\n+'\n+\n+test_expect_success 'update-ref --stdin -z fails update with bad ref name' '\n+\tprintf \"%s\\0\" \"update ~a\" refs/heads/master \"\" >stdin &&\n+\ttest_must_fail git update-ref -z --stdin err &&\n+\tgrep \"fatal: invalid ref format: ~a\" err\n+'\n+\n+test_expect_success 'update-ref --stdin -z fails delete with bad ref name' '\n+\tprintf \"%s\\0\" \"delete ~a\" refs/heads/master >stdin &&\n+\ttest_must_fail git update-ref -z --stdin err &&\n+\tgrep \"fatal: invalid ref format: ~a\" err\n+'\n+\n+test_done\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 8df0445..37c2d63 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -347,36 +347,6 @@ test_expect_success 'B: fail on invalid blob sha1' '\n rm -f .git/objects/pack_* .git/objects/index_*\n \n cat >input < $GIT_COMMITTER_DATE\n-data <input < $GIT_COMMITTER_DATE\n-data <input < $GIT_COMMITTER_DATE\n data <\nDate: Wed, 3 Sep 2014 11:45:43 -0700\n\nWe currently do not handle badly named refs well:\n\n $ cp .git/refs/heads/master .git/refs/heads/master.....@\\*@\\\\.\n $ git branch\n fatal: Reference has invalid format: 'refs/heads/master.....@*@\\.'\n $ git branch -D master.....@\\*@\\\\.\n error: branch 'master.....@*@\\.' not found.\n\nUsers cannot recover from a badly named ref without manually finding\nand deleting the loose ref file or appropriate line in packed-refs.\nMaking that easier will make it easier to tweak the ref naming rules\nin the future, for example to forbid shell metacharacters like '`'\nand '\"', without putting people in a state that is hard to get out of.\n\nSo allow \"branch --list\" to show these refs and allow \"branch -d/-D\"\nand \"update-ref -d\" to delete them. Other commands (for example to\nrename refs) will continue to not handle these refs but can be changed\nin later patches.\n\nDetails:\n\nIn resolving functions, refuse to resolve refs that don't pass the\ngit-check-ref-format(1) check unless the new RESOLVE_REF_ALLOW_BAD_NAME\nflag is passed. Even with RESOLVE_REF_ALLOW_BAD_NAME, refuse to\nresolve refs that escape the refs/ directory and do not match the\npattern [A-Z_]* (think \"HEAD\" and \"MERGE_HEAD\").\n\nIn locking functions, refuse to act on badly named refs unless they\nare being deleted and either are in the refs/ directory or match [A-Z_]*.\n\nJust like other invalid refs, flag resolved, badly named refs with the\nREF_ISBROKEN flag, treat them as resolving to null_sha1, and skip them\nin all iteration functions except for for_each_rawref.\n\nFlag badly named refs (but not symrefs pointing to badly named refs)\nwith a REF_BAD_NAME flag to make it easier for future callers to\nnotice and handle them specially. For example, in a later patch\nfor-each-ref will use this flag to detect refs whose names can confuse\ncallers parsing for-each-ref output.\n\nIn the transaction API, refuse to create or update badly named refs,\nbut allow deleting them (unless they try to escape refs/ and don't match\n[A-Z_]*).\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n builtin/branch.c | 9 +--\n cache.h | 17 +++++-\n refs.c | 148 ++++++++++++++++++++++++++++++++++++++----------\n refs.h | 12 +++-\n t/t1430-bad-ref-name.sh | 125 +++++++++++++++++++++++++++++++++++++++-\n 5 files changed, 273 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 0c7aac0..7e113d6 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tname = mkpathdup(fmt, bname.buf);\n \t\ttarget = resolve_ref_unsafe(name,\n \t\t\t\t\t RESOLVE_REF_READING\n-\t\t\t\t\t | RESOLVE_REF_NO_RECURSE,\n+\t\t\t\t\t | RESOLVE_REF_NO_RECURSE\n+\t\t\t\t\t | RESOLVE_REF_ALLOW_BAD_NAME,\n \t\t\t\t\t sha1, &flags);\n \t\tif (!target) {\n \t\t\terror(remote_branch\n@@ -248,7 +249,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (!(flags & REF_ISSYMREF) &&\n+\t\tif (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&\n \t\t check_branch_commit(bname.buf, name, sha1, head_rev, kinds,\n \t\t\t\t\tforce)) {\n \t\t\tret = 1;\n@@ -268,8 +269,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\t ? _(\"Deleted remote branch %s (was %s).\\n\")\n \t\t\t : _(\"Deleted branch %s (was %s).\\n\"),\n \t\t\t bname.buf,\n-\t\t\t (flags & REF_ISSYMREF)\n-\t\t\t ? target\n+\t\t\t (flags & REF_ISBROKEN) ? \"broken\"\n+\t\t\t : (flags & REF_ISSYMREF) ? target\n \t\t\t : find_unique_abbrev(sha1, DEFAULT_ABBREV));\n \t\t}\n \t\tdelete_branch_config(bname.buf);\ndiff --git a/cache.h b/cache.h\nindex b735f3f..0501f7d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -981,16 +981,29 @@ extern int read_ref(const char *refname, unsigned char *sha1);\n * If flags is non-NULL, set the value that it points to the\n * combination of REF_ISPACKED (if the reference was found among the\n * packed references), REF_ISSYMREF (if the initial reference was a\n- * symbolic reference) and REF_ISBROKEN (if the ref is malformed).\n+ * symbolic reference), REF_BAD_NAME (if the reference name is ill\n+ * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN\n+ * (if the ref is malformed or has a bad name). See refs.h for more detail\n+ * on each flag.\n *\n * If ref is not a properly-formatted, normalized reference, return\n * NULL. If more than MAXDEPTH recursive symbolic lookups are needed,\n * give up and return NULL.\n *\n- * errno is set to something meaningful on error.\n+ * RESOLVE_REF_ALLOW_BAD_NAME allows resolving refs even when their\n+ * name is invalid according to git-check-ref-format(1). If the name\n+ * is bad then the value stored in sha1 will be null_sha1 and the two\n+ * flags REF_ISBROKEN and REF_BAD_NAME will be set.\n+ *\n+ * Even with RESOLVE_REF_ALLOW_BAD_NAME, names that escape the refs/\n+ * directory and do not consist of all caps and underscores cannot be\n+ * resolved. The function returns NULL for such ref names.\n+ * Caps and underscores refers to the special refs, such as HEAD,\n+ * FETCH_HEAD and friends, that all live outside of the refs/ directory.\n */\n #define RESOLVE_REF_READING 0x01\n #define RESOLVE_REF_NO_RECURSE 0x02\n+#define RESOLVE_REF_ALLOW_BAD_NAME 0x04\n extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);\n \ndiff --git a/refs.c b/refs.c\nindex fe1352a..e7000f2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -187,8 +187,8 @@ struct ref_dir {\n \n /*\n * Bit values for ref_entry::flag. REF_ISSYMREF=0x01,\n- * REF_ISPACKED=0x02, and REF_ISBROKEN=0x04 are public values; see\n- * refs.h.\n+ * REF_ISPACKED=0x02, REF_ISBROKEN=0x04 and REF_BAD_NAME=0x08 are\n+ * public values; see refs.h.\n */\n \n /*\n@@ -196,16 +196,16 @@ struct ref_dir {\n * the correct peeled value for the reference, which might be\n * null_sha1 if the reference is not a tag or if it is broken.\n */\n-#define REF_KNOWS_PEELED 0x08\n+#define REF_KNOWS_PEELED 0x10\n \n /* ref_entry represents a directory of references */\n-#define REF_DIR 0x10\n+#define REF_DIR 0x20\n \n /*\n * Entry has not yet been read from disk (used only for REF_DIR\n * entries representing loose references)\n */\n-#define REF_INCOMPLETE 0x20\n+#define REF_INCOMPLETE 0x40\n \n /*\n * A ref_entry represents either a reference or a \"subdirectory\" of\n@@ -274,6 +274,39 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)\n \treturn dir;\n }\n \n+/*\n+ * Check if a refname is safe.\n+ * For refs that start with \"refs/\" we consider it safe as long they do\n+ * not try to resolve to outside of refs/.\n+ *\n+ * For all other refs we only consider them safe iff they only contain\n+ * upper case characters and '_' (like \"HEAD\" AND \"MERGE_HEAD\", and not like\n+ * \"config\").\n+ */\n+static int refname_is_safe(const char *refname)\n+{\n+\tif (starts_with(refname, \"refs/\")) {\n+\t\tchar *buf;\n+\t\tint result;\n+\n+\t\tbuf = xmalloc(strlen(refname) + 1);\n+\t\t/*\n+\t\t * Does the refname try to escape refs/?\n+\t\t * For example: refs/foo/../bar is safe but refs/foo/../../bar\n+\t\t * is not.\n+\t\t */\n+\t\tresult = !normalize_path_copy(buf, refname + strlen(\"refs/\"));\n+\t\tfree(buf);\n+\t\treturn result;\n+\t}\n+\twhile (*refname) {\n+\t\tif (!isupper(*refname) && *refname != '_')\n+\t\t\treturn 0;\n+\t\trefname++;\n+\t}\n+\treturn 1;\n+}\n+\n static struct ref_entry *create_ref_entry(const char *refname,\n \t\t\t\t\t const unsigned char *sha1, int flag,\n \t\t\t\t\t int check_name)\n@@ -284,6 +317,8 @@ static struct ref_entry *create_ref_entry(const char *refname,\n \tif (check_name &&\n \t check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n \t\tdie(\"Reference has invalid format: '%s'\", refname);\n+\tif (!check_name && !refname_is_safe(refname))\n+\t\tdie(\"Reference has invalid name: '%s'\", refname);\n \tlen = strlen(refname) + 1;\n \tref = xmalloc(sizeof(struct ref_entry) + len);\n \thashcpy(ref->u.value.sha1, sha1);\n@@ -1111,7 +1146,13 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)\n \n \t\trefname = parse_ref_line(refline, sha1);\n \t\tif (refname) {\n-\t\t\tlast = create_ref_entry(refname, sha1, REF_ISPACKED, 1);\n+\t\t\tint flag = REF_ISPACKED;\n+\n+\t\t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n+\t\t\t\thashclr(sha1);\n+\t\t\t\tflag |= REF_BAD_NAME | REF_ISBROKEN;\n+\t\t\t}\n+\t\t\tlast = create_ref_entry(refname, sha1, flag, 0);\n \t\t\tif (peeled == PEELED_FULLY ||\n \t\t\t (peeled == PEELED_TAGS && starts_with(refname, \"refs/tags/\")))\n \t\t\t\tlast->flag |= REF_KNOWS_PEELED;\n@@ -1249,8 +1290,13 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)\n \t\t\t\thashclr(sha1);\n \t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t}\n+\t\t\tif (check_refname_format(refname.buf,\n+\t\t\t\t\t\t REFNAME_ALLOW_ONELEVEL)) {\n+\t\t\t\thashclr(sha1);\n+\t\t\t\tflag |= REF_BAD_NAME | REF_ISBROKEN;\n+\t\t\t}\n \t\t\tadd_entry_to_dir(dir,\n-\t\t\t\t\t create_ref_entry(refname.buf, sha1, flag, 1));\n+\t\t\t\t\t create_ref_entry(refname.buf, sha1, flag, 0));\n \t\t}\n \t\tstrbuf_setlen(&refname, dirnamelen);\n \t}\n@@ -1369,10 +1415,10 @@ static struct ref_entry *get_packed_ref(const char *refname)\n * A loose ref file doesn't exist; check for a packed ref. The\n * options are forwarded from resolve_safe_unsafe().\n */\n-static const char *handle_missing_loose_ref(const char *refname,\n-\t\t\t\t\t int resolve_flags,\n-\t\t\t\t\t unsigned char *sha1,\n-\t\t\t\t\t int *flags)\n+static int resolve_missing_loose_ref(const char *refname,\n+\t\t\t\t int resolve_flags,\n+\t\t\t\t unsigned char *sha1,\n+\t\t\t\t int *flags)\n {\n \tstruct ref_entry *entry;\n \n@@ -1385,14 +1431,15 @@ static const char *handle_missing_loose_ref(const char *refname,\n \t\thashcpy(sha1, entry->u.value.sha1);\n \t\tif (flags)\n \t\t\t*flags |= REF_ISPACKED;\n-\t\treturn refname;\n+\t\treturn 0;\n \t}\n \t/* The reference is not a packed reference, either. */\n \tif (resolve_flags & RESOLVE_REF_READING) {\n-\t\treturn NULL;\n+\t\terrno = ENOENT;\n+\t\treturn -1;\n \t} else {\n \t\thashclr(sha1);\n-\t\treturn refname;\n+\t\treturn 0;\n \t}\n }\n \n@@ -1403,13 +1450,29 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \tssize_t len;\n \tchar buffer[256];\n \tstatic char refname_buffer[256];\n+\tint bad_name = 0;\n \n \tif (flags)\n \t\t*flags = 0;\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\terrno = EINVAL;\n-\t\treturn NULL;\n+\t\tif (flags)\n+\t\t\t*flags |= REF_BAD_NAME;\n+\n+\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n+\t\t !refname_is_safe(refname)) {\n+\t\t\terrno = EINVAL;\n+\t\t\treturn NULL;\n+\t\t}\n+\t\t/*\n+\t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n+\t\t * missing refs and refs that were present but invalid,\n+\t\t * to complain about the latter to stderr.\n+\t\t *\n+\t\t * We don't know whether the ref exists, so don't set\n+\t\t * REF_ISBROKEN yet.\n+\t\t */\n+\t\tbad_name = 1;\n \t}\n \tfor (;;) {\n \t\tchar path[PATH_MAX];\n@@ -1435,11 +1498,17 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \t\t */\n \tstat_ref:\n \t\tif (lstat(path, &st) < 0) {\n-\t\t\tif (errno == ENOENT)\n-\t\t\t\treturn handle_missing_loose_ref(refname,\n-\t\t\t\t\t\tresolve_flags, sha1, flags);\n-\t\t\telse\n+\t\t\tif (errno != ENOENT)\n \t\t\t\treturn NULL;\n+\t\t\tif (resolve_missing_loose_ref(refname, resolve_flags,\n+\t\t\t\t\t\t sha1, flags))\n+\t\t\t\treturn NULL;\n+\t\t\tif (bad_name) {\n+\t\t\t\thashclr(sha1);\n+\t\t\t\tif (flags)\n+\t\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t}\n+\t\t\treturn refname;\n \t\t}\n \n \t\t/* Follow \"normalized\" - ie \"refs/..\" symlinks by hand */\n@@ -1512,6 +1581,11 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n+\t\t\tif (bad_name) {\n+\t\t\t\thashclr(sha1);\n+\t\t\t\tif (flags)\n+\t\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t}\n \t\t\treturn refname;\n \t\t}\n \t\tif (flags)\n@@ -1527,8 +1601,13 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned\n \t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\tif (flags)\n \t\t\t\t*flags |= REF_ISBROKEN;\n-\t\t\terrno = EINVAL;\n-\t\t\treturn NULL;\n+\n+\t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n+\t\t\t !refname_is_safe(buf)) {\n+\t\t\t\terrno = EINVAL;\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\t\t\tbad_name = 1;\n \t\t}\n \t}\n }\n@@ -2160,18 +2239,16 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,\n \tint missing = 0;\n \tint attempts_remaining = 3;\n \n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\terrno = EINVAL;\n-\t\treturn NULL;\n-\t}\n-\n \tlock = xcalloc(1, sizeof(struct ref_lock));\n \tlock->lock_fd = -1;\n \n \tif (mustexist)\n \t\tresolve_flags |= RESOLVE_REF_READING;\n-\tif (flags & REF_NODEREF && flags & REF_DELETING)\n-\t\tresolve_flags |= RESOLVE_REF_NO_RECURSE;\n+\tif (flags & REF_DELETING) {\n+\t\tresolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;\n+\t\tif (flags & REF_NODEREF)\n+\t\t\tresolve_flags |= RESOLVE_REF_NO_RECURSE;\n+\t}\n \n \trefname = resolve_ref_unsafe(refname, resolve_flags,\n \t\t\t\t lock->old_sha1, &type);\n@@ -3519,6 +3596,13 @@ int ref_transaction_update(struct ref_transaction *transaction,\n \tif (have_old && !old_sha1)\n \t\tdie(\"BUG: have_old is true but old_sha1 is NULL\");\n \n+\tif (!is_null_sha1(new_sha1) &&\n+\t check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n+\t\tstrbuf_addf(err, \"refusing to update ref with bad name %s\",\n+\t\t\t refname);\n+\t\treturn -1;\n+\t}\n+\n \tupdate = add_update(transaction, refname);\n \thashcpy(update->new_sha1, new_sha1);\n \tupdate->flags = flags;\n@@ -3544,6 +3628,12 @@ int ref_transaction_create(struct ref_transaction *transaction,\n \tif (!new_sha1 || is_null_sha1(new_sha1))\n \t\tdie(\"BUG: create ref with null new_sha1\");\n \n+\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n+\t\tstrbuf_addf(err, \"refusing to create ref with bad name %s\",\n+\t\t\t refname);\n+\t\treturn -1;\n+\t}\n+\n \tupdate = add_update(transaction, refname);\n \n \thashcpy(update->new_sha1, new_sha1);\ndiff --git a/refs.h b/refs.h\nindex c61b8f4..2bc3556 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -56,12 +56,20 @@ struct ref_transaction;\n \n /*\n * Reference cannot be resolved to an object name: dangling symbolic\n- * reference (directly or indirectly), corrupt reference file, or\n- * symbolic reference refers to ill-formatted reference name.\n+ * reference (directly or indirectly), corrupt reference file,\n+ * reference exists but name is bad, or symbolic reference refers to\n+ * ill-formatted reference name.\n */\n #define REF_ISBROKEN 0x04\n \n /*\n+ * Reference name is not well formed.\n+ *\n+ * See git-check-ref-format(1) for the definition of well formed ref names.\n+ */\n+#define REF_BAD_NAME 0x08\n+\n+/*\n * The signature for the callback function for the for_each_*()\n * functions below. The memory pointed to by the refname and sha1\n * arguments is only guaranteed to be valid for the duration of a\ndiff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh\nindex c7b19b0..468e856 100755\n--- a/t/t1430-bad-ref-name.sh\n+++ b/t/t1430-bad-ref-name.sh\n@@ -4,7 +4,8 @@ test_description='Test handling of ref names that check-ref-format rejects'\n . ./test-lib.sh\n \n test_expect_success setup '\n-\ttest_commit one\n+\ttest_commit one &&\n+\ttest_commit two\n '\n \n test_expect_success 'fast-import: fail on invalid branch name \".badbranchname\"' '\n@@ -37,6 +38,107 @@ test_expect_success 'fast-import: fail on invalid branch name \"bad[branch]name\"'\n \ttest_must_fail git fast-import output &&\n+\tgrep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'branch -d can delete badly named ref' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tgit branch -d broken...ref &&\n+\tgit branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'branch -D can delete badly named ref' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tgit branch -D broken...ref &&\n+\tgit branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'branch -D cannot delete non-ref in .git dir' '\n+\techo precious >.git/my-private-file &&\n+\techo precious >expect &&\n+\ttest_must_fail git branch -D ../../my-private-file &&\n+\ttest_cmp expect .git/my-private-file\n+'\n+\n+test_expect_success 'branch -D cannot delete absolute path' '\n+\tgit branch -f extra &&\n+\ttest_must_fail git branch -D \"$(pwd)/.git/refs/heads/extra\" &&\n+\ttest_cmp_rev HEAD extra\n+'\n+\n+test_expect_success 'git branch cannot create a badly named ref' '\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\ttest_must_fail git branch broken...ref &&\n+\tgit branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'branch -m cannot rename to a bad ref name' '\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\ttest_might_fail git branch -D goodref &&\n+\tgit branch goodref &&\n+\ttest_must_fail git branch -m goodref broken...ref &&\n+\ttest_cmp_rev master goodref &&\n+\tgit branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_failure 'branch -m can rename from a bad ref name' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tgit branch -m broken...ref renamed &&\n+\ttest_cmp_rev master renamed &&\n+\tgit branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'push cannot create a badly named ref' '\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\ttest_must_fail git push \"file://$(pwd)\" HEAD:refs/heads/broken...ref &&\n+\tgit branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_failure 'push --mirror can delete badly named ref' '\n+\ttop=$(pwd) &&\n+\tgit init src &&\n+\tgit init dest &&\n+\n+\t(\n+\t\tcd src &&\n+\t\ttest_commit one\n+\t) &&\n+\t(\n+\t\tcd dest &&\n+\t\ttest_commit two &&\n+\t\tgit checkout --detach &&\n+\t\tcp .git/refs/heads/master .git/refs/heads/broken...ref\n+\t) &&\n+\tgit -C src push --mirror \"file://$top/dest\" &&\n+\tgit -C dest branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'rev-parse skips symref pointing to broken name' '\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tgit branch shadow one &&\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\tgit symbolic-ref refs/tags/shadow refs/heads/broken...ref &&\n+\n+\tgit rev-parse --verify one >expect &&\n+\tgit rev-parse --verify shadow >actual 2>err &&\n+\ttest_cmp expect actual &&\n+\ttest_i18ngrep \"ignoring.*refs/tags/shadow\" err\n+'\n+\n test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '\n \tgit symbolic-ref refs/heads/badname refs/heads/broken...ref &&\n \ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n@@ -45,6 +147,27 @@ test_expect_success 'update-ref --no-deref -d can delete reference to broken nam\n \ttest_path_is_missing .git/refs/heads/badname\n '\n \n+test_expect_success 'update-ref -d can delete broken name' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tgit update-ref -d refs/heads/broken...ref &&\n+\tgit branch >output &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '\n+\techo precious >.git/my-private-file &&\n+\techo precious >expect &&\n+\ttest_must_fail git update-ref -d my-private-file &&\n+\ttest_cmp expect .git/my-private-file\n+'\n+\n+test_expect_success 'update-ref -d cannot delete absolute path' '\n+\tgit branch -f extra &&\n+\ttest_must_fail git update-ref -d \"$(pwd)/.git/refs/heads/extra\" &&\n+\ttest_cmp_rev HEAD extra\n+'\n+\n test_expect_success 'update-ref --stdin fails create with bad ref name' '\n \techo \"create ~a refs/heads/master\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250653","messageId":"20141015005421.GY32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 21/25] for-each-ref: skip and warn about broken ref names","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:54:21Z","receivedAt":"2014-10-15T00:54:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Fri, 5 Sep 2014 14:35:17 -0700\n\nPrint a warning message for any bad ref names we find in the repo and\nskip them so callers don't have to deal with parsing them.\n\nIt might be useful in the future to have a flag where we would not\nskip these refs for those callers that want to and are prepared (for\nexample by using a --format argument with %0 as a delimiter after the\nref name).\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \n---\n builtin/for-each-ref.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 492265d..3ee22b9 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -839,6 +839,11 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f\n \tstruct refinfo *ref;\n \tint cnt;\n \n+\tif (flag & REF_BAD_NAME) {\n+\t\t warning(\"ignoring ref with broken name %s\", refname);\n+\t\t return 0;\n+\t}\n+\n \tif (*cb->grab_pattern) {\n \t\tconst char **pattern;\n \t\tint namelen = strlen(refname);\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250654","messageId":"20141015005448.GZ32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 22/25] remote rm/prune: print a message when writing packed-refs fails","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:54:48Z","receivedAt":"2014-10-15T00:54:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Ronnie Sahlberg \nDate: Thu, 11 Sep 2014 08:42:57 -0700\n\nUntil v2.1.0-rc0~22^2~11 (refs.c: add an err argument to\nrepack_without_refs, 2014-06-20), repack_without_refs forgot to\nprovide an error message when commit_packed_refs fails. Even today,\nit only provides a message for callers that pass a non-NULL err\nparameter. Internal callers in refs.c pass non-NULL err but\n\"git remote\" does not.\n\nThat means that \"git remote rm\" and \"git remote prune\" can fail\nwithout printing a message about why. Fix them by passing in a\nnon-NULL err parameter and printing the returned message.\n\nThis is the last caller to a ref handling function passing err ==\nNULL. A later patch can drop support for err == NULL, avoiding such\nproblems in the future.\n\nSigned-off-by: Ronnie Sahlberg \nSigned-off-by: Jonathan Nieder \nReviewed-by: Michael Haggerty \n---\n builtin/remote.c | 15 +++++++++++----\n 1 file changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 42a533a..7f28f92 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -749,13 +749,16 @@ static int mv(int argc, const char **argv)\n \n static int remove_branches(struct string_list *branches)\n {\n+\tstruct strbuf err = STRBUF_INIT;\n \tconst char **branch_names;\n \tint i, result = 0;\n \n \tbranch_names = xmalloc(branches->nr * sizeof(*branch_names));\n \tfor (i = 0; i < branches->nr; i++)\n \t\tbranch_names[i] = branches->items[i].string;\n-\tresult |= repack_without_refs(branch_names, branches->nr, NULL);\n+\tif (repack_without_refs(branch_names, branches->nr, &err))\n+\t\tresult |= error(\"%s\", err.buf);\n+\tstrbuf_release(&err);\n \tfree(branch_names);\n \n \tfor (i = 0; i < branches->nr; i++) {\n@@ -1332,9 +1335,13 @@ static int prune_remote(const char *remote, int dry_run)\n \t\tdelete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));\n \t\tfor (i = 0; i < states.stale.nr; i++)\n \t\t\tdelete_refs[i] = states.stale.items[i].util;\n-\t\tif (!dry_run)\n-\t\t\tresult |= repack_without_refs(delete_refs,\n-\t\t\t\t\t\t states.stale.nr, NULL);\n+\t\tif (!dry_run) {\n+\t\t\tstruct strbuf err = STRBUF_INIT;\n+\t\t\tif (repack_without_refs(delete_refs, states.stale.nr,\n+\t\t\t\t\t\t&err))\n+\t\t\t\tresult |= error(\"%s\", err.buf);\n+\t\t\tstrbuf_release(&err);\n+\t\t}\n \t\tfree(delete_refs);\n \t}\n \n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250655","messageId":"20141015005506.GA32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 23/25] refs.c: do not permit err == NULL","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:55:06Z","receivedAt":"2014-10-15T00:55:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Thu, 28 Aug 2014 16:42:37 -0700\n\nSome functions that take a strbuf argument to append an error treat\n!err as an indication that the message should be suppressed (e.g.,\nref_update_reject_duplicates). Others write the message to stderr on\n!err (e.g., repack_without_refs). Others crash (e.g.,\nref_transaction_update).\n\nSome of these behaviors are for historical reasons and others were\naccidents. Luckily no callers pass err == NULL any more. Simplify\nby consistently requiring the strbuf argument.\n\nSigned-off-by: Jonathan Nieder \nReviewed-by: Ronnie Sahlberg \n---\n refs.c | 46 +++++++++++++++++++++++++++-------------------\n 1 file changed, 27 insertions(+), 19 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e7000f2..097fb4b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2646,6 +2646,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \tstruct string_list_item *ref_to_delete;\n \tint i, ret, removed = 0;\n \n+\tassert(err);\n+\n \t/* Look for a packed ref */\n \tfor (i = 0; i < n; i++)\n \t\tif (get_packed_ref(refnames[i]))\n@@ -2656,13 +2658,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \t\treturn 0; /* no refname exists in packed refs */\n \n \tif (lock_packed_refs(0)) {\n-\t\tif (err) {\n-\t\t\tunable_to_lock_message(git_path(\"packed-refs\"), errno,\n-\t\t\t\t\t err);\n-\t\t\treturn -1;\n-\t\t}\n-\t\tunable_to_lock_error(git_path(\"packed-refs\"), errno);\n-\t\treturn error(\"cannot delete '%s' from packed refs\", refnames[i]);\n+\t\tunable_to_lock_message(git_path(\"packed-refs\"), errno, err);\n+\t\treturn -1;\n \t}\n \tpacked = get_packed_refs(&ref_cache);\n \n@@ -2688,7 +2685,7 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)\n \n \t/* Write what remains */\n \tret = commit_packed_refs();\n-\tif (ret && err)\n+\tif (ret)\n \t\tstrbuf_addf(err, \"unable to overwrite old ref-pack file: %s\",\n \t\t\t strerror(errno));\n \treturn ret;\n@@ -2696,6 +2693,8 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)\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@@ -3551,6 +3550,8 @@ struct ref_transaction {\n \n struct ref_transaction *ref_transaction_begin(struct strbuf *err)\n {\n+\tassert(err);\n+\n \treturn xcalloc(1, sizeof(struct ref_transaction));\n }\n \n@@ -3590,6 +3591,8 @@ int ref_transaction_update(struct ref_transaction *transaction,\n {\n \tstruct ref_update *update;\n \n+\tassert(err);\n+\n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: update called for transaction that is not open\");\n \n@@ -3622,6 +3625,8 @@ int ref_transaction_create(struct ref_transaction *transaction,\n {\n \tstruct ref_update *update;\n \n+\tassert(err);\n+\n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: create called for transaction that is not open\");\n \n@@ -3653,6 +3658,8 @@ int ref_transaction_delete(struct ref_transaction *transaction,\n {\n \tstruct ref_update *update;\n \n+\tassert(err);\n+\n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: delete called for transaction that is not open\");\n \n@@ -3715,13 +3722,14 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \t\t\t\t\tstruct strbuf *err)\n {\n \tint i;\n+\n+\tassert(err);\n+\n \tfor (i = 1; i < n; i++)\n \t\tif (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {\n-\t\t\tconst char *str =\n-\t\t\t\t\"Multiple updates for ref '%s' not allowed.\";\n-\t\t\tif (err)\n-\t\t\t\tstrbuf_addf(err, str, updates[i]->refname);\n-\n+\t\t\tstrbuf_addf(err,\n+\t\t\t\t \"Multiple updates for ref '%s' not allowed.\",\n+\t\t\t\t updates[i]->refname);\n \t\t\treturn 1;\n \t\t}\n \treturn 0;\n@@ -3735,6 +3743,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \tint n = transaction->nr;\n \tstruct ref_update **updates = transaction->updates;\n \n+\tassert(err);\n+\n \tif (transaction->state != REF_TRANSACTION_OPEN)\n \t\tdie(\"BUG: commit called for transaction that is not open\");\n \n@@ -3771,9 +3781,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\tret = (errno == ENOTDIR)\n \t\t\t\t? TRANSACTION_NAME_CONFLICT\n \t\t\t\t: TRANSACTION_GENERIC_ERROR;\n-\t\t\tif (err)\n-\t\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n-\t\t\t\t\t update->refname);\n+\t\t\tstrbuf_addf(err, \"Cannot lock the ref '%s'.\",\n+\t\t\t\t update->refname);\n \t\t\tgoto cleanup;\n \t\t}\n \t}\n@@ -3786,9 +3795,8 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\t\tif (write_ref_sha1(update->lock, update->new_sha1,\n \t\t\t\t\t update->msg)) {\n \t\t\t\tupdate->lock = NULL; /* freed by write_ref_sha1 */\n-\t\t\t\tif (err)\n-\t\t\t\t\tstrbuf_addf(err, \"Cannot update the ref '%s'.\",\n-\t\t\t\t\t\t update->refname);\n+\t\t\t\tstrbuf_addf(err, \"Cannot update the ref '%s'.\",\n+\t\t\t\t\t update->refname);\n \t\t\t\tret = TRANSACTION_GENERIC_ERROR;\n \t\t\t\tgoto cleanup;\n \t\t\t}\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250656","messageId":"20141015005521.GB32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 24/25] lockfile: remove unable_to_lock_error","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:55:21Z","receivedAt":"2014-10-15T00:55:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Thu, 28 Aug 2014 16:41:34 -0700\n\nThe former caller uses unable_to_lock_message now.\n\nSigned-off-by: Jonathan Nieder \nReviewed-by: Ronnie Sahlberg \n---\n lockfile.c | 10 ----------\n lockfile.h | 1 -\n 2 files changed, 11 deletions(-)\n\ndiff --git a/lockfile.c b/lockfile.c\nindex d098ade..4f16ee7 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -162,16 +162,6 @@ void unable_to_lock_message(const char *path, int err, struct strbuf *buf)\n \t\t\t absolute_path(path), strerror(err));\n }\n \n-int unable_to_lock_error(const char *path, int err)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tunable_to_lock_message(path, err, &buf);\n-\terror(\"%s\", buf.buf);\n-\tstrbuf_release(&buf);\n-\treturn -1;\n-}\n-\n NORETURN void unable_to_lock_die(const char *path, int err)\n {\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/lockfile.h b/lockfile.h\nindex dc066d1..cd2ec95 100644\n--- a/lockfile.h\n+++ b/lockfile.h\n@@ -71,7 +71,6 @@ struct lock_file {\n #define LOCK_DIE_ON_ERROR 1\n #define LOCK_NO_DEREF 2\n \n-extern int unable_to_lock_error(const char *path, int err);\n extern void unable_to_lock_message(const char *path, int err,\n \t\t\t\t struct strbuf *buf);\n extern NORETURN void unable_to_lock_die(const char *path, int err);\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250657","messageId":"20141015005540.GC32245@google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"[PATCH 25/25] ref_transaction_commit: bail out on failure to remove a ref","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-10-15T00:55:41Z","receivedAt":"2014-10-15T00:55:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Thu, 28 Aug 2014 17:01:35 -0700\n\nWhen removal of a loose or packed ref fails, bail out instead of\ntrying to finish the transaction. This way, a single error message\ncan be printed (instead of multiple messages being concatenated by\nmistake) and the operator can try to solve the underlying problem\nbefore there is a chance to muck things up even more.\n\nIn particular, when git fails to remove a ref, git goes on to try to\ndelete the reflog. Exiting early lets us keep the reflog.\n\nWhen git succeeds in deleting a ref A and fails to remove a ref B, it\ngoes on to try to delete both reflogs. It would be better to just\nremove the reflog for A, but that would be a more invasive change.\nFailing early means we keep both reflogs, which puts the operator in a\ngood position to understand the problem and recover.\n\nA long term goal is to avoid these problems altogether and roll back\nthe transaction on failure. That kind of transactionality will have\nto wait for a later series (the plan for which is to make all\ndestructive work happen in a single update of the packed-refs file).\n\nSigned-off-by: Jonathan Nieder \nReviewed-by: Ronnie Sahlberg \n---\nThanks for reading.\n\n refs.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 097fb4b..0368ed4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3809,16 +3809,20 @@ int ref_transaction_commit(struct ref_transaction *transaction,\n \t\tstruct ref_update *update = updates[i];\n \n \t\tif (update->lock) {\n-\t\t\tif (delete_ref_loose(update->lock, update->type, err))\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\t}\n \n \t\t\tif (!(update->flags & REF_ISPRUNING))\n \t\t\t\tdelnames[delnum++] = update->lock->ref_name;\n \t\t}\n \t}\n \n-\tif (repack_without_refs(delnames, delnum, err))\n+\tif (repack_without_refs(delnames, delnum, err)) {\n \t\tret = TRANSACTION_GENERIC_ERROR;\n+\t\tgoto cleanup;\n+\t}\n \tfor (i = 0; i < delnum; i++)\n \t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\n \tclear_loose_ref_cache(&ref_cache);\n-- \n2.1.0.rc2.206.gedb03e5\n"},{"id":"250709","messageId":"xmqqk341uiaz.fsf@gitster.dls.corp.google.com","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"Re: [PATCH v23 0/25] rs/ref-transaction (\"Use ref transactions\", part 3)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-15T21:36:20Z","receivedAt":"2014-10-15T21:36:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; queued.\n\nHopefully we can merge to 'next' and go incremental.\n"},{"id":"250782","messageId":"54405457.1040500@alum.mit.edu","threadId":"37729","inReplyTo":"20141015004522.GD32245@google.com","subject":"Re: [PATCH v23 0/25] rs/ref-transaction (\"Use ref transactions\", part 3)","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-10-16T23:27:19Z","receivedAt":"2014-10-16T23:27:19Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 10/15/2014 02:45 AM, Jonathan Nieder wrote:\n> This series by Ronnie was last seen on-list at [1]. It includes some\n> improvements to the ref-transaction API, improves handling of poorly\n> named refs (which should make it easier to tighten the refname format\n> checks in the future), and is a stepping stone toward later series\n> that use the ref-transaction API more and make it support alternate\n> backends (yay!).\n> \n> The changes since (a merge of 'master' and) v22 are very minor and can\n> be seen below[2]. The more important change is that it's rebased\n> against current 'master'.\n> \n> Review comments from Michael and Junio were very helpful in getting\n> this in shape. Thanks much to both.\n> \n> The series can also be found at\n> \n> git://repo.or.cz/git/jrn.git tags/rs/ref-transaction\n> \n> It is based against current 'master' (670a3c1d, 2014-10-14) and\n> intended for 'next'.\n> \n> Thoughts welcome, as always. Improvements preferred in the form of\n> patches on top of the series.\n\nThanks for the new version. I reviewed the previous version pretty\ncarefully in Gerrit and was very happy with the overall series, though\nthere were still some rough spots. It seems to be much more polished now.\n\nGiven that I will be traveling for the next two weeks I probably won't\nbe able to do another thorough review in time. But I just scanned\nthrough patches 01 - 19 and didn't notice any problems. Unfortunately I\nhave to stop there.\n\nCheers,\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\n"}]}