commit 0295e9cebc41020ee84da275549b164a8770ffba upstream. Change refs.c to use a single transaction to copy/rename both the refs and its reflog. Since we are no longer using rename() to move the reflog file we no longer need to disallow rename_ref for refs with a symlink for its reflog so we can remove that test from the testsuite. Change the function to return 1 on failure instead of either -1 or 1. These changes make the rename_ref operation atomic. This also eliminates the need to use rename() to shift the reflog around via a temporary filename. As an extension to this, since we no longer use rename() on the reflog file, we can now safely perform renames even if the reflog is a symbolic link and thus can remove the check and fail for that condition. Change-Id: I59477d410a34298a29cf0cbf32328b9053b158fe Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> --- refs.c | 192 ++++++++++++++++++++---------------------------------- t/t3200-branch.sh | 7 -- 2 files changed, 70 insertions(+), 129 deletions(-) diff --git a/refs.c b/refs.c index b9c8f91..f43fef4 100644 --- a/refs.c +++ b/refs.c @@ -2752,58 +2752,26 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) return 0; } -/* - * People using contrib's git-new-workdir have .git/logs/refs -> - * /some/other/path/.git/logs/refs, and that may live on another device. - * - * IOW, to avoid cross device rename errors, the temporary renamed log must - * live into logs/refs. - */ -#define TMP_RENAMED_LOG "logs/refs/.tmp-renamed-log" +struct rename_reflog_cb { + struct transaction *transaction; + const char *refname; + struct strbuf *err; +}; -static int rename_tmp_log(const char *newrefname) +static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1, + const char *id, unsigned long timestamp, int tz, + const char *message, void *cb_data) { - int attempts_remaining = 4; + struct rename_reflog_cb *cb = cb_data; + struct reflog_committer_info ci; - retry: - switch (safe_create_leading_directories(git_path("logs/%s", newrefname))) { - case SCLD_OK: - break; /* success */ - case SCLD_VANISHED: - if (--attempts_remaining > 0) - goto retry; - /* fall through */ - default: - error("unable to create directory for %s", newrefname); - return -1; - } - - if (rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newrefname))) { - if ((errno==EISDIR || errno==ENOTDIR) && --attempts_remaining > 0) { - /* - * rename(a, b) when b is an existing - * directory ought to result in ISDIR, but - * Solaris 5.8 gives ENOTDIR. Sheesh. - */ - if (remove_empty_directories(git_path("logs/%s", newrefname))) { - error("Directory not empty: logs/%s", newrefname); - return -1; - } - goto retry; - } else if (errno == ENOENT && --attempts_remaining > 0) { - /* - * Maybe another process just deleted one of - * the directories in the path to newrefname. - * Try again from the beginning. - */ - goto retry; - } else { - error("unable to move logfile "TMP_RENAMED_LOG" to logs/%s: %s", - newrefname, strerror(errno)); - return -1; - } - } - return 0; + memset(&ci, 0, sizeof(ci)); + ci.id = id; + ci.timestamp = timestamp; + ci.tz = tz; + return transaction_update_reflog(cb->transaction, cb->refname, + nsha1, osha1, &ci, message, 0, + cb->err); } static int rename_ref_available(const char *oldname, const char *newname) @@ -2823,91 +2791,71 @@ static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { - unsigned char sha1[20], orig_sha1[20]; - int flag = 0, logmoved = 0; - struct ref_lock *lock; - struct stat loginfo; - int log = !lstat(git_path("logs/%s", oldrefname), &loginfo); + unsigned char sha1[20]; + int flag = 0, log; + struct transaction *transaction = NULL; + struct strbuf err = STRBUF_INIT; const char *symref = NULL; + struct rename_reflog_cb cb; + struct reflog_committer_info ci; - if (log && S_ISLNK(loginfo.st_mode)) - return error("reflog for %s is a symlink", oldrefname); + memset(&ci, 0, sizeof(ci)); + ci.committer_info = git_committer_info(0); symref = resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING, - orig_sha1, &flag); - if (flag & REF_ISSYMREF) - return error("refname %s is a symbolic ref, renaming it is not supported", - oldrefname); - if (!symref) - return error("refname %s not found", oldrefname); - - if (!rename_ref_available(oldrefname, newrefname)) + sha1, &flag); + if (flag & REF_ISSYMREF) { + error("refname %s is a symbolic ref, renaming it is not " + "supported", oldrefname); return 1; - - if (log && rename(git_path("logs/%s", oldrefname), git_path(TMP_RENAMED_LOG))) - return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s", - oldrefname, strerror(errno)); - - if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) { - error("unable to delete old %s", oldrefname); - goto rollback; } - - if (!read_ref_full(newrefname, RESOLVE_REF_READING, sha1, NULL) && - delete_ref(newrefname, sha1, REF_NODEREF)) { - if (errno==EISDIR) { - if (remove_empty_directories(git_path("%s", newrefname))) { - error("Directory not empty: %s", newrefname); - goto rollback; - } - } else { - error("unable to delete existing %s", newrefname); - goto rollback; - } + if (!symref) { + error("refname %s not found", oldrefname); + return 1; } - if (log && rename_tmp_log(newrefname)) - goto rollback; - - logmoved = log; + if (!rename_ref_available(oldrefname, newrefname)) + return 1; - lock = lock_ref_sha1_basic(newrefname, NULL, NULL, 0, NULL); - if (!lock) { - error("unable to lock %s for update", newrefname); - goto rollback; - } - lock->force_write = 1; - hashcpy(lock->old_sha1, orig_sha1); - if (write_ref_sha1(lock, orig_sha1, logmsg)) { - error("unable to write current sha1 into %s", newrefname); - goto rollback; + log = reflog_exists(oldrefname); + transaction = transaction_begin(&err); + if (!transaction) + goto fail; + + if (strcmp(oldrefname, newrefname)) { + if (log && transaction_update_reflog(transaction, newrefname, + sha1, sha1, &ci, NULL, + REFLOG_TRUNCATE, &err)) + goto fail; + cb.transaction = transaction; + cb.refname = newrefname; + cb.err = &err; + if (log && for_each_reflog_ent(oldrefname, rename_reflog_ent, + &cb)) + goto fail; + + if (transaction_delete_ref(transaction, oldrefname, sha1, + REF_NODEREF, + 1, NULL, &err)) + goto fail; } - + if (transaction_update_ref(transaction, newrefname, sha1, + NULL, 0, 0, NULL, &err)) + goto fail; + if (log && transaction_update_reflog(transaction, newrefname, sha1, + sha1, &ci, logmsg, + REFLOG_COMMITTER_INFO_IS_VALID, + &err)) + goto fail; + if (transaction_commit(transaction, &err)) + goto fail; + transaction_free(transaction); return 0; - rollback: - lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, 0, NULL); - if (!lock) { - error("unable to lock %s for rollback", oldrefname); - goto rollbacklog; - } - - lock->force_write = 1; - flag = log_all_ref_updates; - log_all_ref_updates = 0; - if (write_ref_sha1(lock, orig_sha1, NULL)) - error("unable to write current sha1 into %s", oldrefname); - log_all_ref_updates = flag; - - rollbacklog: - if (logmoved && rename(git_path("logs/%s", newrefname), git_path("logs/%s", oldrefname))) - error("unable to restore logfile %s from %s: %s", - oldrefname, newrefname, strerror(errno)); - if (!logmoved && log && - rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldrefname))) - error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s", - oldrefname, strerror(errno)); - + fail: + error("rename_ref failed: %s", err.buf); + strbuf_release(&err); + transaction_free(transaction); return 1; } diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 432921b..c6c53e4 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -302,13 +302,6 @@ test_expect_success 'renaming a symref is not allowed' ' test_path_is_missing .git/refs/heads/master3 ' -test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' ' - git branch -l u && - mv .git/logs/refs/heads/u real-u && - ln -s real-u .git/logs/refs/heads/u && - test_must_fail git branch -m u v -' - test_expect_success 'test tracking setup via --track' ' git config remote.local.url . && git config remote.local.fetch refs/heads/*:refs/remotes/local/* && -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html