development/git
development/git
1
0
Fork 0
mirror of https://github.com/git/git synced 2024-07-17 02:57:48 +00:00
Code Issues Actions 9 Packages Projects Releases Wiki Activity

Merge branch 'mh/refs-have-new'

Browse source 
Simplify the ref transaction API around how "the ref should be
pointing at this object" is specified.

* mh/refs-have-new:
  refs.h: remove duplication in function docstrings
  update_ref(): improve documentation
  ref_transaction_verify(): new function to check a reference's value
  ref_transaction_delete(): check that old_sha1 is not null_sha1
  ref_transaction_create(): check that new_sha1 is valid
  commit: avoid race when creating orphan commits
  commit: add tests of commit races
  ref_transaction_delete(): remove "have_old" parameter
  ref_transaction_update(): remove "have_old" parameter
  struct ref_update: move "have_old" into "flags"
  refs.c: change some "flags" to "unsigned int"
  refs: remove the gap in the REF_* constant values
  refs: move REF_DELETING to refs.c
This commit is contained in:
Junio C Hamano 2015-03-05 12:45:39 -08:00
parent 97c12a8b71 d1dd721f11
commit fd9de868c3
14 changed files with 233 additions and 106 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
branch.c
View file

@ -284,8 +284,9 @@ void create_branch(const char *head,
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, sha1,
null_sha1, 0, !forcing, msg, &err) ||
ref_transaction_update(transaction, ref.buf,
sha1, forcing ? NULL : null_sha1,
0, msg, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);

4
builtin/commit.c
View file

@ -1766,8 +1766,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!transaction ||
ref_transaction_update(transaction, "HEAD", sha1,
current_head
? current_head->object.sha1 : NULL,
0, !!current_head, sb.buf, &err) ||
? current_head->object.sha1 : null_sha1,
0, sb.buf, &err) ||
ref_transaction_commit(transaction, &err)) {
rollback_index_files();
die("%s", err.buf);

6
builtin/fetch.c
View file

@ -415,8 +415,10 @@ static int s_update_ref(const char *action,
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref->name, ref->new_sha1,
ref->old_sha1, 0, check_old, msg, &err))
ref_transaction_update(transaction, ref->name,
ref->new_sha1,
check_old ? ref->old_sha1 : NULL,
0, msg, &err))
goto fail;
ret = ref_transaction_commit(transaction, &err);

5
builtin/receive-pack.c
View file

@ -953,8 +953,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
if (ref_transaction_delete(transaction,
namespaced_name,
old_sha1,
0, old_sha1 != NULL,
"push", &err)) {
0, "push", &err)) {
rp_error("%s", err.buf);
strbuf_release(&err);
return "failed to delete";
@ -971,7 +970,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
if (ref_transaction_update(transaction,
namespaced_name,
new_sha1, old_sha1,
0, 1, "push",
0, "push",
&err)) {
rp_error("%s", err.buf);
strbuf_release(&err);

2
builtin/replace.c
View file

@ -172,7 +172,7 @@ static int replace_object_sha1(const char *object_ref,
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref, repl, prev,
0, 1, NULL, &err) ||
0, NULL, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);

2
builtin/tag.c
View file

@ -733,7 +733,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
0, 1, NULL, &err) ||
0, NULL, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);

20
builtin/update-ref.c
View file

