[PATCH v2 01/25] t3600: clean up permissions test properly

2017-05-22 Thread Michael Haggerty
in a `test_when_finished` block so that it can't be skipped. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- t/t3600-rm.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 5f9913ba33..f8568f8841 100755 --- a/t/t3600-rm.sh +

[PATCH v2 12/25] files_transaction_cleanup(): new helper function

2017-05-22 Thread Michael Haggerty
Extract the cleanup functionality from `files_transaction_commit()` into a new function. It will soon have another caller. Use the common cleanup code even on early exit if the transaction is empty, to reduce code duplication. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> ---

[PATCH v2 08/25] ref_store: take a `msg` parameter when deleting references

2017-05-22 Thread Michael Haggerty
()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- builtin/fetch.c| 2 +- builtin/remote.c | 4 ++-- refs.c | 11 ++- refs.h | 12 +++- refs/files-backend.c | 4 ++-- ref

[PATCH v2 02/25] refs.h: clarify docstring for the ref_transaction_update()-related fns

2017-05-22 Thread Michael Haggerty
In particular, make it clear that they make copies of the sha1 arguments. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.h | 13 + 1 file changed, 13 insertions(+) diff --git a/refs.h b/refs.h index 685a979a0e..ec8c6bfbbb 100644 --- a/refs.h +++ b/refs.h @@

[PATCH v2 14/25] ref_transaction_prepare(): new optional step for reference updates

