Skip to content

Commit 19a249b

Browse files
gitstergitster
committed
Merge branch 'rs/ref-transaction-0'
Early part of the "ref transaction" topic. * rs/ref-transaction-0: refs.c: change ref_transaction_update() to do error checking and return status refs.c: remove the onerr argument to ref_transaction_commit update-ref: use err argument to get error from ref_transaction_commit refs.c: make update_ref_write update a strbuf on failure refs.c: make ref_update_reject_duplicates take a strbuf argument for errors refs.c: log_ref_write should try to return meaningful errno refs.c: make resolve_ref_unsafe set errno to something meaningful on error refs.c: commit_packed_refs to return a meaningful errno on failure refs.c: make remove_empty_directories always set errno to something sane refs.c: verify_lock should set errno to something meaningful refs.c: make sure log_ref_setup returns a meaningful errno refs.c: add an err argument to repack_without_refs lockfile.c: make lock_file return a meaningful errno on failurei lockfile.c: add a new public function unable_to_lock_message refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: allow passing NULL to ref_transaction_free refs.c: constify the sha arguments for ref_transaction_create|delete|update refs.c: ref_transaction_commit should not free the transaction refs.c: remove ref_transaction_rollback
2 parents ad25da0 + 8e34800 commit 19a249b

File tree

6 files changed

+199
-100
lines changed

6 files changed

+199
-100
lines changed

‎builtin/remote.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -754,7 +754,7 @@ static int remove_branches(struct string_list *branches)
754754
branch_names = xmalloc(branches->nr * sizeof(*branch_names));
755755
for (i = 0; i < branches->nr; i++)
756756
branch_names[i] = branches->items[i].string;
757-
result |= repack_without_refs(branch_names, branches->nr);
757+
result |= repack_without_refs(branch_names, branches->nr, NULL);
758758
free(branch_names);
759759

760760
for (i = 0; i < branches->nr; i++) {
@@ -1332,7 +1332,8 @@ static int prune_remote(const char *remote, int dry_run)
13321332
for (i = 0; i < states.stale.nr; i++)
13331333
delete_refs[i] = states.stale.items[i].util;
13341334
if (!dry_run)
1335-
result |= repack_without_refs(delete_refs, states.stale.nr);
1335+
result |= repack_without_refs(delete_refs,
1336+
states.stale.nr, NULL);
13361337
free(delete_refs);
13371338
}
13381339

‎builtin/update-ref.c‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ static struct ref_transaction *transaction;
1616

1717
static char line_termination = '\n';
1818
static int update_flags;
19+
static struct strbuf err = STRBUF_INIT;
1920

2021
/*
2122
* Parse one whitespace- or NUL-terminated, possibly C-quoted argument
@@ -197,8 +198,9 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)
197198
if (*next != line_termination)
198199
die("update %s: extra input: %s", refname, next);
199200

200-
ref_transaction_update(transaction, refname, new_sha1, old_sha1,
201-
update_flags, have_old);
201+
if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
202+
update_flags, have_old, &err))
203+
die("%s", err.buf);
202204

203205
update_flags = 0;
204206
free(refname);
@@ -286,8 +288,9 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)
286288
if (*next != line_termination)
287289
die("verify %s: extra input: %s", refname, next);
288290

289-
ref_transaction_update(transaction, refname, new_sha1, old_sha1,
290-
update_flags, have_old);
291+
if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
292+
update_flags, have_old, &err))
293+
die("%s", err.buf);
291294

292295
update_flags = 0;
293296
free(refname);
@@ -359,17 +362,16 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
359362
die("Refusing to perform update with empty message.");
360363

361364
if (read_stdin) {
362-
int ret;
363365
transaction = ref_transaction_begin();
364-
365366
if (delete || no_deref || argc > 0)
366367
usage_with_options(git_update_ref_usage, options);
367368
if (end_null)
368369
line_termination = '\0';
369370
update_refs_stdin();
370-
ret = ref_transaction_commit(transaction, msg,
371-
UPDATE_REFS_DIE_ON_ERR);
372-
return ret;
371+
if (ref_transaction_commit(transaction, msg, &err))
372+
die("%s", err.buf);
373+
ref_transaction_free(transaction);
374+
return 0;
373375
}
374376

375377
if (end_null)

‎cache.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,8 @@ struct lock_file {
578578
#define LOCK_DIE_ON_ERROR 1
579579
#define LOCK_NODEREF 2
580580
extern int unable_to_lock_error(const char *path, int err);
581+
extern void unable_to_lock_message(const char *path, int err,
582+
struct strbuf *buf);
581583
extern NORETURN void unable_to_lock_index_die(const char *path, int err);
582584
extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
583585
extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
@@ -995,7 +997,7 @@ extern int read_ref(const char *refname, unsigned char *sha1);
995997
* NULL. If more than MAXDEPTH recursive symbolic lookups are needed,
996998
* give up and return NULL.
997999
*
998-
* errno is sometimes set on errors, but not always.
1000+
* errno is set to something meaningful on error.
9991001
*/
10001002
extern const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1, int reading, int *flag);
10011003
extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);

