[PATCH v4 3/8] Introduce new function real_path_if_valid()

The function is like real_path(), except that it returns NULL on error instead of dying. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- abspath.c | 5 + cache.h | 1 + 2 files changed, 6 insertions(+) diff --git a/abspath.c b/abspath.c index f8a526f..40cdc46 100644

[PATCH v4 4/8] longest_ancestor_length(): use string_list_split()

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- path.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/path.c b/path.c index cbbdf7d..f455e8e 100644 --- a/path.c +++ b/path.c @@ -12,6 +12,7 @@ */ #include cache.h #include strbuf.h +#include

[PATCH v4 5/8] longest_ancestor_length(): take a string_list argument for prefixes

Change longest_ancestor_length() to take the prefixes argument as a string_list rather than as a colon-separated string. This will make it easier for the caller to alter the entries before calling longest_ancestor_length(). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- cache.h

[PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized

: under Windows, bash mangles arguments that look like absolute POSIX paths into DOS paths. So we have to retain the level of normalization done by normalize_path_copy() to convert the bash-mangled DOS paths (which contain backslashes) into paths that use forward slashes. Signed-off-by: Michael

Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks

On 10/29/2012 01:15 AM, David Aguilar wrote: On Sat, Oct 20, 2012 at 11:51 PM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: This patch series has the side effect that all of the directories listed in GIT_CEILING_DIRECTORIES are accessed

Re: git push tags

file of the repo from which git-push takes objects do not help in enforcing the policy not to update tags in the remote repo. If your remote repository is managed using gitolite, you can institute restrictions on changing tags via the gitolite config. Michael -- Michael Haggerty mhag

[PATCH 00/17] Remove assumptions about refname lifetimes

my comments on these two patches for more information. [1] http://thread.gmane.org/gmane.comp.version-control.git/223299 Michael Haggerty (17): 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

[PATCH 02/17] 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

[PATCH 03/17] 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

[PATCH 08/17] 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

[PATCH 13/17] fsck: don't put a void*-shaped peg in a char*-shaped hole

step: don't write the parent pointer into the name field in the first place. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- I thought that this misuse of the name field was going to be a showstopper for changing how the name's memory is managed, but then I noticed that the value stored

[PATCH 15/17] 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

[RFC 16/17] object_array_entry: copy name before storing in name field

is not defined by the refs API (and which we want to shorten). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- This is the culmination of the last few commits. Since some callers want to store refnames in the name field of object_array elements, but we don't want those callers to assume

[PATCH 06/17] 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 72d99c0..7cac6de 100644 --- a/builtin/diff.c +++ b/builtin/diff.c

[PATCH 14/17] 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

[PATCH 10/17] get_revision_internal(): make check less mysterious

if it is at its allocation limit when a new element is about to be added. So change the check to if (nr == alloc) and add a comment to explain what's going on. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Please check that I have properly described the purpose of this check. The way

Re: [RFC 16/17] object_array_entry: copy name before storing in name field

On 05/20/2013 12:33 PM, Johan Herland wrote: On Sun, May 19, 2013 at 10:27 PM, Michael Haggerty mhag...@alum.mit.edu wrote: This is the culmination of the last few commits. Since some callers want to store refnames in the name field of object_array elements, but we don't want those callers

Re: [RFC 16/17] object_array_entry: copy name before storing in name field

On 05/20/2013 06:44 PM, Jeff King wrote: On Mon, May 20, 2013 at 04:42:38PM +0200, Michael Haggerty wrote: * Many callers store the empty string () as the name; for example, most of the entries created during a run of rev-list have as their name. This means that lots of needless copies

Re: [PATCH v13 02/15] path.c: refactor relative_path(), not only strip prefix

the problem would rarely be encountered in practice. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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

Re: [PATCH 08/17] revision: split some overly-long lines

On 05/21/2013 07:34 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- revision.c | 20 ++-- revision.h | 32 +--- 2 files changed, 35 insertions(+), 17 deletions

Re: [PATCH 10/17] get_revision_internal(): make check less mysterious

On 05/21/2013 07:38 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: The condition under which gc_boundary() is called was previously if (alloc = nr) . But by construction, nr can never exceed alloc, so the check looks unnecessarily mysterious. In fact

Re: [PATCH 09/17] gc_boundary(): move the check alloc = nr to caller

On 05/21/2013 07:49 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: There is no logical reason for this test to be here. At the caller we might be able to figure out its meaning. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- I do not think this change

Re: [PATCH 04/17] builtin_diff_tree(): make it obvious that function wants two entries

On 05/21/2013 07:27 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: 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

Re: [PATCH 05/17] cmd_diff(): use an object_array for holding trees

On 05/21/2013 07:30 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Change cmd_diff() to use a (struct object_array) for holding the trees that it accumulates, rather than rolling its own equivalent. A significant detail missing here is that this lifts

Bug when git rev-list options --first-parent and --ancestry-path are used together?

.) Is this a bug or are my expectations wrong? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ x.sh Description: Bourne shell script

[PATCH v2 00/25] Remove assumptions about each_ref_fn arg lifetimes

(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

[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

[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

[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

[PATCH v2 11/25] object_array_remove_duplicates(): rewrite to reduce copying

. 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

[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

[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

[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

[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

[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

[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

[PATCH v2 25/25] refs: document the lifetime of the args passed to each_ref_fn

-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

[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

[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

[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

[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

[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

[PATCH v2 12/25] fsck: don't put a void*-shaped peg in a char*-shaped hole

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

[PATCH v2 15/25] object_array_entry: fix memory handling of the name field

. 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

[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

[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

[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

[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

[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

[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

[PATCH v2 07/25] cmd_diff(): make it obvious which cases are exclusive of each other

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

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

Re: [PATCH v2 22/25] string_list_add_refs_by_glob(): add a comment about memory management

On 05/29/2013 10:21 AM, Thomas Rast wrote: Michael Haggerty mhag...@alum.mit.edu writes: 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

Re: [PATCH v2 00/25] Remove assumptions about each_ref_fn arg lifetimes

On 05/29/2013 10:25 AM, Thomas Rast wrote: Michael Haggerty mhag...@alum.mit.edu writes: I read the entire series on Monday, and give it an Ack at maybe 90% confidence level -- sorry, I was short on caffeine and sleep ;-) Thanks very much. I'll buy you a coffee the next time I see you

Re: [PATCH v2 11/25] object_array_remove_duplicates(): rewrite to reduce copying

On 05/29/2013 06:18 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: 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

Re: [PATCH v2 24/25] register_ref(): make a copy of the bad reference SHA-1

On 05/29/2013 06:53 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: [...] +current_bad_sha1 = xmalloc(20); +hashcpy(current_bad_sha1, sha1); This, together with 18/25, may hint that we want hashdup()? I thought about it, but was surprised

[PATCH v2 FIXUP 22/25] fixup! string_list_add_refs_by_glob(): add a comment about memory management

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Junio, would you mind squashing this patch onto mh/reflife 22/25? notes.c | 1 + 1 file changed, 1 insertion(+) diff --git a/notes.c b/notes.c index 602d956..b69c0b8 100644 --- a/notes.c +++ b/notes.c @@ -932,6 +932,7 @@ static int

Re: can we prevent reflog deletion when branch is deleted?

. Peff proposed a solution to this problem [1], but AFAIK it is not making progress. Michael [1] http://thread.gmane.org/gmane.comp.version-control.git/201715/focus=201752 -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line

Re: [Administrivia] On ruby and contrib/

. Perhaps ranking the tools based on the results of the Git user surveys would help bring the most popular to the top of each category. Michael [1] https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com

Re: [PATCH 4/4] archive: ignore blob objects when checking reachability

-- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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 2/2] Move sequencer to builtin

and sustainable) community. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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] Documentation/CommunityGuidelines

and specific incidents rather than broad generalizations. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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

[PATCH 1/4] resolve_ref_unsafe(): extract function handle_missing_loose_ref()

The nesting was getting a bit out of hand, and it's about to get worse. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 55 ++- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index d17931a

Re: [PATCH] Documentation/CommunityGuidelines

meltdown. For example, somebody like your mother or father, or a teacher whom you respect, or a member of clergy if you are so inclined. Ask that person's opinion about your email. It is so easy to lose perspective in the Internet. Michael -- Michael Haggerty mhag...@alum.mit.edu http

Re: [PATCH] Documentation/CommunityGuidelines

On 06/11/2013 07:00 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: [...] * When reviewing other peoples' code, be tactful and constructive. Set high expectations, but do what you can to help the submitter achieve them. Don't demand changes based only on your

Re: [PATCH] Documentation/CommunityGuidelines

-- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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] Documentation/CommunityGuidelines

On 06/11/2013 08:29 PM, John Keeping wrote: On Tue, Jun 11, 2013 at 10:00:56AM -0700, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: * When reviewing other peoples' code, be tactful and constructive. Set high expectations, but do what you can to help the submitter achieve

[PATCH 08/12] Extract a struct stat_data from cache_entry

Add public functions fill_stat_data() and match_stat_data() to work with it. This infrastructure will later be used to check the validity of other types of file. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- I'm not too familiar with this part of the code, so please make sure

[PATCH 10/12] get_packed_ref_cache: reload packed-refs file when it changes

loose ref, the new packed-refs file must be in place. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- This is Peff's work, rebased and with some smallish changes to fit it in with the packed_ref_cache data structure. refs.c | 21 - 1 file changed, 16 insertions(+), 5

[PATCH 02/12] pack_refs(): split creation of packed refs and entry writing

place. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 48 ++-- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/refs.c b/refs.c index dccfe14..bb3640f 100644 --- a/refs.c +++ b/refs.c @@ -2019,35 +2019,50 @@ struct

[PATCH 11/12] for_each_ref: load all loose refs before packed refs

put in the packed-refs file by the time we refresh. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Ditto. refs.c | 39 +++ 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 64f72ab..aa4641b 100644 --- a/refs.c +++ b

[PATCH 07/12] packed_ref_cache: increment refcount when locked

Increment the packed_ref_cache reference count while it is locked to prevent its being freed. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index d8e8ce2..92c8e97 100644 --- a/refs.c

[PATCH 04/12] refs: implement simple transactions for the packed-refs file

to add the new references within a transaction. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- The API docs are not clear about whether it is kosher to read lock_file::fd directly. It is only done in one file outside of lockfile.c. So this patch stores the fd of the lockfile separately

[PATCH 09/12] add a stat_validity struct

struct which encapsulates the concept of checking the stat-freshness of a file. It is implemented on top of struct stat_data to reuse the logic about which stat entries to trust for a particular platform, but hides the complexity behind two simple functions: check and update. Signed-off-by: Michael

[PATCH 06/12] do_for_each_entry(): increment the packed refs cache refcount

This function calls a user-supplied callback function which could do something that causes the packed refs cache to be invalidated. So acquire a reference count on the data structure to prevent our copy from being freed while we are iterating over it. Signed-off-by: Michael Haggerty mhag

Re: [PATCH 01/12] repack_without_ref(): split list curation and entry writing

On 06/12/2013 01:38 PM, Jeff King wrote: On Tue, Jun 11, 2013 at 11:48:21PM +0200, Michael Haggerty wrote: Split repack_without_ref() into multiple passes: * collect the list of refnames that should be deleted from packed_refs * delete those refnames from the cache * write the remainder

Re: [PATCH] Documentation/CommunityGuidelines

On 06/12/2013 10:02 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: I would prefer a community standards document that looks more like this: ... * Be welcoming to new community participants. Help them get oriented, and be patient with their questions. Gently

Re: [PATCH 4/4] resolve_ref_unsafe(): close race condition reading loose refs

On 06/13/2013 10:22 AM, Thomas Rast wrote: Michael Haggerty mhag...@alum.mit.edu writes: One race is still possible and undetected: another process could change the file from a regular file into a symlink between the call to lstat and the call to open(). The open() call would silently

Re: [PATCH 00/12] Fix some reference-related races

Thanks for all of the information. On 06/15/2013 10:13 PM, Ramsay Jones wrote: Michael Haggerty wrote: *This patch series must be built on top of mh/reflife.* [...] The other problem was that the for_each_ref() functions will die if the ref cache that they are iterating over is freed

Re: [PATCH 00/12] Fix some reference-related races

On 06/18/2013 08:13 PM, Ramsay Jones wrote: Michael Haggerty wrote: On 06/15/2013 10:13 PM, Ramsay Jones wrote: Now, turning to the new code, t3211-peel-ref.sh test #7 now works, but test #8 still fails... [ ... ] It should be impossible, because the current process

[PATCH v2 3/3] resolve_ref_unsafe(): close race condition reading loose refs

, we may still be depending on a cached view of the packed-refs file; that race will be dealt with in a future patch. This problem was reported and diagnosed by Jeff King p...@peff.net, and this solution is derived from his patch. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 28

[PATCH v2 2/3] resolve_ref_unsafe(): handle the case of an SHA-1 within loop

-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 22 +- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/refs.c b/refs.c index ab1f99e..867cf9f 100644 --- a/refs.c +++ b/refs.c @@ -1300,8 +1300,19 @@ const char *resolve_ref_unsafe(const char *refname, unsigned

[PATCH v2 1/3] resolve_ref_unsafe(): extract function handle_missing_loose_ref()

The nesting was getting a bit out of hand, and it's about to get worse. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 56 +++- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index 42a7e17

[PATCH v2 00/12] Fix some reference-related races

file when it changes for_each_ref: load all loose refs before packed refs Michael Haggerty (10): repack_without_ref(): split list curation and entry writing pack_refs(): split creation of packed refs and entry writing refs: wrap the packed refs cache in a level of indirection refs

[PATCH v2 10/12] get_packed_ref_cache: reload packed-refs file when it changes

loose ref, the new packed-refs file must be in place. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index 6d48b42..d53a172 100644 --- a/refs.c +++ b/refs.c @@ -825,6 +825,9

[PATCH v2 05/12] refs: manage lifetime of packed refs cache via reference counting

it is being iterated over. So far, only the reference in struct ref_cache::packed is counted; other users will be adjusted in separate commits. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 39 --- 1 file changed, 36 insertions(+), 3 deletions

[PATCH v2 03/12] refs: wrap the packed refs cache in a level of indirection

As we know, we can solve any problem in this manner. In this case, the problem is to avoid freeing a packed refs cache while somebody is using it. So add a level of indirection as a prelude to reference-counting the packed refs cache. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu

[PATCH v2 07/12] packed_ref_cache: increment refcount when locked

Increment the packed_ref_cache reference count while it is locked to prevent its being freed. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index d90f487..6d48b42 100644 --- a/refs.c

[PATCH v2 09/12] add a stat_validity struct

struct which encapsulates the concept of checking the stat-freshness of a file. It is implemented on top of struct stat_data to reuse the logic about which stat entries to trust for a particular platform, but hides the complexity behind two simple functions: check and update. Signed-off-by: Michael

[PATCH v2 02/12] pack_refs(): split creation of packed refs and entry writing

place. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 48 ++-- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/refs.c b/refs.c index 1184b99..9f1a007 100644 --- a/refs.c +++ b/refs.c @@ -2023,35 +2023,50 @@ struct

[PATCH v2 06/12] do_for_each_entry(): increment the packed refs cache refcount

This function calls a user-supplied callback function which could do something that causes the packed refs cache to be invalidated. So acquire a reference count on the data structure to prevent our copy from being freed while we are iterating over it. Signed-off-by: Michael Haggerty mhag

[PATCH v2 01/12] repack_without_ref(): split list curation and entry writing

packed_refs * delete those refnames from the cache * write the remainder to the packed-refs file The purpose of this change is to make the write the remainder part reusable. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 62

[PATCH v2 08/12] Extract a struct stat_data from cache_entry

Add public functions fill_stat_data() and match_stat_data() to work with it. This infrastructure will later be used to check the validity of other types of file. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/ls-files.c | 12 +++-- cache.h| 33 +--- read

[PATCH v2 04/12] refs: implement simple transactions for the packed-refs file

to add the new references within a transaction. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- builtin/clone.c | 7 - refs.c | 83 ++--- refs.h | 27 +-- 3 files changed, 98 insertions(+), 19 deletions

[PATCH v2 11/12] for_each_ref: load all loose refs before packed refs

put in the packed-refs file by the time we refresh. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 39 +++ 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index d53a172..350054c 100644 --- a/refs.c +++ b/refs.c

[PATCH v2 12/12] refs: do not invalidate the packed-refs cache unnecessarily

() is called; usually the old copy will still be valid. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index 350054c..4b989b0 100644 --- a/refs.c +++ b/refs.c @@ -2133,11 +2133,14 @@ int

Re: [PATCH v2 04/12] refs: implement simple transactions for the packed-refs file

On 06/19/2013 09:18 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Handle simple transactions for the packed-refs file at the packed_ref_cache level via new functions lock_packed_refs(), commit_packed_refs(), and rollback_packed_refs(). Only allow the packed ref

[PATCH v3 11/12] for_each_ref: load all loose refs before packed refs

put in the packed-refs file by the time we refresh. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu Signed-off-by: Jeff King p...@peff.net --- refs.c | 39 +++ 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 60f2507

[PATCH v3 12/12] refs: do not invalidate the packed-refs cache unnecessarily

() is called; usually the old copy will still be valid. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index 787cc1c..038e5c7 100644 --- a/refs.c +++ b/refs.c @@ -2136,11 +2136,14 @@ int

[PATCH v3 06/12] do_for_each_entry(): increment the packed refs cache refcount

This function calls a user-supplied callback function which could do something that causes the packed refs cache to be invalidated. So acquire a reference count on the data structure to prevent our copy from being freed while we are iterating over it. Signed-off-by: Michael Haggerty mhag

<    4   5   6   7   8   9   10   11   12   13   >