Merge branch 'ab/refs-files-cleanup'

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
This commit is contained in:
Junio C Hamano 2021-10-03 21:49:18 -07:00
commit 842d45d293
8 changed files with 64 additions and 123 deletions

View file

@ -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,

View file

@ -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) {

13
refs.c
View file

@ -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);
}

9
refs.h
View file

@ -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,

View file

@ -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,

View file

@ -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 @@ static int lock_raw_ref(struct files_ref_store *refs,
* 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 @@ static int lock_raw_ref(struct files_ref_store *refs,
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 @@ static int lock_raw_ref(struct files_ref_store *refs,
*/
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;

View file

@ -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;
}

View file

@ -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,