Re: [PATCH v2 0/3] Towards a useable git-branch
On Sat, May 25, 2013 at 5:51 AM, Duy Nguyen pclo...@gmail.com wrote: I just had an idea that might bring pretty stuff to for-each-ref with probably reasonable effort, making for-each-ref format a superset of pretty. But I need to clean up my backlog first. Give me a few days, I will show you something (or give up ;) And I can't get it out of my head. Might as well write it down. Check out https://github.com/pclouds/git.git for-each-ref-pretty It opens a hook in format_commit_message() to plug f-e-r's atom syntax in. I didn't do extensive test or anything, just t6300. The %xx syntax in for-each-ref might override some placeholders in pretty, I did not check. You can add extra %(xx) on top as you have done. I still need one more hook to support %(*) with automatic width detection. After that I'm quite confident we can kill -v/-vv code. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] fast-export: add new --refspec option
On Fri, May 24, 2013 at 10:47 PM, Felipe Contreras felipe.contre...@gmail.com wrote: So that we can covert the exported ref names. s/covert/convert/ Signed-off-by: Felipe Contreras felipe.contre...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 27/48] remote-hg: add test for new bookmark special
On Fri, May 24, 2013 at 10:29 PM, Felipe Contreras felipe.contre...@gmail.com wrote: From the point of view of Mercurial, this creates a new branch head, and requires a forced push. Ideally, whoever, we would want it to work just like in git; new s/whoever/however/ branches can be pushed without problems. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 25/48] remote-hg: add test for diverged push
On Fri, May 24, 2013 at 10:29 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Neither mercurial nor git allows pushing to a remote when it's a non-fast-forward push. We should be able to detect theses errors and s/theses/these/ report them properly. As opposed to throwing an exception stack-trace. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color
On Fri, May 24, 2013 at 10:19 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Since 'git branch' misses important options like --sort, --count, and --format that are present in 'git for-each-ref'. Until we are in a position to fix 'git branch', let us enhance the 'git for-each-ref' format so it can atleast colorize output. s/atleast/at least/ You can use the following format in for-each-ref: %C(green)%(refname:short)%C(reset) To display refs in green. Future patches will attempt to extend the format even more to get useful output. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/25] Remove assumptions about each_ref_fn arg lifetimes
This is version two of the patch series. Aside from addressing Junio's comments about the first version, it goes significantly further than v1: I did a manual audit of the 50 (!) functions that are used as an each_ref_fn callback to the for_each_ref()-style functions. (I hope I haven't missed any.) I checked that they do not make the assumption that the lifetimes of the refname and sha1 arguments extend past the duration of the callback invocation. There were a number of callers that got this wrong; I believe I have fixed them all. I also changed how object_array_entry manages its name field. Like the RFC in the first version of this patch series, I change add_object_array_with_mode() to make a copy of the name before storing it in its field. But (at Peff's suggestion) I also make an optimization like that of strbuf of not copying the name if it is the empty string, but rather using a pointer to a static empty string. After this patch series, the test suite runs without errors under valgrind even when I change refs.c:do_one_ref() to pass temporary copies of refname and sha1 to the callback functions then free them immediately. (Before this patch series, such a test failed in many places.) But it is still not enough to make Peff's jk/packed-refs-race series work correctly under the hyperactive repository stress-test in which the packed-refs file is made to always look stale, refs.c:get_packed_refs(): - if (refs-packed - !stat_validity_check(refs-packed_validity, packed_refs_file)) + if (refs-packed /* + !stat_validity_check(refs-packed_validity, packed_refs_file) */) Michael Haggerty (25): describe: make own copy of refname fetch: make own copies of refnames add_rev_cmdline(): make a copy of the name argument builtin_diff_tree(): make it obvious that function wants two entries cmd_diff(): use an object_array for holding trees cmd_diff(): rename local variable list - entry cmd_diff(): make it obvious which cases are exclusive of each other revision: split some overly-long lines object_array: add function object_array_filter() revision: use object_array_filter() in implementation of gc_boundary() object_array_remove_duplicates(): rewrite to reduce copying fsck: don't put a void*-shaped peg in a char*-shaped hole find_first_merges(): initialize merges variable using initializer find_first_merges(): remove unnecessary code object_array_entry: fix memory handling of the name field do_fetch(): reduce scope of peer_item do_fetch(): clean up existing_refs before exiting add_existing(): do not retain a reference to sha1 show_head_ref(): do not shadow name of argument show_head_ref(): rename first parameter to refname string_list_add_one_ref(): rename first parameter to refname string_list_add_refs_by_glob(): add a comment about memory management exclude_existing(): set existing_refs.strdup_strings register_ref(): make a copy of the bad reference SHA-1 refs: document the lifetime of the args passed to each_ref_fn bisect.c | 5 ++-- builtin/describe.c | 6 +++-- builtin/diff.c | 69 ++--- builtin/fetch.c| 29 -- builtin/fsck.c | 2 +- builtin/show-ref.c | 2 +- bundle.c | 2 +- http-backend.c | 6 ++--- notes.c| 9 --- object.c | 70 -- object.h | 25 +-- refs.h | 22 - revision.c | 64 + revision.h | 32 - submodule.c| 6 ++--- 15 files changed, 228 insertions(+), 121 deletions(-) -- 1.8.2.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/25] describe: make own copy of refname
Do not retain a reference to the refname passed to the each_ref_fn
callback get_name(), because there is no guarantee of the lifetimes of
these names. Instead, make a local copy when needed.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/describe.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 6636a68..3dc09eb 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -42,7 +42,7 @@ struct commit_name {
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
unsigned char sha1[20];
- const char *path;
+ char *path;
};
static const char *prio_names[] = {
head, lightweight, annotated,
@@ -126,12 +126,14 @@ static void add_to_known_names(const char *path,
} else {
e-next = NULL;
}
+ e-path = NULL;
}
e-tag = tag;
e-prio = prio;
e-name_checked = 0;
hashcpy(e-sha1, sha1);
- e-path = path;
+ free(e-path);
+ e-path = xstrdup(path);
}
}
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/25] cmd_diff(): rename local variable list - entry
It's not a list, it's an array entry.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/diff.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index 661fdde..84243d9 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -339,9 +339,9 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
}
for (i = 0; i rev.pending.nr; i++) {
- struct object_array_entry *list = rev.pending.objects+i;
- struct object *obj = list-item;
- const char *name = list-name;
+ struct object_array_entry *entry = rev.pending.objects[i];
+ struct object *obj = entry-item;
+ const char *name = entry-name;
int flags = (obj-flags UNINTERESTING);
if (!obj-parsed)
obj = parse_object(obj-sha1);
@@ -360,7 +360,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
die(_(more than two blobs given: '%s'), name);
hashcpy(blob[blobs].sha1, obj-sha1);
blob[blobs].name = name;
- blob[blobs].mode = list-mode;
+ blob[blobs].mode = entry-mode;
blobs++;
continue;
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/25] revision: use object_array_filter() in implementation of gc_boundary()
Use object_array_filter(), which will soon be made smarter about
cleaning up discarded entries properly. Also add a function comment.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
This version changes the test to nr == alloc for clarity, but
doesn't move the test to the caller as did v1 of the patch series.
revision.c | 32 +++-
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/revision.c b/revision.c
index 8ac88d6..be73cb4 100644
--- a/revision.c
+++ b/revision.c
@@ -2435,25 +2435,23 @@ static struct commit *get_revision_1(struct rev_info
*revs)
return NULL;
}
-static void gc_boundary(struct object_array *array)
+/*
+ * Return true for entries that have not yet been shown. (This is an
+ * object_array_each_func_t.)
+ */
+static int entry_unshown(struct object_array_entry *entry, void
*cb_data_unused)
{
- unsigned nr = array-nr;
- unsigned alloc = array-alloc;
- struct object_array_entry *objects = array-objects;
+ return !(entry-item-flags SHOWN);
+}
- if (alloc = nr) {
- unsigned i, j;
- for (i = j = 0; i nr; i++) {
- if (objects[i].item-flags SHOWN)
- continue;
- if (i != j)
- objects[j] = objects[i];
- j++;
- }
- for (i = j; i nr; i++)
- objects[i].item = NULL;
- array-nr = j;
- }
+/*
+ * If array is on the verge of a realloc, garbage-collect any entries
+ * that have already been shown to try to free up some space.
+ */
+static void gc_boundary(struct object_array *array)
+{
+ if (array-nr == array-alloc)
+ object_array_filter(array, entry_unshown, NULL);
}
static void create_boundary_commit_list(struct rev_info *revs)
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/25] object_array_remove_duplicates(): rewrite to reduce copying
The old version copied one entry to its destination position, then
deleted any matching entries from the tail of the array. This
required the tail of the array to be copied multiple times. It didn't
affect the complexity of the algorithm because the whole tail has to
be searched through anyway. But all the copying was unnecessary.
Instead, check for the existence of an entry with the same name in the
*head* of the list before copying an entry to its final position.
This way each entry has to be copied at most one time.
Extract a helper function contains_name() to do a bit of the work.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
object.c | 32 +---
object.h | 6 +-
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/object.c b/object.c
index fcd4a82..10b5349 100644
--- a/object.c
+++ b/object.c
@@ -294,22 +294,32 @@ void object_array_filter(struct object_array *array,
array-nr = dst;
}
+/*
+ * Return true iff array already contains an entry with name.
+ */
+static int contains_name(struct object_array *array, const char *name)
+{
+ unsigned nr = array-nr, i;
+ struct object_array_entry *object = array-objects;
+
+ for (i = 0; i nr; i++, object++)
+ if (!strcmp(object-name, name))
+ return 1;
+ return 0;
+}
+
void object_array_remove_duplicates(struct object_array *array)
{
- unsigned int ref, src, dst;
+ unsigned nr = array-nr, src;
struct object_array_entry *objects = array-objects;
- for (ref = 0; ref + 1 array-nr; ref++) {
- for (src = ref + 1, dst = src;
-src array-nr;
-src++) {
- if (!strcmp(objects[ref].name, objects[src].name))
- continue;
- if (src != dst)
- objects[dst] = objects[src];
- dst++;
+ array-nr = 0;
+ for (src = 0; src nr; src++) {
+ if (!contains_name(array, objects[src].name)) {
+ if (src != array-nr)
+ objects[array-nr] = objects[src];
+ array-nr++;
}
- array-nr = dst;
}
}
diff --git a/object.h b/object.h
index 0d39ff4..6c1c27f 100644
--- a/object.h
+++ b/object.h
@@ -96,7 +96,11 @@ typedef int (*object_array_each_func_t)(struct
object_array_entry *, void *);
void object_array_filter(struct object_array *array,
object_array_each_func_t want, void *cb_data);
-void object_array_remove_duplicates(struct object_array *);
+/*
+ * Remove from array all but the first entry with a given name.
+ * Warning: this function uses an O(N^2) algorithm.
+ */
+void object_array_remove_duplicates(struct object_array *array);
void clear_object_flags(unsigned flags);
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/25] do_fetch(): reduce scope of peer_item
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/fetch.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f949115..80c6e37 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -694,7 +694,6 @@ static int do_fetch(struct transport *transport,
struct refspec *refs, int ref_count)
{
struct string_list existing_refs = STRING_LIST_INIT_DUP;
- struct string_list_item *peer_item = NULL;
struct ref *ref_map;
struct ref *rm;
int autotags = (transport-remote-fetch_tags == 1);
@@ -724,8 +723,9 @@ static int do_fetch(struct transport *transport,
for (rm = ref_map; rm; rm = rm-next) {
if (rm-peer_ref) {
- peer_item = string_list_lookup(existing_refs,
- rm-peer_ref-name);
+ struct string_list_item *peer_item =
+ string_list_lookup(existing_refs,
+ rm-peer_ref-name);
if (peer_item)
hashcpy(rm-peer_ref-old_sha1,
peer_item-util);
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/25] do_fetch(): clean up existing_refs before exiting
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/fetch.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 80c6e37..48df5fa 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -697,6 +697,7 @@ static int do_fetch(struct transport *transport,
struct ref *ref_map;
struct ref *rm;
int autotags = (transport-remote-fetch_tags == 1);
+ int retcode = 0;
for_each_ref(add_existing, existing_refs);
@@ -712,9 +713,9 @@ static int do_fetch(struct transport *transport,
/* if not appending, truncate FETCH_HEAD */
if (!append !dry_run) {
- int errcode = truncate_fetch_head();
- if (errcode)
- return errcode;
+ retcode = truncate_fetch_head();
+ if (retcode)
+ goto cleanup;
}
ref_map = get_ref_map(transport, refs, ref_count, tags, autotags);
@@ -736,7 +737,8 @@ static int do_fetch(struct transport *transport,
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
if (fetch_refs(transport, ref_map)) {
free_refs(ref_map);
- return 1;
+ retcode = 1;
+ goto cleanup;
}
if (prune) {
/* If --tags was specified, pretend the user gave us the
canonical tags refspec */
@@ -779,7 +781,9 @@ static int do_fetch(struct transport *transport,
free_refs(ref_map);
}
- return 0;
+ cleanup:
+ string_list_clear(existing_refs, 0);
+ return retcode;
}
static void set_option(const char *name, const char *value)
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/25] show_head_ref(): do not shadow name of argument
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
http-backend.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index 6b85ffa..3135835 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -416,8 +416,8 @@ static int show_head_ref(const char *name, const unsigned
char *sha1,
struct strbuf *buf = cb_data;
if (flag REF_ISSYMREF) {
- unsigned char sha1[20];
- const char *target = resolve_ref_unsafe(name, sha1, 1, NULL);
+ unsigned char unused[20];
+ const char *target = resolve_ref_unsafe(name, unused, 1, NULL);
const char *target_nons = strip_namespace(target);
strbuf_addf(buf, ref: %s\n, target_nons);
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 18/25] add_existing(): do not retain a reference to sha1
Its lifetime is not guaranteed, so make a copy. Free the memory when
the string_list is cleared.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/fetch.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 48df5fa..fa6fe44 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -571,7 +571,8 @@ static int add_existing(const char *refname, const unsigned
char *sha1,
{
struct string_list *list = (struct string_list *)cbdata;
struct string_list_item *item = string_list_insert(list, refname);
- item-util = (void *)sha1;
+ item-util = xmalloc(20);
+ hashcpy(item-util, sha1);
return 0;
}
@@ -636,7 +637,7 @@ static void find_non_local_tags(struct transport *transport,
item = string_list_insert(remote_refs, ref-name);
item-util = (void *)ref-old_sha1;
}
- string_list_clear(existing_refs, 0);
+ string_list_clear(existing_refs, 1);
/*
* We may have a final lightweight tag that needs to be
@@ -782,7 +783,7 @@ static int do_fetch(struct transport *transport,
}
cleanup:
- string_list_clear(existing_refs, 0);
+ string_list_clear(existing_refs, 1);
return retcode;
}
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 23/25] exclude_existing(): set existing_refs.strdup_strings
The each_ref_fn add_existing() adds refnames to the existing_refs
list. But the lifetimes of these refnames is not guaranteed by the
refs API, so configure the string_list to make copies as it adds them.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/show-ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 8d9b76a..4a0310d 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -103,7 +103,7 @@ static int add_existing(const char *refname, const unsigned
char *sha1, int flag
*/
static int exclude_existing(const char *match)
{
- static struct string_list existing_refs = STRING_LIST_INIT_NODUP;
+ static struct string_list existing_refs = STRING_LIST_INIT_DUP;
char buf[1024];
int matchlen = match ? strlen(match) : 0;
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 24/25] register_ref(): make a copy of the bad reference SHA-1
The lifetime of the sha1 parameter passed to an each_ref_fn callback
is not guaranteed, so make a copy for later use.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
bisect.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/bisect.c b/bisect.c
index 374d9e2..71c1958 100644
--- a/bisect.c
+++ b/bisect.c
@@ -15,7 +15,7 @@
static struct sha1_array good_revs;
static struct sha1_array skipped_revs;
-static const unsigned char *current_bad_sha1;
+static unsigned char *current_bad_sha1;
static const char *argv_checkout[] = {checkout, -q, NULL, --, NULL};
static const char *argv_show_branch[] = {show-branch, NULL, NULL};
@@ -404,7 +404,8 @@ static int register_ref(const char *refname, const unsigned
char *sha1,
int flags, void *cb_data)
{
if (!strcmp(refname, bad)) {
- current_bad_sha1 = sha1;
+ current_bad_sha1 = xmalloc(20);
+ hashcpy(current_bad_sha1, sha1);
} else if (!prefixcmp(refname, good-)) {
sha1_array_append(good_revs, sha1);
} else if (!prefixcmp(refname, skip-)) {
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 25/25] refs: document the lifetime of the args passed to each_ref_fn
The lifetime of the memory pointed to by the refname and sha1
arguments to each_ref_fn was never documented, but some callers used
to assume that it was essentially permanent. In fact the API does
*not* guarantee that these objects live beyond a single callback
invocation.
In the current code, the lifetimes are bound together with the
lifetimes of the ref_caches. Since these are usually long, the
callers usually got away with their sloppiness. But even today, if a
ref_cache is invalidated the memory can be freed. And planned changes
to reference caching, needed to eliminate race conditions, will
probably need to shorten the lifetimes of these objects.
The commits leading up to this have (hopefully) fixed all of the
callers of the for_each_ref()-like functions. This commit does the
last step: documents what each_ref_fn callbacks can assume about
object lifetimes.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.h | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/refs.h b/refs.h
index a35eafc..122ec03 100644
--- a/refs.h
+++ b/refs.h
@@ -15,13 +15,23 @@ struct ref_lock {
#define REF_ISBROKEN 0x04
/*
- * Calls the specified function for each ref file until it returns
- * nonzero, and returns the value. Please note that it is not safe to
- * modify references while an iteration is in progress, unless the
- * same callback function invocation that modifies the reference also
- * returns a nonzero value to immediately stop the iteration.
+ * The signature for the callback function for the for_each_*()
+ * functions below. The memory pointed to by the refname and sha1
+ * arguments is only guaranteed to be valid for the duration of a
+ * single callback invocation.
+ */
+typedef int each_ref_fn(const char *refname,
+ const unsigned char *sha1, int flags, void *cb_data);
+
+/*
+ * The following functions invoke the specified callback function for
+ * each reference indicated. If the function ever returns a nonzero
+ * value, stop the iteration and return that value. Please note that
+ * it is not safe to modify references while an iteration is in
+ * progress, unless the same callback function invocation that
+ * modifies the reference also returns a nonzero value to immediately
+ * stop the iteration.
*/
-typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int
flags, void *cb_data);
extern int head_ref(each_ref_fn, void *);
extern int for_each_ref(each_ref_fn, void *);
extern int for_each_ref_in(const char *, each_ref_fn, void *);
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/25] fetch: make own copies of refnames
Do not retain references to refnames passed to the each_ref_fn
callback add_existing(), because their lifetime is not guaranteed.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/fetch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4b6b1df..f949115 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -590,7 +590,7 @@ static void find_non_local_tags(struct transport *transport,
struct ref **head,
struct ref ***tail)
{
- struct string_list existing_refs = STRING_LIST_INIT_NODUP;
+ struct string_list existing_refs = STRING_LIST_INIT_DUP;
struct string_list remote_refs = STRING_LIST_INIT_NODUP;
const struct ref *ref;
struct string_list_item *item = NULL;
@@ -693,7 +693,7 @@ static int truncate_fetch_head(void)
static int do_fetch(struct transport *transport,
struct refspec *refs, int ref_count)
{
- struct string_list existing_refs = STRING_LIST_INIT_NODUP;
+ struct string_list existing_refs = STRING_LIST_INIT_DUP;
struct string_list_item *peer_item = NULL;
struct ref *ref_map;
struct ref *rm;
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/25] add_rev_cmdline(): make a copy of the name argument
Instead of assuming that the memory pointed to by the name argument will live forever, make a local copy of it before storing it in the ref_cmdline_info. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- revision.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/revision.c b/revision.c index a67b615..25e424c 100644 --- a/revision.c +++ b/revision.c @@ -898,6 +898,10 @@ static int limit_list(struct rev_info *revs) return 0; } +/* + * Add an entry to refs-cmdline with the specified information. + * *name is copied. + */ static void add_rev_cmdline(struct rev_info *revs, struct object *item, const char *name, @@ -909,7 +913,7 @@ static void add_rev_cmdline(struct rev_info *revs, ALLOC_GROW(info-rev, nr + 1, info-alloc); info-rev[nr].item = item; - info-rev[nr].name = name; + info-rev[nr].name = xstrdup(name); info-rev[nr].whence = whence; info-rev[nr].flags = flags; info-nr++; -- 1.8.2.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/25] builtin_diff_tree(): make it obvious that function wants two entries
Instead of accepting an array and using exactly two elements from the
array, take two single (struct object_array_entry *) arguments.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/diff.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index 8c2af6c..abdd613 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -153,7 +153,8 @@ static int builtin_diff_index(struct rev_info *revs,
static int builtin_diff_tree(struct rev_info *revs,
int argc, const char **argv,
-struct object_array_entry *ent)
+struct object_array_entry *ent0,
+struct object_array_entry *ent1)
{
const unsigned char *(sha1[2]);
int swap = 0;
@@ -161,13 +162,14 @@ static int builtin_diff_tree(struct rev_info *revs,
if (argc 1)
usage(builtin_diff_usage);
- /* We saw two trees, ent[0] and ent[1].
-* if ent[1] is uninteresting, they are swapped
+ /*
+* We saw two trees, ent0 and ent1. If ent1 is uninteresting,
+* swap them.
*/
- if (ent[1].item-flags UNINTERESTING)
+ if (ent1-item-flags UNINTERESTING)
swap = 1;
- sha1[swap] = ent[0].item-sha1;
- sha1[1-swap] = ent[1].item-sha1;
+ sha1[swap] = ent0-item-sha1;
+ sha1[1-swap] = ent1-item-sha1;
diff_tree_sha1(sha1[0], sha1[1], , revs-diffopt);
log_tree_diff_flush(revs);
return 0;
@@ -403,7 +405,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
else if (ents == 1)
result = builtin_diff_index(rev, argc, argv);
else if (ents == 2)
- result = builtin_diff_tree(rev, argc, argv, ent);
+ result = builtin_diff_tree(rev, argc, argv, ent[0], ent[1]);
else if (ent[0].item-flags UNINTERESTING) {
/*
* diff A...B where there is at least one merge base
@@ -412,8 +414,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
* between the base and B. Note that we pick one
* merge base at random if there are more than one.
*/
- ent[1] = ent[ents-1];
- result = builtin_diff_tree(rev, argc, argv, ent);
+ result = builtin_diff_tree(rev, argc, argv, ent[0],
ent[ents-1]);
} else
result = builtin_diff_combined(rev, argc, argv,
ent, ents);
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/25] cmd_diff(): use an object_array for holding trees
Change cmd_diff() to use a (struct object_array) for holding the trees
that it accumulates, rather than rolling its own equivalent.
Incidentally, this change removes a hard-coded limit of 100 trees in
combined diff, not that it matters in practice.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/diff.c | 37 ++---
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index abdd613..661fdde 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -253,8 +253,8 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
{
int i;
struct rev_info rev;
- struct object_array_entry ent[100];
- int ents = 0, blobs = 0, paths = 0;
+ struct object_array ent = OBJECT_ARRAY_INIT;
+ int blobs = 0, paths = 0;
const char *path = NULL;
struct blobinfo blob[2];
int nongit;
@@ -351,13 +351,8 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
if (obj-type == OBJ_COMMIT)
obj = ((struct commit *)obj)-tree-object;
if (obj-type == OBJ_TREE) {
- if (ARRAY_SIZE(ent) = ents)
- die(_(more than %d trees given: '%s'),
- (int) ARRAY_SIZE(ent), name);
obj-flags |= flags;
- ent[ents].item = obj;
- ent[ents].name = name;
- ents++;
+ add_object_array(obj, name, ent);
continue;
}
if (obj-type == OBJ_BLOB) {
@@ -381,7 +376,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
/*
* Now, do the arguments look reasonable?
*/
- if (!ents) {
+ if (!ent.nr) {
switch (blobs) {
case 0:
result = builtin_diff_files(rev, argc, argv);
@@ -402,22 +397,26 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
}
else if (blobs)
usage(builtin_diff_usage);
- else if (ents == 1)
+ else if (ent.nr == 1)
result = builtin_diff_index(rev, argc, argv);
- else if (ents == 2)
- result = builtin_diff_tree(rev, argc, argv, ent[0], ent[1]);
- else if (ent[0].item-flags UNINTERESTING) {
+ else if (ent.nr == 2)
+ result = builtin_diff_tree(rev, argc, argv,
+ ent.objects[0], ent.objects[1]);
+ else if (ent.objects[0].item-flags UNINTERESTING) {
/*
* diff A...B where there is at least one merge base
-* between A and B. We have ent[0] == merge-base,
-* ent[ents-2] == A, and ent[ents-1] == B. Show diff
-* between the base and B. Note that we pick one
-* merge base at random if there are more than one.
+* between A and B. We have ent.objects[0] ==
+* merge-base, ent.objects[ents-2] == A, and
+* ent.objects[ents-1] == B. Show diff between the
+* base and B. Note that we pick one merge base at
+* random if there are more than one.
*/
- result = builtin_diff_tree(rev, argc, argv, ent[0],
ent[ents-1]);
+ result = builtin_diff_tree(rev, argc, argv,
+ ent.objects[0],
+ ent.objects[ent.nr-1]);
} else
result = builtin_diff_combined(rev, argc, argv,
- ent, ents);
+ ent.objects, ent.nr);
result = diff_result_code(rev.diffopt, result);
if (1 rev.diffopt.skip_stat_unmatch)
refresh_index_quietly();
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/25] find_first_merges(): initialize merges variable using initializer
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
submodule.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/submodule.c b/submodule.c
index e728025..b837c04 100644
--- a/submodule.c
+++ b/submodule.c
@@ -846,7 +846,7 @@ static int find_first_merges(struct object_array *result,
const char *path,
struct commit *a, struct commit *b)
{
int i, j;
- struct object_array merges;
+ struct object_array merges = OBJECT_ARRAY_INIT;
struct commit *commit;
int contains_another;
@@ -856,7 +856,6 @@ static int find_first_merges(struct object_array *result,
const char *path,
struct rev_info revs;
struct setup_revision_opt rev_opts;
- memset(merges, 0, sizeof(merges));
memset(result, 0, sizeof(struct object_array));
memset(rev_opts, 0, sizeof(rev_opts));
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/25] fsck: don't put a void*-shaped peg in a char*-shaped hole
The source of this nonsense was 04d3975937 fsck: reduce stack footprint , which wedged a pointer to parent into the object_array_entry's name field. The parent pointer was passed to traverse_one_object(), even though that function *didn't use it*. The useless code has been deleted over time. Commit a1cdc25172 fsck: drop unused parameter from traverse_one_object() removed the parent pointer from traverse_one_object()'s signature. Commit c0aa335c95 Remove unused variables removed the code that read the parent pointer back out of the name field. This commit takes the last step: don't write the parent pointer into the name field in the first place. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/fsck.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index bb9a2cd..9909b6d 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -112,7 +112,7 @@ static int mark_object(struct object *obj, int type, void *data) return 1; } - add_object_array(obj, (void *) parent, pending); + add_object_array(obj, NULL, pending); return 0; } -- 1.8.2.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 15/25] object_array_entry: fix memory handling of the name field
Previously, the memory management of the object_array_entry::name
field was inconsistent and undocumented. object_array_entries are
ultimately created by a single function, add_object_array_with_mode(),
which has an argument const char *name. This function used to
simply set the name field to reference the string pointed to by the
name parameter, and nobody on the object_array side ever freed the
memory. Thus, it assumed that the memory for the name field would be
managed by the caller, and that the lifetime of that string would be
at least as long as the lifetime of the object_array_entry. But
callers were inconsistent:
* Some passed pointers to constant strings or argv entries, which was
OK.
* Some passed pointers to newly-allocated memory, but didn't arrange
for the memory ever to be freed.
* Some passed the return value of sha1_to_hex(), which is a pointer to
a statically-allocated buffer that can be overwritten at any time.
* Some passed pointers to refnames that they received from a
for_each_ref()-type iteration, but the lifetimes of such refnames is
not guaranteed by the refs API.
Bring consistency to this mess by changing object_array to make its
own copy for the object_array_entry::name field and free this memory
when an object_array_entry is deleted from the array.
Many callers were passing the empty string as the name parameter, so
as a performance optimization, treat the empty string specially.
Instead of making a copy, store a pointer to a statically-allocated
empty string to object_array_entry::name. When deleting such an
entry, skip the free().
Change the callers that were already passing copies to
add_object_array_with_mode() to either skip the copy, or (if the
memory needed to be allocated anyway) freeing the memory itself.
A part of this commit effectively reverts
70d26c6e76 read_revisions_from_stdin: make copies for handle_revision_arg
because the copying introduced by that commit (which is still
necessary) is now done at a deeper level.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
bundle.c | 2 +-
object.c | 26 +++---
object.h | 8 +++-
revision.c | 6 --
4 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/bundle.c b/bundle.c
index 4b0e5cd..3d64311 100644
--- a/bundle.c
+++ b/bundle.c
@@ -281,7 +281,7 @@ int create_bundle(struct bundle_header *header, const char
*path,
if (!get_sha1_hex(buf.buf + 1, sha1)) {
struct object *object =
parse_object_or_die(sha1, buf.buf);
object-flags |= UNINTERESTING;
- add_pending_object(revs, object,
xstrdup(buf.buf));
+ add_pending_object(revs, object, buf.buf);
}
} else if (!get_sha1_hex(buf.buf, sha1)) {
struct object *object = parse_object_or_die(sha1,
buf.buf);
diff --git a/object.c b/object.c
index 10b5349..e4ff714 100644
--- a/object.c
+++ b/object.c
@@ -260,11 +260,18 @@ void add_object_array(struct object *obj, const char
*name, struct object_array
add_object_array_with_mode(obj, name, array, S_IFINVALID);
}
+/*
+ * A zero-length string to which object_array_entry::name can be
+ * initialized without requiring a malloc/free.
+ */
+char object_array_slopbuf[1];
+
void add_object_array_with_mode(struct object *obj, const char *name, struct
object_array *array, unsigned mode)
{
unsigned nr = array-nr;
unsigned alloc = array-alloc;
struct object_array_entry *objects = array-objects;
+ struct object_array_entry *entry;
if (nr = alloc) {
alloc = (alloc + 32) * 2;
@@ -272,9 +279,16 @@ void add_object_array_with_mode(struct object *obj, const
char *name, struct obj
array-alloc = alloc;
array-objects = objects;
}
- objects[nr].item = obj;
- objects[nr].name = name;
- objects[nr].mode = mode;
+ entry = objects[nr];
+ entry-item = obj;
+ if (!name)
+ entry-name = NULL;
+ else if (!*name)
+ /* Use our own empty string instead of allocating one: */
+ entry-name = object_array_slopbuf;
+ else
+ entry-name = xstrdup(name);
+ entry-mode = mode;
array-nr = ++nr;
}
@@ -289,6 +303,9 @@ void object_array_filter(struct object_array *array,
if (src != dst)
objects[dst] = objects[src];
dst++;
+ } else {
+ if (objects[src].name != object_array_slopbuf)
+ free(objects[src].name);
}
}
array-nr = dst;
@@ -319,6 +336,9 @@ void object_array_remove_duplicates(struct object_array
*array)
if (src != array-nr)
[PATCH v2 20/25] show_head_ref(): rename first parameter to refname
This is the usual convention.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
http-backend.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index 3135835..0324417 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -410,14 +410,14 @@ static void get_info_refs(char *arg)
strbuf_release(buf);
}
-static int show_head_ref(const char *name, const unsigned char *sha1,
+static int show_head_ref(const char *refname, const unsigned char *sha1,
int flag, void *cb_data)
{
struct strbuf *buf = cb_data;
if (flag REF_ISSYMREF) {
unsigned char unused[20];
- const char *target = resolve_ref_unsafe(name, unused, 1, NULL);
+ const char *target = resolve_ref_unsafe(refname, unused, 1,
NULL);
const char *target_nons = strip_namespace(target);
strbuf_addf(buf, ref: %s\n, target_nons);
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 22/25] string_list_add_refs_by_glob(): add a comment about memory management
Since string_list_add_one_ref() adds refname to the string list, but
the lifetime of refname is limited, it is important that the
string_list passed to string_list_add_one_ref() has strdup_strings
set. Document this fact.
All current callers do the right thing.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
notes.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/notes.c b/notes.c
index fa7cdf7..602d956 100644
--- a/notes.c
+++ b/notes.c
@@ -927,6 +927,9 @@ static int string_list_add_one_ref(const char *refname,
const unsigned char *sha
return 0;
}
+/*
+ * The list argument must have strdup_strings set on it.
+ */
void string_list_add_refs_by_glob(struct string_list *list, const char *glob)
{
if (has_glob_specials(glob)) {
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 21/25] string_list_add_one_ref(): rename first parameter to refname
This is the usual convention.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
notes.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/notes.c b/notes.c
index f63fd57..fa7cdf7 100644
--- a/notes.c
+++ b/notes.c
@@ -918,12 +918,12 @@ out:
return ret;
}
-static int string_list_add_one_ref(const char *path, const unsigned char *sha1,
+static int string_list_add_one_ref(const char *refname, const unsigned char
*sha1,
int flag, void *cb)
{
struct string_list *refs = cb;
- if (!unsorted_string_list_has_string(refs, path))
- string_list_append(refs, path);
+ if (!unsorted_string_list_has_string(refs, refname))
+ string_list_append(refs, refname);
return 0;
}
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/25] find_first_merges(): remove unnecessary code
No names are ever set for the object_array_entries in merges, so there is no need to pretend to copy them to the result array. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- submodule.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/submodule.c b/submodule.c index b837c04..ad476ce 100644 --- a/submodule.c +++ b/submodule.c @@ -893,8 +893,7 @@ static int find_first_merges(struct object_array *result, const char *path, } if (!contains_another) - add_object_array(merges.objects[i].item, -merges.objects[i].name, result); + add_object_array(merges.objects[i].item, NULL, result); } free(merges.objects); -- 1.8.2.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/25] object_array: add function object_array_filter()
Add a function that allows unwanted entries in an object_array to be
removed. This encapsulation is a step towards giving object_array
ownership of its entries' name memory.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
object.c | 16
object.h | 11 +++
2 files changed, 27 insertions(+)
diff --git a/object.c b/object.c
index 20703f5..fcd4a82 100644
--- a/object.c
+++ b/object.c
@@ -278,6 +278,22 @@ void add_object_array_with_mode(struct object *obj, const
char *name, struct obj
array-nr = ++nr;
}
+void object_array_filter(struct object_array *array,
+object_array_each_func_t want, void *cb_data)
+{
+ unsigned nr = array-nr, src, dst;
+ struct object_array_entry *objects = array-objects;
+
+ for (src = dst = 0; src nr; src++) {
+ if (want(objects[src], cb_data)) {
+ if (src != dst)
+ objects[dst] = objects[src];
+ dst++;
+ }
+ }
+ array-nr = dst;
+}
+
void object_array_remove_duplicates(struct object_array *array)
{
unsigned int ref, src, dst;
diff --git a/object.h b/object.h
index 97d384b..0d39ff4 100644
--- a/object.h
+++ b/object.h
@@ -85,6 +85,17 @@ int object_list_contains(struct object_list *list, struct
object *obj);
/* Object array handling .. */
void add_object_array(struct object *obj, const char *name, struct
object_array *array);
void add_object_array_with_mode(struct object *obj, const char *name, struct
object_array *array, unsigned mode);
+
+typedef int (*object_array_each_func_t)(struct object_array_entry *, void *);
+
+/*
+ * Apply want to each entry in array, retaining only the entries for
+ * which the function returns true. Preserve the order of the entries
+ * that are retained.
+ */
+void object_array_filter(struct object_array *array,
+object_array_each_func_t want, void *cb_data);
+
void object_array_remove_duplicates(struct object_array *);
void clear_object_flags(unsigned flags);
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/25] revision: split some overly-long lines
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
revision.c | 20 ++--
revision.h | 32 +---
2 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/revision.c b/revision.c
index 25e424c..8ac88d6 100644
--- a/revision.c
+++ b/revision.c
@@ -70,7 +70,8 @@ static int show_path_truncated(FILE *out, const struct
name_path *path)
return ours || emitted;
}
-void show_object_with_name(FILE *out, struct object *obj, const struct
name_path *path, const char *component)
+void show_object_with_name(FILE *out, struct object *obj,
+ const struct name_path *path, const char *component)
{
struct name_path leaf;
leaf.up = (struct name_path *)path;
@@ -186,7 +187,9 @@ void mark_parents_uninteresting(struct commit *commit)
}
}
-static void add_pending_object_with_mode(struct rev_info *revs, struct object
*obj, const char *name, unsigned mode)
+static void add_pending_object_with_mode(struct rev_info *revs,
+struct object *obj,
+const char *name, unsigned mode)
{
if (!obj)
return;
@@ -209,7 +212,8 @@ static void add_pending_object_with_mode(struct rev_info
*revs, struct object *o
add_object_array_with_mode(obj, name, revs-pending, mode);
}
-void add_pending_object(struct rev_info *revs, struct object *obj, const char
*name)
+void add_pending_object(struct rev_info *revs,
+ struct object *obj, const char *name)
{
add_pending_object_with_mode(revs, obj, name, S_IFINVALID);
}
@@ -226,7 +230,9 @@ void add_head_to_pending(struct rev_info *revs)
add_pending_object(revs, obj, HEAD);
}
-static struct object *get_reference(struct rev_info *revs, const char *name,
const unsigned char *sha1, unsigned int flags)
+static struct object *get_reference(struct rev_info *revs, const char *name,
+ const unsigned char *sha1,
+ unsigned int flags)
{
struct object *object;
@@ -247,7 +253,8 @@ void add_pending_sha1(struct rev_info *revs, const char
*name,
add_pending_object(revs, object, name);
}
-static struct commit *handle_commit(struct rev_info *revs, struct object
*object, const char *name)
+static struct commit *handle_commit(struct rev_info *revs,
+ struct object *object, const char *name)
{
unsigned long flags = object-flags;
@@ -368,7 +375,8 @@ static void file_change(struct diff_options *options,
DIFF_OPT_SET(options, HAS_CHANGES);
}
-static int rev_compare_tree(struct rev_info *revs, struct commit *parent,
struct commit *commit)
+static int rev_compare_tree(struct rev_info *revs,
+ struct commit *parent, struct commit *commit)
{
struct tree *t1 = parent-tree;
struct tree *t2 = commit-tree;
diff --git a/revision.h b/revision.h
index 01bd2b7..9628465 100644
--- a/revision.h
+++ b/revision.h
@@ -195,19 +195,23 @@ struct setup_revision_opt {
};
extern void init_revisions(struct rev_info *revs, const char *prefix);
-extern int setup_revisions(int argc, const char **argv, struct rev_info *revs,
struct setup_revision_opt *);
+extern int setup_revisions(int argc, const char **argv, struct rev_info *revs,
+ struct setup_revision_opt *);
extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t
*ctx,
-const struct option *options,
-const char * const usagestr[]);
+ const struct option *options,
+ const char * const usagestr[]);
#define REVARG_CANNOT_BE_FILENAME 01
#define REVARG_COMMITTISH 02
-extern int handle_revision_arg(const char *arg, struct rev_info *revs, int
flags, unsigned revarg_opt);
+extern int handle_revision_arg(const char *arg, struct rev_info *revs,
+ int flags, unsigned revarg_opt);
extern void reset_revision_walk(void);
extern int prepare_revision_walk(struct rev_info *revs);
extern struct commit *get_revision(struct rev_info *revs);
-extern char *get_revision_mark(const struct rev_info *revs, const struct
commit *commit);
-extern void put_revision_mark(const struct rev_info *revs, const struct commit
*commit);
+extern char *get_revision_mark(const struct rev_info *revs,
+ const struct commit *commit);
+extern void put_revision_mark(const struct rev_info *revs,
+ const struct commit *commit);
extern void mark_parents_uninteresting(struct commit *commit);
extern void mark_tree_uninteresting(struct tree *tree);
@@ -220,15 +224,19 @@ struct name_path {
char *path_name(const struct name_path *path, const char *name);
-extern void show_object_with_name(FILE *, struct object *, const
[PATCH v2 07/25] cmd_diff(): make it obvious which cases are exclusive of each other
At first glance the OBJ_COMMIT, OBJ_TREE, and OBJ_BLOB cases look like
they might be mutually exclusive. But the OBJ_COMMIT case doesn't end
the loop iteration with continue like the other two cases, but
rather falls through. So use if...else if...else construct to make it
more obvious that only the last two cases are mutually exclusive.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/diff.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index 84243d9..9fc273d 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -350,22 +350,21 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
die(_(invalid object '%s' given.), name);
if (obj-type == OBJ_COMMIT)
obj = ((struct commit *)obj)-tree-object;
+
if (obj-type == OBJ_TREE) {
obj-flags |= flags;
add_object_array(obj, name, ent);
- continue;
- }
- if (obj-type == OBJ_BLOB) {
+ } else if (obj-type == OBJ_BLOB) {
if (2 = blobs)
die(_(more than two blobs given: '%s'), name);
hashcpy(blob[blobs].sha1, obj-sha1);
blob[blobs].name = name;
blob[blobs].mode = entry-mode;
blobs++;
- continue;
+ } else {
+ die(_(unhandled object '%s' given.), name);
}
- die(_(unhandled object '%s' given.), name);
}
if (rev.prune_data.nr) {
if (!path)
--
1.8.2.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Towards a useable git-branch
On Sat, May 25, 2013 at 1:26 PM, Duy Nguyen pclo...@gmail.com wrote: On Sat, May 25, 2013 at 5:51 AM, Duy Nguyen pclo...@gmail.com wrote: I just had an idea that might bring pretty stuff to for-each-ref with probably reasonable effort, making for-each-ref format a superset of pretty. But I need to clean up my backlog first. Give me a few days, I will show you something (or give up ;) And I can't get it out of my head. Might as well write it down. Check out https://github.com/pclouds/git.git for-each-ref-pretty Ram, fetch the url above again. Its tip now is 5b4aa27 (for-each-ref: introduce format specifier %(*) and %(*) - 2013-05-25). Those changes make for-each-ref --format a superset of pretty. You can add new %(xxx) on top and resend the whole thing to the list for review. You can remove branch -v/-vv code as well or I'll add some patches on top to do that later. I have some compatibility concerns about the superset thing. But let's wait until the series hits git@vger. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] config: allow inaccessible configuration under $HOME
Jonathan's patch would indeed be nice. In cgit, we are forced to do ugly things like this: /* Do not look in /etc/ for gitconfig and gitattributes. */ setenv(GIT_CONFIG_NOSYSTEM, 1, 1); setenv(GIT_ATTR_NOSYSTEM, 1, 1); /* We unset HOME and XDG_CONFIG_HOME before calling the git setup function * so that we don't make unneccessary filesystem accesses. */ user_home = getenv(HOME); xdg_home = getenv(XDG_CONFIG_HOME); unsetenv(HOME); unsetenv(XDG_CONFIG_HOME); /* Setup the git directory and initialize the notes system. Both of these * load local configuration from the git repository, so we do them both while * the HOME variables are unset. */ setup_git_directory_gently(nongit); init_display_notes(NULL); /* We restore the unset variables afterward. */ if (user_home) setenv(HOME, user_home, 1); if (xdg_home) setenv(XDG_CONFIG_HOME, xdg_home, 1); A big nasty guard around the git setup function, so that we don't error out. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Towards a useable git-branch
Duy Nguyen wrote: Ram, fetch the url above again. Its tip now is 5b4aa27 (for-each-ref: introduce format specifier %(*) and %(*) - 2013-05-25). Those changes make for-each-ref --format a superset of pretty. You can add new %(xxx) on top and resend the whole thing to the list for review. You can remove branch -v/-vv code as well or I'll add some patches on top to do that later. I have some compatibility concerns about the superset thing. But let's wait until the series hits git@vger. Great work Duy! I see that you've used format_commit_message(), but there are some concerns about pretty-formats conflicting with f-e-r's format. We'll iron it out slowly, but I like the overall approach. Thanks. (Very sleep deprived at the moment; will review and collaborate after I wake up) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color
Antoine Pelisse wrote: Is it not possible for color to be used uninitialized here ? My compiler didn't complain; what am I missing? Doesn't the declaration char color[COLOR_MAXLEN]; initialize an empty string? More importantly, aren't there numerous instances of this in the codebase? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color
David Aguilar wrote: Can you please also update Documentation/? Yeah, will do in the re-roll. Duy is bringing in pretty-formats. We'll probably need a separate document called pretty-ref-formats or some such thing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #07; Fri, 24)
On Fri, May 24, 2013 at 02:15:55PM -0700, Junio C Hamano wrote: * jk/submodule-subdirectory-ok (2013-04-24) 3 commits (merged to 'next' on 2013-04-24 at 6306b29) + submodule: fix quoting in relative_path() (merged to 'next' on 2013-04-22 at f211e25) + submodule: drop the top-level requirement + rev-parse: add --prefix option Allow various subcommands of git submodule to be run not from the top of the working tree of the superproject. What's the status of this one? As far as I'm concerned this is done. If you're re-rolling next you could squash the top two together but I don't think that's really necessary. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color
On Sat, May 25, 2013 at 05:20:29PM +0530, Ramkumar Ramachandra wrote: Antoine Pelisse wrote: Is it not possible for color to be used uninitialized here ? My compiler didn't complain; what am I missing? Doesn't the declaration char color[COLOR_MAXLEN]; initialize an empty string? Why would it? The variable's begin allocated on the stack and the C standard only zero-initializes variables with static storage duration; Section 6.7.9 of the C11 standard says: If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. I suspect the compiler doesn't complain because there is a path through the function that initializes color before reading it (if we hit the if branch in the loop before the else branch) and the compile assumes that there is something in the function's contract that guarantees that we follow this path. But I don't think that's correct so you do need to initialize color to the empty string. More importantly, aren't there numerous instances of this in the codebase? Care to point at one? I had a quick look and all places I inspected are either static or write to the array before reading it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color
On Sat, May 25, 2013 at 1:50 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Antoine Pelisse wrote: Is it not possible for color to be used uninitialized here ? My compiler didn't complain; what am I missing? Doesn't the declaration char color[COLOR_MAXLEN]; initialize an empty string? As John said, this is allocated on the stack. What do you want it to be initialized to ? Filled with zeros ? That's overkill because only the first char needs to be zeroed. The compiler can't know what to do and it lets you to take the best action. More importantly, aren't there numerous instances of this in the codebase? I think Valgrind would be mad at us if there was many instances of this. By the way Ramkumar, could you check if Valgrind complains with your patch ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color
John Keeping wrote: Section 6.7.9 of the C11 standard says: If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. Ah, thanks. I'll initialize it to an empty string. More importantly, aren't there numerous instances of this in the codebase? Care to point at one? I had a quick look and all places I inspected are either static or write to the array before reading it. I'll run some Valgrind tests to see what it yields. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Commit with an empty message broken since v1.8.2.1
Commit a24a41ea9a928ccde2db074ab0835c4817223c9d introduces a bug which is still present in latest master. This command git commit -m --allow-empty --allow-empty-message should create an empty commit with an empty message and never open a text editor for the commit message. Since the change, the editor is always opened. My current workaround to skip the editor is setting the environment variable: GIT_EDITOR=true-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug when git rev-list options --first-parent and --ancestry-path are used together?
On 05/24/2013 08:12 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Now assume a slightly more complicated situation, in which master has been merged to feature branch at some point: o - 0 - 1 - 2 - 3 - 4← master \ \ A - B - C - D ← branch \ / X - Y Now when we do an incremental merge branch into master, how do we generate the list of commits to put one each axis of the table? The merge base is 2, so I think the best answer is 1- 2 - 3 - 4 ← master | || C - C3 - C4 | || D - D3 - D4 ← final merge ↑ branch I am not sure if that is the best answer, though. After managing to create Cn, if a change between C and D (which come from X and Y) is too complex, wouldn't you want to break down the task to come up with Dn recursively into a smaller subtask of merging X first and then Y on top and finally D? OK, so let's assume that C3 is done and D3 is giving us problems: o - 0 - 1 - 2 - 3 - 4← master \ \ \ \ \ C3- ? \ \ / / A - B - C - D ← branch \ / X - Y Your proposal is not to merge D directly but rather to merge X then Y. o - 0 - 1 - 2 - 3 - 4← master \ \ \ \ \ C3 --- X3 - Y3 - D3 \ \ / /// A - B - C - D- / -- / ---' ← the lines here... \ / // X - Y- / ---' ← ...and here don't intersect \/ `--' The problem is that the merges that would be required are not able to take advantage of the conflict resolution that was done in D: The merge base for X3 would be B. This is a little bit wrong because X3 includes C among its ancestors, so creating X3 requires some of the conflicts between X and C (which were resolved once in D) to be resolved again. The merge base for Y3 would be X. Note that Y3 already includes C and Y among its ancestors. Therefore, resolving Y3 involves resolving the same conflicts between Y and C that were already resolved in D. But since merge Y3 doesn't know about D, the user would be forced to resolve those conflicts again (albeit maybe helped by something like rerere). And merge D3 would have two merge bases, C and Y. This is related to the fact that there are now two independent known resolutions for merging C and Y, namely D and Y3. Given that Y3 in the above scenario needs to include include C (via C3) and also Y, it seems to me that this merge is superfluous. It should have exactly the same content as D3, assuming that the conflicts are resolved the same way. Therefore one could skip Y3 and proceed directly to D3: o - 0 - 1 - 2 - 3 - 4← master \ \ \ \ \ C3 --- X3 - D3 \ \ / // A - B - C - D- / ---'← the lines here... \ / / X - Y / ← ...and here don't intersect \/ `--' This merge could take advantage of the conflict resolution that was done in D. It would have an unambiguous merge base, C. But I still think that this approach is not as clean as an incremental merge of two linear branches, because X3 requires some of the same conflicts to be resolved as were already resolved in D. Incidentally, if merge D had been done incrementally and the full incremental merge resolution had been kept, then we would have the missing merge CX that would allow us to compute D3 incrementally: o - 0 - 1 - 2 - 3 - 4← master | | | A - B - C - C3 | | | X - CX- C3X | | | Y - D - D3 I think that all of the required merges have sane merge-bases and take advantage of all of the merges that have been done previously. This is another case where an incremental merges contains information that can be useful for the future. The simplest way I can think of to generate the list C,D is git rev-list --first-parent --ancestry-path 2..D We need --ancestry-path to avoid getting commits A and B. It's still not clear that this is always the best approach but at least it seems safe. Hmm, while I agree that A and B will be omitted by using ancestry path on the example topology, I need to be convinced that it is impossible to end up with disjoint segments of a history in any ancestry graph by combining -f-p and -a-p that way to feel safe. If by disjoint you mean that the history contains gaps, that is exactly what happens in the case I described in my last email: o - 0 - 1 - 2 - 3 - 4← master \ \ A - B - C --. \ \ X - Y - D← branch The result of git rev-list --first-parent --ancestry-path 2..D would be only D and commit C would disappear into the gap. I am willing to accept that for my application, because the incremental merge algorithm can work despite gaps
[PATCH] trivial: Add missing period in documentation
--- Documentation/diff-options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 104579d..b8a9b86 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -480,7 +480,7 @@ endif::git-format-patch[] --ignore-submodules[=when]:: Ignore changes to submodules in the diff generation. when can be - either none, untracked, dirty or all, which is the default + either none, untracked, dirty or all, which is the default. Using none will consider the submodule modified when it either contains untracked or modified files or its HEAD differs from the commit recorded in the superproject and can be used to override any settings of the -- 1.8.3.426.gea353ce.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git clone does not understand insteadOf URLs
Hello, it seems that `git clone` does not understand keywords used `insteadOf` longer URLs. $ git clone remote-repo/ProjectA.git fatal repository 'remote-repo/ProjectA.git' does not exist I suppose that git interprets the argument as a local directory. Git should see if the argument matches one of the known URLs before raising an error. Regards, -- Gioele Barabucci gio...@svario.it -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] commit: don't start editor if empty message is given with -m
If an empty message is specified with the option -m of git commit then
the editor is started. That's unexpected and unnecessary. Instead of
using the length of the message string for checking if the user
specified one, directly remember if the option -m was given.
Reported-by: Mislav Marohnić mislav.maroh...@gmail.com
Signed-off-by: René Scharfe rene.scha...@lsrfire.ath.cx
---
builtin/commit.c | 10 ++
t/t7502-commit.sh | 17 +
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d2f30d9..1621dfc 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -107,7 +107,7 @@ static const char *cleanup_arg;
static enum commit_whence whence;
static int use_editor = 1, include_status = 1;
-static int show_ignored_in_status;
+static int show_ignored_in_status, have_option_m;
static const char *only_include_assumed;
static struct strbuf message = STRBUF_INIT;
@@ -121,9 +121,11 @@ static enum {
static int opt_parse_m(const struct option *opt, const char *arg, int unset)
{
struct strbuf *buf = opt-value;
- if (unset)
+ if (unset) {
+ have_option_m = 0;
strbuf_setlen(buf, 0);
- else {
+ } else {
+ have_option_m = 1;
if (buf-len)
strbuf_addch(buf, '\n');
strbuf_addstr(buf, arg);
@@ -975,7 +977,7 @@ static int parse_and_validate_options(int argc, const char
*argv[],
if (force_author renew_authorship)
die(_(Using both --reset-author and --author does not make
sense));
- if (logfile || message.len || use_message || fixup_message)
+ if (logfile || have_option_m || use_message || fixup_message)
use_editor = 0;
if (0 = edit_flag)
use_editor = edit_flag;
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index a4938b1..6313da2 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -361,6 +361,23 @@ test_expect_success !AUTOIDENT 'do not fire editor when
committer is bogus' '
test_cmp expect .git/result
'
+test_expect_success 'do not fire editor if -m msg was given' '
+ echo tick file
+ git add file
+ echo editor not started .git/result
+ (GIT_EDITOR=\$(pwd)/.git/FAKE_EDITOR\ git commit -m tick)
+ test $(cat .git/result) = editor not started
+'
+
+test_expect_success 'do not fire editor if -m was given' '
+ echo tock file
+ git add file
+ echo editor not started .git/result
+ (GIT_EDITOR=\$(pwd)/.git/FAKE_EDITOR\ \
+git commit -m --allow-empty-message)
+ test $(cat .git/result) = editor not started
+'
+
test_expect_success 'do not fire editor in the presence of conflicts' '
git clean -f
--
1.8.3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 02/15] path.c: refactor relative_path(), not only strip prefix
2013/5/22 Michael Haggerty mhag...@alum.mit.edu:
Sorry for coming late to the party.
I am on a business travel, and respond late also. ;-)
On 05/22/2013 03:40 AM, Jiang Xin wrote:
Different results for relative_path() before and after this refactor:
abs path base path relative (original) relative (refactor)
= === ===
/a/b/c/ /a/b c/ c/
/a/b//c/ //a///b/ c/ c/
/a/b /a/b ../
/a/b/ /a/b ../
/a/a/b/ /a ../
/ /a/b/ /../../
/a/c /a/b/ /a/c ../c
/a/b (empty)/a/b /a/b
/a/b (null) /a/b /a/b
(empty) /a/b (empty) ./
(null)(empty)(null) ./
(null)/a/b (segfault) ./
The old and new versions both seem to be (differently) inconsistent
about when the output has a trailing slash. What is the rule?
The reason for introducing patch 02/15 is that we don't want to reinvent
the wheel. Patch 06/15 (git-clean: refactor git-clean into two phases)
needs to save relative_path of each git-clean candidate file/directory in
del_list, but the public method in path.c (i.e. relative_path) is not
powerful, and static method in quote.c (i.e. path_relative) can note be
used directly. One way is to enhanced relative_path in path.c, like this
patch.
Since we combine the two methods (relative_path in path.c and
path_relative in quote.c), the new relative_path must be compatible
with the original two methods.
relative_path in path.c
===
relative_path is called in one place:
if (getenv(GIT_WORK_TREE_ENVIRONMENT))
setenv(GIT_WORK_TREE_ENVIRONMENT, ., 1);
set_git_dir(relative_path(git_dir, work_tree));
initialized = 1;
and set_git_dir only set the environment GIT_DIR_ENVIRONMENT
like this:
int set_git_dir(const char *path)
{
if (setenv(GIT_DIR_ENVIRONMENT, path, 1))
return error(Could not set GIT_DIR to '%s', path);
setup_git_env();
return 0;
}
So the only restraint for relative_path is that the return value can
not be blank. If the abs and base arguments for relative_path are
the same, the return value should be . (./ is also OK), then
set the envionment GIT_DIR_ENVIRONMENT to . (or ./).
path_relative in quote.c
We can not simply move path_relative in quote.c to relative_path
in path.c directly. It is because:
* The arguments for relative_path are from user input. So must
validate (remove duplicate slash) before use. But path_relative
does not check duplicate slash in arguments.
* path_relative will return blank string, if abs and base are the same.
Also I noticed that quote_path_relative of quote.c (which calls
path_relative) will transform the blank string from path_relative to
./ (not .)
char *quote_path_relative(const char *in, int len,
...
const char *rel = path_relative(in, len, sb, prefix, -1);
...
if (!out-len)
strbuf_addstr(out, ./);
That's why the path_relative in path.c refactor the output of . into ./.
diff --git a/path.c b/path.c
index 04ff..0174d 100644
--- a/path.c
+++ b/path.c
@@ -441,42 +441,104 @@ int adjust_shared_perm(const char *path)
return 0;
}
-const char *relative_path(const char *abs, const char *base)
+/*
+ * Give path as relative to prefix.
+ *
+ * The strbuf may or may not be used, so do not assume it contains the
+ * returned path.
+ */
+const char *relative_path(const char *abs, const char *base,
+ struct strbuf *sb)
Thanks for adding documentation. But I think it could be improved:
* The comment refers to path and prefix but the function parameters
are abs and base. I suggest making them agree.
Yes, it will be nice to update the comments.
* Who owns the memory pointed to by the return value?
* The comment says that the strbuf may or may not be used. So why is
it part of the interface? (I suppose it is because the strbuf might be
given ownership of the returned memory if it has to be allocated.)
Would it be more straightforward to *always* return the result in the
strbuf?
* Please document when the return value contains a trailing slash and
also that superfluous internal slashes are (apparently) normalized away.
* Leading double-slashes have a special meaning on some operating
systems. The old and new versions of this function both seem to ignore
differences between initial slashes. Perhaps somebody who knows the
rules better could say whether this is an issue but I guess the problem
would rarely be encountered in
Re: [PATCH v3 2/4] fetch-pack: prepare updated shallow file before fetching the pack
On Tue, May 7, 2013 at 10:59 PM, Junio C Hamano gits...@pobox.com wrote:
if (args-depth 0) {
+ struct stat st;
+ if (!fstat(shallow_lock.fd, st)
+ st.st_size == 0) {
+ unlink_or_warn(git_path(shallow));
Are we unlinking the right file here?
Yes we are. when st.st_size is 0, the new shallow file is empty (i.e.
--unshallow), so we want to remove the old file. I should check
!*alternate_shallow_file here instead of based on st.st_size.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/4] nd/clone-connectivity-shortcut updates
This addresses the comments from Junio and Eric in v3 [1]. [1] http://thread.gmane.org/gmane.comp.version-control.git/219611/focus=223584 Nguyễn Thái Ngọc Duy (4): clone: let the user know when check_everything_connected is run fetch-pack: prepare updated shallow file before fetching the pack index-pack: remove dead code (it should never happen) clone: open a shortcut for connectivity check Documentation/git-index-pack.txt | 3 ++ builtin/clone.c | 15 +-- builtin/index-pack.c | 38 -- commit.h | 2 + connected.c | 34 +++- connected.h | 5 +++ fetch-pack.c | 84 ++-- fetch-pack.h | 4 +- git.c| 7 shallow.c| 42 +++- t/t5500-fetch-pack.sh| 7 transport.c | 4 ++ transport.h | 2 + 13 files changed, 192 insertions(+), 55 deletions(-) -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] clone: let the user know when check_everything_connected is run
check_everything_connected could take a long time, especially in the
clone case where the whole DAG is traversed. The user deserves to know
what's going on.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/clone.c | 4
1 file changed, 4 insertions(+)
diff --git a/builtin/clone.c b/builtin/clone.c
index 035ab64..dad4265 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -546,8 +546,12 @@ static void update_remote_refs(const struct ref *refs,
{
const struct ref *rm = mapped_refs;
+ if (0 = option_verbosity)
+ printf(_(Checking connectivity... ));
if (check_everything_connected(iterate_ref_map, 0, rm))
die(_(remote did not send all necessary objects));
+ if (0 = option_verbosity)
+ printf(_(done\n));
if (refs) {
write_remote_refs(mapped_refs);
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/4] fetch-pack: prepare updated shallow file before fetching the pack
index-pack --strict looks up and follows parent commits. If shallow
information is not ready by the time index-pack is run, index-pack may
be led to non-existent objects. Make fetch-pack save shallow file to
disk before invoking index-pack.
git learns new global option --shallow-file to pass on the alternate
shallow file path. Undocumented (and not even support --shallow-file=
syntax) because it's unlikely to be used again elsewhere.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
commit.h | 2 ++
fetch-pack.c | 73 ++-
git.c | 7 +
shallow.c | 42 +++--
t/t5500-fetch-pack.sh | 7 +
5 files changed, 93 insertions(+), 38 deletions(-)
diff --git a/commit.h b/commit.h
index 67bd509..6e9c7cd 100644
--- a/commit.h
+++ b/commit.h
@@ -176,6 +176,8 @@ extern int for_each_commit_graft(each_commit_graft_fn, void
*);
extern int is_repository_shallow(void);
extern struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
+extern void check_shallow_file_for_update(void);
+extern void set_alternate_shallow_file(const char *path);
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit *);
diff --git a/fetch-pack.c b/fetch-pack.c
index f156dd4..6b5467c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -20,6 +20,8 @@ static int no_done;
static int fetch_fsck_objects = -1;
static int transfer_fsck_objects = -1;
static int agent_supported;
+static struct lock_file shallow_lock;
+static const char *alternate_shallow_file;
#define COMPLETE (1U 0)
#define COMMON (1U 1)
@@ -683,7 +685,7 @@ static int get_pack(struct fetch_pack_args *args,
int xd[2], char **pack_lockfile)
{
struct async demux;
- const char *argv[20];
+ const char *argv[22];
char keep_arg[256];
char hdr_arg[256];
const char **av;
@@ -724,6 +726,11 @@ static int get_pack(struct fetch_pack_args *args,
do_keep = 1;
}
+ if (alternate_shallow_file) {
+ *av++ = --shallow-file;
+ *av++ = alternate_shallow_file;
+ }
+
if (do_keep) {
if (pack_lockfile)
cmd.out = -1;
@@ -779,6 +786,27 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
return strcmp(a-name, b-name);
}
+static void setup_alternate_shallow(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int fd;
+
+ check_shallow_file_for_update();
+ fd = hold_lock_file_for_update(shallow_lock, git_path(shallow),
+ LOCK_DIE_ON_ERROR);
+ if (write_shallow_commits(sb, 0)) {
+ if (write_in_full(fd, sb.buf, sb.len) != sb.len)
+ die_errno(failed to write to %s,
shallow_lock.filename);
+ alternate_shallow_file = shallow_lock.filename;
+ } else
+ /*
+* is_repository_shallow() sees empty string as no
+* shallow file.
+*/
+ alternate_shallow_file = ;
+ strbuf_release(sb);
+}
+
static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int fd[2],
const struct ref *orig_ref,
@@ -858,6 +886,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
if (args-stateless_rpc)
packet_flush(fd[1]);
+ if (args-depth 0)
+ setup_alternate_shallow();
if (get_pack(args, fd, pack_lockfile))
die(git fetch-pack: fetch failed.);
@@ -936,15 +966,9 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
struct ref **sought, int nr_sought,
char **pack_lockfile)
{
- struct stat st;
struct ref *ref_cpy;
fetch_pack_setup();
- if (args-depth 0) {
- if (stat(git_path(shallow), st))
- st.st_mtime = 0;
- }
-
if (nr_sought)
nr_sought = remove_duplicates_in_refs(sought, nr_sought);
@@ -954,35 +978,12 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
}
ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
pack_lockfile);
- if (args-depth 0) {
- static struct lock_file lock;
- struct cache_time mtime;
- struct strbuf sb = STRBUF_INIT;
- char *shallow = git_path(shallow);
- int fd;
-
- mtime.sec = st.st_mtime;
- mtime.nsec = ST_MTIME_NSEC(st);
- if (stat(shallow, st)) {
- if (mtime.sec)
- die(shallow file was removed during fetch);
- } else if (st.st_mtime
[PATCH v4 3/4] index-pack: remove dead code (it should never happen)
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/index-pack.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 79dfe47..f52a04f 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -747,8 +747,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry, int eaten; void *buf = (void *) data; - if (!buf) - buf = new_data = get_data_from_pack(obj_entry); + assert(data data can only be NULL for large _blobs_); /* * we do not need to free the memory here, as the -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] clone: open a shortcut for connectivity check
In order to make sure the cloned repository is good, we run rev-list
--objects --not --all $new_refs on the repository. This is expensive
on large repositories. This patch attempts to mitigate the impact in
this special case.
In the good clone case, we only have one pack. If all of the
following are met, we can be sure that all objects reachable from the
new refs exist, which is the intention of running rev-list ...:
- all refs point to an object in the pack
- there are no dangling pointers in any object in the pack
- no objects in the pack point to objects outside the pack
The second and third checks can be done with the help of index-pack as
a slight variation of --strict check (which introduces a new condition
for the shortcut: pack transfer must be used and the number of objects
large enough to call index-pack). The first is checked in
check_everything_connected after we get an ok from index-pack.
index-pack + new checks is still faster than the current index-pack
+ rev-list, which is the whole point of this patch. If any of the
conditions fail, we fall back to the good old but expensive rev-list
... In that case it's even more expensive because we have to pay for
the new checks in index-pack. But that should only happen when the
other side is either buggy or malicious.
Cloning linux-2.6 over file://
before after
real3m25.693s 2m53.050s
user5m2.037s 4m42.396s
sys 0m13.750s 0m16.574s
A more realistic test with ssh:// over wireless
before after
real11m26.629s 10m4.213s
user5m43.196s 5m19.444s
sys 0m35.812s 0m37.630s
This shortcut is not applied to shallow clones, partly because shallow
clones should have no more objects than a usual fetch and the cost of
rev-list is acceptable, partly to avoid dealing with corner cases when
grafting is involved.
This shortcut does not apply to unpack-objects code path either
because the number of objects must be small in order to trigger that
code path.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/git-index-pack.txt | 3 +++
builtin/clone.c | 11 ---
builtin/index-pack.c | 35 ++-
connected.c | 34 +-
connected.h | 5 +
fetch-pack.c | 11 ++-
fetch-pack.h | 4 +++-
transport.c | 4
transport.h | 2 ++
9 files changed, 94 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index bde8eec..7a4e055 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -74,6 +74,9 @@ OPTIONS
--strict::
Die, if the pack contains broken objects or links.
+--check-self-contained-and-connected::
+ Die if the pack contains broken links. For internal use only.
+
--threads=n::
Specifies the number of threads to spawn when resolving
deltas. This requires that index-pack be compiled with
diff --git a/builtin/clone.c b/builtin/clone.c
index dad4265..069e81e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -542,13 +542,15 @@ static void update_remote_refs(const struct ref *refs,
const struct ref *mapped_refs,
const struct ref *remote_head_points_at,
const char *branch_top,
- const char *msg)
+ const char *msg,
+ struct transport *transport)
{
const struct ref *rm = mapped_refs;
if (0 = option_verbosity)
printf(_(Checking connectivity... ));
- if (check_everything_connected(iterate_ref_map, 0, rm))
+ if (check_everything_connected_with_transport(iterate_ref_map,
+ 0, rm, transport))
die(_(remote did not send all necessary objects));
if (0 = option_verbosity)
printf(_(done\n));
@@ -893,6 +895,9 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
+
+ if (transport-smart_options !option_depth)
+
transport-smart_options-check_self_contained_and_connected = 1;
}
refs = transport_get_remote_refs(transport);
@@ -954,7 +959,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
transport_fetch_refs(transport, mapped_refs);
update_remote_refs(refs, mapped_refs, remote_head_points_at,
- branch_top.buf, reflog_msg.buf);
+ branch_top.buf, reflog_msg.buf,
GIT Architecture
Hello. I´m postgraduate student in distributed computing and I'm searching on git. I would like to know is there any website where I can find the main components and connectors of the architecture of GIT. Is there any website where I can find components-connector view point. I appreciate any help! Thank you! Best regards, Rafael Abraão Rodrigues Lago -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
