Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

2018-11-16 Thread Michael Haggerty
On Fri, Nov 16, 2018 at 11:38 AM Ævar Arnfjörð Bjarmason wrote: > [...] > A follow-up on this: We should really fix this for other > reasons. I.e. compile in some "this is stuff we ourselves think is in > git". > > There's other manifestations of this, e.g.: > > git-sizer --help # => shows

Re: [PATCH 00/17] object_id part 14

2018-07-14 Thread Michael Haggerty
spot anything odd, except for > > the question about reasoning for why we use memcmp directly over using > > hashcmp. I don't think that's any sort of blocker, it just seemed an > > odd decision to me. > > I also read through the series and only found the 100/200 constants &g

Re: [PATCH] xdiff: reduce indent heuristic overhead

2018-07-03 Thread Michael Haggerty
On Mon, Jul 2, 2018 at 7:27 PM Stefan Beller wrote: > On Sun, Jul 1, 2018 at 8:57 AM Michael Haggerty wrote: > [...] > So this suggests to have MAX_BORING to be > "hunk size + some small constant offset" ? That would be my suggestion, yes. There are cases where it will

Re: [PATCH] xdiff: reduce indent heuristic overhead

2018-07-01 Thread Michael Haggerty
On 06/29/2018 10:28 PM, Stefan Beller wrote: > [...] > Adds some threshold to avoid expensive cases, like: > > ``` > #!python > open('a', 'w').write(" \n" * 100) > open('b', 'w').write(" \n" * 101) > ``` > > The indent heuristic is O(N * 20) (N = 100) for

Re: [PATCH] t9104: kosherly remove remote refs

2018-06-02 Thread Michael Haggerty
ef backend is used. > > Suggested-by: Michael Haggerty > Helped-by: Jeff King > Signed-off-by: Christian Couder > --- > This was suggested and discussed in: > > https://public-inbox.org/git/20180525085906.ga2...@sigill.intra.peff.net/ > > t/t9104-git-svn-follow-pa

Re: [PATCH 1/3] refs/packed-backend.c: close fd of empty file

2018-05-31 Thread Michael Haggerty
On Wed, May 30, 2018 at 7:03 PM, Stefan Beller wrote: > Signed-off-by: Stefan Beller > --- > > This was an oversight in 01caf20d57a (load_contents(): don't try to mmap an > empty file, 2018-01-24). > > This and the following 2 patches apply on master. > > refs/packed-backend.c | 1 + > 1 file

Re: [PATCH] t990X: use '.git/objects' as 'deep inside .git' path

2018-05-26 Thread Michael Haggerty
On Sat, May 26, 2018 at 8:47 AM, Christian Couder wrote: > Tests t9902-completion.sh and t9903-bash-prompt.sh each have tests > that check what happens when we are "in the '.git' directory" and > when we are "deep inside the '.git' directory". > > To test the case when

Re: [PATCH v2] t: make many tests depend less on the refs being files

2018-05-25 Thread Michael Haggerty
On Fri, May 25, 2018 at 10:59 AM, Jeff King <p...@peff.net> wrote: > On Fri, May 25, 2018 at 10:48:04AM +0200, Michael Haggerty wrote: > >> > test_expect_success "multi-fetch works off a 'clean' repository" ' >> > - rm -r "$GIT_DIR/svn" "$G

Re: [PATCH v2] t: make many tests depend less on the refs being files

2018-05-25 Thread Michael Haggerty
On 05/23/2018 07:25 AM, Christian Couder wrote: > From: David Turner > > Many tests are very focused on the file system representation of the > loose and packed refs code. As there are plans to implement other > ref storage systems, let's migrate these tests to a form

Re: commit-graph: change in "best" merge-base when ambiguous

2018-05-25 Thread Michael Haggerty
On 05/25/2018 12:08 AM, Jakub Narebski wrote: > Derrick Stolee <sto...@gmail.com> writes: >> On 5/22/2018 1:39 AM, Michael Haggerty wrote: >>> On 05/21/2018 08:10 PM, Derrick Stolee wrote: >>>> [...] >>> This may be beyond the scope of what you

Re: commit-graph: change in "best" merge-base when ambiguous

2018-05-21 Thread Michael Haggerty
On 05/21/2018 08:10 PM, Derrick Stolee wrote: > [...] > In the Discussion section of the `git merge-base` docs [1], we have the > following: > >     When the history involves criss-cross merges, there can be more than > one best common ancestor for two commits. For example, with this topology: >

Re: Implementing reftable in Git

2018-05-11 Thread Michael Haggerty
On Wed, May 9, 2018 at 4:33 PM, Christian Couder wrote: > I might start working on implementing reftable in Git soon. > [...] Nice. It'll be great to have a reftable implementation in git core (and ideally libgit2, as well). It seems to me that it could someday become

Re: [PATCH] refs: handle null-oid for pseudorefs

2018-05-07 Thread Michael Haggerty
On 05/06/2018 03:35 PM, Martin Ågren wrote: > According to the documentation on `git update-ref`, it is possible to > "specify 40 '0' or an empty string as to make sure that the > ref you are creating does not exist." But in the code for pseudorefs, we > do not implement this. If we fail to read

Re: [PATCH 12/16] refs: store the main ref store inside the repository struct

2018-04-10 Thread Michael Haggerty
On 04/10/2018 12:45 AM, Stefan Beller wrote: > Signed-off-by: Stefan Beller > --- > refs.c | 13 + > refs.h | 4 +--- > repository.h | 3 +++ > 3 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/refs.c b/refs.c > index

Re: [PATCH 07/16] refs: add repository argument to get_main_ref_store

2018-04-10 Thread Michael Haggerty
On 04/10/2018 12:45 AM, Stefan Beller wrote: > Add a repository argument to allow the get_main_ref_store caller > to be more specific about which repository to handle. This is a small > mechanical change; it doesn't change the implementation to handle > repositories other than the_repository yet.

[RFC 1/1] packed-backend: ignore broken headers

2018-03-26 Thread Michael Haggerty
file. But the file format isn't actually documented to allow comments, and comments make little sense in this file, so we won't go that far.) Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- I *don't* think we actually want to merge this patch. See the cover letter for my reasoning

