* [PATCH 12/12] is_refname_available(): reimplement using do_for_each_ref_in_array()
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
This implementation will survive upcoming changes to the ref_array
data structure.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 44 ++++++++++++++++++++++++++++++++------------
1 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index 4bc6041..c41d995 100644
--- a/refs.c
+++ b/refs.c
@@ -1102,6 +1102,25 @@ static int names_conflict(const char *refname1, const char *refname2)
|| (*refname1 == '/' && *refname2 == '\0');
}
+struct name_conflict_cb {
+ const char *refname;
+ const char *oldrefname;
+ const char *conflicting_refname;
+};
+
+static int name_conflict_fn(const char *existingrefname, const unsigned char *sha1,
+ int flags, void *cb_data)
+{
+ struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
+ if (data->oldrefname && !strcmp(data->oldrefname, existingrefname))
+ return 0;
+ if (names_conflict(data->refname, existingrefname)) {
+ data->conflicting_refname = existingrefname;
+ return 1;
+ }
+ return 0;
+}
+
/*
* Return true iff a reference named refname could be created without
* conflicting with the name of an existing reference. If oldrefname
@@ -1112,18 +1131,19 @@ static int names_conflict(const char *refname1, const char *refname2)
static int is_refname_available(const char *refname, const char *oldrefname,
struct ref_array *array)
{
- int i;
- for (i = 0; i < array->nr; i++ ) {
- struct ref_entry *entry = array->refs[i];
- if (oldrefname && !strcmp(oldrefname, entry->name))
- continue;
- if (names_conflict(refname, entry->name)) {
- error("'%s' exists; cannot create '%s'",
- entry->name, refname);
- return 0;
- }
- }
- return 1;
+ struct name_conflict_cb data;
+ data.refname = refname;
+ data.oldrefname = oldrefname;
+ data.conflicting_refname = NULL;
+
+ if (do_for_each_ref_in_array(array, 0, "", name_conflict_fn,
+ 0, DO_FOR_EACH_INCLUDE_BROKEN,
+ &data)) {
+ error("'%s' exists; cannot create '%s'",
+ data.conflicting_refname, refname);
+ return 0;
+ }
+ return 1;
}
static struct ref_lock *lock_ref_sha1_basic(const char *refname,
--
1.7.7
^ permalink raw reply related
* Re: [RFC 04/13] add_ref(): move the call of check_refname_format() to callers
From: Junio C Hamano @ 2011-10-19 21:49 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <1319057716-28094-5-git-send-email-mhagger@alum.mit.edu>
mhagger@alum.mit.edu writes:
> I'm still not clear on how extra_refs are used. Are they generated
> from local refs or are they generated from remote refs? If the
> latter, then it is probably irresponsible not to do *some* sanity
> checking in add_extra_ref() to prevent any chance of refnames like
> "../../../etc/passwd".
No, add_extra_ref() already tells us what their values are, these are
never used to actually read from filesystem. Their refname field has
almost no value other than for debugging and we probably shouldn't even
insist on uniqueness among extra refs or for that matter collision with
the real refs. As I mentioned in an earlier message, their only raison
d'être is to be found by for_each_ref() that feeds revision machinery with
up to which commits we know we have complete histories for. A sample call
chain looks like this:
- cmd_clone()
- setup_reference()
- add_one_reference()
- add_extra_ref()
adds refs in other repositories we borrow from as "extra"
- transport_fetch_refs()
- fetch_refs_via_pack()
- fetch_pack()
- do_fetch_pack()
- find_common()
- for_each_ref(rev_list_insert_ref)
That way find_common() thinks histories leading to these extra refs are
already complete on our end (i.e. we have all the necessary objects), and
by subtracting that from what we are asking from the other end, we can
reduce the amount of history that needs to be transferred.
^ permalink raw reply
* Re: Compiling on Windows
From: Philip Oakley @ 2011-10-19 21:46 UTC (permalink / raw)
To: Andrew Ardill, Vincent van Ravesteijn; +Cc: Git MsysGit, git
In-Reply-To: <4E9E811C.10205@lyx.org>
From: "Vincent van Ravesteijn" <vfr@lyx.org>
> Op 18-10-2011 6:08, Andrew Ardill schreef:
>> Hi list, I have been searching for details on what is required to
>> compile on Windows, but haven't found anything conclusive. Perhaps
>> there is something on the wiki, but unfortunately it is down at the
>> moment.
>>
>> Can anyone point me in the right direction? I would like to be able to
>> compile and test topic branches, and perhaps even do some dev work on
>> my windows machine.
>
> I once wrote a little step-by-step tutorial on how to compile the native
> Windows Git with MSVC (Express).
>
> http://blog.vfrconsultancy.nl/#post0
The blog post filled in a few gaps in the Msysgit README instructions about
where to place the various downloads described.
>
> Be aware that git runs without apparent problems, but that all
> functionality written in shell scripts can't be used.
>
> HTH,
>
> Vincent
> --
Many thanks, it saved me asking.
Philip
^ permalink raw reply
* [PATCH 07/12] do_for_each_ref_in_array(): new function
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Extract function do_for_each_ref_in_array() from do_for_each_ref().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 39 +++++++++++++++++++++++----------------
1 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/refs.c b/refs.c
index 7aef76c..4e60edb 100644
--- a/refs.c
+++ b/refs.c
@@ -696,21 +696,31 @@ fallback:
return -1;
}
+static int do_for_each_ref_in_array(struct ref_array *array, int offset,
+ const char *base,
+ each_ref_fn fn, int trim, int flags, void *cb_data)
+{
+ int i;
+ for (i = offset; i < array->nr; i++) {
+ int retval = do_one_ref(base, fn, trim, flags, cb_data, array->refs[i]);
+ if (retval)
+ return retval;
+ }
+ return 0;
+}
+
static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
int trim, int flags, void *cb_data)
{
- int retval = 0, i, p = 0, l = 0;
+ int retval = 0, p = 0, l = 0;
struct ref_cache *refs = get_ref_cache(submodule);
struct ref_array *packed = get_packed_refs(refs);
struct ref_array *loose = get_loose_refs(refs);
- struct ref_array *extra = &extra_refs;
-
- for (i = 0; i < extra->nr; i++) {
- retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
- if (retval)
- goto end_each;
- }
+ retval = do_for_each_ref_in_array(&extra_refs, 0,
+ base, fn, trim, flags, cb_data);
+ if (retval)
+ goto end_each;
while (p < packed->nr && l < loose->nr) {
struct ref_entry *entry;
@@ -730,14 +740,11 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
}
if (l < loose->nr) {
- p = l;
- packed = loose;
- }
-
- for (; p < packed->nr; p++) {
- retval = do_one_ref(base, fn, trim, flags, cb_data, packed->refs[p]);
- if (retval)
- goto end_each;
+ retval = do_for_each_ref_in_array(loose, l,
+ base, fn, trim, flags, cb_data);
+ } else {
+ retval = do_for_each_ref_in_array(packed, p,
+ base, fn, trim, flags, cb_data);
}
end_each:
--
1.7.7
^ permalink raw reply related
* [PATCH 04/12] create_ref_entry(): extract function from add_ref()
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Separate the creation of the ref_entry from its addition to a ref_array.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 36 +++++++++++++++++++++---------------
1 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/refs.c b/refs.c
index 752938c..acb098c 100644
--- a/refs.c
+++ b/refs.c
@@ -57,27 +57,33 @@ 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)
+static struct ref_entry *create_ref_entry(const char *refname,
+ const unsigned char *sha1, int flag)
{
int len;
- struct ref_entry *entry;
+ struct ref_entry *ref;
- /* Allocate it and add it in.. */
- len = strlen(refname) + 1;
- entry = xmalloc(sizeof(struct ref_entry) + len);
- hashcpy(entry->sha1, sha1);
- hashclr(entry->peeled);
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;
+ len = strlen(refname) + 1;
+ ref = xmalloc(sizeof(struct ref_entry) + len);
+ hashcpy(ref->sha1, sha1);
+ hashclr(ref->peeled);
+ memcpy(ref->name, refname, len);
+ ref->flag = flag;
+ return ref;
+}
+
+/* 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_ref)
+{
+ struct ref_entry *ref = create_ref_entry(refname, sha1, flag);
+ if (new_ref)
+ *new_ref = ref;
ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
- refs->refs[refs->nr++] = entry;
+ refs->refs[refs->nr++] = ref;
}
static int ref_entry_cmp(const void *a, const void *b)
--
1.7.7
^ permalink raw reply related
* [PATCH 06/12] do_for_each_ref(): correctly terminate while processesing extra_refs
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
If the user-supplied function returns a nonzero value while processing
extra_refs, terminate without processing the rest of the list.
This probably has no practical importance, but makes the handling of
extra_refs a little bit more consistent with the handling of other
refs.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index ae90993..7aef76c 100644
--- a/refs.c
+++ b/refs.c
@@ -706,8 +706,11 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
struct ref_array *extra = &extra_refs;
- for (i = 0; i < extra->nr; i++)
+ for (i = 0; i < extra->nr; i++) {
retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
+ if (retval)
+ goto end_each;
+ }
while (p < packed->nr && l < loose->nr) {
struct ref_entry *entry;
--
1.7.7
^ permalink raw reply related
* [PATCH 09/12] repack_without_ref(): reimplement using do_for_each_ref_in_array()
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
It costs a bit of boilerplate, but it means that the function can be
ignorant of how cached refs are stored.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 46 ++++++++++++++++++++++++++++------------------
1 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/refs.c b/refs.c
index cd3acf8..7c2bcab 100644
--- a/refs.c
+++ b/refs.c
@@ -1213,36 +1213,46 @@ struct ref_lock *lock_any_ref_for_update(const char *refname,
return lock_ref_sha1_basic(refname, old_sha1, flags, NULL);
}
+struct repack_without_ref_sb {
+ const char *refname;
+ int fd;
+};
+
+static int repack_without_ref_fn(const char *refname, const unsigned char *sha1,
+ int flags, void *cb_data)
+{
+ struct repack_without_ref_sb *data = cb_data;
+ char line[PATH_MAX + 100];
+ int len;
+
+ if (!strcmp(data->refname, refname))
+ return 0;
+ len = snprintf(line, sizeof(line), "%s %s\n",
+ sha1_to_hex(sha1), refname);
+ /* this should not happen but just being defensive */
+ if (len > sizeof(line))
+ die("too long a refname '%s'", refname);
+ write_or_die(data->fd, line, len);
+ return 0;
+}
+
static struct lock_file packlock;
static int repack_without_ref(const char *refname)
{
+ struct repack_without_ref_sb data;
struct ref_array *packed;
- int fd, i;
packed = get_packed_refs(get_ref_cache(NULL));
if (search_ref_array(packed, refname) == NULL)
return 0;
- fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
- if (fd < 0) {
+ data.refname = refname;
+ data.fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
+ if (data.fd < 0) {
unable_to_lock_error(git_path("packed-refs"), errno);
return error("cannot delete '%s' from packed refs", refname);
}
-
- for (i = 0; i < packed->nr; i++) {
- char line[PATH_MAX + 100];
- int len;
- struct ref_entry *ref = packed->refs[i];
-
- if (!strcmp(refname, ref->name))
- continue;
- len = snprintf(line, sizeof(line), "%s %s\n",
- sha1_to_hex(ref->sha1), ref->name);
- /* this should not happen but just being defensive */
- if (len > sizeof(line))
- die("too long a refname '%s'", ref->name);
- write_or_die(fd, line, len);
- }
+ do_for_each_ref_in_array(packed, 0, "", repack_without_ref_fn, 0, 0, &data);
return commit_lock_file(&packlock);
}
--
1.7.7
^ permalink raw reply related
* [PATCH 08/12] do_for_each_ref_in_arrays(): new function
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Extract function do_for_each_ref_in_arrays() from do_for_each_ref().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 71 +++++++++++++++++++++++++++++++++++++--------------------------
1 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/refs.c b/refs.c
index 4e60edb..cd3acf8 100644
--- a/refs.c
+++ b/refs.c
@@ -709,45 +709,58 @@ static int do_for_each_ref_in_array(struct ref_array *array, int offset,
return 0;
}
-static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
- int trim, int flags, void *cb_data)
+static int do_for_each_ref_in_arrays(struct ref_array *array1,
+ struct ref_array *array2,
+ const char *base, each_ref_fn fn, int trim,
+ int flags, void *cb_data)
{
- int retval = 0, p = 0, l = 0;
- struct ref_cache *refs = get_ref_cache(submodule);
- struct ref_array *packed = get_packed_refs(refs);
- struct ref_array *loose = get_loose_refs(refs);
-
- retval = do_for_each_ref_in_array(&extra_refs, 0,
- base, fn, trim, flags, cb_data);
- if (retval)
- goto end_each;
+ int retval;
+ int i1 = 0, i2 = 0;
- while (p < packed->nr && l < loose->nr) {
- struct ref_entry *entry;
- int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
- if (!cmp) {
- p++;
+ while (1) {
+ struct ref_entry *e1, *e2;
+ int cmp;
+ if (i1 == array1->nr) {
+ return do_for_each_ref_in_array(array2, i2,
+ base, fn, trim, flags, cb_data);
+ }
+ if (i2 == array2->nr) {
+ return do_for_each_ref_in_array(array1, i1,
+ base, fn, trim, flags, cb_data);
+ }
+ e1 = array1->refs[i1];
+ e2 = array2->refs[i2];
+ cmp = strcmp(e1->name, e2->name);
+ if (cmp == 0) {
+ /* Two refs with the same name; ignore the one from array1. */
+ i1++;
continue;
}
- if (cmp > 0) {
- entry = loose->refs[l++];
+ if (cmp < 0) {
+ retval = do_one_ref(base, fn, trim, flags, cb_data, e1);
+ i1++;
} else {
- entry = packed->refs[p++];
+ retval = do_one_ref(base, fn, trim, flags, cb_data, e2);
+ i2++;
}
- retval = do_one_ref(base, fn, trim, flags, cb_data, entry);
if (retval)
- goto end_each;
+ return retval;
}
+}
- if (l < loose->nr) {
- retval = do_for_each_ref_in_array(loose, l,
- base, fn, trim, flags, cb_data);
- } else {
- retval = do_for_each_ref_in_array(packed, p,
- base, fn, trim, flags, cb_data);
- }
+static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
+ int trim, int flags, void *cb_data)
+{
+ int retval = 0;
+ struct ref_cache *refs = get_ref_cache(submodule);
+
+ retval = do_for_each_ref_in_array(&extra_refs, 0,
+ base, fn, trim, flags, cb_data);
+ if (!retval)
+ retval = do_for_each_ref_in_arrays(get_packed_refs(refs),
+ get_loose_refs(refs),
+ base, fn, trim, flags, cb_data);
-end_each:
current_ref = NULL;
return retval;
}
--
1.7.7
^ permalink raw reply related
* [PATCH 05/12] add_ref(): take a (struct ref_entry *) parameter
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Take a pointer to the ref_entry to add to the array, rather than
creating the ref_entry within the function. This opens the way to
having multiple kinds of ref_entries.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 14 +++++---------
1 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index acb098c..ae90993 100644
--- a/refs.c
+++ b/refs.c
@@ -75,13 +75,8 @@ static struct ref_entry *create_ref_entry(const char *refname,
}
/* 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_ref)
+static void add_ref(struct ref_array *refs, struct ref_entry *ref)
{
- struct ref_entry *ref = create_ref_entry(refname, sha1, flag);
- if (new_ref)
- *new_ref = ref;
ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
refs->refs[refs->nr++] = ref;
}
@@ -266,7 +261,8 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
refname = parse_ref_line(refline, sha1);
if (refname) {
- add_ref(refname, sha1, flag, array, &last);
+ last = create_ref_entry(refname, sha1, flag);
+ add_ref(array, last);
continue;
}
if (last &&
@@ -281,7 +277,7 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
{
- add_ref(refname, sha1, flag, &extra_refs, NULL);
+ add_ref(&extra_refs, create_ref_entry(refname, sha1, flag));
}
void clear_extra_refs(void)
@@ -368,7 +364,7 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
hashclr(sha1);
flag |= REF_BROKEN;
}
- add_ref(refname, sha1, flag, array, NULL);
+ add_ref(array, create_ref_entry(refname, sha1, flag));
}
free(refname);
closedir(dir);
--
1.7.7
^ permalink raw reply related
* [PATCH 03/12] parse_ref_line(): add a check that the refname is properly formatted
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-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 | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index 431d3c6..752938c 100644
--- a/refs.c
+++ b/refs.c
@@ -51,6 +51,9 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)
return NULL;
line[len] = 0;
+ if (check_refname_format(line, REFNAME_ALLOW_ONELEVEL))
+ return NULL;
+
return line;
}
--
1.7.7
^ permalink raw reply related
* [PATCH 01/12] Rename another local variable name -> refname
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
I missed this one earlier. (There are probably others too. I hope
you don't feel that I'm morally obliged to find every single analogous
example...)
refs.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index 7e6cea5..f6752a4 100644
--- a/refs.c
+++ b/refs.c
@@ -317,11 +317,11 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
if (dir) {
struct dirent *de;
int baselen = strlen(base);
- char *ref = xmalloc(baselen + 257);
+ char *refname = xmalloc(baselen + 257);
- memcpy(ref, base, baselen);
+ memcpy(refname, base, baselen);
if (baselen && base[baselen-1] != '/')
- ref[baselen++] = '/';
+ refname[baselen++] = '/';
while ((de = readdir(dir)) != NULL) {
unsigned char sha1[20];
@@ -337,31 +337,31 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
continue;
if (has_extension(de->d_name, ".lock"))
continue;
- memcpy(ref + baselen, de->d_name, namelen+1);
+ memcpy(refname + baselen, de->d_name, namelen+1);
refdir = *refs->name
- ? git_path_submodule(refs->name, "%s", ref)
- : git_path("%s", ref);
+ ? git_path_submodule(refs->name, "%s", refname)
+ : git_path("%s", refname);
if (stat(refdir, &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
- get_ref_dir(refs, ref, array);
+ get_ref_dir(refs, refname, array);
continue;
}
if (*refs->name) {
hashclr(sha1);
flag = 0;
- if (resolve_gitlink_ref(refs->name, ref, sha1) < 0) {
+ if (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {
hashclr(sha1);
flag |= REF_BROKEN;
}
} else
- if (!resolve_ref(ref, sha1, 1, &flag)) {
+ if (!resolve_ref(refname, sha1, 1, &flag)) {
hashclr(sha1);
flag |= REF_BROKEN;
}
- add_ref(ref, sha1, flag, array, NULL);
+ add_ref(refname, sha1, flag, array, NULL);
}
- free(ref);
+ free(refname);
closedir(dir);
}
}
--
1.7.7
^ permalink raw reply related
* [PATCH 02/12] repack_without_ref(): remove temporary
From: mhagger @ 2011-10-19 21:44 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: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Trivial and boring.
refs.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index f6752a4..431d3c6 100644
--- a/refs.c
+++ b/refs.c
@@ -1190,12 +1190,10 @@ static struct lock_file packlock;
static int repack_without_ref(const char *refname)
{
struct ref_array *packed;
- struct ref_entry *ref;
int fd, i;
packed = get_packed_refs(get_ref_cache(NULL));
- ref = search_ref_array(packed, refname);
- if (ref == NULL)
+ if (search_ref_array(packed, refname) == NULL)
return 0;
fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
if (fd < 0) {
@@ -1206,8 +1204,7 @@ static int repack_without_ref(const char *refname)
for (i = 0; i < packed->nr; i++) {
char line[PATH_MAX + 100];
int len;
-
- ref = packed->refs[i];
+ struct ref_entry *ref = packed->refs[i];
if (!strcmp(refname, ref->name))
continue;
--
1.7.7
^ permalink raw reply related
* [PATCH 00/12] Use refs API more consistently
From: mhagger @ 2011-10-19 21:44 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 patch series primarily has to do with using the refs API more
consistently within refs.c itself. We want to minimize the surface
area for accessing the ref_cache data structure so that when it is
changed into a tree (we're on the verge of that change now, I
promise!) we don't have to change callers more than necessary. There
is also a bit of an eat-your-own-dogfood thing going on here; if
for_each_ref() and its friends are good enough for "outsiders", they
should be good enough for most internal use.
The first two patches are trivial preparation cleanups.
The third verifies that refnames read from packed-ref files are
properly formatted. When the REFNAME_FULL changes becomes available,
this check can be tightened up.
The do_for_each_ref_in_{array,arrays}() functions provide an
iterator-like interface to iterating over a single ref_array and
iterating over two ref_arrays in parallel. These are useful
abstractions in and of themselves (the former is used to reimplement
repack_without_ref() and is_refname_available() in patches 9 and 12
respectively). And they will be even more useful when references are
stored hierarchically.
This series applies on top of mh/ref-api-2. It conflicts with the
"Checking full vs. partial refnames" series that I just submitted but
not in any fundamental way. I'll be happy to reconcile them for you
in a later round when it is clear if and when each series is accepted.
Michael Haggerty (12):
Rename another local variable name -> refname
repack_without_ref(): remove temporary
parse_ref_line(): add a check that the refname is properly formatted
create_ref_entry(): extract function from add_ref()
add_ref(): take a (struct ref_entry *) parameter
do_for_each_ref(): correctly terminate while processesing extra_refs
do_for_each_ref_in_array(): new function
do_for_each_ref_in_arrays(): new function
repack_without_ref(): reimplement using do_for_each_ref_in_array()
names_conflict(): new function, extracted from is_refname_available()
names_conflict(): simplify implementation
is_refname_available(): reimplement using do_for_each_ref_in_array()
refs.c | 259 ++++++++++++++++++++++++++++++++++++++++------------------------
1 files changed, 162 insertions(+), 97 deletions(-)
--
1.7.7
^ permalink raw reply
* [PATCH 6/6] revert: simplify communicating command-line arguments
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
From: Jonathan Nieder <jrnieder@gmail.com>
Currently, command-line arguments are communicated using (argc, argv)
until a prepare_revs() turns it into a terse structure. However,
since we plan to expose the cherry-picking machinery through a public
API in the future, we want callers to be able to call in with a
filled-in structure. For the revert builtin, this means that the
chief argument parser, parse_args(), should parse into such a
structure. Make this change.
[rr: minor improvements, commit message]
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 53 +++++++++++++++++++++++++++++------------------------
1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0201e98..576b9e7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,13 +54,14 @@ struct replay_opts {
int allow_rerere_auto;
int mainline;
- int commit_argc;
- const char **commit_argv;
/* Merge strategy */
const char *strategy;
const char **xopts;
size_t xopts_nr, xopts_alloc;
+
+ /* Only used by REPLAY_NONE */
+ struct rev_info *revs;
};
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -161,9 +162,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
die(_("program error"));
}
- opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
- PARSE_OPT_KEEP_ARGV0 |
- PARSE_OPT_KEEP_UNKNOWN);
+ argc = parse_options(argc, argv, NULL, options, usage_str,
+ PARSE_OPT_KEEP_ARGV0 |
+ PARSE_OPT_KEEP_UNKNOWN);
/* Check for incompatible subcommands */
verify_opt_mutually_compatible(me,
@@ -198,9 +199,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
NULL);
}
- else if (opts->commit_argc < 2)
- usage_with_options(usage_str, options);
-
if (opts->allow_ff)
verify_opt_compatible(me, "--ff",
"--signoff", opts->signoff,
@@ -208,7 +206,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
"-x", opts->record_origin,
"--edit", opts->edit,
NULL);
- opts->commit_argv = argv;
+
+ if (opts->subcommand == REPLAY_NONE) {
+ opts->revs = xmalloc(sizeof(*opts->revs));
+ init_revisions(opts->revs, NULL);
+ opts->revs->no_walk = 1;
+ if (argc < 2)
+ usage_with_options(usage_str, options);
+ argc = setup_revisions(argc, argv, opts->revs, NULL);
+ } else
+ opts->revs = NULL;
+
+ /* Forbid stray command-line arguments */
+ if (argc > 1)
+ usage_with_options(usage_str, options);
}
struct commit_message {
@@ -605,23 +616,15 @@ static int do_pick_commit(struct commit *commit, enum replay_action action,
return res;
}
-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
+static void prepare_revs(struct replay_opts *opts)
{
- int argc;
-
- init_revisions(revs, NULL);
- revs->no_walk = 1;
if (opts->action != REPLAY_REVERT)
- revs->reverse = 1;
+ opts->revs->reverse ^= 1;
- argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
- if (argc > 1)
- usage(*revert_or_cherry_pick_usage(opts));
-
- if (prepare_revision_walk(revs))
+ if (prepare_revision_walk(opts->revs))
die(_("revision walk setup failed"));
- if (!revs->commits)
+ if (!opts->revs->commits)
die(_("empty commit set passed"));
}
@@ -809,14 +812,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
struct replay_opts *opts)
{
- struct rev_info revs;
struct commit *commit;
struct replay_insn_list **next;
- prepare_revs(&revs, opts);
+ prepare_revs(opts);
next = todo_list;
- while ((commit = get_revision(&revs)))
+ while ((commit = get_revision(opts->revs)))
next = replay_insn_list_append(opts->action, commit, next);
}
@@ -940,6 +942,9 @@ static int pick_revisions(struct replay_opts *opts)
struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
+ if (opts->subcommand == REPLAY_NONE)
+ assert(opts->revs);
+
read_and_refresh_cache(opts);
/*
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 5/6] revert: allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
Parse the instruction sheet in '.git/sequencer/todo' as a list of
(action, operand) pairs, instead of assuming that all instructions use
the same action. Now you can do:
pick fdc0b12 picked
revert 965fed4 anotherpick
For cherry-pick and revert, this means that a 'git cherry-pick
--continue' can continue an ongoing revert operation and viceversa.
This patch lays the foundation for extending the parser to support
more actions so 'git rebase -i' can reuse this machinery in the
future. While at it, also improve the error messages reported by the
insn sheet parser.
Helped-by: Jonathan Nieder <jrnider@gmail.com>
Acked-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 130 +++++++++++++++++++--------------------
sequencer.h | 8 +++
t/t3510-cherry-pick-sequence.sh | 58 +++++++++++++++++
3 files changed, 129 insertions(+), 67 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8e34dc0..0201e98 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,6 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_action { REVERT, CHERRY_PICK };
enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
struct replay_opts {
@@ -68,14 +67,14 @@ struct replay_opts {
static const char *action_name(const struct replay_opts *opts)
{
- return opts->action == REVERT ? "revert" : "cherry-pick";
+ return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
}
static char *get_encoding(const char *message);
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
- return opts->action == REVERT ? revert_usage : cherry_pick_usage;
+ return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
}
static int option_parse_x(const struct option *opt,
@@ -152,7 +151,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
OPT_END(),
};
- if (opts->action == CHERRY_PICK) {
+ if (opts->action == REPLAY_PICK) {
struct option cp_extra[] = {
OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -346,7 +345,7 @@ static int error_dirty_index(struct replay_opts *opts)
return error_resolve_conflict(action_name(opts));
/* Different translation strings for cherry-pick and revert */
- if (opts->action == CHERRY_PICK)
+ if (opts->action == REPLAY_PICK)
error(_("Your local changes would be overwritten by cherry-pick."));
else
error(_("Your local changes would be overwritten by revert."));
@@ -450,7 +449,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
return run_command_v_opt(args, RUN_GIT_CMD);
}
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+ struct replay_opts *opts)
{
unsigned char head[20];
struct commit *base, *next, *parent;
@@ -525,7 +525,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REVERT) {
+ if (action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -568,7 +568,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
write_cherry_pick_head(commit);
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -587,7 +587,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (res) {
- error(opts->action == REVERT
+ error(action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -611,7 +611,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
init_revisions(revs, NULL);
revs->no_walk = 1;
- if (opts->action != REVERT)
+ if (opts->action != REPLAY_REVERT)
revs->reverse = 1;
argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
@@ -657,49 +657,53 @@ static void read_and_refresh_cache(struct replay_opts *opts)
* assert(commit_list_count(list) == 2);
* return list;
*/
-static struct commit_list **commit_list_append(struct commit *commit,
- struct commit_list **next)
+static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+ struct commit *operand,
+ struct replay_insn_list **next)
{
- struct commit_list *new = xmalloc(sizeof(struct commit_list));
- new->item = commit;
+ struct replay_insn_list *new = xmalloc(sizeof(*new));
+ new->action = action;
+ new->operand = operand;
*next = new;
new->next = NULL;
return &new->next;
}
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
- struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
{
- struct commit_list *cur = NULL;
- const char *sha1_abbrev = NULL;
- const char *action_str = opts->action == REVERT ? "revert" : "pick";
- const char *subject;
- int subject_len;
+ struct replay_insn_list *cur;
for (cur = todo_list; cur; cur = cur->next) {
- sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
- subject_len = find_commit_subject(cur->item->buffer, &subject);
+ const char *sha1_abbrev, *action_str, *subject;
+ int subject_len;
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ subject_len = find_commit_subject(cur->operand->buffer, &subject);
strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
subject_len, subject);
}
return 0;
}
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
+static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
{
unsigned char commit_sha1[20];
- enum replay_action action;
char *end_of_object_name;
int saved, status;
if (!prefixcmp(bol, "pick ")) {
- action = CHERRY_PICK;
+ item->action = REPLAY_PICK;
bol += strlen("pick ");
} else if (!prefixcmp(bol, "revert ")) {
- action = REVERT;
+ item->action = REPLAY_REVERT;
bol += strlen("revert ");
- } else
- return NULL;
+ } else {
+ size_t len = strchrnul(bol, '\n') - bol;
+ if (len > 255)
+ len = 255;
+ return error(_("Unrecognized action: %.*s"), (int)len, bol);
+ }
end_of_object_name = bol + strcspn(bol, " \n");
saved = *end_of_object_name;
@@ -707,37 +711,29 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
status = get_sha1(bol, commit_sha1);
*end_of_object_name = saved;
- /*
- * Verify that the action matches up with the one in
- * opts; we don't support arbitrary instructions
- */
- if (action != opts->action) {
- const char *action_str;
- action_str = action == REVERT ? "revert" : "cherry-pick";
- error(_("Cannot %s during a %s"), action_str, action_name(opts));
- return NULL;
- }
-
if (status < 0)
- return NULL;
+ return error(_("Malformed object name: %s"), bol);
- return lookup_commit_reference(commit_sha1);
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return error(_("Not a valid commit: %s"), bol);
+
+ item->next = NULL;
+ return 0;
}
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
- struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
{
- struct commit_list **next = todo_list;
- struct commit *commit;
+ struct replay_insn_list **next = todo_list;
+ struct replay_insn_list item = {0, NULL, NULL};
char *p = buf;
int i;
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- commit = parse_insn_line(p, eol, opts);
- if (!commit)
- return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ if (parse_insn_line(p, eol, &item) < 0)
+ return error(_("on line %d."), i);
+ next = replay_insn_list_append(item.action, item.operand, next);
p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
@@ -745,8 +741,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
return 0;
}
-static void read_populate_todo(struct commit_list **todo_list,
- struct replay_opts *opts)
+static void read_populate_todo(struct replay_insn_list **todo_list)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
struct strbuf buf = STRBUF_INIT;
@@ -762,7 +757,7 @@ static void read_populate_todo(struct commit_list **todo_list,
}
close(fd);
- res = parse_insn_buffer(buf.buf, todo_list, opts);
+ res = parse_insn_buffer(buf.buf, todo_list);
strbuf_release(&buf);
if (res)
die(_("Unusable instruction sheet: %s"), todo_file);
@@ -811,18 +806,18 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
die(_("Malformed options sheet: %s"), opts_file);
}
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
struct replay_opts *opts)
{
struct rev_info revs;
struct commit *commit;
- struct commit_list **next;
+ struct replay_insn_list **next;
prepare_revs(&revs, opts);
next = todo_list;
while ((commit = get_revision(&revs)))
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(opts->action, commit, next);
}
static int create_seq_dir(void)
@@ -851,7 +846,7 @@ static void save_head(const char *head)
die(_("Error wrapping up %s."), head_file);
}
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static void save_todo(struct replay_insn_list *todo_list)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
static struct lock_file todo_lock;
@@ -859,7 +854,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
int fd;
fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
- if (format_todo(&buf, todo_list, opts) < 0)
+ if (format_todo(&buf, todo_list) < 0)
die(_("Could not format %s."), todo_file);
if (write_in_full(fd, buf.buf, buf.len) < 0) {
strbuf_release(&buf);
@@ -903,9 +898,10 @@ static void save_opts(struct replay_opts *opts)
}
}
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list,
+ struct replay_opts *opts)
{
- struct commit_list *cur;
+ struct replay_insn_list *cur;
int res;
setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -915,8 +911,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
read_and_refresh_cache(opts);
for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur, opts);
- res = do_pick_commit(cur->item, opts);
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, opts);
if (res) {
if (!cur->next)
/*
@@ -941,7 +937,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
static int pick_revisions(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
read_and_refresh_cache(opts);
@@ -958,7 +954,7 @@ static int pick_revisions(struct replay_opts *opts)
if (!file_exists(git_path(SEQ_TODO_FILE)))
goto error;
read_populate_opts(&opts);
- read_populate_todo(&todo_list, opts);
+ read_populate_todo(&todo_list);
/* Verify that the conflict has been resolved */
if (!index_differs_from("HEAD", 0))
@@ -978,7 +974,7 @@ static int pick_revisions(struct replay_opts *opts)
return -1;
}
if (get_sha1("HEAD", sha1)) {
- if (opts->action == REVERT)
+ if (opts->action == REPLAY_REVERT)
return error(_("Can't revert as initial commit"));
return error(_("Can't cherry-pick into empty head"));
}
@@ -998,7 +994,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
- opts.action = REVERT;
+ opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
@@ -1013,7 +1009,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
int res;
memset(&opts, 0, sizeof(opts));
- opts.action = CHERRY_PICK;
+ opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
diff --git a/sequencer.h b/sequencer.h
index 905d295..f4db257 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,6 +7,14 @@
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
+enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+
+struct replay_insn_list {
+ enum replay_action action;
+ struct commit *operand;
+ struct replay_insn_list *next;
+};
+
/*
* Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
* any errors. Intended to be used by 'git reset'.
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 39b55c1..4b12244 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -239,4 +239,62 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_line_count = 4 commits
'
+test_expect_success 'revert --continue continues after cherry-pick' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git revert --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'mixed pick and revert instructions' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ oldsha=`git rev-parse --short HEAD~1` &&
+ echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 4/6] revert: make commit subjects in insn sheet optional
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
Change the instruction sheet format subtly so that the subject of the
commit message that follows the object name is optional. As a result,
an instruction sheet like this is now perfectly valid:
pick 35b0426
pick fbd5bbcbc2e
pick 7362160f
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 37 ++++++++++++++++---------------------
t/t3510-cherry-pick-sequence.sh | 14 ++++++++++++++
2 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 474dda1..8e34dc0 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -685,31 +685,27 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
return 0;
}
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
{
unsigned char commit_sha1[20];
- char sha1_abbrev[40];
enum replay_action action;
- int insn_len = 0;
- char *p, *q;
+ char *end_of_object_name;
+ int saved, status;
- if (!prefixcmp(start, "pick ")) {
+ if (!prefixcmp(bol, "pick ")) {
action = CHERRY_PICK;
- insn_len = strlen("pick");
- p = start + insn_len + 1;
- } else if (!prefixcmp(start, "revert ")) {
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ")) {
action = REVERT;
- insn_len = strlen("revert");
- p = start + insn_len + 1;
+ bol += strlen("revert ");
} else
return NULL;
- q = strchr(p, ' ');
- if (!q || q - p + 1 > sizeof(sha1_abbrev))
- return NULL;
- q++;
-
- strlcpy(sha1_abbrev, p, q - p);
+ end_of_object_name = bol + strcspn(bol, " \n");
+ saved = *end_of_object_name;
+ *end_of_object_name = '\0';
+ status = get_sha1(bol, commit_sha1);
+ *end_of_object_name = saved;
/*
* Verify that the action matches up with the one in
@@ -722,7 +718,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
return NULL;
}
- if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+ if (status < 0)
return NULL;
return lookup_commit_reference(commit_sha1);
@@ -737,13 +733,12 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
int i;
for (i = 1; *p; i++) {
- commit = parse_insn_line(p, opts);
+ char *eol = strchrnul(p, '\n');
+ commit = parse_insn_line(p, eol, opts);
if (!commit)
return error(_("Could not parse line %d."), i);
next = commit_list_append(commit, next);
- p = strchrnul(p, '\n');
- if (*p)
- p++;
+ p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
return error(_("No commits parsed."));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 2113308..39b55c1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -225,4 +225,18 @@ test_expect_success 'malformed instruction sheet 3' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'commit descriptions in insn sheet are optional' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git rev-list HEAD >commits
+ test_line_count = 4 commits
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 3/6] revert: fix buffer overflow in insn sheet parser
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
Check that the commit name argument to a "pick" or "revert" action in
'.git/sequencer/todo' is not too long, to avoid overflowing an
on-stack buffer. This fixes a regression introduced by 5a5d80f4
(revert: Introduce --continue to continue the operation, 2011-08-04).
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 2 +-
t/t3510-cherry-pick-sequence.sh | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index acb357d..474dda1 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -705,7 +705,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
return NULL;
q = strchr(p, ' ');
- if (!q)
+ if (!q || q - p + 1 > sizeof(sha1_abbrev))
return NULL;
q++;
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3..2113308 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -12,6 +12,9 @@ test_description='Test cherry-pick continuation features
. ./test-lib.sh
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
pristine_detach () {
git cherry-pick --reset &&
git checkout -f "$1^0" &&
@@ -211,4 +214,15 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'malformed instruction sheet 3' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git cherry-pick --continue
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 2/6] revert: simplify getting commit subject in format_todo()
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
format_todo() calls get_message(), but uses only the subject line of
the commit message. Save work and unnecessary memory allocations by
using find_commit_subject() instead.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 94b7c50..acb357d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -671,16 +671,16 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
struct replay_opts *opts)
{
struct commit_list *cur = NULL;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
const char *sha1_abbrev = NULL;
const char *action_str = opts->action == REVERT ? "revert" : "pick";
+ const char *subject;
+ int subject_len;
for (cur = todo_list; cur; cur = cur->next) {
sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
- if (get_message(cur->item, &msg))
- return error(_("Cannot get commit message for %s"), sha1_abbrev);
- strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
- free_message(&msg);
+ subject_len = find_commit_subject(cur->item->buffer, &subject);
+ strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+ subject_len, subject);
}
return 0;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 1/6] revert: free msg in format_todo()
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
Memory allocated to the fields of msg by get_message() isn't freed.
This is potentially a big leak, because fresh memory is allocated to
store the commit message for each commit. Fix this using
free_message().
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 010508d..94b7c50 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -680,6 +680,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
if (get_message(cur->item, &msg))
return error(_("Cannot get commit message for %s"), sha1_abbrev);
strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+ free_message(&msg);
}
return 0;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH v2 0/6] Sequencer fixups mini-series
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
Hi,
This is the second iteration of series I first posted in
$gmane/183157. The improvements can be summarized as follows:
1. Junio wrote some extra code to squash into the fourth part. It
took me some time to resolve conflicts correctly while rebasing.
2. Jonathan and Tay (off-list) have helped improve all the commit
messages.
Thanks.
Jonathan Nieder (1):
revert: simplify communicating command-line arguments
Ramkumar Ramachandra (5):
revert: free msg in format_todo()
revert: simplify getting commit subject in format_todo()
revert: fix buffer overflow in insn sheet parser
revert: make commit subjects in insn sheet optional
revert: allow mixed pick and revert instructions
builtin/revert.c | 219 +++++++++++++++++++--------------------
sequencer.h | 8 ++
t/t3510-cherry-pick-sequence.sh | 86 +++++++++++++++
3 files changed, 202 insertions(+), 111 deletions(-)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply
* [RFC 12/13] resolve_ref: use check_refname_format(..., REFNAME_FULL)
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
...instead of own inline code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index a40dfa5..67df3a6 100644
--- a/refs.c
+++ b/refs.c
@@ -548,8 +548,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (len < 0)
return NULL;
buffer[len] = 0;
- if (!prefixcmp(buffer, "refs/") &&
- !check_refname_format(buffer, 0)) {
+ if (!check_refname_format(buffer, REFNAME_FULL)) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.7
^ permalink raw reply related
* [RFC 13/13] filter_refs(): the refname is full, so use REFNAME_FULL option
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/fetch-pack.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index c6bc8eb..d72aa44 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -546,7 +546,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5) &&
- check_refname_format(ref->name + 5, 0))
+ check_refname_format(ref->name, REFNAME_FULL))
; /* trash */
else if (args.fetch_all &&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
--
1.7.7
^ permalink raw reply related
* [RFC 11/13] replace_object(): the refname is full, so use REFNAME_FULL option
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/replace.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 517fa10..f5d4f77 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
"refs/replace/%s",
sha1_to_hex(object)) > sizeof(ref) - 1)
die("replace ref name too long: %.*s...", 50, ref);
- if (check_refname_format(ref, 0))
+ if (check_refname_format(ref, REFNAME_FULL))
die("'%s' is not a valid ref name.", ref);
if (!resolve_ref(ref, prev, 1, NULL))
--
1.7.7
^ permalink raw reply related
* [RFC 10/13] strbuf_check_tag_ref(): the refname is full, so use REFNAME_FULL option
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/tag.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 9b6fd95..abf3cb0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -412,7 +412,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
strbuf_reset(sb);
strbuf_addf(sb, "refs/tags/%s", name);
- return check_refname_format(sb->buf, 0);
+ return check_refname_format(sb->buf, REFNAME_FULL);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
--
1.7.7
^ permalink raw reply related
* [RFC 01/13] check_refname_component(): iterate via index rather than via pointer
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
This will make later changes easier.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 0d74d70..3b41252 100644
--- a/refs.c
+++ b/refs.c
@@ -912,11 +912,11 @@ static inline int bad_ref_char(int ch)
*/
static int check_refname_component(const char *ref, int flags)
{
- const char *cp;
+ int i, len = strlen(ref);
char last = '\0';
- for (cp = ref; ; cp++) {
- char ch = *cp;
+ for (i = 0; i < len; i++) {
+ char ch = ref[i];
if (ch == '\0' || ch == '/')
break;
if (bad_ref_char(ch))
@@ -927,7 +927,7 @@ static int check_refname_component(const char *ref, int flags)
return -1; /* Refname contains "@{". */
last = ch;
}
- if (cp == ref)
+ if (i == 0)
return -1; /* Component has zero length. */
if (ref[0] == '.') {
if (!(flags & REFNAME_DOT_COMPONENT))
@@ -936,12 +936,12 @@ static int check_refname_component(const char *ref, int flags)
* Even if leading dots are allowed, don't allow "."
* as a component (".." is prevented by a rule above).
*/
- if (ref[1] == '\0')
+ if (i == 1)
return -1; /* Component equals ".". */
}
- if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
+ if (i >= 5 && !memcmp(&ref[i - 5], ".lock", 5))
return -1; /* Refname ends with ".lock". */
- return cp - ref;
+ return i;
}
int check_refname_format(const char *ref, int flags)
--
1.7.7
^ permalink raw reply related
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