2017-05-22 Thread Michael Haggerty
`. A `ref_transaction_commit()` now basically calls methods `transaction_prepare` then `transaction_finish`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 74 -- refs.h | 124 --- refs

[PATCH v2 16/25] ref_update_reject_duplicates(): use `size_t` rather than `int`

2017-05-22 Thread Michael Haggerty
Eliminate a theoretical risk of integer overflow if the two types have different sizes. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index beb49fb297..143936a9c3 100644 --- a/refs.c

[PATCH v2 07/25] refs: use `size_t` indexes when iterating over ref transaction updates

2017-05-22 Thread Michael Haggerty
Eliminate any chance of integer overflow on platforms where the two types have different sizes. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 2 +- refs/files-backend.c | 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/refs.c b/

[PATCH v2 06/25] refs_ref_iterator_begin(): don't check prefixes redundantly

2017-05-22 Thread Michael Haggerty
The backend already correctly restricts its output to references whose names start with the prefix. By passing the prefix again to `prefix_ref_iterator`, we were forcing that iterator to do redundant prefix comparisons. So set it to the empty string. Signed-off-by: Michael Haggerty <m

[PATCH v2 03/25] ref_iterator_begin_fn(): fix docstring

2017-05-22 Thread Michael Haggerty
, `cache_ref_iterator_begin()` (from which the files reference iterator gets its values) automatically wraps its output using `prefix_ref_iterator_begin()` when necessary, so it has the stricter behavior. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/refs-internal.h | 7 ---

[PATCH v2 09/25] lockfile: add a new method, is_lock_file_locked()

2017-05-22 Thread Michael Haggerty
It will soon prove useful. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- lockfile.h | 8 1 file changed, 8 insertions(+) diff --git a/lockfile.h b/lockfile.h index 7b715f9e77..572064939c 100644 --- a/lockfile.h +++ b/lockfile.h @@ -175,6 +175,14 @@ static inli

Re: [PATCH 22/23] ref-filter: limit traversal to prefix

2017-05-19 Thread Michael Haggerty
On 05/17/2017 03:38 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:45PM +0200, Michael Haggerty wrote: > >> From: Jeff King <p...@peff.net> > > This patch did originate with me, but I know you had to fix several > things to integrate it in your series. So I'll r

Re: [PATCH 12/23] ref_transaction_commit(): break into multiple functions

2017-05-19 Thread Michael Haggerty
On 05/17/2017 07:44 PM, Stefan Beller wrote: > On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> Break the function `ref_transaction_commit()` into two functions, >> `ref_transaction_prepare()` and `ref_transaction_finish()`,

Re: [PATCH 11/23] files_transaction_cleanup(): new helper function

2017-05-18 Thread Michael Haggerty
On 05/17/2017 03:19 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:34PM +0200, Michael Haggerty wrote: > >> Extract function from `files_transaction_commit()`. It will soon have >> another caller. >> [...] >> @@ -2868,10 +2889,8 @@ static int files_trans

Re: [PATCH 11/23] files_transaction_cleanup(): new helper function

2017-05-18 Thread Michael Haggerty
On 05/17/2017 07:26 PM, Stefan Beller wrote: > On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> Extract function from `files_transaction_commit()`. It will soon have >> another caller. > > This sounds odd to me. Maybe it is

Re: [PATCH 10/23] files_ref_store: put the packed files lock directly in this struct

2017-05-18 Thread Michael Haggerty
On 05/17/2017 07:18 PM, Stefan Beller wrote: > On Wed, May 17, 2017 at 6:17 AM, Jeff King <p...@peff.net> wrote: >> On Wed, May 17, 2017 at 02:05:33PM +0200, Michael Haggerty wrote: >> >>> Instead of using a global `lock_file` instance for the main >>>

Re: [PATCH 01/23] t3600: clean up permissions test properly

2017-05-18 Thread Michael Haggerty
On 05/18/2017 06:10 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> The test of failing `git rm -f` removes the write permissions on the >> test directory, but fails to restore them if the test fails. This >> means that the t

Re: [PATCH 06/23] refs: use `size_t` indexes when iterating over ref transaction updates

2017-05-17 Thread Michael Haggerty
On 05/17/2017 06:59 PM, Stefan Beller wrote: > On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: > > Now this would want to have some selling words for it? > I do not see an advantage of this patch as-is. > > I mean technically we d

Re: [PATCH 04/23] prefix_ref_iterator: don't trim too much

2017-05-17 Thread Michael Haggerty
On 05/18/2017 06:19 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> The `trim` parameter can be set independently of `prefix`. So if some >> caller were to set `trim` to be greater than `strlen(prefix)`, we >> could end u

Re: [PATCH 09/23] files-backend: move `lock` member to `files_ref_store`

2017-05-17 Thread Michael Haggerty
On 05/17/2017 03:15 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:32PM +0200, Michael Haggerty wrote: > >> @@ -70,6 +61,13 @@ struct files_ref_store { >> >> struct ref_cache *loose; >> struct packed_ref_cache *packed; >> + >> +

Re: [PATCH 19/23] read_packed_refs(): report unexpected fopen() failures

2017-05-17 Thread Michael Haggerty
On 05/17/2017 03:28 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:42PM +0200, Michael Haggerty wrote: > >> The old code ignored any errors encountered when trying to fopen the >> "packed-refs" file, treating all such failures as if the file d

Re: [PATCH 10/23] files_ref_store: put the packed files lock directly in this struct

2017-05-17 Thread Michael Haggerty
On 05/17/2017 03:17 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:33PM +0200, Michael Haggerty wrote: > >> Instead of using a global `lock_file` instance for the main >> "packed-refs" file and using a pointer in `files_ref_store` to keep >> tr

Re: [PATCH 07/23] ref_store: take `logmsg` parameter when deleting references

2017-05-17 Thread Michael Haggerty
On 05/17/2017 03:12 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:30PM +0200, Michael Haggerty wrote: > >> Just because the files backend can't retain reflogs for deleted >> references is no reason that they shouldn't be supported by the >> virtual method inter

Re: [PATCH 05/23] refs_ref_iterator_begin(): don't check prefixes redundantly

2017-05-17 Thread Michael Haggerty
On 05/17/2017 02:59 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:28PM +0200, Michael Haggerty wrote: > >> The backend already takes care of the prefix. By passing the prefix >> again to `prefix_ref_iterator`, we were forcing that iterator to do >> redundant pr

Re: [PATCH 04/23] prefix_ref_iterator: don't trim too much

2017-05-17 Thread Michael Haggerty
On 05/17/2017 02:55 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:27PM +0200, Michael Haggerty wrote: > >> diff --git a/refs/iterator.c b/refs/iterator.c >> index bce1f192f7..f33d1b3a39 100644 >> --- a/refs/iterator.c >> +++ b/refs/iterator.c >

Re: [PATCH 01/23] t3600: clean up permissions test properly

2017-05-17 Thread Michael Haggerty
On 05/17/2017 02:42 PM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:24PM +0200, Michael Haggerty wrote: > >> The test of failing `git rm -f` removes the write permissions on the >> test directory, but fails to restore them if the test fails. This >> means that the

Re: [PATCH v3 10/12] files-backend: make reflog iterator go through per-worktree reflog

2017-05-17 Thread Michael Haggerty
Hi, I put off reviewing this patch, thinking that it would appear in a re-roll, then never came back to it. :-( On 04/23/2017 06:44 AM, Duy Nguyen wrote: > On Sat, Apr 22, 2017 at 10:05:02AM +0200, Michael Haggerty wrote: >> I find this implementation confusing: >>

[PATCH 13/23] ref_update_reject_duplicates(): expose function to whole refs module

2017-05-17 Thread Michael Haggerty
It will soon have some other users. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 17 + refs/files-backend.c | 17 - refs/refs-internal.h | 8 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/re

[PATCH 11/23] files_transaction_cleanup(): new helper function

2017-05-17 Thread Michael Haggerty
Extract function from `files_transaction_commit()`. It will soon have another caller. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/refs/files-backend.c

[PATCH 21/23] create_ref_entry(): remove `check_name` option

2017-05-17 Thread Michael Haggerty
Only one caller was using it, so move the check to that caller. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 12 refs/ref-cache.c | 6 +- refs/ref-cache.h | 3 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff

[PATCH 08/23] lockfile: add a new method, is_lock_file_locked()

2017-05-17 Thread Michael Haggerty
It will soon prove useful. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- lockfile.h | 8 1 file changed, 8 insertions(+) diff --git a/lockfile.h b/lockfile.h index 7b715f9e77..572064939c 100644 --- a/lockfile.h +++ b/lockfile.h @@ -175,6 +175,14 @@ static inli

[PATCH 15/23] ref_update_reject_duplicates(): add a sanity check

2017-05-17 Thread Michael Haggerty
It's pretty cheap to make sure that the caller didn't pass us an unsorted list by accident, so do so. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index ffc9bd0be5..68a0

[PATCH 20/23] refs_ref_iterator_begin(): handle `GIT_REF_PARANOIA`

2017-05-17 Thread Michael Haggerty
Instead of handling `GIT_REF_PARANOIA` in `files_ref_iterator_begin()`, handle it in `refs_ref_iterator_begin()`, where it will cover all reference stores. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 5 + refs/files-backend.c | 11 --- 2

[PATCH 22/23] ref-filter: limit traversal to prefix

2017-05-17 Thread Michael Haggerty
ultiple patterns, but (a) it probably doesn't come up that often, and (b) it is more awkward to deal with multiple patterns (e.g., the patterns might not be disjoint). So, since this is just an optimization, punt on the case of multiple patterns. Signed-off-by: Jeff King <p...@peff.net> Signed-off-by:

[PATCH 23/23] cache_ref_iterator_begin(): avoid priming unneeded directories

2017-05-17 Thread Michael Haggerty
e useful for another patch series that I'm working on. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/ref-cache.c | 93 ++-- 1 file changed, 83 insertions(+), 10 deletions(-) diff --git a/refs/ref-cache.c b/refs/ref-cache.c in

[PATCH 14/23] ref_update_reject_duplicates(): use `size_t` rather than `int`

2017-05-17 Thread Michael Haggerty
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 43d65bc9c6..ffc9bd0be5 100644 --- a/refs.c +++ b/refs.c @@ -1692,7 +1692,7 @@ int create_symref(const char *ref_target, cons

[PATCH 18/23] read_packed_refs(): do more of the work of reading packed refs

2017-05-17 Thread Michael Haggerty
Teach `read_packed_refs()` to also * Allocate and initialize the new `packed_ref_cache` * Open and close the `packed-refs` file * Update the `validity` field of the new object This decreases the coupling between `packed_refs_cache` and `files_ref_store` by a little bit. Signed-off-by: Michael

[PATCH 06/23] refs: use `size_t` indexes when iterating over ref transaction updates

2017-05-17 Thread Michael Haggerty
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 2 +- refs/files-backend.c | 6 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index f4a485cd8a..ea8233c67d 100644 --- a/refs.c +++ b/refs.c @@ -848,7 +848,7 @@

[PATCH 10/23] files_ref_store: put the packed files lock directly in this struct

2017-05-17 Thread Michael Haggerty
keep track of whether it is locked. This keeps related data together and makes the main reference store less of a special case. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 29 +++-- 1 file changed, 11 insertions(+), 18 deletions(-) d

[PATCH 16/23] should_pack_ref(): new function, extracted from `files_pack_refs()`

2017-05-17 Thread Michael Haggerty
Extract a function for deciding whether a reference should be packed. It is a self-contained bit of logic, so splitting it out improves readability. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 42 -- 1 file chang

[PATCH 09/23] files-backend: move `lock` member to `files_ref_store`

2017-05-17 Thread Michael Haggerty
Move the `lock` member from `packed_ref_cache` to `files_ref_store`, since at most one cache can have a locked "packed-refs" file associated with it. Rename it to `packlock` to make its purpose clearer in its new home. More changes are coming here shortly. Signed-off-by: Michael Hagg

[PATCH 12/23] ref_transaction_commit(): break into multiple functions

2017-05-17 Thread Michael Haggerty
sub-transactions. Only if all of the "prepare" steps succeed would we "finish" each of them. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 34 ++--- refs.h | 3

[PATCH 05/23] refs_ref_iterator_begin(): don't check prefixes redundantly

2017-05-17 Thread Michael Haggerty
The backend already takes care of the prefix. By passing the prefix again to `prefix_ref_iterator`, we were forcing that iterator to do redundant prefix comparisons. So set it to the empty string. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 8 +++- 1 file chan

[PATCH 17/23] get_packed_ref_cache(): assume "packed-refs" won't change while locked

2017-05-17 Thread Michael Haggerty
If we've got the "packed-refs" file locked, then it can't change; there's no need to keep calling `stat_validity_check()` on it. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletion

[PATCH 19/23] read_packed_refs(): report unexpected fopen() failures

2017-05-17 Thread Michael Haggerty
oblems. So report any failures that are not due to ENOENT. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 6a037e1d61..e

[PATCH 02/23] refs.h: clarify docstring for the ref_transaction_update()-related fns

2017-05-17 Thread Michael Haggerty
In particular, make it clear that they make copies of the sha1 arguments. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.h | 13 + 1 file changed, 13 insertions(+) diff --git a/refs.h b/refs.h index d18ef47128..a7d7b1afdf 100644 --- a/refs.h +++ b/refs.h @@

[PATCH 07/23] ref_store: take `logmsg` parameter when deleting references

2017-05-17 Thread Michael Haggerty
Just because the files backend can't retain reflogs for deleted references is no reason that they shouldn't be supported by the virtual method interface. Let's add them now before the interface becomes truly polymorphic and increases the work. Signed-off-by: Michael Haggerty <mhag...@alum.mit.

[PATCH 00/23] Prepare to separate out a packed_ref_store

2017-05-17 Thread Michael Haggerty
rest of the changes (which works but is not yet polished), checkout the `mmap-packed-refs` branch from the same place. Michael [1] http://public-inbox.org/git/cover.1490026594.git.mhag...@alum.mit.edu/ http://public-inbox.org/git/cover.1490966385.git.mhag...@alum.mit.edu/ http://public

[PATCH 01/23] t3600: clean up permissions test properly

2017-05-17 Thread Michael Haggerty
in a `test_must_fail` block so that it can't be skipped. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- t/t3600-rm.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 5f9913ba33..4a35c378c8 100755 --- a/t/t3600-rm.sh +++ b/t

[PATCH 04/23] prefix_ref_iterator: don't trim too much

2017-05-17 Thread Michael Haggerty
here. Skip over any references whose names are not longer than `trim`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/iterator.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/refs/iterator.c b/refs/iterator.c index bce1f192f7..f33d1b3a39

[PATCH 03/23] ref_iterator_begin_fn(): fix docstring

2017-05-17 Thread Michael Haggerty
, `cache_ref_iterator_begin()` (from which the files reference iterator gets its values) automatically wraps its output using `prefix_ref_iterator_begin()` when necessary, so it has the stricter behavior. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/refs-internal.h | 4 ++--

Performance regression in `git branch` due to ref-filter usage

2017-05-17 Thread Michael Haggerty
other commands have similar regressions. Michael [1] One wonders why the file has to be read more than once, but that's a different story and probably harder to fix.

Re: Git 2.13.0 segfaults on Solaris SPARC due to DC_SHA1=YesPlease being on by default

2017-05-15 Thread Michael Kebe
Hi Marc, works like a charm! Michael 2017-05-15 16:33 GMT+02:00 Marc Stevens <m...@marc-stevens.nl>: > Hi Michael, > > > > See the latest commit to the SHA1DC repo: > > https://github.com/cr-marcstevens/sha1collisiondetection/commit/33a694a9ee1b79c24be45f9eab5a

Re: [PATCHv2 0/3] Make diff plumbing commands respect the indentHeuristic.

2017-04-29 Thread Michael Haggerty
gt; diff: have the diff-* builtins configure diff before initializing >> revisions >> >> Stefan Beller (1): >> diff: enable indent heuristic by default > > Thanks, these look fine to me. I'd like to get an ACK from Michael, in > case he had some other rea

Re: [PATCH 39/53] refs/files-backend: convert many internals to struct object_id

2017-04-26 Thread Michael Haggerty
strbuf will be NUL-terminated and that parse_oid_hex will fail on > truncated input to avoid the need to check for an explicit length. > > This is a requirement to convert parse_object later on. > [...] This patch also looks fine to me. Michael

Re: [PATCH 38/53] refs: convert struct ref_update to use struct object_id

2017-04-26 Thread Michael Haggerty
On 04/23/2017 11:34 PM, brian m. carlson wrote: > Convert struct ref_array_item to use struct object_id by changing the > definition and applying the following semantic patch, plus the standard > object_id transforms: > [...] This commit LGTM. Michael

Re: [PATCH] tests: remove the GETTEXT_POISON compile-time option to test i18n marking

2017-04-26 Thread Michael J Gruber
[Turns out I still can't operate gmail's web interface. Sorry for the dupe.] 2017-04-24 13:04 GMT+02:00 Ævar Arnfjörð Bjarmason : > Remove the GETTEXT_POISON=YesPlease compile-time which turns all of > git's LC_*=C output into strings like "# GETTEXT POISON #" instead of >

Re: [PATCH v3 00/12] Fix git-gc losing objects in multi worktree

2017-04-22 Thread Michael Haggerty
leaner distinction between "logical" and "physical" ref_stores. But given the current state of the code, your implementation is reasonable. Michael

Re: [PATCH v3 10/12] files-backend: make reflog iterator go through per-worktree reflog

2017-04-22 Thread Michael Haggerty
continue; if (refs_read_ref_full(iter->ref_store, iter->base.refname, 0, iter->oid.hash, )) { error("bad ref for %s", (*diter)->path.buf); continue; } iter->base.oid = >oid; iter->base.flags = flags; return ITER_OK; } if (ref_iterator_abort(ref_iterator) == ITER_ERROR) return ITER_ERROR; return ok; } > if (ref_iterator_abort(ref_iterator) == ITER_ERROR) > ok = ITER_ERROR; > return ok; > [...] Michael

Re: [PATCH v3 06/12] refs: add refs_head_ref()

2017-04-22 Thread Michael Haggerty
; > int refs_for_each_ref(struct ref_store *refs, > each_ref_fn fn, void *cb_data); > int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, > I'm seeing segfaults in t3600 after this patch, apparently because `refs==NULL` gets passed from `head_ref_submodule()` to `refs_head_ref()`. Michael

Re: [PATCH v3 09/12] revision.c: --all adds HEAD from all worktrees

2017-04-21 Thread Michael Haggerty
f that's not possible now, maybe they will have done it with some future version of git or with another tool). If the problem is only that this version of git is too stupid to handle pruning safely in that situation, it would be more appropriate to use something more like if (!refs->single_worktree) die("error: git is currently unable to handle submodules that use linked worktrees"); > refs = get_submodule_ref_store(submodule); > } else > refs = get_main_ref_store(); > [...] Michael

Re: [PATCH v3 08/12] refs: remove dead for_each_*_submodule()

2017-04-21 Thread Michael Haggerty
On 04/19/2017 01:01 PM, Nguyễn Thái Ngọc Duy wrote: > These are used in revision.c. After the last patch they are replaced > with the refs_ version. Delete them (except for_each_remote_ref_submodule > which is still used by submodule.c) ❤❤ I love the way this is going. Michael

Re: [PATCH v3 05/12] refs: move submodule slash stripping code to get_submodule_ref_store

2017-04-21 Thread Michael Haggerty
derstand that's what the code did before this patch, but it seems to me more like an accident of the old design rather than something worth supporting. In other words, if a caller would really pass us such a string, it seems like we could declare the caller buggy, no? > [...] Otherwise, looks good and makes a lot of sense. Michael

Re: [PATCH v3 04/12] refs.c: refactor get_submodule_ref_store(), share common free block

2017-04-21 Thread Michael Haggerty
strbuf_release(_sb); > - return NULL; > - } > + if (ret) > + goto done; After this change, the temporary variable `ret` could be eliminated. > [...] Michael

Re: [PATCH v4 0/5] Kill manual ref parsing code in worktree.c

2017-04-21 Thread Michael Haggerty
>> The commit message in 1/5 is rephrased a bit, hopefully clearer. > > Michael, does this look good to replace what has been queued? I finally reviewed this patch series. The refs-related changes look fine, and the submodule-related changes (which I'm not so familiar with) looks plausible. It's a nice cleanup :-) Michael

Re: [PATCH v4 3/5] refs: introduce get_worktree_ref_store()

2017-04-21 Thread Michael Haggerty
REF_STORE_MAIN; This constant appears another place, too. It might make sense to define a constant `REF_STORE_ALL_CAPABILITIES` in `refs-internal.h` alongside the individual bit values. If you prefer not to, please at least declare this variable `const` to spare the reader the trouble of looking to see whether it is modified before it is used. Otherwise, looks fine to me. > [...] Michael

Re: [BUG] test suite broken with GETTEXT_POISON=YesPlease

2017-04-21 Thread Michael J Gruber
olves l10n, but usually I run specific tests only. To be honest: I have to make sure not to get confused by (nor forget one of) the build flag GETTEXT_POISON and the environment variable GIT_GETTEXT_POISON. I'm not sure I always tested what I meant to test... Michael

Re: [PATCH] refs.h: rename submodule arguments to submodule_path

2017-04-21 Thread Michael Haggerty
On 04/21/2017 08:32 AM, Michael Haggerty wrote: > [...] > I've CCed Duy because I don't know whether he has more plans regarding > submodule references [...] get rid of the > `for_each_ref_submodule()` family of functions entirely. > > So perhaps the code that this patch touch

Re: [PATCH] refs.h: rename submodule arguments to submodule_path

2017-04-21 Thread Michael Haggerty
On 04/21/2017 03:12 AM, Junio C Hamano wrote: > Stefan Beller <sbel...@google.com> writes: > >> + Junio > > Just like Michael, I do not have strong enough opinion for or > against this patch to comment on it. > > I do agree with you that it would be a go

Re: [PATCH v10 4/5] dir_iterator: rewrite state machine model

2017-04-20 Thread Michael Haggerty
refs/files-backend.c to pass > the flags parameter introduced, as well as handle the case in which it > fails to open the directory. > > Improve t/t0065-dir-iterator.sh and t/helper/test-dir-iterator.c to > test "post-order" and "iterate-over-root" modes. > &

Re: Draft of Git Rev News edition 26

2017-04-19 Thread Michael J Gruber
not installed) because the main problem is mixed installations of gpg1 and gpg2.1+, and we don't want to use a library instead of the command line API for the reasons mentioned by Linus and others. Michael

Re: [PATCH] files_for_each_reflog_ent_reverse(): close stream and free strbuf on error

2017-04-17 Thread Michael Haggerty
%d bytes from reflog for %s: > %s", > - cnt, refname, strerror(errno)); > + if (nread != 1) { > + ret = error("cannot read %d bytes from reflog for %s: > %s", > + cnt, refname, strerror(errno)); > + break; > + } > pos -= cnt; > > scanp = endp = buf + cnt; > Reviewed-by: Michael Haggerty <mhag...@alum.mit.edu> Michael

Re: [PATCH v8 4/5] dir_iterator: refactor state machine model

2017-04-17 Thread Michael Haggerty
RAVERSAL; > + else if (!strcmp(*myargv, "--post-order")) > + flag |= DIR_ITERATOR_POST_ORDER_TRAVERSAL; > + else if (!strcmp(*myargv, "--list-root-dir")) > + flag |= DIR_ITERATOR_LIST_ROOT_DIR; > + else if (!strcmp(*myargv, "--")) { > + myargc--; > + myargv++; > + break; > + } else > +die("Unrecognized option: %s", *myargv); The indentation above is wrong. > + } > > - strbuf_add(, argv[1], strlen(argv[1])); > + if (myargc != 1) > + die("expected exactly one non-option argument"); > + strbuf_addstr(, *myargv); > > - diter = dir_iterator_begin(path.buf); > + diter = dir_iterator_begin(path.buf, flag); > > while (dir_iterator_advance(diter) == ITER_OK) { > if (S_ISDIR(diter->st.st_mode)) > [...] Michael

Re: [PATCH v8 0/5] [GSoC] remove_subtree(): reimplement using iterators

2017-04-17 Thread Michael Haggerty
-21504-1-git-send-email-bnm...@gmail.com/T/#t > v7: > https://public-inbox.org/git/1491163388-41255-1-git-send-email-bnm...@gmail.com/T/#t > > Travis CI build: https://travis-ci.org/theiostream/git/jobs/21982 > > In this version, I applied pretty much all suggestions Michael an

[PATCH v3 14/20] do_for_each_entry_in_dir(): eliminate `offset` argument

2017-04-16 Thread Michael Haggerty
It was never used. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- refs/ref-cache.c | 6 +++--- refs/ref-cache.h | 11 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/refs/files-backend.c b/refs/files-bac

[PATCH v3 12/20] ref-cache: use a callback function to fill the cache

2017-04-16 Thread Michael Haggerty
` to supply a pointer to function `read_loose_refs` (renamed to `loose_fill_ref_dir`) when creating the ref cache for its loose refs. This means that we can generify the type of the back-pointer in `struct ref_cache` from `files_ref_store` to `ref_store`. Signed-off-by: Michael Haggerty <m