@ -198,8 +198,9 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,
if (*next != line_termination)
die("update %s: extra input: %s", refname, next);
if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
update_flags, have_old, msg, &err))
if (ref_transaction_update(transaction, refname,
new_sha1, have_old ? old_sha1 : NULL,
update_flags, msg, &err))
die("%s", err.buf);
update_flags = 0;
@ -264,8 +265,9 @@ static const char *parse_cmd_delete(struct ref_transaction *transaction,
if (*next != line_termination)
die("delete %s: extra input: %s", refname, next);
if (ref_transaction_delete(transaction, refname, old_sha1,
update_flags, have_old, msg, &err))
if (ref_transaction_delete(transaction, refname,
have_old ? old_sha1 : NULL,
update_flags, msg, &err))
die("%s", err.buf);
update_flags = 0;
@ -280,7 +282,6 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,
{
struct strbuf err = STRBUF_INIT;
char *refname;
unsigned char new_sha1[20];
unsigned char old_sha1[20];
refname = parse_refname(input, &next);
@ -291,13 +292,11 @@ static const char *parse_cmd_verify(struct ref_transaction *transaction,
PARSE_SHA1_OLD))
hashclr(old_sha1);
hashcpy(new_sha1, old_sha1);
if (*next != line_termination)
die("verify %s: extra input: %s", refname, next);
if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
update_flags, 1, msg, &err))
if (ref_transaction_verify(transaction, refname, old_sha1,
update_flags, &err))
die("%s", err.buf);
update_flags = 0;
@ -353,7 +352,8 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
{
const char *refname, *oldval;
unsigned char sha1[20], oldsha1[20];
int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;
int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0;
unsigned int flags = 0;
struct option options[] = {
OPT_STRING( 'm', NULL, &msg, N_("reason"), N_("reason of the update")),
OPT_BOOL('d', NULL, &delete, N_("delete the reference")),

2
cache.h
View file

@ -568,7 +568,7 @@ extern void update_index_if_able(struct index_state *, struct lock_file *);
extern int hold_locked_index(struct lock_file *, int);
extern void set_alternate_index_output(const char *);
extern int delete_ref(const char *, const unsigned char *sha1, int delopt);
extern int delete_ref(const char *, const unsigned char *sha1, unsigned int flags);
/* Environment bits from configuration mechanism */
extern int trust_executable_bit;

6
fast-import.c
View file

@ -1720,7 +1720,7 @@ static int update_branch(struct branch *b)
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, b->name, b->sha1, old_sha1,
0, 1, msg, &err) ||
0, msg, &err) ||
ref_transaction_commit(transaction, &err)) {
ref_transaction_free(transaction);
error("%s", err.buf);
@ -1760,8 +1760,8 @@ static void dump_tags(void)
strbuf_reset(&ref_name);
strbuf_addf(&ref_name, "refs/tags/%s", t->name);
if (ref_transaction_update(transaction, ref_name.buf, t->sha1,
NULL, 0, 0, msg, &err)) {
if (ref_transaction_update(transaction, ref_name.buf,
t->sha1, NULL, 0, msg, &err)) {
failure |= error("%s", err.buf);
goto cleanup;
}

140
refs.c
View file

@ -35,10 +35,29 @@ static unsigned char refname_disposition[256] = {
};
/*
* Used as a flag to ref_transaction_delete when a loose ref is being
* Flag passed to lock_ref_sha1_basic() telling it to tolerate broken
* refs (i.e., because the reference is about to be deleted anyway).
*/
#define REF_DELETING 0x02
/*
* Used as a flag in ref_update::flags when a loose ref is being
* pruned.
*/
#define REF_ISPRUNING 0x0100
#define REF_ISPRUNING 0x04
/*
* Used as a flag in ref_update::flags when the reference should be
* updated to new_sha1.
*/
#define REF_HAVE_NEW 0x08
/*
* Used as a flag in ref_update::flags when old_sha1 should be
* checked.
*/
#define REF_HAVE_OLD 0x10
/*
* Try to read one refname component from the front of refname.
* Return the length of the component found, or -1 if the component is
@ -2249,7 +2268,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
const struct string_list *skip,
int flags, int *type_p)
unsigned int flags, int *type_p)
{
char *ref_file;
const char *orig_refname = refname;
@ -2563,7 +2582,7 @@ static void prune_ref(struct ref_to_prune *r)
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_delete(transaction, r->name, r->sha1,
REF_ISPRUNING, 1, NULL, &err) ||
REF_ISPRUNING, NULL, &err) ||
ref_transaction_commit(transaction, &err)) {
ref_transaction_free(transaction);
error("%s", err.buf);
@ -2733,15 +2752,16 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)
return 0;
}
int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
int delete_ref(const char *refname, const unsigned char *sha1, unsigned int flags)
{
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_delete(transaction, refname, sha1, delopt,
sha1 && !is_null_sha1(sha1), NULL, &err) ||
ref_transaction_delete(transaction, refname,
(sha1 && !is_null_sha1(sha1)) ? sha1 : NULL,
flags, NULL, &err) ||
ref_transaction_commit(transaction, &err)) {
error("%s", err.buf);
ref_transaction_free(transaction);
@ -3556,16 +3576,27 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
}
/**
* Information needed for a single ref update. Set new_sha1 to the
* new value or to zero to delete the ref. To check the old value
* while locking the ref, set have_old to 1 and set old_sha1 to the
* value or to zero to ensure the ref does not exist before update.
* Information needed for a single ref update. Set new_sha1 to the new
* value or to null_sha1 to delete the ref. To check the old value
* while the ref is locked, set (flags & REF_HAVE_OLD) and set
* old_sha1 to the old value, or to null_sha1 to ensure the ref does
* not exist before update.
*/
struct ref_update {
/*
* If (flags & REF_HAVE_NEW), set the reference to this value:
*/
unsigned char new_sha1[20];
/*
* If (flags & REF_HAVE_OLD), check that the reference
* previously had this value:
*/
unsigned char old_sha1[20];
int flags; /* REF_NODEREF? */
int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
/*
* One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,
* REF_DELETING, and REF_ISPRUNING:
*/
unsigned int flags;
struct ref_lock *lock;
int type;
char *msg;
@ -3637,7 +3668,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
int flags, int have_old, const char *msg,
unsigned int flags, const char *msg,
struct strbuf *err)
{
struct ref_update *update;
@ -3647,10 +3678,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
if (transaction->state != REF_TRANSACTION_OPEN)
die("BUG: update called for transaction that is not open");
if (have_old && !old_sha1)
die("BUG: have_old is true but old_sha1 is NULL");
if (!is_null_sha1(new_sha1) &&
if (new_sha1 && !is_null_sha1(new_sha1) &&
check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
strbuf_addf(err, "refusing to update ref with bad name %s",
refname);
@ -3658,11 +3686,15 @@ int ref_transaction_update(struct ref_transaction *transaction,
}
update = add_update(transaction, refname);
hashcpy(update->new_sha1, new_sha1);
update->flags = flags;
update->have_old = have_old;
if (have_old)
if (new_sha1) {
hashcpy(update->new_sha1, new_sha1);
flags |= REF_HAVE_NEW;
}
if (old_sha1) {
hashcpy(update->old_sha1, old_sha1);
flags |= REF_HAVE_OLD;
}
update->flags = flags;
if (msg)
update->msg = xstrdup(msg);
return 0;
@ -3671,34 +3703,52 @@ int ref_transaction_update(struct ref_transaction *transaction,
int ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
int flags, const char *msg,
unsigned int flags, const char *msg,
struct strbuf *err)
{
if (!new_sha1 || is_null_sha1(new_sha1))
die("BUG: create called without valid new_sha1");
return ref_transaction_update(transaction, refname, new_sha1,
null_sha1, flags, 1, msg, err);
null_sha1, flags, msg, err);
}
int ref_transaction_delete(struct ref_transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
int flags, int have_old, const char *msg,
unsigned int flags, const char *msg,
struct strbuf *err)
{
return ref_transaction_update(transaction, refname, null_sha1,
old_sha1, flags, have_old, msg, err);
if (old_sha1 && is_null_sha1(old_sha1))
die("BUG: delete called with old_sha1 set to zeros");
return ref_transaction_update(transaction, refname,
null_sha1, old_sha1,
flags, msg, err);
}
int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr)
int ref_transaction_verify(struct ref_transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
unsigned int flags,
struct strbuf *err)
{
if (!old_sha1)
die("BUG: verify called with old_sha1 set to NULL");
return ref_transaction_update(transaction, refname,
NULL, old_sha1,
flags, NULL, err);
}
int update_ref(const char *msg, const char *refname,
const unsigned char *new_sha1, const unsigned char *old_sha1,
unsigned int flags, enum action_on_err onerr)
{
struct ref_transaction *t;
struct strbuf err = STRBUF_INIT;
t = ref_transaction_begin(&err);
if (!t ||
ref_transaction_update(t, refname, sha1, oldval, flags,
!!oldval, action, &err) ||
ref_transaction_update(t, refname, new_sha1, old_sha1,
flags, msg, &err) ||
ref_transaction_commit(t, &err)) {
const char *str = "update_ref failed for ref '%s': %s";
@ -3774,17 +3824,17 @@ int ref_transaction_commit(struct ref_transaction *transaction,
/* Acquire all locks while verifying old values */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
int flags = update->flags;
unsigned int flags = update->flags;
if (is_null_sha1(update->new_sha1))
if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
flags |= REF_DELETING;
update->lock = lock_ref_sha1_basic(update->refname,
(update->have_old ?
update->old_sha1 :
NULL),
NULL,
flags,
&update->type);
update->lock = lock_ref_sha1_basic(
update->refname,
((update->flags & REF_HAVE_OLD) ?
update->old_sha1 : NULL),
NULL,
flags,
&update->type);
if (!update->lock) {
ret = (errno == ENOTDIR)
? TRANSACTION_NAME_CONFLICT
@ -3798,8 +3848,9 @@ int ref_transaction_commit(struct ref_transaction *transaction,
/* Perform updates first so live commits remain referenced */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
int flags = update->flags;
if (!is_null_sha1(update->new_sha1)) {
if ((flags & REF_HAVE_NEW) && !is_null_sha1(update->new_sha1)) {
if (write_ref_sha1(update->lock, update->new_sha1,
update->msg)) {
update->lock = NULL; /* freed by write_ref_sha1 */
@ -3815,14 +3866,15 @@ int ref_transaction_commit(struct ref_transaction *transaction,
/* Perform deletes now that updates are safely completed */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
int flags = update->flags;
if (update->lock) {
if ((flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1)) {
if (delete_ref_loose(update->lock, update->type, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
if (!(update->flags & REF_ISPRUNING))
if (!(flags & REF_ISPRUNING))
string_list_append(&refs_to_delete,
update->lock->ref_name);
}

113
refs.h
View file

@ -183,12 +183,10 @@ extern int peel_ref(const char *refname, unsigned char *sha1);
* Flags controlling ref_transaction_update(), ref_transaction_create(), etc.
* REF_NODEREF: act on the ref directly, instead of dereferencing
* symbolic references.
* REF_DELETING: tolerate broken refs
*
* Flags >= 0x100 are reserved for internal use.
* Other flags are reserved for internal use.
*/
#define REF_NODEREF 0x01
#define REF_DELETING 0x02
/*
* Setup reflog before using. Set errno to something meaningful on failure.
@ -257,57 +255,95 @@ enum action_on_err {
struct ref_transaction *ref_transaction_begin(struct strbuf *err);
/*
* The following functions add a reference check or update to a
* ref_transaction. In all of them, refname is the name of the
* reference to be affected. The functions make internal copies of
* refname and msg, so the caller retains ownership of these parameters.
* flags can be REF_NODEREF; it is passed to update_ref_lock().
* Reference transaction updates
*
* The following four functions add a reference check or update to a
* ref_transaction. They have some common similar parameters:
*
* transaction -- a pointer to an open ref_transaction, obtained
* from ref_transaction_begin().
*
* refname -- the name of the reference to be affected.
*
* flags -- flags affecting the update, passed to
* update_ref_lock(). Can be REF_NODEREF, which means that
* symbolic references should not be followed.
*
* msg -- a message describing the change (for the reflog).
*
* err -- a strbuf for receiving a description of any error that
* might have occured.
*
* The functions make internal copies of refname and msg, so the
* caller retains ownership of these parameters.
*
* The functions return 0 on success and non-zero on failure. A
* failure means that the transaction as a whole has failed and needs
* to be rolled back.
*/
/*
* Add a reference update to transaction. new_sha1 is the value that
* the reference should have after the update, or null_sha1 if it should
* be deleted. If have_old is true, then old_sha1 holds the value
* that the reference should have had before the update, or zeros if
* it must not have existed beforehand.
* Function returns 0 on success and non-zero on failure. A failure to update
* means that the transaction as a whole has failed and will need to be
* rolled back.
* Add a reference update to transaction. new_sha1 is the value that
* the reference should have after the update, or null_sha1 if it
* should be deleted. If new_sha1 is NULL, then the reference is not
* changed at all. old_sha1 is the value that the reference must have
* before the update, or null_sha1 if it must not have existed
* beforehand. The old value is checked after the lock is taken to
* prevent races. If the old value doesn't agree with old_sha1, the
* whole transaction fails. If old_sha1 is NULL, then the previous
* value is not checked.
*
* See the above comment "Reference transaction updates" for more
* information.
*/
int ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
const unsigned char *old_sha1,
int flags, int have_old, const char *msg,
unsigned int flags, const char *msg,
struct strbuf *err);
/*
* Add a reference creation to transaction. new_sha1 is the value
* that the reference should have after the update; it must not be the
* null SHA-1. It is verified that the reference does not exist
* Add a reference creation to transaction. new_sha1 is the value that
* the reference should have after the update; it must not be
* null_sha1. It is verified that the reference does not exist
* already.
* Function returns 0 on success and non-zero on failure. A failure to create
* means that the transaction as a whole has failed and will need to be
* rolled back.
*
* See the above comment "Reference transaction updates" for more
* information.
*/
int ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
const unsigned char *new_sha1,
int flags, const char *msg,
unsigned int flags, const char *msg,
struct strbuf *err);
/*
* Add a reference deletion to transaction. If have_old is true, then
* old_sha1 holds the value that the reference should have had before
* the update (which must not be the null SHA-1).
* Function returns 0 on success and non-zero on failure. A failure to delete
* means that the transaction as a whole has failed and will need to be
* rolled back.
* Add a reference deletion to transaction. If old_sha1 is non-NULL,
* then it holds the value that the reference should have had before
* the update (which must not be null_sha1).
*
* See the above comment "Reference transaction updates" for more
* information.
*/
int ref_transaction_delete(struct ref_transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
int flags, int have_old, const char *msg,
unsigned int flags, const char *msg,
struct strbuf *err);
/*
* Verify, within a transaction, that refname has the value old_sha1,
* or, if old_sha1 is null_sha1, then verify that the reference
* doesn't exist. old_sha1 must be non-NULL.
*
* See the above comment "Reference transaction updates" for more
* information.
*/
int ref_transaction_verify(struct ref_transaction *transaction,
const char *refname,
const unsigned char *old_sha1,
unsigned int flags,
struct strbuf *err);
/*
@ -328,10 +364,17 @@ int ref_transaction_commit(struct ref_transaction *transaction,
*/
void ref_transaction_free(struct ref_transaction *transaction);
/** Lock a ref and then write its file */
int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr);
/**
* Lock, update, and unlock a single reference. This function
* basically does a transaction containing a single call to
* ref_transaction_update(). The parameters to this function have the
* same meaning as the corresponding parameters to
* ref_transaction_update(). Handle errors as requested by the `onerr`
* argument.
*/
int update_ref(const char *msg, const char *refname,
const unsigned char *new_sha1, const unsigned char *old_sha1,
unsigned int flags, enum action_on_err onerr);
extern int parse_hide_refs_config(const char *var, const char *value, const char *);
extern int ref_is_hidden(const char *);

2
sequencer.c
View file

@ -252,7 +252,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
if (!transaction ||
ref_transaction_update(transaction, "HEAD",
to, unborn ? null_sha1 : from,
0, 1, sb.buf, &err) ||
0, sb.buf, &err) ||
ref_transaction_commit(transaction, &err)) {
ref_transaction_free(transaction);
error("%s", err.buf);

30
t/t7516-commit-races.sh Executable file
View file

@ -0,0 +1,30 @@
#!/bin/sh
test_description='git commit races'
. ./test-lib.sh
test_expect_success 'race to create orphan commit' '
write_script hare-editor <<-\EOF &&
git commit --allow-empty -m hare
EOF
test_must_fail env EDITOR=./hare-editor git commit --allow-empty -m tortoise -e &&
git show -s --pretty=format:%s >subject &&
grep hare subject &&
test -z "$(git show -s --pretty=format:%P)"
'
test_expect_success 'race to create non-orphan commit' '
write_script airplane-editor <<-\EOF &&
git commit --allow-empty -m airplane
EOF
git checkout --orphan branch &&
git commit --allow-empty -m base &&
git rev-parse HEAD >base &&
test_must_fail env EDITOR=./airplane-editor git commit --allow-empty -m ship -e &&
git show -s --pretty=format:%s >subject &&
grep airplane subject &&
git rev-parse HEAD^ >parent &&
test_cmp base parent
'
test_done

2
walker.c
View file

@ -299,7 +299,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
strbuf_reset(&refname);
strbuf_addf(&refname, "refs/%s", write_ref[i]);
if (ref_transaction_update(transaction, refname.buf,
&sha1[20 * i], NULL, 0, 0,
&sha1[20 * i], NULL, 0,
msg ? msg : "fetch (unknown)",
&err)) {
error("%s", err.buf);