Merge branch 'mh/expire-updateref-fixes'
Various issues around "reflog expire", e.g. using --updateref when expiring a reflog for a symbolic reference, have been corrected and/or made saner. * mh/expire-updateref-fixes: reflog_expire(): never update a reference to null_sha1 reflog_expire(): ignore --updateref for symbolic references reflog: improve and update documentation struct ref_lock: delete the force_write member lock_ref_sha1_basic(): do not set force_write for missing references write_ref_sha1(): move write elision test to callers write_ref_sha1(): remove check for lock == NULL
This commit is contained in:
65
refs.c
65
refs.c
@ -12,7 +12,6 @@ struct ref_lock {
|
||||
struct lock_file *lk;
|
||||
unsigned char old_sha1[20];
|
||||
int lock_fd;
|
||||
int force_write;
|
||||
};
|
||||
|
||||
/*
|
||||
@ -2277,7 +2276,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
|
||||
int type, lflags;
|
||||
int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
|
||||
int resolve_flags = 0;
|
||||
int missing = 0;
|
||||
int attempts_remaining = 3;
|
||||
|
||||
lock = xcalloc(1, sizeof(struct ref_lock));
|
||||
@ -2316,13 +2314,13 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
|
||||
orig_refname, strerror(errno));
|
||||
goto error_return;
|
||||
}
|
||||
missing = is_null_sha1(lock->old_sha1);
|
||||
/* When the ref did not exist and we are creating it,
|
||||
* make sure there is no existing ref that is packed
|
||||
* whose name begins with our refname, nor a ref whose
|
||||
* name is a proper prefix of our refname.
|
||||
/*
|
||||
* If the ref did not exist and we are creating it, make sure
|
||||
* there is no existing packed ref whose name begins with our
|
||||
* refname, nor a packed ref whose name is a proper prefix of
|
||||
* our refname.
|
||||
*/
|
||||
if (missing &&
|
||||
if (is_null_sha1(lock->old_sha1) &&
|
||||
!is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {
|
||||
last_errno = ENOTDIR;
|
||||
goto error_return;
|
||||
@ -2338,10 +2336,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
|
||||
lock->ref_name = xstrdup(refname);
|
||||
lock->orig_ref_name = xstrdup(orig_refname);
|
||||
ref_file = git_path("%s", refname);
|
||||
if (missing)
|
||||
lock->force_write = 1;
|
||||
if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
|
||||
lock->force_write = 1;
|
||||
|
||||
retry:
|
||||
switch (safe_create_leading_directories(ref_file)) {
|
||||
@ -2897,7 +2891,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
|
||||
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);
|
||||
@ -2913,7 +2906,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
|
||||
goto rollbacklog;
|
||||
}
|
||||
|
||||
lock->force_write = 1;
|
||||
flag = log_all_ref_updates;
|
||||
log_all_ref_updates = 0;
|
||||
if (write_ref_sha1(lock, orig_sha1, NULL))
|
||||
@ -3099,14 +3091,6 @@ static int write_ref_sha1(struct ref_lock *lock,
|
||||
static char term = '\n';
|
||||
struct object *o;
|
||||
|
||||
if (!lock) {
|
||||
errno = EINVAL;
|
||||
return -1;
|
||||
}
|
||||
if (!lock->force_write && !hashcmp(lock->old_sha1, sha1)) {
|
||||
unlock_ref(lock);
|
||||
return 0;
|
||||
}
|
||||
o = parse_object(sha1);
|
||||
if (!o) {
|
||||
error("Trying to write ref %s with nonexistent object %s",
|
||||
@ -3851,15 +3835,28 @@ int ref_transaction_commit(struct ref_transaction *transaction,
|
||||
int flags = update->flags;
|
||||
|
||||
if ((flags & REF_HAVE_NEW) && !is_null_sha1(update->new_sha1)) {
|
||||
if (write_ref_sha1(update->lock, update->new_sha1,
|
||||
update->msg)) {
|
||||
int overwriting_symref = ((update->type & REF_ISSYMREF) &&
|
||||
(update->flags & REF_NODEREF));
|
||||
|
||||
if (!overwriting_symref
|
||||
&& !hashcmp(update->lock->old_sha1, update->new_sha1)) {
|
||||
/*
|
||||
* The reference already has the desired
|
||||
* value, so we don't need to write it.
|
||||
*/
|
||||
unlock_ref(update->lock);
|
||||
update->lock = NULL;
|
||||
} else if (write_ref_sha1(update->lock, update->new_sha1,
|
||||
update->msg)) {
|
||||
update->lock = NULL; /* freed by write_ref_sha1 */
|
||||
strbuf_addf(err, "Cannot update the ref '%s'.",
|
||||
update->refname);
|
||||
ret = TRANSACTION_GENERIC_ERROR;
|
||||
goto cleanup;
|
||||
} else {
|
||||
/* freed by write_ref_sha1(): */
|
||||
update->lock = NULL;
|
||||
}
|
||||
update->lock = NULL; /* freed by write_ref_sha1 */
|
||||
}
|
||||
}
|
||||
|
||||
@ -4083,6 +4080,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
|
||||
struct ref_lock *lock;
|
||||
char *log_file;
|
||||
int status = 0;
|
||||
int type;
|
||||
|
||||
memset(&cb, 0, sizeof(cb));
|
||||
cb.flags = flags;
|
||||
@ -4094,7 +4092,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
|
||||
* reference itself, plus we might need to update the
|
||||
* reference if --updateref was specified:
|
||||
*/
|
||||
lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, NULL);
|
||||
lock = lock_ref_sha1_basic(refname, sha1, NULL, 0, &type);
|
||||
if (!lock)
|
||||
return error("cannot lock ref '%s'", refname);
|
||||
if (!reflog_exists(refname)) {
|
||||
@ -4131,10 +4129,21 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
|
||||
(*cleanup_fn)(cb.policy_cb);
|
||||
|
||||
if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
|
||||
/*
|
||||
* It doesn't make sense to adjust a reference pointed
|
||||
* to by a symbolic ref based on expiring entries in
|
||||
* the symbolic reference's reflog. Nor can we update
|
||||
* a reference if there are no remaining reflog
|
||||
* entries.
|
||||
*/
|
||||
int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
|
||||
!(type & REF_ISSYMREF) &&
|
||||
!is_null_sha1(cb.last_kept_sha1);
|
||||
|
||||
if (close_lock_file(&reflog_lock)) {
|
||||
status |= error("couldn't write %s: %s", log_file,
|
||||
strerror(errno));
|
||||
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&
|
||||
} else if (update &&
|
||||
(write_in_full(lock->lock_fd,
|
||||
sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
|
||||
write_str_in_full(lock->lock_fd, "\n") != 1 ||
|
||||
@ -4145,7 +4154,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
|
||||
} else if (commit_lock_file(&reflog_lock)) {
|
||||
status |= error("unable to commit reflog '%s' (%s)",
|
||||
log_file, strerror(errno));
|
||||
} else if ((flags & EXPIRE_REFLOGS_UPDATE_REF) && commit_ref(lock)) {
|
||||
} else if (update && commit_ref(lock)) {
|
||||
status |= error("couldn't set %s", lock->ref_name);
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user