[PATCH v3 16/20] get_loose_ref_cache(): new function

2017-04-16 Thread Michael Haggerty
Extract a new function, `get_loose_ref_cache()`, from get_loose_ref_dir(). The function returns the `ref_cache` for the loose refs of a `files_ref_store`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 9 +++-- 1 file changed, 7 insertions(+), 2 del

[PATCH v3 02/20] refs_read_raw_ref(): new function

2017-04-16 Thread Michael Haggerty
Extract a new function from `refs_resolve_ref_unsafe()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 11 +-- refs/refs-internal.h | 4 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/refs.c b/

[PATCH v3 09/20] refs: split `ref_cache` code into separate files

2017-04-16 Thread Michael Haggerty
, adds declarations, and changes the visibility of some functions, but doesn't change any code. The modules are still too tightly coupled, but the situation will be improved in subsequent commits. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- Makefile | 1 + refs

[PATCH v3 17/20] cache_ref_iterator_begin(): make function smarter

2017-04-16 Thread Michael Haggerty
ator_begin()` to be made more ignorant of the internals of `ref_cache`, and `find_containing_dir()` and `prime_ref_dir()` to be made private. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 44 +--- refs/ref-

[PATCH v3 04/20] refs_verify_refname_available(): implement once for all backends

2017-04-16 Thread Michael Haggerty
It turns out that we can now implement `refs_verify_refname_available()` based on the other virtual functions, so there is no need for it to be defined at the backend level. Instead, define it once in `refs.c` and remove the `files_backend` definition. Signed-off-by: Michael Haggerty <m