[RFC 0/1] Tolerate broken headers in `packed-refs` files

2018-03-26 Thread Michael Haggerty
atch would be counterproductive. But I wanted to share it with the list anyway. Michael Michael Haggerty (1): packed-backend: ignore broken headers refs/packed-backend.c | 21 + t/t3210-pack-refs.sh | 33 - 2 files changed, 41 insertions(+), 13 deletions(-) -- 2.14.2

Re: [ANNOUNCE] git-sizer: compute various size-related metrics for your Git repository

2018-03-18 Thread Michael Haggerty
On Fri, Mar 16, 2018 at 10:29 PM, Jeff King wrote: > On Fri, Mar 16, 2018 at 09:01:42PM +0100, Ævar Arnfjörð Bjarmason wrote: >> One thing that can make repositories very pathological is if the ratio >> of trees to commits is too low. >> >> I was dealing with a repo the other day

[ANNOUNCE] git-sizer: compute various size-related metrics for your Git repository

2018-03-16 Thread Michael Haggerty
What makes a Git repository unwieldy to work with and host? It turns out that the respository's on-disk size in gigabytes is only part of the story. From our experience at GitHub, repositories cause problems because of poor internal layout at least as often as because of their overall size. For

Re: [git-sizer] Implications of a large commit object

2018-03-14 Thread Michael Haggerty
On Wed, Mar 14, 2018 at 9:14 AM, Lars Schneider wrote: > I am using Michael's fantastic Git repo analyzer tool "git-sizer" [*] > and it detected a very large commit of 7.33 MiB in my repo (see chart > below). > > This large commit is expected. I've imported that repo

Re: [PATCH] sq_dequote: fix extra consumption of source string

2018-02-18 Thread Michael Haggerty
On Wed, Feb 14, 2018 at 12:41 AM, Jeff King wrote: > This fixes a (probably harmless) parsing problem in > sq_dequote_step(), in which we parse some bogus input > incorrectly rather than complaining that it's bogus. > [...] LGTM. Thanks for taking care of this. Michael

Re: make git diff output easier to read - use better diff heuristics

2018-02-13 Thread Michael Haggerty
On Tue, Feb 13, 2018 at 7:25 PM, Σπύρος Βαζαίος wrote: > While I din't have the experience to express an opinion on this > matter, I have to say that the --no-indent-heuristic that Jeff > suggested worked great. > There were more than a handful of cases that this issue

[PATCH 4/6] packed_ref_iterator_begin(): make optimization more general

2018-01-24 Thread Michael Haggerty
the determined start position is the same as `snapshot->eof`. (This is possible now because the previous commit made `find_reference_location()` robust against empty snapshots.) Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 12 ++-- 1 file c

[PATCH 5/6] load_contents(): don't try to mmap an empty file

2018-01-24 Thread Michael Haggerty
without trying to read or mmap the file. Returning 0 also makes `create_snapshot()` exit early, which avoids the technically undefined comparison `NULL < NULL`. Reported-by: Kim Gybels <kgyb...@infogroep.be> Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed

[PATCH 0/6] Yet another approach to handling empty snapshots

2018-01-24 Thread Michael Haggerty
4 from this patch series would be worthwhile improvements. Michael Kim Gybels (1): packed_ref_cache: don't use mmap() for small files Michael Haggerty (5): struct snapshot: store `start` rather than `header_len` create_snapshot(): use `xmemdupz()` rather than a strbuf find_reference_location()

[PATCH 2/6] create_snapshot(): use `xmemdupz()` rather than a strbuf

2018-01-24 Thread Michael Haggerty
It's lighter weight. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index b872267f02..08698de6ea 100644 --- a/refs/packed-backend.c

[PATCH 6/6] packed_ref_cache: don't use mmap() for small files

2018-01-24 Thread Michael Haggerty
ts...@pobox.com> Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index e829cf206d..8b4b45da67 100644 --- a/refs/packed-backend.c +++

[PATCH 3/6] find_reference_location(): make function safe for empty snapshots

2018-01-24 Thread Michael Haggerty
ed NULL when `mustexist` was false, contrary to its docstring. Change the check and fix the docstring. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/refs/packed-backend.

[PATCH 1/6] struct snapshot: store `start` rather than `header_len`

2018-01-24 Thread Michael Haggerty
Store a pointer to the start of the actual references within the `packed-refs` contents rather than storing the length of the header. This is more convenient for most users of this field. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.

Re: [PATCH v3] packed_ref_cache: don't use mmap() for small files

2018-01-24 Thread Michael Haggerty
On 01/22/2018 08:31 PM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> `snapshot->buf` can still be NULL if the `packed-refs` file didn't exist >> (see the earlier code path in `load_contents()`). So either that code >> pa

Re: [PATCH] files_initial_transaction_commit(): only unlock if locked

2018-01-22 Thread Michael Haggerty
On 01/19/2018 11:14 PM, Junio C Hamano wrote: > Jeff King writes: > >> On Thu, Jan 18, 2018 at 02:38:41PM +0100, Mathias Rav wrote: >> >>> Running git clone --single-branch --mirror -b TAGNAME previously >>> triggered the following error message: >>> >>> fatal: multiple

Re: [PATCH v3] packed_ref_cache: don't use mmap() for small files

2018-01-20 Thread Michael Haggerty
On 01/17/2018 11:09 PM, Jeff King wrote: > On Tue, Jan 16, 2018 at 08:38:15PM +0100, Kim Gybels wrote: > >> Take a hint from commit ea68b0ce9f8 (hash-object: don't use mmap() for >> small files, 2010-02-21) and use read() instead of mmap() for small >> packed-refs files. >> >> This also fixes the

[PATCH 2/3] create_snapshot(): exit early if the file was empty

2018-01-15 Thread Michael Haggerty
puting `NULL - NULL` and `NULL + 0`, which are probably safe in real life but are technically undefined in C. Sidestep both issues by exiting early if `snapshot->buf` is NULL. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 2 +- 1 file changed, 1

[PATCH 3/3] find_reference_location(): don't invoke if `snapshot->buf` is NULL

2018-01-15 Thread Michael Haggerty
s more straightforward to document that the function should only be called if `snapshot->buf` is non-NULL. Adjust `packed_read_raw_ref()` to return early if `snapshot->buf` is NULL rather than calling `find_reference_location()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu>

[PATCH 1/3] SQUASH? Mention that `snapshot::buf` can be NULL for empty files

2018-01-15 Thread Michael Haggerty
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 01a13cb817..f20f05b4df 100644 --- a/refs/packed-backend.c +++ b/refs/packed-bac

[PATCH 0/3] Supplements to "packed_ref_cache: don't use mmap() for small files"

2018-01-15 Thread Michael Haggerty
, they are totally legitimate and shouldn't cause Git to fail.) With or without the additions mentioned below, Reviewed-by: Michael Haggerty <mhag...@alum.mit.edu> While reviewing your patch, I realized that some areas of the existing code use constructs that are undefined according to the C st

Re: [PATCH] refs: drop "clear packed-refs while locked" assertion

2018-01-01 Thread Michael Haggerty
"maint". But I > figured it was worth sharing here in case somebody else runs across it, > and in case we ever do a v2.14.4 release. I forgot to respond to this. +1 Reviewed-by: Michael Haggerty <mhag...@alum.mit.edu> Michael > -- >8 -- > In clear_packed_ref_cache(), we a

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

2017-11-05 Thread Michael Haggerty
On 11/05/2017 07:17 AM, Junio C Hamano wrote: > Junio C Hamano writes: >> Rafael Ascensão writes: >> ... >>> Because changing the default behavior of that function has >>> implications on multiple commands which I think shouldn't change. But >>> at the

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Michael Haggerty
On 11/06/2017 02:23 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> [1] I say "almost entirely" because putting them in one function means >> that only `pattern` needs to be scanned for glob characters. But that is >>

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Michael Haggerty
On 11/04/2017 01:41 AM, Rafael Ascensão wrote: > `for_each_glob_ref_in` has some code built into it that converts > partial refs like 'heads/master' to their full qualified form > 'refs/heads/master'. It also assume a trailing '/*' if no glob > characters are present in the pattern. > > Extract

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Michael Haggerty
On 11/04/2017 11:45 PM, Kevin Daudt wrote: > On Sat, Nov 04, 2017 at 11:27:39AM +0900, Junio C Hamano wrote: >> I however notice that addition of /* to the tail is trying to be >> careful by using strbuf_complete('/'), but prefixing with "refs/" >> does not and we would end up with a double-slash

[PATCH v2 2/9] prune_ref(): call `ref_transaction_add_update()` directly

2017-11-05 Thread Michael Haggerty
parentheses to its definition to avoid potential future mishaps. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.h | 4 +--- refs/files-backend.c | 25 - 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/refs.h b/refs.h

[PATCH v2 4/9] ref_transaction_add_update(): remove a check

2017-11-05 Thread Michael Haggerty
We want to make `REF_ISPRUNING` internal to the files backend. For this to be possible, `ref_transaction_add_update()` mustn't know about it. So move the check that `REF_ISPRUNING` is only used with `REF_NODEREF` from this function to `files_transaction_prepare()`. Signed-off-by: Michael Haggerty

[PATCH v2 6/9] refs: rename constant `REF_NODEREF` to `REF_NO_DEREF`

2017-11-05 Thread Michael Haggerty
Even after working with this code for years, I still see this constant name as "ref node ref". Rename it to make it's meaning clearer. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- builtin/am.c | 2 +- builtin/branch.c | 2 +- builtin/che

[PATCH v2 3/9] ref_transaction_update(): die on disallowed flags

2017-11-05 Thread Michael Haggerty
Callers shouldn't be passing disallowed flags into `ref_transaction_update()`. So instead of masking them off, treat it as a bug if any are set. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refs.c b/

[PATCH v2 9/9] refs: update some more docs to use "oid" rather than "sha1"

2017-11-05 Thread Michael Haggerty
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c| 2 +- refs.h| 8 refs/files-backend.c | 19 +-- refs/packed-backend.c | 2 +- refs/ref-cache.c | 4 ++-- refs/refs-internal.h | 18 +- 6

[PATCH v2 5/9] refs: tidy up and adjust visibility of the `ref_update` flags

2017-11-05 Thread Michael Haggerty
`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.h | 67 ++-- refs/files-backend.c | 45 +++ refs/refs-internal.h | 67 3 files c

[PATCH v2 0/9] Tidy up the constants related to ref_update::flags

2017-11-05 Thread Michael Haggerty
as branch `tidy-ref-update-flags` [2]. Michael [1] https://public-inbox.org/git/cover.1509183413.git.mhag...@alum.mit.edu/ [2] https://github.com/mhagger/git Michael Haggerty (9): files_transaction_prepare(): don't leak flags to packed transaction prune_ref(): call `ref_transaction_add_update

[PATCH v2 1/9] files_transaction_prepare(): don't leak flags to packed transaction

2017-11-05 Thread Michael Haggerty
logically what we want, so include it. In fact it is actually ignored by the packed backend (which doesn't support symbolic references), but that's its own business. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 del

[PATCH v2 7/9] refs: rename constant `REF_ISPRUNING` to `REF_IS_PRUNING`

2017-11-05 Thread Michael Haggerty
Underscores are cheap, and help readability. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 71e088e811..bb10b715a8

[PATCH v2 8/9] write_packed_entry(): take `object_id` arguments

2017-11-05 Thread Michael Haggerty
Change `write_packed_entry()` to take `struct object_id *` rather than `unsigned char *` arguments. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/refs/packed-backend.c

Re: [PATCH 5/7] refs: tidy up and adjust visibility of the `ref_update` flags

2017-11-05 Thread Michael Haggerty
On 11/01/2017 09:18 AM, Martin Ågren wrote: > On 28 October 2017 at 11:49, Michael Haggerty <mhag...@alum.mit.edu> wrote: >> The constants used for `ref_update::flags` were rather disorganized: > >> * The documentation wasn't very consistent and partly still referred &

Re: [PATCH 1/7] files_transaction_prepare(): don't leak flags to packed transaction

2017-11-05 Thread Michael Haggerty
On 10/30/2017 05:44 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> The files backend uses `ref_update::flags` for several internal flags. >> But those flags have no meaning to the packed backend. So when adding >> updates for the pac

Re: [PATCH 2/2] refs_resolve_ref_unsafe: handle d/f conflicts for writes

2017-11-04 Thread Michael Haggerty
On 10/07/2017 06:36 AM, Michael Haggerty wrote: > On 10/06/2017 07:16 PM, Jeff King wrote: >> On Fri, Oct 06, 2017 at 07:09:10PM +0200, Michael Haggerty wrote: >> >>> I do have one twinge of uneasiness at a deeper level, that I haven't had >>> time to chec

[PATCH 7/7] refs: rename constant `REF_ISPRUNING` to `REF_IS_PRUNING`

2017-10-28 Thread Michael Haggerty
Underscores are cheap, and help readability. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 71e088e811..bb10b715a8

[PATCH 0/7] Tidy up the constants related to ref_update::flags

2017-10-28 Thread Michael Haggerty
can talk about OIDs rather than SHA-1s. These patches are also available as branch `tidy-ref-update-flags` from my GitHub fork [1]. Michael [1] https://github.com/mhagger/git Michael Haggerty (7): files_transaction_prepare(): don't leak flags to packed transaction prune_ref(): call

[PATCH 1/7] files_transaction_prepare(): don't leak flags to packed transaction

2017-10-28 Thread Michael Haggerty
logically what we want, so include it. In fact it is actually ignored by the packed backend (which doesn't support symbolic references), but that's its own business. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 del

[PATCH 6/7] refs: rename constant `REF_NODEREF` to `REF_NO_DEREF`

2017-10-28 Thread Michael Haggerty
Even after working with this code for years, I still see this constant name as "ref node ref". Rename it to make it's meaning clearer. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- builtin/am.c | 2 +- builtin/branch.c | 2 +- builtin/che

[PATCH 5/7] refs: tidy up and adjust visibility of the `ref_update` flags

2017-10-28 Thread Michael Haggerty
`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.h | 65 +- refs/files-backend.c | 45 +++ refs/refs-internal.h | 67 3 files chang

[PATCH 2/7] prune_ref(): call `ref_transaction_add_update()` directly

2017-10-28 Thread Michael Haggerty
parentheses to its definition to avoid potential future mishaps. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.h | 4 +--- refs/files-backend.c | 25 - 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/refs.h b/refs.h

[PATCH 4/7] ref_transaction_add_update(): remove a check

2017-10-28 Thread Michael Haggerty
We want to make `REF_ISPRUNING` internal to the files backend. For this to be possible, `ref_transaction_add_update()` mustn't know about it. So move the check that `REF_ISPRUNING` is only used with `REF_NODEREF` from this function to `files_transaction_prepare()`. Signed-off-by: Michael Haggerty

[PATCH 3/7] ref_transaction_update(): die on disallowed flags

2017-10-28 Thread Michael Haggerty
Callers shouldn't be passing disallowed flags into `ref_transaction_update()`. So instead of masking them off, treat it as a bug if any are set. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refs.c b/

[PATCH v2 1/2] t1409: check that `packed-refs` is not rewritten unnecessarily

2017-10-28 Thread Michael Haggerty
loose reference and no `packed-refs` file previously existed. * The `packed-refs` file is rewritten unnecessarily when deleting a loose reference that has no packed counterpart. Both problems will be fixed in the next commit. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- t

[PATCH v2 0/2] Avoid rewriting "packed-refs" unnecessarily

2017-10-28 Thread Michael Haggerty
-refs` on my GitHub fork [2]. They now use `mh/packed-ref-transactions` as the base, since that is where Junio chose to apply v1. Michael [1] https://public-inbox.org/git/cover.1508924577.git.mhag...@alum.mit.edu/ [2] https://github.com/mhagger/git Michael Haggerty (2): t1409: check that `packed

[PATCH v2 2/2] files-backend: don't rewrite the `packed-refs` file unnecessarily

2017-10-28 Thread Michael Haggerty
and `N` is the total number of packed references. This commit fixes two tests in t1409. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 18 - refs/packed-backend.c | 94 +++ refs/packed-bac

Re: [PATCH 0/2] Avoid rewriting "packed-refs" unnecessarily

2017-10-27 Thread Michael Haggerty
On 10/25/2017 10:10 PM, Eric Sunshine wrote: > On Wed, Oct 25, 2017 at 5:53 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> Since commit dc39e09942 (files_ref_store: use a transaction to update >> packed refs, 2017-09-08), we've been rewriting the `packed-refs` fi

Re: [PATCH] t0000: check whether the shell supports the "local" keyword

2017-10-26 Thread Michael Haggerty
On 10/26/2017 10:40 AM, Jacob Keller wrote: > On Thu, Oct 26, 2017 at 1:28 AM, Eric Sunshine <sunsh...@sunshineco.com> > wrote: >> On Thu, Oct 26, 2017 at 4:18 AM, Michael Haggerty <mhag...@alum.mit.edu> >> wrote: >>> Add a test balloon to see if we get c

[PATCH] t0000: check whether the shell supports the "local" keyword

2017-10-26 Thread Michael Haggerty
Add a test balloon to see if we get complaints from anybody who is using a shell that doesn't support the "local" keyword. If so, this test can be reverted. If not, we might want to consider using "local" in shell code throughout the git code base. Signed-off-by: Mi

Re: [PATCH 2/2] files-backend: don't rewrite the `packed-refs` file unnecessarily

2017-10-26 Thread Michael Haggerty
On 10/26/2017 08:46 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> Even when we are deleting references, we needn't overwrite the >> `packed-refs` file if the references that we are deleting are not >> present in that file. Implemen

[PATCH 0/2] Avoid rewriting "packed-refs" unnecessarily

2017-10-25 Thread Michael Haggerty
[1] as branch `avoid-rewriting-packed-refs`. Michael [1] https://github.com/mhagger/git Michael Haggerty (2): t1409: check that `packed-refs` is not rewritten unnecessarily files-backend: don't rewrite the `packed-refs` file unnecessarily refs/files-backend.c | 18 ++- refs

[PATCH 2/2] files-backend: don't rewrite the `packed-refs` file unnecessarily

2017-10-25 Thread Michael Haggerty
of the transaction to avoid races. This fixes two tests in t1409. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 18 +++- refs/packed-backend.c | 68 +++ refs/packed-backend.h

[PATCH 1/2] t1409: check that `packed-refs` is not rewritten unnecessarily

2017-10-25 Thread Michael Haggerty
loose reference and no `packed-refs` file previously existed. * The `packed-refs` file is rewritten unnecessarily when deleting a loose reference that has no packed counterpart. Both problems will be fixed in the next commit. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- t

Re: [PATCH 1/2] t1404: add a bunch of tests of D/F conflicts

2017-10-25 Thread Michael Haggerty
On 10/25/2017 10:03 AM, Michael Haggerty wrote: > On 10/24/2017 09:46 PM, Jeff King wrote: >> On Tue, Oct 24, 2017 at 12:19:26PM -0400, Eric Sunshine wrote: >> >>> On Tue, Oct 24, 2017 at 11:16 AM, Michael Haggerty <mhag...@alum.mit.edu> >>> wrote: >>&

Re: [PATCH 1/2] t1404: add a bunch of tests of D/F conflicts

2017-10-25 Thread Michael Haggerty
On 10/24/2017 09:46 PM, Jeff King wrote: > On Tue, Oct 24, 2017 at 12:19:26PM -0400, Eric Sunshine wrote: > >> On Tue, Oct 24, 2017 at 11:16 AM, Michael Haggerty <mhag...@alum.mit.edu> >> wrote: >>> diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-

[PATCH 2/2] files_transaction_prepare(): fix handling of ref lock failure

2017-10-24 Thread Michael Haggerty
Since dc39e09942 (files_ref_store: use a transaction to update packed refs, 2017-09-08), failure to lock a reference has been handled incorrectly by `files_transaction_prepare()`. If `lock_ref_for_update()` fails in the lock-acquisition loop of that function, it sets `ret` then breaks out of that

[PATCH 1/2] t1404: add a bunch of tests of D/F conflicts

2017-10-24 Thread Michael Haggerty
fact, many of the new tests currently fail. They will be fixed in the next commit along with an explanation. Reported-by: Jeff King <p...@peff.net> Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- t/t1404-update-ref-errors.sh | 146 ++

[PATCH 0/2] Fix an error-handling path when locking refs

2017-10-24 Thread Michael Haggerty
something that I would like to fix (though maybe not for 2.15? I'm open to input about its urgency.) [1] https://public-inbox.org/git/20171024082409.smwsd6pla64jj...@sigill.intra.peff.net/ [2] https://github.com/mhagger/git Michael Haggerty (2): t1404: add a bunch of tests of D/F conflicts

Re: v2.15.0-rc2 ref deletion bug

2017-10-24 Thread Michael Haggerty
On 10/24/2017 01:05 PM, Michael Haggerty wrote: > On 10/24/2017 10:24 AM, Jeff King wrote: >> I found a potentially serious bug in v2.15.0-rc2 (and earlier release >> candidates, too) that we may want to deal with before the release. >> >> If I do: >> [...] &

Re: v2.15.0-rc2 ref deletion bug

2017-10-24 Thread Michael Haggerty
On 10/24/2017 10:24 AM, Jeff King wrote: > I found a potentially serious bug in v2.15.0-rc2 (and earlier release > candidates, too) that we may want to deal with before the release. > > If I do: > > git init -q repo > cd repo > obj=$(git hash-object -w /dev/null) > git update-ref refs/tags/foo

Re: [PATCH v2 00/24] object_id part 10

2017-10-11 Thread Michael Haggerty
On 10/09/2017 03:11 AM, brian m. carlson wrote: > This is the tenth in a series of patches to convert from unsigned char > [20] to struct object_id. This series mostly involves changes to the > refs code. After these changes, there are almost no references to > unsigned char in the main refs

Re: [PATCH v2 24/24] refs/files-backend: convert static functions to object_id

2017-10-11 Thread Michael Haggerty
On 10/09/2017 03:11 AM, brian m. carlson wrote: > Convert several static functions to take pointers to struct object_id. > Change the relevant parameters to write_packed_entry to be const, as we > don't modify them. Rename lock_ref_sha1_basic to lock_ref_oid_basic to > reflect its new argument. >

Re: [PATCH v2 23/24] refs: convert read_raw_ref backends to struct object_id

2017-10-11 Thread Michael Haggerty
On 10/09/2017 03:11 AM, brian m. carlson wrote: > Convert the unsigned char * parameter to struct object_id * for > files_read_raw_ref and packed_read_raw-ref. Update the documentation. Nit: s/packed_read_raw-ref/packed_read_raw_ref/ > Switch from using get_sha1_hex and a hard-coded 40 to

Re: [PATCH v2 21/24] refs: convert resolve_ref_unsafe to struct object_id

2017-10-11 Thread Michael Haggerty
On 10/09/2017 03:11 AM, brian m. carlson wrote: > Convert resolve_ref_unsafe to take a pointer to struct object_id by > converting one remaining caller to use struct object_id, converting the > declaration and definition, and applying the following semantic patch: > > @@ > expression E1, E2, E3,

Re: [PATCH v2 14/24] refs: convert peel_ref to struct object_id

2017-10-11 Thread Michael Haggerty
On 10/09/2017 03:11 AM, brian m. carlson wrote: > Convert peel_ref (and its corresponding backend) to struct object_id. > > This transformation was done with an update to the declaration, > definition, and test helper and the following semantic patch: > > @@ > expression E1, E2; > @@ > -

Re: [PATCH v2 05/24] refs: update ref transactions to use struct object_id

2017-10-11 Thread Michael Haggerty
On 10/09/2017 03:11 AM, brian m. carlson wrote: > Update the ref transaction code to use struct object_id. Remove one > NULL pointer check which was previously inserted around a dereference; > since we now pass a pointer to struct object_id directly through, the > code we're calling handles this

Re: [PATCH v2 04/24] refs: convert update_ref and refs_update_ref to use struct object_id

2017-10-11 Thread Michael Haggerty
On 10/09/2017 03:11 AM, brian m. carlson wrote: > Convert update_ref, refs_update_ref, and write_pseudoref to use struct > object_id. Update the existing callers as well. Remove update_ref_oid, > as it is no longer needed. > > Signed-off-by: brian m. carlson > ---

Re: [PATCH 2/2] refs_resolve_ref_unsafe: handle d/f conflicts for writes

2017-10-06 Thread Michael Haggerty
On 10/06/2017 07:16 PM, Jeff King wrote: > On Fri, Oct 06, 2017 at 07:09:10PM +0200, Michael Haggerty wrote: > >> I do have one twinge of uneasiness at a deeper level, that I haven't had >> time to check... >> >> Does this patch make it easier to *set* HEAD t

Re: [PATCH 2/2] refs_resolve_ref_unsafe: handle d/f conflicts for writes

2017-10-06 Thread Michael Haggerty
On 10/06/2017 04:42 PM, Jeff King wrote: > If our call to refs_read_raw_ref() fails, we check errno to > see if the ref is simply missing, or if we encountered a > more serious error. If it's just missing, then in "write" > mode (i.e., when RESOLVE_REFS_READING is not set), this is > perfectly

Re: RFC: Design and code of partial clones (now, missing commits and trees OK)

2017-09-26 Thread Michael Haggerty
On 09/22/2017 12:42 AM, Jonathan Tan wrote: > On Thu, 21 Sep 2017 13:57:30 Jeff Hostetler wrote: > [...] >> I struggled with the terms here a little when looking at the source. >> () A remote responding to a partial-clone is termed a >> "promisor-remote". () Packfiles

[PATCH v3 08/21] read_packed_refs(): read references with minimal copying

2017-09-25 Thread Michael Haggerty
that this change slightly tightens up the parsing of the `packed-refs` file. Previously, the parser would have accepted multiple "peeled" lines for a single reference (ignoring all but the last one). Now it would reject that. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu>

[PATCH v3 14/21] read_packed_refs(): ensure that references are ordered when read

2017-09-25 Thread Michael Haggerty
that is currently mmapped to be replaced using `rename()`, then it would be bad for us to keep the file mmapped for any longer than necessary. So, on such systems, always make a copy of the file contents, either as part of the sorting process, or afterwards. Signed-off-by: Michael Haggerty <m

[PATCH v3 15/21] packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator`

2017-09-25 Thread Michael Haggerty
` entirely. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 109 -- 1 file changed, 106 insertions(+), 3 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a7fc613c06..abf14a1405

[PATCH v3 19/21] ref_cache: remove support for storing peeled values

2017-09-25 Thread Michael Haggerty
Now that the `packed-refs` backend doesn't use `ref_cache`, there is nobody left who might want to store peeled values of references in `ref_cache`. So remove that feature. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 9 - refs/ref-cache.c

[PATCH v3 21/21] packed-backend.c: rename a bunch of things and update comments

2017-09-25 Thread Michael Haggerty
changes that the code has undergone. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 422 +++--- 1 file changed, 232 insertions(+), 190 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c

[PATCH v3 20/21] mmapped_ref_iterator: inline into `packed_ref_iterator`

2017-09-25 Thread Michael Haggerty
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 284 -- 1 file changed, 114 insertions(+), 170 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 66e5525174..1ed52d7eca 100644 -

[PATCH v3 11/21] mmapped_ref_iterator_advance(): no peeled value for broken refs

2017-09-25 Thread Michael Haggerty
If a reference is broken, suppress its peeled value. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 312116a99d..724c88631d

[PATCH v3 09/21] packed_ref_cache: remember the file-wide peeling state

2017-09-25 Thread Michael Haggerty
Rather than store the peeling state (i.e., the one defined by traits in the `packed-refs` file header line) in a local variable in `read_packed_refs()`, store it permanently in `packed_ref_cache`. This will be needed when we stop reading all packed refs at once. Signed-off-by: Michael Haggerty

[PATCH v3 18/21] packed_ref_store: get rid of the `ref_cache` entirely

2017-09-25 Thread Michael Haggerty
if the file starts out sorted). Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 29 ++--- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index dbbba45502..3829e9c294

[PATCH v3 16/21] packed_read_raw_ref(): read the reference from the mmapped buffer

2017-09-25 Thread Michael Haggerty
Instead of reading the reference from the `ref_cache`, read it directly from the mmapped buffer. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/refs/packed-backend.c b/refs/

[PATCH v3 10/21] mmapped_ref_iterator: add iterator over a packed-refs file

2017-09-25 Thread Michael Haggerty
. That doesn't matter for now, because the packed refs cache doesn't care what order it is filled. This change adds a lot of boilerplate without providing any obvious benefits. The benefits will come soon, when we get rid of the `ref_cache` for packed references altogether. Signed-off-by: Michael Haggerty

  1   2   3   4   5   6   7   8   9   10   >