‎lockfile.c‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ static char *resolve_symlink(char *p, size_t s)
120120
return p;
121121
}
122122

123-
123+
/* Make sure errno contains a meaningful value on error */
124124
static int lock_file(struct lock_file *lk, const char *path, int flags)
125125
{
126126
/*
@@ -129,8 +129,10 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
129129
*/
130130
static const size_t max_path_len = sizeof(lk->filename) - 5;
131131

132-
if (strlen(path) >= max_path_len)
132+
if (strlen(path) >= max_path_len) {
133+
errno = ENAMETOOLONG;
133134
return -1;
135+
}
134136
strcpy(lk->filename, path);
135137
if (!(flags & LOCK_NODEREF))
136138
resolve_symlink(lk->filename, max_path_len);
@@ -147,44 +149,51 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
147149
lock_file_list = lk;
148150
lk->on_list = 1;
149151
}
150-
if (adjust_shared_perm(lk->filename))
151-
return error("cannot fix permission bits on %s",
152-
lk->filename);
152+
if (adjust_shared_perm(lk->filename)) {
153+
int save_errno = errno;
154+
error("cannot fix permission bits on %s",
155+
lk->filename);
156+
errno = save_errno;
157+
return -1;
158+
}
153159
}
154160
else
155161
lk->filename[0] = 0;
156162
return lk->fd;
157163
}
158164

159-
static char *unable_to_lock_message(const char *path, int err)
165+
void unable_to_lock_message(const char *path, int err, struct strbuf *buf)
160166
{
161-
struct strbuf buf = STRBUF_INIT;
162-
163167
if (err == EEXIST) {
164-
strbuf_addf(&buf, "Unable to create '%s.lock': %s.\n\n"
168+
strbuf_addf(buf, "Unable to create '%s.lock': %s.\n\n"
165169
"If no other git process is currently running, this probably means a\n"
166170
"git process crashed in this repository earlier. Make sure no other git\n"
167171
"process is running and remove the file manually to continue.",
168172
absolute_path(path), strerror(err));
169173
} else
170-
strbuf_addf(&buf, "Unable to create '%s.lock': %s",
174+
strbuf_addf(buf, "Unable to create '%s.lock': %s",
171175
absolute_path(path), strerror(err));
172-
return strbuf_detach(&buf, NULL);
173176
}
174177

175178
int unable_to_lock_error(const char *path, int err)
176179
{
177-
char *msg = unable_to_lock_message(path, err);
178-
error("%s", msg);
179-
free(msg);
180+
struct strbuf buf = STRBUF_INIT;
181+
182+
unable_to_lock_message(path, err, &buf);
183+
error("%s", buf.buf);
184+
strbuf_release(&buf);
180185
return -1;
181186
}
182187

183188
NORETURN void unable_to_lock_index_die(const char *path, int err)
184189
{
185-
die("%s", unable_to_lock_message(path, err));
190+
struct strbuf buf = STRBUF_INIT;
191+
192+
unable_to_lock_message(path, err, &buf);
193+
die("%s", buf.buf);
186194
}
187195

196+
/* This should return a meaningful errno on failure */
188197
int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
189198
{
190199
int fd = lock_file(lk, path, flags);

0 commit comments

Comments
 (0)