[PATCH v3 05/20] refs_verify_refname_available(): use function in more places

2017-04-16 Thread Michael Haggerty
(and will be regained later). These were the last callers of `verify_refname_available_dir()`, so also delete that (very complicated) function. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 171 --- 1 file chang

[PATCH v3 20/20] do_for_each_entry_in_dir(): delete function

2017-04-16 Thread Michael Haggerty
Its only remaining caller was itself. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/ref-cache.c | 21 - refs/ref-cache.h | 11 --- 2 files changed, 32 deletions(-) diff --git a/refs/ref-cache.c b/refs/ref-cache.c index b3a30350d7..6059362f1d

[PATCH v3 06/20] ref-cache: rename `add_ref()` to `add_ref_entry()`

2017-04-16 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH v3 07/20] ref-cache: rename `find_ref()` to `find_ref_entry()`

2017-04-16 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH v3 03/20] refs_ref_iterator_begin(): new function

2017-04-16 Thread Michael Haggerty
Extract a new function from `do_for_each_ref()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 15 +-- refs/refs-internal.h | 11 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/re

[PATCH v3 19/20] files_pack_refs(): use reference iteration

2017-04-16 Thread Michael Haggerty
`pack_refs_cb_data` to preserve intermediate state. This removes the last callers of `entry_resolves_to_object()` and `get_loose_ref_dir()`, so delete those functions. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c

[PATCH v3 13/20] refs: handle "refs/bisect/" in `loose_fill_ref_dir()`

2017-04-16 Thread Michael Haggerty
That "refs/bisect/" has to be handled specially when filling the ref_cache for loose references is a peculiarity of the files backend, and the ref-cache code shouldn't need to know about it. So move this code to the callback function, `loose_fill_ref_dir()`. Signed-off-by: Michael Hagg

[PATCH v3 11/20] refs: record the ref_store in ref_cache, not ref_dir

2017-04-16 Thread Michael Haggerty
`. So change `create_dir_entry()` to take a `ref_cache` parameter, and change its callers to pass the correct `ref_cache` depending on the purpose of the new `dir_entry`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- refs/ref-cache.c

