* [PATCH 13/14] Pass a (cached_refs *) to the resolve_gitlink_*() functions
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
And remove some redundant arguments from resolve_gitlink_packed_ref().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 01b8efd..8776195 100644
--- a/refs.c
+++ b/refs.c
@@ -414,12 +414,12 @@ static struct ref_array *get_loose_refs(struct cached_refs *refs)
#define MAXDEPTH 5
#define MAXREFLEN (1024)
-static int resolve_gitlink_packed_ref(char *name, int pathlen,
+static int resolve_gitlink_packed_ref(struct cached_refs *refs,
const char *refname, unsigned char *sha1)
{
int retval = -1;
struct ref_entry *ref;
- struct ref_array *array = get_packed_refs(get_cached_refs(name));
+ struct ref_array *array = get_packed_refs(refs);
ref = search_ref_array(array, refname);
if (ref != NULL) {
@@ -429,7 +429,8 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen,
return retval;
}
-static int resolve_gitlink_ref_recursive(char *name, int pathlen,
+static int resolve_gitlink_ref_recursive(struct cached_refs *refs,
+ char *name, int pathlen,
const char *refname, unsigned char *sha1,
int recursion)
{
@@ -441,7 +442,7 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
memcpy(name + pathlen, refname, len+1);
fd = open(name, O_RDONLY);
if (fd < 0)
- return resolve_gitlink_packed_ref(name, pathlen, refname, sha1);
+ return resolve_gitlink_packed_ref(refs, refname, sha1);
len = read(fd, buffer, sizeof(buffer)-1);
close(fd);
@@ -462,19 +463,24 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
while (isspace(*p))
p++;
- return resolve_gitlink_ref_recursive(name, pathlen, p, sha1, recursion+1);
+ return resolve_gitlink_ref_recursive(refs, name, pathlen, p, sha1, recursion+1);
}
int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sha1)
{
int len = strlen(path), retval;
- char *gitdir;
+ char *submodule, *gitdir;
+ struct cached_refs *refs;
const char *tmp;
while (len && path[len-1] == '/')
len--;
if (!len)
return -1;
+ submodule = xstrndup(path, len);
+ refs = get_cached_refs(submodule);
+ free(submodule);
+
gitdir = xmalloc(len + MAXREFLEN + 8);
memcpy(gitdir, path, len);
memcpy(gitdir + len, "/.git", 6);
@@ -489,7 +495,7 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sh
}
gitdir[len] = '/';
gitdir[++len] = '\0';
- retval = resolve_gitlink_ref_recursive(gitdir, len, refname, sha1, 0);
+ retval = resolve_gitlink_ref_recursive(refs, gitdir, len, refname, sha1, 0);
free(gitdir);
return retval;
}
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 10/14] is_dup_ref(): extract function from sort_ref_list()
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Giving it a name makes the code easier to understand. And the new
function will be convenient later when it has to be called from
multiple places.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 27 +++++++++++++++++++++------
1 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index cb5bb18..715a41a 100644
--- a/refs.c
+++ b/refs.c
@@ -84,6 +84,25 @@ static int ref_entry_cmp(const void *a, const void *b)
return strcmp(one->name, two->name);
}
+/*
+ * Emit a warning and return true iff ref1 and ref2 have the same name
+ * and the same sha1. Die if they have the same name but different
+ * sha1s.
+ */
+static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2)
+{
+ if (!strcmp(ref1->name, ref2->name)) {
+ /* Duplicate name; make sure that the SHA1s match: */
+ if (hashcmp(ref1->sha1, ref2->sha1))
+ die("Duplicated ref, and SHA1s don't match: %s",
+ ref1->name);
+ warning("Duplicated ref: %s", ref1->name);
+ return 1;
+ } else {
+ return 0;
+ }
+}
+
static void sort_ref_array(struct ref_array *array)
{
int i = 0, j = 1;
@@ -94,15 +113,11 @@ static void sort_ref_array(struct ref_array *array)
qsort(array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
- /* Remove any duplicates from the ref_array */
+ /* Remove any duplicates from the ref_list */
for (; j < array->nr; j++) {
struct ref_entry *a = array->refs[i];
struct ref_entry *b = array->refs[j];
- if (!strcmp(a->name, b->name)) {
- if (hashcmp(a->sha1, b->sha1))
- die("Duplicated ref, and SHA1s don't match: %s",
- a->name);
- warning("Duplicated ref: %s", a->name);
+ if (is_dup_ref(a, b)) {
free(b);
continue;
}
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 12/14] get_ref_dir(): change signature
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Change get_ref_dir() to take a (struct cached_refs *) in place of the
submodule name.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 1109c9f..01b8efd 100644
--- a/refs.c
+++ b/refs.c
@@ -300,14 +300,14 @@ static struct ref_array *get_packed_refs(struct cached_refs *refs)
return &refs->packed;
}
-static void get_ref_dir(const char *submodule, const char *base,
+static void get_ref_dir(struct cached_refs *refs, const char *base,
struct ref_array *array)
{
DIR *dir;
const char *path;
- if (*submodule)
- path = git_path_submodule(submodule, "%s", base);
+ if (*refs->name)
+ path = git_path_submodule(refs->name, "%s", base);
else
path = git_path("%s", base);
@@ -338,19 +338,19 @@ static void get_ref_dir(const char *submodule, const char *base,
if (has_extension(de->d_name, ".lock"))
continue;
memcpy(ref + baselen, de->d_name, namelen+1);
- refdir = submodule
- ? git_path_submodule(submodule, "%s", ref)
+ refdir = *refs->name
+ ? git_path_submodule(refs->name, "%s", ref)
: git_path("%s", ref);
if (stat(refdir, &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
- get_ref_dir(submodule, ref, array);
+ get_ref_dir(refs, ref, array);
continue;
}
- if (submodule) {
+ if (*refs->name) {
hashclr(sha1);
flag = 0;
- if (resolve_gitlink_ref(submodule, ref, sha1) < 0) {
+ if (resolve_gitlink_ref(refs->name, ref, sha1) < 0) {
hashclr(sha1);
flag |= REF_BROKEN;
}
@@ -403,7 +403,7 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
static struct ref_array *get_loose_refs(struct cached_refs *refs)
{
if (!refs->did_loose) {
- get_ref_dir(refs->name, "refs", &refs->loose);
+ get_ref_dir(refs, "refs", &refs->loose);
sort_ref_array(&refs->loose);
refs->did_loose = 1;
}
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 11/14] refs: change signatures of get_packed_refs() and get_loose_refs()
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Change get_packed_refs() and get_loose_refs() to take a (struct
cached_refs *) instead of the name of the submodule.
Change get_ref_dir() to take a submodule name (i.e., "" for the main
module) rather than a submodule pointer (i.e., NULL for the main
module) so that refs->name can be used as its argument. (In a moment
this function will also be changed to take a (struct cached_refs *),
too.)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 36 +++++++++++++++++-------------------
1 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/refs.c b/refs.c
index 715a41a..1109c9f 100644
--- a/refs.c
+++ b/refs.c
@@ -280,16 +280,14 @@ void clear_extra_refs(void)
clear_ref_array(&extra_refs);
}
-static struct ref_array *get_packed_refs(const char *submodule)
+static struct ref_array *get_packed_refs(struct cached_refs *refs)
{
- struct cached_refs *refs = get_cached_refs(submodule);
-
if (!refs->did_packed) {
const char *packed_refs_file;
FILE *f;
- if (submodule)
- packed_refs_file = git_path_submodule(submodule, "packed-refs");
+ if (*refs->name)
+ packed_refs_file = git_path_submodule(refs->name, "packed-refs");
else
packed_refs_file = git_path("packed-refs");
f = fopen(packed_refs_file, "r");
@@ -308,7 +306,7 @@ static void get_ref_dir(const char *submodule, const char *base,
DIR *dir;
const char *path;
- if (submodule)
+ if (*submodule)
path = git_path_submodule(submodule, "%s", base);
else
path = git_path("%s", base);
@@ -402,12 +400,10 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
for_each_rawref(warn_if_dangling_symref, &data);
}
-static struct ref_array *get_loose_refs(const char *submodule)
+static struct ref_array *get_loose_refs(struct cached_refs *refs)
{
- struct cached_refs *refs = get_cached_refs(submodule);
-
if (!refs->did_loose) {
- get_ref_dir(submodule, "refs", &refs->loose);
+ get_ref_dir(refs->name, "refs", &refs->loose);
sort_ref_array(&refs->loose);
refs->did_loose = 1;
}
@@ -423,7 +419,7 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen,
{
int retval = -1;
struct ref_entry *ref;
- struct ref_array *array = get_packed_refs(name);
+ struct ref_array *array = get_packed_refs(get_cached_refs(name));
ref = search_ref_array(array, refname);
if (ref != NULL) {
@@ -504,7 +500,7 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sh
*/
static int get_packed_ref(const char *refname, unsigned char *sha1)
{
- struct ref_array *packed = get_packed_refs(NULL);
+ struct ref_array *packed = get_packed_refs(get_cached_refs(NULL));
struct ref_entry *entry = search_ref_array(packed, refname);
if (entry) {
hashcpy(sha1, entry->sha1);
@@ -682,7 +678,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
return -1;
if ((flag & REF_ISPACKED)) {
- struct ref_array *array = get_packed_refs(NULL);
+ struct ref_array *array = get_packed_refs(get_cached_refs(NULL));
struct ref_entry *r = search_ref_array(array, refname);
if (r != NULL && r->flag & REF_KNOWS_PEELED) {
@@ -707,8 +703,9 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
int trim, int flags, void *cb_data)
{
int retval = 0, i, p = 0, l = 0;
- struct ref_array *packed = get_packed_refs(submodule);
- struct ref_array *loose = get_loose_refs(submodule);
+ struct cached_refs *refs = get_cached_refs(submodule);
+ struct ref_array *packed = get_packed_refs(refs);
+ struct ref_array *loose = get_loose_refs(refs);
struct ref_array *extra = &extra_refs;
@@ -1144,7 +1141,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
* name is a proper prefix of our refname.
*/
if (missing &&
- !is_refname_available(refname, NULL, get_packed_refs(NULL))) {
+ !is_refname_available(refname, NULL, get_packed_refs(get_cached_refs(NULL)))) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -1204,7 +1201,7 @@ static int repack_without_ref(const char *refname)
struct ref_entry *ref;
int fd, i;
- packed = get_packed_refs(NULL);
+ packed = get_packed_refs(get_cached_refs(NULL));
ref = search_ref_array(packed, refname);
if (ref == NULL)
return 0;
@@ -1288,6 +1285,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
struct stat loginfo;
int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
const char *symref = NULL;
+ struct cached_refs *refs = get_cached_refs(NULL);
if (log && S_ISLNK(loginfo.st_mode))
return error("reflog for %s is a symlink", oldrefname);
@@ -1299,10 +1297,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
if (!symref)
return error("refname %s not found", oldrefname);
- if (!is_refname_available(newrefname, oldrefname, get_packed_refs(NULL)))
+ if (!is_refname_available(newrefname, oldrefname, get_packed_refs(refs)))
return 1;
- if (!is_refname_available(newrefname, oldrefname, get_loose_refs(NULL)))
+ if (!is_refname_available(newrefname, oldrefname, get_loose_refs(refs)))
return 1;
lock = lock_ref_sha1_basic(renamed_ref, NULL, 0, NULL);
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 08/14] parse_ref_line(): add docstring
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index 7e84b63..abb03f7 100644
--- a/refs.c
+++ b/refs.c
@@ -21,6 +21,11 @@ struct ref_array {
struct ref_entry **refs;
};
+/*
+ * Parse one line from a packed-refs file. Return a pointer to the
+ * name within the line (null-terminated), or NULL if there is a
+ * problem.
+ */
static const char *parse_ref_line(char *line, unsigned char *sha1)
{
/*
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 09/14] add_ref(): add docstring
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index abb03f7..cb5bb18 100644
--- a/refs.c
+++ b/refs.c
@@ -54,6 +54,7 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
return line;
}
+/* Add a ref_entry to the end of the ref_array (unsorted). */
static void add_ref(const char *refname, const unsigned char *sha1,
int flag, struct ref_array *refs,
struct ref_entry **new_entry)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 02/14] struct ref_list: document name member
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index 409314d..e4e4bcd 100644
--- a/refs.c
+++ b/refs.c
@@ -12,6 +12,7 @@ struct ref_entry {
unsigned char flag; /* ISSYMREF? ISPACKED? */
unsigned char sha1[20];
unsigned char peeled[20];
+ /* The full name of the reference (e.g., "refs/heads/master"): */
char name[FLEX_ARRAY];
};
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 04/14] refs: rename some parameters result -> sha1
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 16 ++++++++--------
refs.h | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 2ae5d0d..c466fcd 100644
--- a/refs.c
+++ b/refs.c
@@ -398,7 +398,7 @@ static struct ref_array *get_loose_refs(const char *submodule)
#define MAXREFLEN (1024)
static int resolve_gitlink_packed_ref(char *name, int pathlen,
- const char *refname, unsigned char *result)
+ const char *refname, unsigned char *sha1)
{
int retval = -1;
struct ref_entry *ref;
@@ -406,14 +406,14 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen,
ref = search_ref_array(array, refname);
if (ref != NULL) {
- memcpy(result, ref->sha1, 20);
+ memcpy(sha1, ref->sha1, 20);
retval = 0;
}
return retval;
}
static int resolve_gitlink_ref_recursive(char *name, int pathlen,
- const char *refname, unsigned char *result,
+ const char *refname, unsigned char *sha1,
int recursion)
{
int fd, len = strlen(refname);
@@ -424,7 +424,7 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
memcpy(name + pathlen, refname, len+1);
fd = open(name, O_RDONLY);
if (fd < 0)
- return resolve_gitlink_packed_ref(name, pathlen, refname, result);
+ return resolve_gitlink_packed_ref(name, pathlen, refname, sha1);
len = read(fd, buffer, sizeof(buffer)-1);
close(fd);
@@ -435,7 +435,7 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
buffer[len] = 0;
/* Was it a detached head or an old-fashioned symlink? */
- if (!get_sha1_hex(buffer, result))
+ if (!get_sha1_hex(buffer, sha1))
return 0;
/* Symref? */
@@ -445,10 +445,10 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
while (isspace(*p))
p++;
- return resolve_gitlink_ref_recursive(name, pathlen, p, result, recursion+1);
+ return resolve_gitlink_ref_recursive(name, pathlen, p, sha1, recursion+1);
}
-int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *result)
+int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sha1)
{
int len = strlen(path), retval;
char *gitdir;
@@ -472,7 +472,7 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
}
gitdir[len] = '/';
gitdir[++len] = '\0';
- retval = resolve_gitlink_ref_recursive(gitdir, len, refname, result, 0);
+ retval = resolve_gitlink_ref_recursive(gitdir, len, refname, sha1, 0);
free(gitdir);
return retval;
}
diff --git a/refs.h b/refs.h
index 13e2aa3..c6b8749 100644
--- a/refs.h
+++ b/refs.h
@@ -133,7 +133,7 @@ extern char *shorten_unambiguous_ref(const char *refname, int strict);
extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);
/** resolve ref in nested "gitlink" repository */
-extern int resolve_gitlink_ref(const char *name, const char *refname, unsigned char *result);
+extern int resolve_gitlink_ref(const char *name, const char *refname, unsigned char *sha1);
/** lock a ref and then write its file */
enum action_on_err { MSG_ON_ERR, DIE_ON_ERR, QUIET_ON_ERR };
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 07/14] is_refname_available(): remove the "quiet" argument
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
quiet was always set to 0, so get rid of it. Add a function docstring
for good measure.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index a2e48e4..7e84b63 100644
--- a/refs.c
+++ b/refs.c
@@ -1049,8 +1049,15 @@ static int remove_empty_directories(const char *file)
return result;
}
+/*
+ * Return true iff a reference named refname could be created without
+ * conflicting with the name of an existing reference. If oldrefname
+ * is non-NULL, ignore potential conflicts with oldrefname (e.g.,
+ * because oldrefname is scheduled for deletion in the same
+ * operation).
+ */
static int is_refname_available(const char *refname, const char *oldrefname,
- struct ref_array *array, int quiet)
+ struct ref_array *array)
{
int i, namlen = strlen(refname); /* e.g. 'foo/bar' */
for (i = 0; i < array->nr; i++ ) {
@@ -1062,9 +1069,8 @@ static int is_refname_available(const char *refname, const char *oldrefname,
const char *lead = (namlen < len) ? entry->name : refname;
if (!strncmp(refname, entry->name, cmplen) &&
lead[cmplen] == '/') {
- if (!quiet)
- error("'%s' exists; cannot create '%s'",
- entry->name, refname);
+ error("'%s' exists; cannot create '%s'",
+ entry->name, refname);
return 0;
}
}
@@ -1117,7 +1123,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
* name is a proper prefix of our refname.
*/
if (missing &&
- !is_refname_available(refname, NULL, get_packed_refs(NULL), 0)) {
+ !is_refname_available(refname, NULL, get_packed_refs(NULL))) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -1272,10 +1278,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
if (!symref)
return error("refname %s not found", oldrefname);
- if (!is_refname_available(newrefname, oldrefname, get_packed_refs(NULL), 0))
+ if (!is_refname_available(newrefname, oldrefname, get_packed_refs(NULL)))
return 1;
- if (!is_refname_available(newrefname, oldrefname, get_loose_refs(NULL), 0))
+ if (!is_refname_available(newrefname, oldrefname, get_loose_refs(NULL)))
return 1;
lock = lock_ref_sha1_basic(renamed_ref, NULL, 0, NULL);
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 03/14] refs.c: rename some local "refname" variables
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Try to consistently use the variable name "refname" when referring to
a string naming a reference.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 276 +++++++++++++++++++++++++++++++++-------------------------------
refs.h | 26 ++++---
2 files changed, 157 insertions(+), 145 deletions(-)
diff --git a/refs.c b/refs.c
index e4e4bcd..2ae5d0d 100644
--- a/refs.c
+++ b/refs.c
@@ -49,7 +49,7 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
return line;
}
-static void add_ref(const char *name, const unsigned char *sha1,
+static void add_ref(const char *refname, const unsigned char *sha1,
int flag, struct ref_array *refs,
struct ref_entry **new_entry)
{
@@ -57,13 +57,13 @@ static void add_ref(const char *name, const unsigned char *sha1,
struct ref_entry *entry;
/* Allocate it and add it in.. */
- len = strlen(name) + 1;
+ len = strlen(refname) + 1;
entry = xmalloc(sizeof(struct ref_entry) + len);
hashcpy(entry->sha1, sha1);
hashclr(entry->peeled);
- if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
- die("Reference has invalid format: '%s'", name);
- memcpy(entry->name, name, len);
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
+ die("Reference has invalid format: '%s'", refname);
+ memcpy(entry->name, refname, len);
entry->flag = flag;
if (new_entry)
*new_entry = entry;
@@ -106,20 +106,20 @@ static void sort_ref_array(struct ref_array *array)
array->nr = i + 1;
}
-static struct ref_entry *search_ref_array(struct ref_array *array, const char *name)
+static struct ref_entry *search_ref_array(struct ref_array *array, const char *refname)
{
struct ref_entry *e, **r;
int len;
- if (name == NULL)
+ if (refname == NULL)
return NULL;
if (!array->nr)
return NULL;
- len = strlen(name) + 1;
+ len = strlen(refname) + 1;
e = xmalloc(sizeof(struct ref_entry) + len);
- memcpy(e->name, name, len);
+ memcpy(e->name, refname, len);
r = bsearch(&e, array->refs, array->nr, sizeof(*array->refs), ref_entry_cmp);
@@ -223,7 +223,7 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
while (fgets(refline, sizeof(refline), f)) {
unsigned char sha1[20];
- const char *name;
+ const char *refname;
static const char header[] = "# pack-refs with:";
if (!strncmp(refline, header, sizeof(header)-1)) {
@@ -234,9 +234,9 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
continue;
}
- name = parse_ref_line(refline, sha1);
- if (name) {
- add_ref(name, sha1, flag, array, &last);
+ refname = parse_ref_line(refline, sha1);
+ if (refname) {
+ add_ref(refname, sha1, flag, array, &last);
continue;
}
if (last &&
@@ -249,9 +249,9 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
sort_ref_array(array);
}
-void add_extra_ref(const char *name, const unsigned char *sha1, int flag)
+void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
{
- add_ref(name, sha1, flag, &extra_refs, NULL);
+ add_ref(refname, sha1, flag, &extra_refs, NULL);
}
void clear_extra_refs(void)
@@ -397,7 +397,8 @@ static struct ref_array *get_loose_refs(const char *submodule)
#define MAXDEPTH 5
#define MAXREFLEN (1024)
-static int resolve_gitlink_packed_ref(char *name, int pathlen, const char *refname, unsigned char *result)
+static int resolve_gitlink_packed_ref(char *name, int pathlen,
+ const char *refname, unsigned char *result)
{
int retval = -1;
struct ref_entry *ref;
@@ -411,7 +412,9 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen, const char *refna
return retval;
}
-static int resolve_gitlink_ref_recursive(char *name, int pathlen, const char *refname, unsigned char *result, int recursion)
+static int resolve_gitlink_ref_recursive(char *name, int pathlen,
+ const char *refname, unsigned char *result,
+ int recursion)
{
int fd, len = strlen(refname);
char buffer[128], *p;
@@ -478,10 +481,10 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
* Try to read ref from the packed references. On success, set sha1
* and return 0; otherwise, return -1.
*/
-static int get_packed_ref(const char *ref, unsigned char *sha1)
+static int get_packed_ref(const char *refname, unsigned char *sha1)
{
struct ref_array *packed = get_packed_refs(NULL);
- struct ref_entry *entry = search_ref_array(packed, ref);
+ struct ref_entry *entry = search_ref_array(packed, refname);
if (entry) {
hashcpy(sha1, entry->sha1);
return 0;
@@ -489,18 +492,18 @@ static int get_packed_ref(const char *ref, unsigned char *sha1)
return -1;
}
-const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag)
+const char *resolve_ref(const char *refname, unsigned char *sha1, int reading, int *flag)
{
int depth = MAXDEPTH;
ssize_t len;
char buffer[256];
- static char ref_buffer[256];
+ static char refname_buffer[256];
char path[PATH_MAX];
if (flag)
*flag = 0;
- if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
return NULL;
for (;;) {
@@ -511,7 +514,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (--depth < 0)
return NULL;
- git_snpath(path, sizeof(path), "%s", ref);
+ git_snpath(path, sizeof(path), "%s", refname);
if (lstat(path, &st) < 0) {
if (errno != ENOENT)
@@ -520,17 +523,17 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
* The loose reference file does not exist;
* check for a packed reference.
*/
- if (!get_packed_ref(ref, sha1)) {
+ if (!get_packed_ref(refname, sha1)) {
if (flag)
*flag |= REF_ISPACKED;
- return ref;
+ return refname;
}
/* The reference is not a packed reference, either. */
if (reading) {
return NULL;
} else {
hashclr(sha1);
- return ref;
+ return refname;
}
}
@@ -542,8 +545,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
buffer[len] = 0;
if (!prefixcmp(buffer, "refs/") &&
!check_refname_format(buffer, 0)) {
- strcpy(ref_buffer, buffer);
- ref = ref_buffer;
+ strcpy(refname_buffer, buffer);
+ refname = refname_buffer;
if (flag)
*flag |= REF_ISSYMREF;
continue;
@@ -584,7 +587,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
path);
return NULL;
}
- ref = strcpy(ref_buffer, buf);
+ refname = strcpy(refname_buffer, buf);
if (flag)
*flag |= REF_ISSYMREF;
}
@@ -593,7 +596,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
warning("reference in %s is formatted incorrectly", path);
return NULL;
}
- return ref;
+ return refname;
}
/* The argument to filter_refs */
@@ -603,9 +606,9 @@ struct ref_filter {
void *cb_data;
};
-int read_ref(const char *ref, unsigned char *sha1)
+int read_ref(const char *refname, unsigned char *sha1)
{
- if (resolve_ref(ref, sha1, 1, NULL))
+ if (resolve_ref(refname, sha1, 1, NULL))
return 0;
return -1;
}
@@ -629,23 +632,23 @@ static int do_one_ref(const char *base, each_ref_fn fn, int trim,
return fn(entry->name + trim, entry->sha1, entry->flag, cb_data);
}
-static int filter_refs(const char *ref, const unsigned char *sha, int flags,
+static int filter_refs(const char *refname, const unsigned char *sha, int flags,
void *data)
{
struct ref_filter *filter = (struct ref_filter *)data;
- if (fnmatch(filter->pattern, ref, 0))
+ if (fnmatch(filter->pattern, refname, 0))
return 0;
- return filter->fn(ref, sha, flags, filter->cb_data);
+ return filter->fn(refname, sha, flags, filter->cb_data);
}
-int peel_ref(const char *ref, unsigned char *sha1)
+int peel_ref(const char *refname, unsigned char *sha1)
{
int flag;
unsigned char base[20];
struct object *o;
- if (current_ref && (current_ref->name == ref
- || !strcmp(current_ref->name, ref))) {
+ if (current_ref && (current_ref->name == refname
+ || !strcmp(current_ref->name, refname))) {
if (current_ref->flag & REF_KNOWS_PEELED) {
hashcpy(sha1, current_ref->peeled);
return 0;
@@ -654,12 +657,12 @@ int peel_ref(const char *ref, unsigned char *sha1)
goto fallback;
}
- if (!resolve_ref(ref, base, 1, &flag))
+ if (!resolve_ref(refname, base, 1, &flag))
return -1;
if ((flag & REF_ISPACKED)) {
struct ref_array *array = get_packed_refs(NULL);
- struct ref_entry *r = search_ref_array(array, ref);
+ struct ref_entry *r = search_ref_array(array, refname);
if (r != NULL && r->flag & REF_KNOWS_PEELED) {
hashcpy(sha1, r->peeled);
@@ -670,7 +673,7 @@ int peel_ref(const char *ref, unsigned char *sha1)
fallback:
o = parse_object(base);
if (o && o->type == OBJ_TAG) {
- o = deref_tag(o, ref, 0);
+ o = deref_tag(o, refname, 0);
if (o) {
hashcpy(sha1, o->sha1);
return 0;
@@ -900,16 +903,16 @@ static inline int bad_ref_char(int ch)
}
/*
- * Try to read one refname component from the front of ref. Return
+ * Try to read one refname component from the front of refname. Return
* the length of the component found, or -1 if the component is not
* legal.
*/
-static int check_refname_component(const char *ref, int flags)
+static int check_refname_component(const char *refname, int flags)
{
const char *cp;
char last = '\0';
- for (cp = ref; ; cp++) {
+ for (cp = refname; ; cp++) {
char ch = *cp;
if (ch == '\0' || ch == '/')
break;
@@ -921,34 +924,34 @@ static int check_refname_component(const char *ref, int flags)
return -1; /* Refname contains "@{". */
last = ch;
}
- if (cp == ref)
+ if (cp == refname)
return -1; /* Component has zero length. */
- if (ref[0] == '.') {
+ if (refname[0] == '.') {
if (!(flags & REFNAME_DOT_COMPONENT))
return -1; /* Component starts with '.'. */
/*
* Even if leading dots are allowed, don't allow "."
* as a component (".." is prevented by a rule above).
*/
- if (ref[1] == '\0')
+ if (refname[1] == '\0')
return -1; /* Component equals ".". */
}
- if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
+ if (cp - refname >= 5 && !memcmp(cp - 5, ".lock", 5))
return -1; /* Refname ends with ".lock". */
- return cp - ref;
+ return cp - refname;
}
-int check_refname_format(const char *ref, int flags)
+int check_refname_format(const char *refname, int flags)
{
int component_len, component_count = 0;
while (1) {
/* We are at the start of a path component. */
- component_len = check_refname_component(ref, flags);
+ component_len = check_refname_component(refname, flags);
if (component_len < 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
- ref[0] == '*' &&
- (ref[1] == '\0' || ref[1] == '/')) {
+ refname[0] == '*' &&
+ (refname[1] == '\0' || refname[1] == '/')) {
/* Accept one wildcard as a full refname component. */
flags &= ~REFNAME_REFSPEC_PATTERN;
component_len = 1;
@@ -957,13 +960,13 @@ int check_refname_format(const char *ref, int flags)
}
}
component_count++;
- if (ref[component_len] == '\0')
+ if (refname[component_len] == '\0')
break;
/* Skip to next component. */
- ref += component_len + 1;
+ refname += component_len + 1;
}
- if (ref[component_len - 1] == '.')
+ if (refname[component_len - 1] == '.')
return -1; /* Refname ends with '.'. */
if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
return -1; /* Refname has only one component. */
@@ -1046,22 +1049,22 @@ static int remove_empty_directories(const char *file)
return result;
}
-static int is_refname_available(const char *ref, const char *oldref,
+static int is_refname_available(const char *refname, const char *oldrefname,
struct ref_array *array, int quiet)
{
- int i, namlen = strlen(ref); /* e.g. 'foo/bar' */
+ int i, namlen = strlen(refname); /* e.g. 'foo/bar' */
for (i = 0; i < array->nr; i++ ) {
struct ref_entry *entry = array->refs[i];
/* entry->name could be 'foo' or 'foo/bar/baz' */
- if (!oldref || strcmp(oldref, entry->name)) {
+ if (!oldrefname || strcmp(oldrefname, entry->name)) {
int len = strlen(entry->name);
int cmplen = (namlen < len) ? namlen : len;
- const char *lead = (namlen < len) ? entry->name : ref;
- if (!strncmp(ref, entry->name, cmplen) &&
+ const char *lead = (namlen < len) ? entry->name : refname;
+ if (!strncmp(refname, entry->name, cmplen) &&
lead[cmplen] == '/') {
if (!quiet)
error("'%s' exists; cannot create '%s'",
- entry->name, ref);
+ entry->name, refname);
return 0;
}
}
@@ -1069,10 +1072,12 @@ static int is_refname_available(const char *ref, const char *oldref,
return 1;
}
-static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int flags, int *type_p)
+static struct ref_lock *lock_ref_sha1_basic(const char *refname,
+ const unsigned char *old_sha1,
+ int flags, int *type_p)
{
char *ref_file;
- const char *orig_ref = ref;
+ const char *orig_refname = refname;
struct ref_lock *lock;
int last_errno = 0;
int type, lflags;
@@ -1082,27 +1087,27 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
lock = xcalloc(1, sizeof(struct ref_lock));
lock->lock_fd = -1;
- ref = resolve_ref(ref, lock->old_sha1, mustexist, &type);
- if (!ref && errno == EISDIR) {
+ refname = resolve_ref(refname, lock->old_sha1, mustexist, &type);
+ if (!refname && 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.
*/
- ref_file = git_path("%s", orig_ref);
+ ref_file = git_path("%s", orig_refname);
if (remove_empty_directories(ref_file)) {
last_errno = errno;
- error("there are still refs under '%s'", orig_ref);
+ error("there are still refs under '%s'", orig_refname);
goto error_return;
}
- ref = resolve_ref(orig_ref, lock->old_sha1, mustexist, &type);
+ refname = resolve_ref(orig_refname, lock->old_sha1, mustexist, &type);
}
if (type_p)
*type_p = type;
- if (!ref) {
+ if (!refname) {
last_errno = errno;
error("unable to resolve reference %s: %s",
- orig_ref, strerror(errno));
+ orig_refname, strerror(errno));
goto error_return;
}
missing = is_null_sha1(lock->old_sha1);
@@ -1112,7 +1117,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
* name is a proper prefix of our refname.
*/
if (missing &&
- !is_refname_available(ref, NULL, get_packed_refs(NULL), 0)) {
+ !is_refname_available(refname, NULL, get_packed_refs(NULL), 0)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -1121,12 +1126,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
lflags = LOCK_DIE_ON_ERROR;
if (flags & REF_NODEREF) {
- ref = orig_ref;
+ refname = orig_refname;
lflags |= LOCK_NODEREF;
}
- lock->ref_name = xstrdup(ref);
- lock->orig_ref_name = xstrdup(orig_ref);
- ref_file = git_path("%s", ref);
+ 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))
@@ -1147,20 +1152,21 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
return NULL;
}
-struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
+struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1)
{
char refpath[PATH_MAX];
- if (check_refname_format(ref, 0))
+ if (check_refname_format(refname, 0))
return NULL;
- strcpy(refpath, mkpath("refs/%s", ref));
+ strcpy(refpath, mkpath("refs/%s", refname));
return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
}
-struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
+struct ref_lock *lock_any_ref_for_update(const char *refname,
+ const unsigned char *old_sha1, int flags)
{
- if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
return NULL;
- return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
+ return lock_ref_sha1_basic(refname, old_sha1, flags, NULL);
}
static struct lock_file packlock;
@@ -1246,30 +1252,30 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
*/
#define TMP_RENAMED_LOG "logs/refs/.tmp-renamed-log"
-int rename_ref(const char *oldref, const char *newref, const char *logmsg)
+int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg)
{
static const char renamed_ref[] = "RENAMED-REF";
unsigned char sha1[20], orig_sha1[20];
int flag = 0, logmoved = 0;
struct ref_lock *lock;
struct stat loginfo;
- int log = !lstat(git_path("logs/%s", oldref), &loginfo);
+ int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
const char *symref = NULL;
if (log && S_ISLNK(loginfo.st_mode))
- return error("reflog for %s is a symlink", oldref);
+ return error("reflog for %s is a symlink", oldrefname);
- symref = resolve_ref(oldref, orig_sha1, 1, &flag);
+ symref = resolve_ref(oldrefname, orig_sha1, 1, &flag);
if (flag & REF_ISSYMREF)
return error("refname %s is a symbolic ref, renaming it is not supported",
- oldref);
+ oldrefname);
if (!symref)
- return error("refname %s not found", oldref);
+ return error("refname %s not found", oldrefname);
- if (!is_refname_available(newref, oldref, get_packed_refs(NULL), 0))
+ if (!is_refname_available(newrefname, oldrefname, get_packed_refs(NULL), 0))
return 1;
- if (!is_refname_available(newref, oldref, get_loose_refs(NULL), 0))
+ if (!is_refname_available(newrefname, oldrefname, get_loose_refs(NULL), 0))
return 1;
lock = lock_ref_sha1_basic(renamed_ref, NULL, 0, NULL);
@@ -1279,71 +1285,71 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
if (write_ref_sha1(lock, orig_sha1, logmsg))
return error("unable to save current sha1 in %s", renamed_ref);
- if (log && rename(git_path("logs/%s", oldref), git_path(TMP_RENAMED_LOG)))
+ if (log && rename(git_path("logs/%s", oldrefname), git_path(TMP_RENAMED_LOG)))
return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
- oldref, strerror(errno));
+ oldrefname, strerror(errno));
- if (delete_ref(oldref, orig_sha1, REF_NODEREF)) {
- error("unable to delete old %s", oldref);
+ if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
+ error("unable to delete old %s", oldrefname);
goto rollback;
}
- if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1, REF_NODEREF)) {
+ if (resolve_ref(newrefname, sha1, 1, &flag) && delete_ref(newrefname, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
- if (remove_empty_directories(git_path("%s", newref))) {
- error("Directory not empty: %s", newref);
+ if (remove_empty_directories(git_path("%s", newrefname))) {
+ error("Directory not empty: %s", newrefname);
goto rollback;
}
} else {
- error("unable to delete existing %s", newref);
+ error("unable to delete existing %s", newrefname);
goto rollback;
}
}
- if (log && safe_create_leading_directories(git_path("logs/%s", newref))) {
- error("unable to create directory for %s", newref);
+ if (log && safe_create_leading_directories(git_path("logs/%s", newrefname))) {
+ error("unable to create directory for %s", newrefname);
goto rollback;
}
retry:
- if (log && rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newref))) {
+ if (log && rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", newrefname))) {
if (errno==EISDIR || errno==ENOTDIR) {
/*
* rename(a, b) when b is an existing
* directory ought to result in ISDIR, but
* Solaris 5.8 gives ENOTDIR. Sheesh.
*/
- if (remove_empty_directories(git_path("logs/%s", newref))) {
- error("Directory not empty: logs/%s", newref);
+ if (remove_empty_directories(git_path("logs/%s", newrefname))) {
+ error("Directory not empty: logs/%s", newrefname);
goto rollback;
}
goto retry;
} else {
error("unable to move logfile "TMP_RENAMED_LOG" to logs/%s: %s",
- newref, strerror(errno));
+ newrefname, strerror(errno));
goto rollback;
}
}
logmoved = log;
- lock = lock_ref_sha1_basic(newref, NULL, 0, NULL);
+ lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL);
if (!lock) {
- error("unable to lock %s for update", newref);
+ 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", newref);
+ error("unable to write current sha1 into %s", newrefname);
goto rollback;
}
return 0;
rollback:
- lock = lock_ref_sha1_basic(oldref, NULL, 0, NULL);
+ lock = lock_ref_sha1_basic(oldrefname, NULL, 0, NULL);
if (!lock) {
- error("unable to lock %s for rollback", oldref);
+ error("unable to lock %s for rollback", oldrefname);
goto rollbacklog;
}
@@ -1351,17 +1357,17 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
flag = log_all_ref_updates;
log_all_ref_updates = 0;
if (write_ref_sha1(lock, orig_sha1, NULL))
- error("unable to write current sha1 into %s", oldref);
+ error("unable to write current sha1 into %s", oldrefname);
log_all_ref_updates = flag;
rollbacklog:
- if (logmoved && rename(git_path("logs/%s", newref), git_path("logs/%s", oldref)))
+ if (logmoved && rename(git_path("logs/%s", newrefname), git_path("logs/%s", oldrefname)))
error("unable to restore logfile %s from %s: %s",
- oldref, newref, strerror(errno));
+ oldrefname, newrefname, strerror(errno));
if (!logmoved && log &&
- rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldref)))
+ rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldrefname)))
error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s",
- oldref, strerror(errno));
+ oldrefname, strerror(errno));
return 1;
}
@@ -1418,16 +1424,16 @@ static int copy_msg(char *buf, const char *msg)
return cp - buf;
}
-int log_ref_setup(const char *ref_name, char *logfile, int bufsize)
+int log_ref_setup(const char *refname, char *logfile, int bufsize)
{
int logfd, oflags = O_APPEND | O_WRONLY;
- git_snpath(logfile, bufsize, "logs/%s", ref_name);
+ git_snpath(logfile, bufsize, "logs/%s", refname);
if (log_all_ref_updates &&
- (!prefixcmp(ref_name, "refs/heads/") ||
- !prefixcmp(ref_name, "refs/remotes/") ||
- !prefixcmp(ref_name, "refs/notes/") ||
- !strcmp(ref_name, "HEAD"))) {
+ (!prefixcmp(refname, "refs/heads/") ||
+ !prefixcmp(refname, "refs/remotes/") ||
+ !prefixcmp(refname, "refs/notes/") ||
+ !strcmp(refname, "HEAD"))) {
if (safe_create_leading_directories(logfile) < 0)
return error("unable to create directory for %s",
logfile);
@@ -1457,7 +1463,7 @@ int log_ref_setup(const char *ref_name, char *logfile, int bufsize)
return 0;
}
-static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
+static int log_ref_write(const char *refname, const unsigned char *old_sha1,
const unsigned char *new_sha1, const char *msg)
{
int logfd, result, written, oflags = O_APPEND | O_WRONLY;
@@ -1470,7 +1476,7 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
if (log_all_ref_updates < 0)
log_all_ref_updates = !is_bare_repository();
- result = log_ref_setup(ref_name, log_file, sizeof(log_file));
+ result = log_ref_setup(refname, log_file, sizeof(log_file));
if (result)
return result;
@@ -1641,7 +1647,9 @@ static char *ref_msg(const char *line, const char *endp)
return xmemdupz(line, ep - line);
}
-int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
+int read_ref_at(const char *refname, unsigned long at_time, int cnt,
+ unsigned char *sha1, char **msg,
+ unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
{
const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
char *tz_c;
@@ -1652,7 +1660,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
void *log_mapped;
size_t mapsz;
- logfile = git_path("logs/%s", ref);
+ logfile = git_path("logs/%s", refname);
logfd = open(logfile, O_RDONLY, 0);
if (logfd < 0)
die_errno("Unable to read log '%s'", logfile);
@@ -1745,14 +1753,14 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
return 1;
}
-int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long ofs, void *cb_data)
+int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long ofs, void *cb_data)
{
const char *logfile;
FILE *logfp;
struct strbuf sb = STRBUF_INIT;
int ret = 0;
- logfile = git_path("logs/%s", ref);
+ logfile = git_path("logs/%s", refname);
logfp = fopen(logfile, "r");
if (!logfp)
return -1;
@@ -1803,9 +1811,9 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long ofs,
return ret;
}
-int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
+int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_data)
{
- return for_each_recent_reflog_ent(ref, fn, 0, cb_data);
+ return for_each_recent_reflog_ent(refname, fn, 0, cb_data);
}
static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
@@ -1925,7 +1933,7 @@ static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
return;
}
-char *shorten_unambiguous_ref(const char *ref, int strict)
+char *shorten_unambiguous_ref(const char *refname, int strict)
{
int i;
static char **scanf_fmts;
@@ -1954,10 +1962,10 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
/* bail out if there are no rules */
if (!nr_rules)
- return xstrdup(ref);
+ return xstrdup(refname);
- /* buffer for scanf result, at most ref must fit */
- short_name = xstrdup(ref);
+ /* buffer for scanf result, at most refname must fit */
+ short_name = xstrdup(refname);
/* skip first rule, it will always match */
for (i = nr_rules - 1; i > 0 ; --i) {
@@ -1965,7 +1973,7 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
int rules_to_fail = i;
int short_name_len;
- if (1 != sscanf(ref, scanf_fmts[i], short_name))
+ if (1 != sscanf(refname, scanf_fmts[i], short_name))
continue;
short_name_len = strlen(short_name);
@@ -2010,5 +2018,5 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
}
free(short_name);
- return xstrdup(ref);
+ return xstrdup(refname);
}
diff --git a/refs.h b/refs.h
index f439c54..13e2aa3 100644
--- a/refs.h
+++ b/refs.h
@@ -59,14 +59,16 @@ extern void add_extra_ref(const char *refname, const unsigned char *sha1, int fl
extern void clear_extra_refs(void);
extern int ref_exists(const char *);
-extern int peel_ref(const char *, unsigned char *);
+extern int peel_ref(const char *refname, unsigned char *sha1);
/** Locks a "refs/" ref returning the lock on success and NULL on failure. **/
-extern struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1);
+extern struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char *old_sha1);
/** Locks any ref (for 'HEAD' type refs). */
#define REF_NODEREF 0x01
-extern struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags);
+extern struct ref_lock *lock_any_ref_for_update(const char *refname,
+ const unsigned char *old_sha1,
+ int flags);
/** Close the file descriptor owned by a lock and return the status */
extern int close_ref(struct ref_lock *lock);
@@ -92,12 +94,14 @@ extern void invalidate_ref_cache(const char *submodule);
int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
/** Reads log for the value of ref during at_time. **/
-extern int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt);
+extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,
+ unsigned char *sha1, char **msg,
+ unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt);
/* iterate over reflog entries */
typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, unsigned long, int, const char *, void *);
-int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data);
-int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, void *cb_data);
+int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_data);
+int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long, void *cb_data);
/*
* Calls the specified function for each reflog file until it returns nonzero,
@@ -110,9 +114,9 @@ extern int for_each_reflog(each_ref_fn, void *);
#define REFNAME_DOT_COMPONENT 4
/*
- * Return 0 iff ref has the correct format for a refname according to
- * the rules described in Documentation/git-check-ref-format.txt. If
- * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
+ * Return 0 iff refname has the correct format for a refname according
+ * to the rules described in Documentation/git-check-ref-format.txt.
+ * If REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
* reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
* allow a "*" wildcard character in place of one of the name
* components. No leading or repeated slashes are accepted. If
@@ -120,10 +124,10 @@ extern int for_each_reflog(each_ref_fn, void *);
* components to start with "." (but not a whole component equal to
* "." or "..").
*/
-extern int check_refname_format(const char *ref, int flags);
+extern int check_refname_format(const char *refname, int flags);
extern const char *prettify_refname(const char *refname);
-extern char *shorten_unambiguous_ref(const char *ref, int strict);
+extern char *shorten_unambiguous_ref(const char *refname, int strict);
/** rename ref, return 0 on success **/
extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 05/14] clear_ref_list(): rename from free_ref_list()
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Rename the function since it doesn't actually free the array object
that is passed to it.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index c466fcd..a2e48e4 100644
--- a/refs.c
+++ b/refs.c
@@ -149,7 +149,7 @@ static struct ref_entry *current_ref;
static struct ref_array extra_refs;
-static void free_ref_array(struct ref_array *array)
+static void clear_ref_array(struct ref_array *array)
{
int i;
for (i = 0; i < array->nr; i++)
@@ -162,14 +162,14 @@ static void free_ref_array(struct ref_array *array)
static void clear_cached_packed_refs(struct cached_refs *refs)
{
if (refs->did_packed)
- free_ref_array(&refs->packed);
+ clear_ref_array(&refs->packed);
refs->did_packed = 0;
}
static void clear_cached_loose_refs(struct cached_refs *refs)
{
if (refs->did_loose)
- free_ref_array(&refs->loose);
+ clear_ref_array(&refs->loose);
refs->did_loose = 0;
}
@@ -256,7 +256,7 @@ void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
void clear_extra_refs(void)
{
- free_ref_array(&extra_refs);
+ clear_ref_array(&extra_refs);
}
static struct ref_array *get_packed_refs(const char *submodule)
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 06/14] resolve_gitlink_ref(): improve docstring
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.h | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/refs.h b/refs.h
index c6b8749..d9c90cf 100644
--- a/refs.h
+++ b/refs.h
@@ -132,7 +132,11 @@ extern char *shorten_unambiguous_ref(const char *refname, int strict);
/** rename ref, return 0 on success **/
extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);
-/** resolve ref in nested "gitlink" repository */
+/**
+ * Resolve refname in the nested "gitlink" repository that is located
+ * at name. If the resolution is successful, return 0 and set sha1 to
+ * the name of the object; otherwise, return a non-zero value.
+ */
extern int resolve_gitlink_ref(const char *name, const char *refname, unsigned char *sha1);
/** lock a ref and then write its file */
--
1.7.7.rc2
^ permalink raw reply related
* [PATCH 00/14] Tidying up references code
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This is the next installment of the reference changes that I have been
working on. This batch includes a lot of tidying up in preparation
for the real changes.
The last few patches have a little bit of meat on them. They start
changing the innards of refs.c to work less with strings and more with
objects. This work will continue in later patches with the ultimate
goal of swapping the data structure used to store cached references
out from under the module--changing it from a sorted array of pointers
into a hierarchical tree shaped like the reference namespace
tree.
This patch series should be applied on top of "[PATCH v3] Provide API
to invalidate refs cache". It has textual dependencies on that patch
series, though logically I don't think that they interact.
Michael Haggerty (14):
cache.h: add comments for git_path() and git_path_submodule()
struct ref_list: document name member
refs.c: rename some local "refname" variables
refs: rename some parameters result -> sha1
clear_ref_list(): rename from free_ref_list()
resolve_gitlink_ref(): improve docstring
is_refname_available(): remove the "quiet" argument
parse_ref_line(): add docstring
add_ref(): add docstring
is_dup_ref(): extract function from sort_ref_list()
refs: change signatures of get_packed_refs() and get_loose_refs()
get_ref_dir(): change signature
Pass a (cached_refs *) to the resolve_gitlink_*() functions
resolve_gitlink_ref_recursive(): change to work with struct
cached_refs
cache.h | 15 +++
refs.c | 418 +++++++++++++++++++++++++++++++++-----------------------------
refs.h | 34 +++--
3 files changed, 258 insertions(+), 209 deletions(-)
--
1.7.7.rc2
^ permalink raw reply
* [PATCH 01/14] cache.h: add comments for git_path() and git_path_submodule()
From: mhagger @ 2011-10-13 7:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318492715-5931-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
cache.h | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/cache.h b/cache.h
index e39e160..b94855e 100644
--- a/cache.h
+++ b/cache.h
@@ -660,7 +660,22 @@ extern char *git_pathdup(const char *fmt, ...)
/* Return a statically allocated filename matching the sha1 signature */
extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+
+/*
+ * Return the path of a file within get_git_dir(). The arguments
+ * should be printf-like arguments that produce the filename relative
+ * to get_git_dir(). Return the resulting path, or "/bad-path/" if
+ * there is an error.
+ */
extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+
+/*
+ * Return the path of a file within the submodule located at path.
+ * The other arguments should be printf-like arguments that produce
+ * the filename relative to "<path>/.git". If "<path>/.git" is a
+ * gitlink file, follow it to find the actual submodule git path.
+ * Return the resulting path, or "/bad-path/" if there is an error.
+ */
extern char *git_path_submodule(const char *path, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
--
1.7.7.rc2
^ permalink raw reply related
* Re: [PATCH] http_init: accept separate URL parameter
From: Michael J Gruber @ 2011-10-13 7:26 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20111012224625.GA11408@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 13.10.2011 00:46:
> On Wed, Oct 12, 2011 at 03:38:27PM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> On Wed, Oct 12, 2011 at 05:43:16PM -0400, Jeff King wrote:
>>> ...
>>>> Instead, let's just add a separate URL parameter to
>>>> http_init, and all three callsites can pass in the
>>>> appropriate information.
>>>>
>>>> Signed-off-by: Jeff King <peff@peff.net>
>>>
>>> Sorry, I forgot to mention: this is meant to go on top of the
>>> http-auth-keyring topic.
>>
>> Hmm, of course the patch was written to help http-auth-keyring topic, but
>> wouldn't this be an improvement that is general enough? I.e. it could
>> even go to the bottom of the topic, no?
>
> Yes, it could, and probably should. I suspect it might need some
> rebasing to do that.
>
> I'm going to float some other possible designs for the topic as soon as
> I put enough polish on them. So I'll try to move this down when I
> re-roll. In the meantime, if you want to throw it on top, great. If you
> want to ignore it until then, no problem. :)
>
> -Peff
Thanks, Jeff.
To clarify:
Without http-auth-keyring, this helps in the sense that git reads the
username from a user@host URL and asks for the password only. When using
GIT_ASKPASS or such, the askpass helper is called with "Password:" only.
With (parts of) http-auth-keyring, the askpass helper is called with
"Password for:user@host", which helps the user identify the request, and
which helps helpers such as ksshaskpass to store the password with a
meaningful key in a wallet.
I'm not sure whether it's feasible/worth taking these bits of
http-auth-keyring (improved prompt) out and apply them early. That is,
I'm sure it's worth it (it would alleviate the need for credential
helpers for some users at least), I haven't looked at feasibility ;)
Michael
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Nguyen Thai Ngoc Duy @ 2011-10-13 7:02 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, git, Ilari Liusvaara, Junio C Hamano, Johannes Sixt
In-Reply-To: <CACsJy8C6o_-SM4oCM6o5-VDXFy5PBXsE0oL_uhYH1_Zk9h06QQ@mail.gmail.com>
On Thu, Oct 13, 2011 at 5:56 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> Translation could be fun to do
By translation, I don't mean inter-language translation. More like
personification. Instead of "service not enabled" you may want
"service is off, you want to attack me or what?"
--
Duy
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Nguyen Thai Ngoc Duy @ 2011-10-13 6:56 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Jeff King, git, Ilari Liusvaara, Junio C Hamano, Johannes Sixt
In-Reply-To: <20111013055924.GA24019@elie.hsd1.il.comcast.net>
On Thu, Oct 13, 2011 at 4:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyen Thai Ngoc Duy wrote:
>
>> How about allow users to select which messages they want to print? We
>> can even go further, allowing users to specify the messages themselves..
> [...]
>> + { "service not enabled", "message.serviceNotEnabled" },
>> + { "no such repository", "message.noSuchRepository" },
>> + { "repository not exported", "message.repositoryNotExported" },
>
> I administer a private server that is only accessible as "localhost".
> :) This much customization would leave me confused about what the
> right choices are and what the choices mean (even if I were to make
> the server public and start having security worries).
--informative-errors is your friend. All errors are enabled.
> What is the intended use --- translation? The idealist in me thinks
> that should be taken care of on the client side, if at all. (This
> way, we would not be preventing especially friendly clients from
> offering pertinent detailed advice for each error condition.
> Alternatively, maybe some day the protocol will want to provide a way
> for clients to indicate a preferred language and message verbosity.)
Translation could be fun to do, but it's more about how much admins
want to reveal. For example, I may only want to show "service not
enabled" and "no such repository", not the last one, which simply
becomes "access denied".
Again I'm not real admin and this may be just bogus.
--
Duy
^ permalink raw reply
* Re: [PATCH 1/6] Recognize magic pathspec as filenames
From: Junio C Hamano @ 2011-10-13 6:06 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8CY=myfLpAbEA0=LCm+tCgS7jzEAxH3rnwQt4-RXyjW9w@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> It's unfortunate that ":" has so many meanings attached to it.
Yeah, I still recall that there were people who said ":/$path looks
similar to the way how the object at the $path in the index is named" as
if it were an advantage. Maybe they were all misguided ;-).
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2011, #04; Wed, 12)
From: Jakub Narebski @ 2011-10-13 6:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Cord Seele, Cord Seele
In-Reply-To: <7vipnu9hbj.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> * cs/perl-config-path-send-email (2011-09-30) 2 commits
> (merged to 'next' on 2011-10-06 at 93c00f0)
> + use new Git::config_path() for aliasesfile
> + Add Git::config_path()
>
> Originally merged to 'next' on 2011-10-05.
> Will merge to 'master' in the second wave.
What about
. Refactor Git::config_*
from
[PATCH/RFC 3/2] Refactor Git::config_*
http://thread.gmane.org/gmane.comp.version-control.git/182310/focus=183111
Ahhh... sorry, it is marked as an RFC. Should I resend?
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jonathan Nieder @ 2011-10-13 5:59 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Jeff King, git, Ilari Liusvaara, Junio C Hamano, Johannes Sixt
In-Reply-To: <20111013044544.GA27890@duynguyen-vnpc.dek-tpc.internal>
Nguyen Thai Ngoc Duy wrote:
> How about allow users to select which messages they want to print? We
> can even go further, allowing users to specify the messages themselves..
[...]
> + { "service not enabled", "message.serviceNotEnabled" },
> + { "no such repository", "message.noSuchRepository" },
> + { "repository not exported", "message.repositoryNotExported" },
I administer a private server that is only accessible as "localhost".
:) This much customization would leave me confused about what the
right choices are and what the choices mean (even if I were to make
the server public and start having security worries).
What is the intended use --- translation? The idealist in me thinks
that should be taken care of on the client side, if at all. (This
way, we would not be preventing especially friendly clients from
offering pertinent detailed advice for each error condition.
Alternatively, maybe some day the protocol will want to provide a way
for clients to indicate a preferred language and message verbosity.)
^ permalink raw reply
* Re: [PATCH 3/3] t5403: do not use access repos with GIT_DIR when worktree is involved
From: Nguyen Thai Ngoc Duy @ 2011-10-13 4:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwrc95xe5.fsf@alter.siamese.dyndns.org>
2011/10/13 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Setting GIT_DIR alone means worktree is current directory for legacy
>> reasons. Avoid using that, instead go to the worktree and execute
>> commands there.
>>
>> The troublesome command is "GIT_DIR=clone2/.git git add clone2/b". The
>> real worktree is clone2, but that command tells git worktree is $(pwd).
>> What does user expect to add then? Should the new entry in index be "b"
>> or "clone2/b"?
>
> There is no troublesomeness here, as the semantics has been clearly
> defined and (hopefully) stayed constant before the days when GIT_WORK_TREE
> was invented.
It's not wrong per se. The trouble comes from the people who reads the
test. It's not clear the author wants to add "b" or "clone2/b".
Another thing, because "clone2" has .git inside, should "clone2" in
this case be considered a submodule and "git add" refuse to add
anything "clone2/*" but "clone2" itself?
> Unless we are trying to break them without knowing, and declare that we
> deprecated it after the fact, which is not exactly the way we want to
> remove existing (mis)feature.
I had problems with my read_directory() rewrite but it's been some
time since I touched the code. I don't remember what went wrong. Need
to have another look.
--
Duy
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Nguyen Thai Ngoc Duy @ 2011-10-13 4:45 UTC (permalink / raw)
To: Jeff King
Cc: git, Ilari Liusvaara, Junio C Hamano, Johannes Sixt,
Jonathan Nieder
In-Reply-To: <20111012200916.GA1502@sigill.intra.peff.net>
On Wed, Oct 12, 2011 at 04:09:16PM -0400, Jeff King wrote:
> On Tue, Oct 04, 2011 at 08:55:09AM +1100, Nguyen Thai Ngoc Duy wrote:
>
> > The message is chosen to avoid leaking information, yet let users know
> > that they are deliberately not allowed to use the service, not a fault
> > in service configuration or the service itself.
>
> I do think this is an improvement, but I wonder if the verbosity should
> be configurable. Then open sites like kernel.org could be friendlier to
> their users. Something like this instead:
How about allow users to select which messages they want to print? We
can even go further, allowing users to specify the messages themselves..
I don't know. I'm not a real server admin so maybe I'm just too
paranoid. Any admins care to speak up?
On the other hand, grouping all messages at one place may be easier to
audit, even if we don't allow customization.
Anyway, two cents on top of your patch..
-- 8< --
diff --git a/daemon.c b/daemon.c
index ec88fd0..a846ef1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -17,10 +17,25 @@
#define initgroups(x, y) (0) /* nothing */
#endif
+/* Must match messages[] order below */
+#define MSG_SERVICE_NOT_ENABLED 0
+#define MSG_NO_SUCH_REPOSITORY 1
+#define MSG_REPOSITORY_NOT_EXPORTED 2
+
+static struct daemon_message
+{
+ const char *message;
+ const char *config;
+ int enabled;
+} messages[] = {
+ { "service not enabled", "message.serviceNotEnabled" },
+ { "no such repository", "message.noSuchRepository" },
+ { "repository not exported", "message.repositoryNotExported" },
+};
+
static int log_syslog;
static int verbose;
static int reuseaddr;
-static int informative_errors;
static const char daemon_usage[] =
"git daemon [--verbose] [--syslog] [--export-all]\n"
@@ -238,20 +253,31 @@ static int service_enabled;
static int git_daemon_config(const char *var, const char *value, void *cb)
{
+ int i;
+
if (!prefixcmp(var, "daemon.") &&
!strcmp(var + 7, service_looking_at->config_name)) {
service_enabled = git_config_bool(var, value);
return 0;
}
+ for (i = 0; i < ARRAY_SIZE(messages); i++)
+ if (!strcmp(var, messages[i].config)) {
+ messages[i].enabled = git_config_bool(var, value);
+ return 0;
+ }
+
/* we are not interested in parsing any other configuration here */
return 0;
}
-static int daemon_error(const char *dir, const char *msg)
+static int daemon_error(const char *dir, int msg_id)
{
- if (!informative_errors)
+ const char *msg;
+ if (!messages[msg_id].enabled)
msg = "access denied";
+ else
+ msg = messages[msg_id].message;
packet_write(1, "ERR %s: %s", dir, msg);
return -1;
}
@@ -266,11 +292,11 @@ static int run_service(char *dir, struct daemon_service *service)
if (!enabled && !service->overridable) {
logerror("'%s': service not enabled.", service->name);
errno = EACCES;
- return daemon_error(dir, "service not enabled");
+ return daemon_error(dir, MSG_SERVICE_NOT_ENABLED);
}
if (!(path = path_ok(dir)))
- return daemon_error(dir, "no such repository");
+ return daemon_error(dir, MSG_NO_SUCH_REPOSITORY);
/*
* Security on the cheap.
@@ -286,7 +312,7 @@ static int run_service(char *dir, struct daemon_service *service)
if (!export_all_trees && access("git-daemon-export-ok", F_OK)) {
logerror("'%s': repository not exported.", path);
errno = EACCES;
- return daemon_error(dir, "repository not exported");
+ return daemon_error(dir, MSG_REPOSITORY_NOT_EXPORTED);
}
if (service->overridable) {
@@ -300,7 +326,7 @@ static int run_service(char *dir, struct daemon_service *service)
logerror("'%s': service not enabled for '%s'",
service->name, path);
errno = EACCES;
- return daemon_error(dir, "service not enabled");
+ return daemon_error(dir, MSG_SERVICE_NOT_ENABLED);
}
/*
@@ -1177,7 +1203,9 @@ int main(int argc, char **argv)
continue;
}
if (!prefixcmp(arg, "--informative-errors")) {
- informative_errors = 1;
+ int i;
+ for (i = 0; i < ARRAY_SIZE(messages); i++)
+ messages[i].enabled = 1;
continue;
}
if (!strcmp(arg, "--")) {
-- 8< --
^ permalink raw reply related
* Re: [PATCH 3/3] t5403: do not use access repos with GIT_DIR when worktree is involved
From: Junio C Hamano @ 2011-10-13 4:27 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1318412105-13595-3-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Setting GIT_DIR alone means worktree is current directory for legacy
> reasons. Avoid using that, instead go to the worktree and execute
> commands there.
>
> The troublesome command is "GIT_DIR=clone2/.git git add clone2/b". The
> real worktree is clone2, but that command tells git worktree is $(pwd).
> What does user expect to add then? Should the new entry in index be "b"
> or "clone2/b"?
There is no troublesomeness here, as the semantics has been clearly
defined and (hopefully) stayed constant before the days when GIT_WORK_TREE
was invented.
Assuming that there is no core.worktree in clone2/.git/config and there is
no GIT_WORK_TREE environment variable, "GIT_DIR=$anything git add
clone2/b" should add a path "clone2/b" to the index controlled by that
$GIT_DIR no matter where $anything is.
I would like to find out the motivation behind this patch. Even though I
find the ancient style stale and unsightly, we would want to keep the
(GIT_DIR is set, GIT_WORK_TREE is not set anywhere) combination working
for people who have work habits (read: old scripts) that rely on it. So we
would discourage new tests from using ancient style, but at the same time,
we would not want to remove _all_ existing ones.
Unless we are trying to break them without knowing, and declare that we
deprecated it after the fact, which is not exactly the way we want to
remove existing (mis)feature.
Same comment applies to the other patch that removes the test that uses
update-index && update-ref combination to a lessor degree.
The [PATCH 2/3] is a genuine improvement, though.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/6] Recognize magic pathspec as filenames
From: Nguyen Thai Ngoc Duy @ 2011-10-13 4:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nze7x61.fsf@alter.siamese.dyndns.org>
2011/10/13 Junio C Hamano <gitster@pobox.com>:
> "git log :/foo" is ambiguous no matter how you slice it, if you are going
> to look at only the first character in the string. It could be asking to
> show only commits that touch the path in the top-level directory "foo" and
> its subdirectories, or it could be asking to start traversal at a commit
> with "foo" in the log message.
Yeah, I forgot the ":/" ref syntax. But because it's ambiguous anyway,
should we disallow "git log :/foo"? Either "git log :/foo --" or "git
log -- :/foo" is OK.
> Longhand magic pathspecs e.g. ":(icase)foo" might have better chance, but
> not by a wide margin. It can be a rev that names the blob object in the
> index registered for the literal path "'(icase)foo", or it could be an
> element in the pathspec to match [Ff][Oo][[Oo].
It's unfortunate that ":" has so many meanings attached to it. I hope
that our ambiguation detection code can be smart, based on context, to
avoid unnecessary "--". For example, "git log :(icase)foo" can only
mean pathspec magic (I hope..)
--
Duy
^ permalink raw reply
* Re: [PATCH 1/3] t5403.1: simplify commit creation
From: Nguyen Thai Ngoc Duy @ 2011-10-13 3:54 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Junio C Hamano
In-Reply-To: <4E95A0BF.2060003@viscovery.net>
2011/10/13 Johannes Sixt <j.sixt@viscovery.net>:
> Am 10/12/2011 11:35, schrieb Nguyễn Thái Ngọc Duy:
>> test_expect_success setup '
>> echo Data for commit0. >a &&
>> echo Data for commit0. >b &&
>> - git update-index --add a &&
>> - git update-index --add b &&
>> - tree0=$(git write-tree) &&
>> - commit0=$(echo setup | git commit-tree $tree0) &&
>> - git update-ref refs/heads/master $commit0 &&
>> + git add a b &&
>> + git commit -m setup &&
>> git clone ./. clone1 &&
>> git clone ./. clone2 &&
>> GIT_DIR=clone2/.git git branch new2 &&
>
> I don't think this change is necessary. It doesn't hurt to use plumbing
> commands here and there in the test suite to exercise them to a degree
> that they deserve.
I'm fine either way, for the record.
--
Duy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox