summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2021-09-23 20:45:14 (GMT)
committerJunio C Hamano <gitster@pobox.com>2021-09-23 20:45:14 (GMT)
commit5b0547fd25d66a50bfff2c546b8c36e3641ef65c (patch)
treee72296d05fd80651638443c6fc2e1c52ded03e2c
parentcabc945f299ec9681f2f34f86d613acf66c6b188 (diff)
parent48cdcd9ca0daa7904f299a00433a593a5941a2d3 (diff)
downloadgit-5b0547fd25d66a50bfff2c546b8c36e3641ef65c.zip
git-5b0547fd25d66a50bfff2c546b8c36e3641ef65c.tar.gz
git-5b0547fd25d66a50bfff2c546b8c36e3641ef65c.tar.bz2
Merge branch 'ab/refs-files-cleanup' into jch
Continued work on top of the hn/refs-errno-cleanup topic. * ab/refs-files-cleanup: refs/files: remove unused "errno != ENOTDIR" condition refs/files: remove unused "errno == EISDIR" code refs/files: remove unused "oid" in lock_ref_oid_basic() refs API: remove OID argument to reflog_expire() reflog expire: don't lock reflogs using previously seen OID refs/files: add a comment about refs_reflog_exists() call refs: make repo_dwim_log() accept a NULL oid refs/debug: re-indent argument list for "prepare" refs/files: remove unused "skip" in lock_raw_ref() too refs/files: remove unused "extras/skip" in lock_ref_oid_basic() refs: drop unused "flags" parameter to lock_ref_oid_basic() refs/files: remove unused REF_DELETING in lock_ref_oid_basic() refs/packet: add missing BUG() invocations to reflog callbacks
-rw-r--r--builtin/reflog.c13
-rw-r--r--reflog-walk.c3
-rw-r--r--refs.c13
-rw-r--r--refs.h9
-rw-r--r--refs/debug.c8
-rw-r--r--refs/files-backend.c132
-rw-r--r--refs/packed-backend.c7
-rw-r--r--refs/refs-internal.h2
8 files changed, 64 insertions, 123 deletions
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 09541d1..bd4c669 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -629,8 +629,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
free_worktrees(worktrees);
for (i = 0; i < collected.nr; i++) {
struct collected_reflog *e = collected.e[i];
+
set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);
- status |= reflog_expire(e->reflog, &e->oid, flags,
+ status |= reflog_expire(e->reflog, flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
@@ -642,13 +643,12 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
for (; i < argc; i++) {
char *ref;
- struct object_id oid;
- if (!dwim_log(argv[i], strlen(argv[i]), &oid, &ref)) {
+ if (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) {
status |= error(_("%s points nowhere!"), argv[i]);
continue;
}
set_reflog_expiry_param(&cb.cmd, explicit_expiry, ref);
- status |= reflog_expire(ref, &oid, flags,
+ status |= reflog_expire(ref, flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
@@ -700,7 +700,6 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
for ( ; i < argc; i++) {
const char *spec = strstr(argv[i], "@{");
- struct object_id oid;
char *ep, *ref;
int recno;
@@ -709,7 +708,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
continue;
}
- if (!dwim_log(argv[i], spec - argv[i], &oid, &ref)) {
+ if (!dwim_log(argv[i], spec - argv[i], NULL, &ref)) {
status |= error(_("no reflog for '%s'"), argv[i]);
continue;
}
@@ -724,7 +723,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
cb.cmd.expire_total = 0;
}
- status |= reflog_expire(ref, &oid, flags,
+ status |= reflog_expire(ref, flags,
reflog_expiry_prepare,
should_expire_reflog_ent,
reflog_expiry_cleanup,
diff --git a/reflog-walk.c b/reflog-walk.c
index e9cd328..8ac4b28 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -158,10 +158,9 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
}
reflogs = read_complete_reflog(branch);
if (!reflogs || reflogs->nr == 0) {
- struct object_id oid;
char *b;
int ret = dwim_log(branch, strlen(branch),
- &oid, &b);
+ NULL, &b);
if (ret > 1)
free(b);
else if (ret == 1) {
diff --git a/refs.c b/refs.c
index 8b9f7c3..05944d8 100644
--- a/refs.c
+++ b/refs.c
@@ -698,7 +698,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
strbuf_addf(&path, *p, len, str);
ref = refs_resolve_ref_unsafe(refs, path.buf,
RESOLVE_REF_READING,
- &hash, NULL);
+ oid ? &hash : NULL, NULL);
if (!ref)
continue;
if (refs_reflog_exists(refs, path.buf))
@@ -710,7 +710,8 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
continue;
if (!logs_found++) {
*log = xstrdup(it);
- oidcpy(oid, &hash);
+ if (oid)
+ oidcpy(oid, &hash);
}
if (!warn_ambiguous_refs)
break;
@@ -2370,19 +2371,19 @@ int delete_reflog(const char *refname)
}
int refs_reflog_expire(struct ref_store *refs,
- const char *refname, const struct object_id *oid,
+ const char *refname,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,
reflog_expiry_cleanup_fn cleanup_fn,
void *policy_cb_data)
{
- return refs->be->reflog_expire(refs, refname, oid, flags,
+ return refs->be->reflog_expire(refs, refname, flags,
prepare_fn, should_prune_fn,
cleanup_fn, policy_cb_data);
}
-int reflog_expire(const char *refname, const struct object_id *oid,
+int reflog_expire(const char *refname,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,
@@ -2390,7 +2391,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
void *policy_cb_data)
{
return refs_reflog_expire(get_main_ref_store(the_repository),
- refname, oid, flags,
+ refname, flags,
prepare_fn, should_prune_fn,
cleanup_fn, policy_cb_data);
}
diff --git a/refs.h b/refs.h
index 48970df..fda8aef 100644
--- a/refs.h
+++ b/refs.h
@@ -796,7 +796,7 @@ enum expire_reflog_flags {
* expiration policy that is desired.
*
* reflog_expiry_prepare_fn -- Called once after the reference is
- * locked.
+ * locked. Called with the OID of the locked reference.
*
* reflog_expiry_should_prune_fn -- Called once for each entry in the
* existing reflog. It should return true iff that entry should be
@@ -816,20 +816,19 @@ typedef int reflog_expiry_should_prune_fn(struct object_id *ooid,
typedef void reflog_expiry_cleanup_fn(void *cb_data);
/*
- * Expire reflog entries for the specified reference. oid is the old
- * value of the reference. flags is a combination of the constants in
+ * Expire reflog entries for the specified reference.
+ * flags is a combination of the constants in
* enum expire_reflog_flags. The three function pointers are described
* above. On success, return zero.
*/
int refs_reflog_expire(struct ref_store *refs,
const char *refname,
- const struct object_id *oid,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,
reflog_expiry_cleanup_fn cleanup_fn,
void *policy_cb_data);
-int reflog_expire(const char *refname, const struct object_id *oid,
+int reflog_expire(const char *refname,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,
diff --git a/refs/debug.c b/refs/debug.c
index 1a7a9e1..bf4a82b 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -365,8 +365,8 @@ struct debug_reflog_expiry_should_prune {
};
static void debug_reflog_expiry_prepare(const char *refname,
- const struct object_id *oid,
- void *cb_data)
+ const struct object_id *oid,
+ void *cb_data)
{
struct debug_reflog_expiry_should_prune *prune = cb_data;
trace_printf_key(&trace_refs, "reflog_expire_prepare: %s\n", refname);
@@ -392,7 +392,7 @@ static void debug_reflog_expiry_cleanup(void *cb_data)
}
static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,
- const struct object_id *oid, unsigned int flags,
+ unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,
reflog_expiry_cleanup_fn cleanup_fn,
@@ -405,7 +405,7 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,
.should_prune = should_prune_fn,
.cb_data = policy_cb_data,
};
- int res = drefs->refs->be->reflog_expire(drefs->refs, refname, oid,
+ int res = drefs->refs->be->reflog_expire(drefs->refs, refname,
flags, &debug_reflog_expiry_prepare,
&debug_reflog_expiry_should_prune_fn,
&debug_reflog_expiry_cleanup,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 74c0385..2c8cac2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -531,7 +531,6 @@ static void unlock_ref(struct ref_lock *lock)
static int lock_raw_ref(struct files_ref_store *refs,
const char *refname, int mustexist,
const struct string_list *extras,
- const struct string_list *skip,
struct ref_lock **lock_p,
struct strbuf *referent,
unsigned int *type,
@@ -568,7 +567,7 @@ retry:
* reason to expect this error to be transitory.
*/
if (refs_verify_refname_available(&refs->base, refname,
- extras, skip, err)) {
+ extras, NULL, err)) {
if (mustexist) {
/*
* To the user the relevant error is
@@ -673,7 +672,7 @@ retry:
REMOVE_DIR_EMPTY_ONLY)) {
if (refs_verify_refname_available(
&refs->base, refname,
- extras, skip, err)) {
+ extras, NULL, err)) {
/*
* The error message set by
* verify_refname_available() is OK.
@@ -710,7 +709,7 @@ retry:
*/
if (refs_verify_refname_available(
refs->packed_ref_store, refname,
- extras, skip, err))
+ extras, NULL, err))
goto error_return;
}
@@ -853,42 +852,6 @@ static struct ref_iterator *files_ref_iterator_begin(
return ref_iterator;
}
-/*
- * Verify that the reference locked by lock has the value old_oid
- * (unless it is NULL). Fail if the reference doesn't exist and
- * mustexist is set. Return 0 on success. On error, write an error
- * message to err, set errno, and return a negative value.
- */
-static int verify_lock(struct ref_store *ref_store, struct ref_lock *lock,
- const struct object_id *old_oid, int mustexist,
- struct strbuf *err)
-{
- assert(err);
-
- if (refs_read_ref_full(ref_store, lock->ref_name,
- mustexist ? RESOLVE_REF_READING : 0,
- &lock->old_oid, NULL)) {
- if (old_oid) {
- int save_errno = errno;
- strbuf_addf(err, "can't verify ref '%s'", lock->ref_name);
- errno = save_errno;
- return -1;
- } else {
- oidclr(&lock->old_oid);
- return 0;
- }
- }
- if (old_oid && !oideq(&lock->old_oid, old_oid)) {
- strbuf_addf(err, "ref '%s' is at %s but expected %s",
- lock->ref_name,
- oid_to_hex(&lock->old_oid),
- oid_to_hex(old_oid));
- errno = EBUSY;
- return -1;
- }
- return 0;
-}
-
static int remove_empty_directories(struct strbuf *path)
{
/*
@@ -913,59 +876,25 @@ static int create_reflock(const char *path, void *cb)
* On failure errno is set to something meaningful.
*/
static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
- const char *refname,
- const struct object_id *old_oid,
- const struct string_list *extras,
- const struct string_list *skip,
- unsigned int flags, int *type,
+ const char *refname, int *type,
struct strbuf *err)
{
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
int last_errno = 0;
- int mustexist = (old_oid && !is_null_oid(old_oid));
- int resolve_flags = RESOLVE_REF_NO_RECURSE;
- int resolved;
files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
CALLOC_ARRAY(lock, 1);
- if (mustexist)
- resolve_flags |= RESOLVE_REF_READING;
- if (flags & REF_DELETING)
- resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;
-
files_ref_path(refs, &ref_file, refname);
- resolved = !!refs_resolve_ref_unsafe(&refs->base,
- refname, resolve_flags,
- &lock->old_oid, type);
- if (!resolved && errno == EISDIR) {
- /*
- * we are trying to lock foo but we used to
- * have foo/bar which now does not exist;
- * it is normal for the empty directory 'foo'
- * to remain.
- */
- if (remove_empty_directories(&ref_file)) {
- last_errno = errno;
- if (!refs_verify_refname_available(
- &refs->base,
- refname, extras, skip, err))
- strbuf_addf(err, "there are still refs under '%s'",
- refname);
- goto error_return;
- }
- resolved = !!refs_resolve_ref_unsafe(&refs->base,
- refname, resolve_flags,
- &lock->old_oid, type);
- }
- if (!resolved) {
+ if (!refs_resolve_ref_unsafe(&refs->base, refname,
+ RESOLVE_REF_NO_RECURSE,
+ &lock->old_oid, type)) {
last_errno = errno;
- if (last_errno != ENOTDIR ||
- !refs_verify_refname_available(&refs->base, refname,
- extras, skip, err))
+ if (!refs_verify_refname_available(&refs->base, refname,
+ NULL, NULL, err))
strbuf_addf(err, "unable to resolve reference '%s': %s",
refname, strerror(last_errno));
@@ -980,7 +909,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
*/
if (is_null_oid(&lock->old_oid) &&
refs_verify_refname_available(refs->packed_ref_store, refname,
- extras, skip, err)) {
+ NULL, NULL, err)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -993,10 +922,10 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
goto error_return;
}
- if (verify_lock(&refs->base, lock, old_oid, mustexist, err)) {
- last_errno = errno;
- goto error_return;
- }
+ if (refs_read_ref_full(&refs->base, lock->ref_name,
+ 0,
+ &lock->old_oid, NULL))
+ oidclr(&lock->old_oid);
goto out;
error_return:
@@ -1415,8 +1344,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
logmoved = log;
- lock = lock_ref_oid_basic(refs, newrefname, NULL, NULL, NULL,
- REF_NO_DEREF, NULL, &err);
+ lock = lock_ref_oid_basic(refs, newrefname, NULL, &err);
if (!lock) {
if (copy)
error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf);
@@ -1438,8 +1366,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
goto out;
rollback:
- lock = lock_ref_oid_basic(refs, oldrefname, NULL, NULL, NULL,
- REF_NO_DEREF, NULL, &err);
+ lock = lock_ref_oid_basic(refs, oldrefname, NULL, &err);
if (!lock) {
error("unable to lock %s for rollback: %s", oldrefname, err.buf);
strbuf_release(&err);
@@ -1846,9 +1773,7 @@ static int files_create_symref(struct ref_store *ref_store,
struct ref_lock *lock;
int ret;
- lock = lock_ref_oid_basic(refs, refname, NULL,
- NULL, NULL, REF_NO_DEREF, NULL,
- &err);
+ lock = lock_ref_oid_basic(refs, refname, NULL, &err);
if (!lock) {
error("%s", err.buf);
strbuf_release(&err);
@@ -2416,7 +2341,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
}
ret = lock_raw_ref(refs, update->refname, mustexist,
- affected_refnames, NULL,
+ affected_refnames,
&lock, &referent,
&update->type, err);
if (ret) {
@@ -3037,7 +2962,7 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
}
static int files_reflog_expire(struct ref_store *ref_store,
- const char *refname, const struct object_id *oid,
+ const char *refname,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,
@@ -3054,6 +2979,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
int status = 0;
int type;
struct strbuf err = STRBUF_INIT;
+ const struct object_id *oid;
memset(&cb, 0, sizeof(cb));
cb.flags = flags;
@@ -3065,14 +2991,26 @@ static int files_reflog_expire(struct ref_store *ref_store,
* reference itself, plus we might need to update the
* reference if --updateref was specified:
*/
- lock = lock_ref_oid_basic(refs, refname, oid,
- NULL, NULL, REF_NO_DEREF,
- &type, &err);
+ lock = lock_ref_oid_basic(refs, refname, &type, &err);
if (!lock) {
error("cannot lock ref '%s': %s", refname, err.buf);
strbuf_release(&err);
return -1;
}
+ oid = &lock->old_oid;
+
+ /*
+ * When refs are deleted, their reflog is deleted before the
+ * ref itself is deleted. This is because there is no separate
+ * lock for reflog; instead we take a lock on the ref with
+ * lock_ref_oid_basic().
+ *
+ * If a race happens and the reflog doesn't exist after we've
+ * acquired the lock that's OK. We've got nothing more to do;
+ * We were asked to delete the reflog, but someone else
+ * deleted it! The caller doesn't care that we deleted it,
+ * just that it is deleted. So we can return successfully.
+ */
if (!refs_reflog_exists(ref_store, refname)) {
unlock_ref(lock);
return 0;
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index f8aa97d..65ba421 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1600,6 +1600,7 @@ static int packed_for_each_reflog_ent(struct ref_store *ref_store,
const char *refname,
each_reflog_ent_fn fn, void *cb_data)
{
+ BUG("packed reference store does not support reflogs");
return 0;
}
@@ -1608,12 +1609,14 @@ static int packed_for_each_reflog_ent_reverse(struct ref_store *ref_store,
each_reflog_ent_fn fn,
void *cb_data)
{
+ BUG("packed reference store does not support reflogs");
return 0;
}
static int packed_reflog_exists(struct ref_store *ref_store,
const char *refname)
{
+ BUG("packed reference store does not support reflogs");
return 0;
}
@@ -1627,17 +1630,19 @@ static int packed_create_reflog(struct ref_store *ref_store,
static int packed_delete_reflog(struct ref_store *ref_store,
const char *refname)
{
+ BUG("packed reference store does not support reflogs");
return 0;
}
static int packed_reflog_expire(struct ref_store *ref_store,
- const char *refname, const struct object_id *oid,
+ const char *refname,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,
reflog_expiry_cleanup_fn cleanup_fn,
void *policy_cb_data)
{
+ BUG("packed reference store does not support reflogs");
return 0;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 3155708..d496c5e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -593,7 +593,7 @@ typedef int create_reflog_fn(struct ref_store *ref_store, const char *refname,
int force_create, struct strbuf *err);
typedef int delete_reflog_fn(struct ref_store *ref_store, const char *refname);
typedef int reflog_expire_fn(struct ref_store *ref_store,
- const char *refname, const struct object_id *oid,
+ const char *refname,
unsigned int flags,
reflog_expiry_prepare_fn prepare_fn,
reflog_expiry_should_prune_fn should_prune_fn,