[PATCH v3 10/20] ref-cache: introduce a new type, ref_cache

2017-04-16 Thread Michael Haggerty
For now, it just wraps a `ref_entry *` that points at the root of the tree. Soon it will hold more information. Add two new functions, `create_ref_cache()` and `free_ref_cache()`. Make `free_ref_entry()` private. Change files-backend to use this type to hold its caches. Signed-off-by: Michael

[PATCH v3 18/20] commit_packed_refs(): use reference iteration

2017-04-16 Thread Michael Haggerty
-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 38 +- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 8c360869c1..1419512d51 100644 --- a/refs/files-backend.c +++ b/refs

[PATCH v3 08/20] ref-cache: rename `remove_entry()` to `remove_entry_from_dir()`

2017-04-16 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH v3 15/20] get_loose_ref_dir(): function renamed from get_loose_refs()

2017-04-16 Thread Michael Haggerty
The new name is more analogous to `get_packed_ref_dir()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index c0550ad9d6..3beab0b752

[PATCH v3 01/20] get_ref_dir(): don't call read_loose_refs() for "refs/bisect"

2017-04-16 Thread Michael Haggerty
nism, and this time the read was done correctly. This code has been broken since it was first introduced. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 4d

[PATCH v3 00/20] Separate `ref_cache` into a separate module

2017-04-16 Thread Michael Haggerty
cache". These patches depend on Duy's nd/files-backend-git-dir branch. [1] http://public-inbox.org/git/cover.1490966385.git.mhag...@alum.mit.edu/ [2] http://public-inbox.org/git/cover.1490026594.git.mhag...@alum.mit.edu/ [3] https://github.com/mhagger/git Michael Haggerty (20): get_ref_dir():

Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module

2017-04-16 Thread Michael Haggerty
On 04/07/2017 01:53 PM, Duy Nguyen wrote: > On Wed, Apr 5, 2017 at 9:03 PM, Duy Nguyen <pclo...@gmail.com> wrote: >> On Sat, Apr 1, 2017 at 12:16 PM, Michael Haggerty <mhag...@alum.mit.edu> >> wrote: >>> Duy, have you looked over my patch series? Since you'v

Re: [PATCH v2 19/20] files_pack_refs(): use reference iteration

2017-04-16 Thread Michael Haggerty
On 04/07/2017 01:51 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> Use reference iteration rather than do_for_each_entry_in_dir() in the >> definition of files_pack_refs(). > > A "why"

Re: [PATCH v2 11/20] refs: record the ref_store in ref_cache, not ref_dir

2017-04-15 Thread Michael Haggerty
On 04/07/2017 01:38 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> Instead of keeping a pointer to the ref_store in every ref_dir entry, >> store it once in `struct ref_cache`, and change `struct ref_dir` to >

Re: [PATCH v2 10/20] ref-cache: introduce a new type, ref_cache

2017-04-15 Thread Michael Haggerty
On 04/07/2017 01:32 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> +void free_ref_cache(struct ref_cache *cache) >> +{ >> + free_ref_entry(cache->root); >> + free(cache); >> +}

Re: [PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends

2017-04-15 Thread Michael Haggerty
On 04/07/2017 01:20 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> It turns out that we can now implement >> `refs_verify_refname_available()` based on the other virtual >> functions, so there i

Re: [PATCH v2 03/20] refs_ref_iterator_begin(): new function

2017-04-15 Thread Michael Haggerty
On 04/07/2017 12:57 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> Extract a new function from `do_for_each_ref()`. It will be useful >> elsewhere. >> >> Signed-off-by: Michael Haggerty <

<    1   2   3   4   5   6   7   8   9   10   >