summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2021-09-23 20:45:17 (GMT)
committerJunio C Hamano <gitster@pobox.com>2021-09-23 20:45:17 (GMT)
commitdb1e533058289ddb577e10d623c1482400b45d91 (patch)
tree073a27df8b072251a66140d9f5cd352b8a63f36b
parent4938d6859b7723197b1912f8d7340915aff1d526 (diff)
parent3fde557f7f3fe1c6b6ab1e64a0551ba250026c09 (diff)
downloadgit-db1e533058289ddb577e10d623c1482400b45d91.zip
git-db1e533058289ddb577e10d623c1482400b45d91.tar.gz
git-db1e533058289ddb577e10d623c1482400b45d91.tar.bz2
Merge branch 'ab/refs-errno-cleanup' into jch
The "remainder" of hn/refs-errno-cleanup topic. * ab/refs-errno-cleanup: refs: make errno output explicit for refs_resolve_ref_unsafe refs: explicitly return failure_errno from parse_loose_ref_contents branch tests: test for errno propagating on failing read refs: add failure_errno to refs_read_raw_ref() signature
-rw-r--r--refs.c69
-rw-r--r--refs.h11
-rw-r--r--refs/files-backend.c51
-rw-r--r--refs/packed-backend.c7
-rw-r--r--refs/refs-internal.h12
-rwxr-xr-xt/t3200-branch.sh21
6 files changed, 123 insertions, 48 deletions
diff --git a/refs.c b/refs.c
index da0cc82..e3b6d8f 100644
--- a/refs.c
+++ b/refs.c
@@ -1654,7 +1654,8 @@ int for_each_fullref_in_prefixes(const char *namespace,
static int refs_read_special_head(struct ref_store *ref_store,
const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type)
+ struct strbuf *referent, unsigned int *type,
+ int *failure_errno)
{
struct strbuf full_path = STRBUF_INIT;
struct strbuf content = STRBUF_INIT;
@@ -1664,7 +1665,8 @@ static int refs_read_special_head(struct ref_store *ref_store,
if (strbuf_read_file(&content, full_path.buf, 0) < 0)
goto done;
- result = parse_loose_ref_contents(content.buf, oid, referent, type);
+ result = parse_loose_ref_contents(content.buf, oid, referent, type,
+ failure_errno);
done:
strbuf_release(&full_path);
@@ -1672,30 +1674,33 @@ done:
return result;
}
-int refs_read_raw_ref(struct ref_store *ref_store,
- const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type)
+int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
+ struct object_id *oid, struct strbuf *referent,
+ unsigned int *type, int *failure_errno)
{
+ assert(failure_errno);
if (!strcmp(refname, "FETCH_HEAD") || !strcmp(refname, "MERGE_HEAD")) {
return refs_read_special_head(ref_store, refname, oid, referent,
- type);
+ type, failure_errno);
}
return ref_store->be->read_raw_ref(ref_store, refname, oid, referent,
- type, &errno);
+ type, failure_errno);
}
-/* This function needs to return a meaningful errno on failure */
-const char *refs_resolve_ref_unsafe(struct ref_store *refs,
- const char *refname,
- int resolve_flags,
- struct object_id *oid, int *flags)
+const char *refs_resolve_ref_unsafe_with_errno(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ struct object_id *oid,
+ int *flags, int *failure_errno)
{
static struct strbuf sb_refname = STRBUF_INIT;
struct object_id unused_oid;
int unused_flags;
int symref_count;
+ assert(failure_errno);
+
if (!oid)
oid = &unused_oid;
if (!flags)
@@ -1706,7 +1711,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(refname)) {
- errno = EINVAL;
+ *failure_errno = EINVAL;
return NULL;
}
@@ -1724,8 +1729,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
for (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {
unsigned int read_flags = 0;
- if (refs_read_raw_ref(refs, refname,
- oid, &sb_refname, &read_flags)) {
+ if (refs_read_raw_ref(refs, refname, oid, &sb_refname,
+ &read_flags, failure_errno)) {
*flags |= read_flags;
/* In reading mode, refs must eventually resolve */
@@ -1737,9 +1742,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
* may show errors besides ENOENT if there are
* similarly-named refs.
*/
- if (errno != ENOENT &&
- errno != EISDIR &&
- errno != ENOTDIR)
+ if (*failure_errno != ENOENT &&
+ *failure_errno != EISDIR &&
+ *failure_errno != ENOTDIR)
return NULL;
oidclr(oid);
@@ -1766,7 +1771,7 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
!refname_is_safe(refname)) {
- errno = EINVAL;
+ *failure_errno = EINVAL;
return NULL;
}
@@ -1774,10 +1779,24 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
}
}
- errno = ELOOP;
+ *failure_errno = ELOOP;
return NULL;
}
+const char *refs_resolve_ref_unsafe(struct ref_store *refs, const char *refname,
+ int resolve_flags, struct object_id *oid,
+ int *flags)
+{
+ int failure_errno = 0;
+ const char *refn;
+ refn = refs_resolve_ref_unsafe_with_errno(refs, refname, resolve_flags,
+ oid, flags, &failure_errno);
+ if (!refn)
+ /* For unmigrated legacy callers */
+ errno = failure_errno;
+ return refn;
+}
+
/* backend functions */
int refs_init_db(struct strbuf *err)
{
@@ -2228,6 +2247,13 @@ int refs_verify_refname_available(struct ref_store *refs,
strbuf_grow(&dirname, strlen(refname) + 1);
for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1, '/')) {
+ /*
+ * Just saying "Is a directory" when we e.g. can't
+ * lock some multi-level ref isn't very informative,
+ * the user won't be told *what* is a directory, so
+ * let's not use strerror() below.
+ */
+ int ignore_errno;
/* Expand dirname to the new prefix, not including the trailing slash: */
strbuf_add(&dirname, refname + dirname.len, slash - refname - dirname.len);
@@ -2239,7 +2265,8 @@ int refs_verify_refname_available(struct ref_store *refs,
if (skip && string_list_has_string(skip, dirname.buf))
continue;
- if (!refs_read_raw_ref(refs, dirname.buf, &oid, &referent, &type)) {
+ if (!refs_read_raw_ref(refs, dirname.buf, &oid, &referent,
+ &type, &ignore_errno)) {
strbuf_addf(err, _("'%s' exists; cannot create '%s'"),
dirname.buf, refname);
goto cleanup;
diff --git a/refs.h b/refs.h
index fda8aef..a5685c8 100644
--- a/refs.h
+++ b/refs.h
@@ -68,6 +68,17 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
int resolve_flags,
struct object_id *oid,
int *flags);
+/**
+ * refs_resolve_ref_unsafe_with_errno() is like
+ * refs_resolve_ref_unsafe(), but provide access to errno code that
+ * lead to a failure. We guarantee that errno is set to a meaningful
+ * value on non-zero return.
+ */
+const char *refs_resolve_ref_unsafe_with_errno(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ struct object_id *oid,
+ int *flags, int *failure_errno);
const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
struct object_id *oid, int *flags);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index e461787..7243e15 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -355,6 +355,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, const char *refname,
int fd;
int ret = -1;
int remaining_retries = 3;
+ int myerr = 0;
*type = 0;
strbuf_reset(&sb_path);
@@ -381,11 +382,14 @@ stat_ref:
goto out;
if (lstat(path, &st) < 0) {
- if (errno != ENOENT)
+ int ignore_errno;
+ myerr = errno;
+ errno = 0;
+ if (myerr != ENOENT)
goto out;
- if (refs_read_raw_ref(refs->packed_ref_store, refname,
- oid, referent, type)) {
- errno = ENOENT;
+ if (refs_read_raw_ref(refs->packed_ref_store, refname, oid,
+ referent, type, &ignore_errno)) {
+ myerr = ENOENT;
goto out;
}
ret = 0;
@@ -396,7 +400,9 @@ stat_ref:
if (S_ISLNK(st.st_mode)) {
strbuf_reset(&sb_contents);
if (strbuf_readlink(&sb_contents, path, st.st_size) < 0) {
- if (errno == ENOENT || errno == EINVAL)
+ myerr = errno;
+ errno = 0;
+ if (myerr == ENOENT || myerr == EINVAL)
/* inconsistent with lstat; retry */
goto stat_ref;
else
@@ -418,14 +424,15 @@ stat_ref:
/* Is it a directory? */
if (S_ISDIR(st.st_mode)) {
+ int ignore_errno;
/*
* Even though there is a directory where the loose
* ref is supposed to be, there could still be a
* packed ref:
*/
- if (refs_read_raw_ref(refs->packed_ref_store, refname,
- oid, referent, type)) {
- errno = EISDIR;
+ if (refs_read_raw_ref(refs->packed_ref_store, refname, oid,
+ referent, type, &ignore_errno)) {
+ myerr = EISDIR;
goto out;
}
ret = 0;
@@ -438,7 +445,8 @@ stat_ref:
*/
fd = open(path, O_RDONLY);
if (fd < 0) {
- if (errno == ENOENT && !S_ISLNK(st.st_mode))
+ myerr = errno;
+ if (myerr == ENOENT && !S_ISLNK(st.st_mode))
/* inconsistent with lstat; retry */
goto stat_ref;
else
@@ -446,26 +454,29 @@ stat_ref:
}
strbuf_reset(&sb_contents);
if (strbuf_read(&sb_contents, fd, 256) < 0) {
- int save_errno = errno;
+ myerr = errno;
close(fd);
- errno = save_errno;
goto out;
}
close(fd);
strbuf_rtrim(&sb_contents);
buf = sb_contents.buf;
- ret = parse_loose_ref_contents(buf, oid, referent, type);
+ ret = parse_loose_ref_contents(buf, oid, referent, type, &myerr);
out:
- *failure_errno = errno;
+ if (ret && !myerr)
+ BUG("returning non-zero %d, should have set myerr!", ret);
+ *failure_errno = myerr;
+
strbuf_release(&sb_path);
strbuf_release(&sb_contents);
return ret;
}
int parse_loose_ref_contents(const char *buf, struct object_id *oid,
- struct strbuf *referent, unsigned int *type)
+ struct strbuf *referent, unsigned int *type,
+ int *failure_errno)
{
const char *p;
if (skip_prefix(buf, "ref:", &buf)) {
@@ -484,7 +495,7 @@ int parse_loose_ref_contents(const char *buf, struct object_id *oid,
if (parse_oid_hex(buf, oid, &p) ||
(*p != '\0' && !isspace(*p))) {
*type |= REF_ISBROKEN;
- errno = EINVAL;
+ *failure_errno = EINVAL;
return -1;
}
return 0;
@@ -990,6 +1001,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
{
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
+ int resolve_errno = 0;
files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
@@ -997,13 +1009,14 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
CALLOC_ARRAY(lock, 1);
files_ref_path(refs, &ref_file, refname);
- if (!refs_resolve_ref_unsafe(&refs->base, refname,
- RESOLVE_REF_NO_RECURSE,
- &lock->old_oid, type)) {
+ if (!refs_resolve_ref_unsafe_with_errno(&refs->base, refname,
+ RESOLVE_REF_NO_RECURSE,
+ &lock->old_oid, type,
+ &resolve_errno)) {
if (!refs_verify_refname_available(&refs->base, refname,
NULL, NULL, err))
strbuf_addf(err, "unable to resolve reference '%s': %s",
- refname, strerror(errno));
+ refname, strerror(resolve_errno));
goto error_return;
}
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 47247a1..52cdc94 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1347,6 +1347,7 @@ int is_packed_transaction_needed(struct ref_store *ref_store,
ret = 0;
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = transaction->updates[i];
+ int failure_errno;
unsigned int type;
struct object_id oid;
@@ -1357,9 +1358,9 @@ int is_packed_transaction_needed(struct ref_store *ref_store,
*/
continue;
- if (!refs_read_raw_ref(ref_store, update->refname,
- &oid, &referent, &type) ||
- errno != ENOENT) {
+ if (!refs_read_raw_ref(ref_store, update->refname, &oid,
+ &referent, &type, &failure_errno) ||
+ failure_errno != ENOENT) {
/*
* We have to actually delete that reference
* -> this transaction is needed.
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index ddd6d7f..7a3a61a 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -149,9 +149,9 @@ struct ref_update {
const char refname[FLEX_ARRAY];
};
-int refs_read_raw_ref(struct ref_store *ref_store,
- const char *refname, struct object_id *oid,
- struct strbuf *referent, unsigned int *type);
+int refs_read_raw_ref(struct ref_store *ref_store, const char *refname,
+ struct object_id *oid, struct strbuf *referent,
+ unsigned int *type, int *failure_errno);
/*
* Write an error to `err` and return a nonzero value iff the same
@@ -692,10 +692,12 @@ struct ref_store {
};
/*
- * Parse contents of a loose ref file.
+ * Parse contents of a loose ref file. *failure_errno maybe be set to EINVAL for
+ * invalid contents.
*/
int parse_loose_ref_contents(const char *buf, struct object_id *oid,
- struct strbuf *referent, unsigned int *type);
+ struct strbuf *referent, unsigned int *type,
+ int *failure_errno);
/*
* Fill in the generic part of refs and add it to our collection of
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e575ffb..7486688 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -731,6 +731,27 @@ test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for
test_must_fail git branch -m u v
'
+test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
+ test_when_finished "rm -rf subdir" &&
+ git init subdir &&
+
+ (
+ cd subdir &&
+ for d in refs objects packed-refs
+ do
+ rm -rf .git/$d &&
+ ln -s ../../.git/$d .git/$d
+ done
+ ) &&
+ git --git-dir subdir/.git/ branch rename-src &&
+ git rev-parse rename-src >expect &&
+ # Tests a BUG() assertion in files_read_raw_ref()
+ git --git-dir subdir/.git/ branch -m rename-src rename-dest &&
+ git rev-parse rename-dest >actual &&
+ test_cmp expect actual &&
+ git branch -D rename-dest
+'
+
test_expect_success 'test tracking setup via --track' '
git config remote.local.url . &&
git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&