Re: [PATCH 1/2] test-lib: group system specific FIFO tests by system

2017-09-15 Thread Michael J Gruber
Johannes Schindelin venit, vidit, dixit 15.09.2017 00:21: > Hi Michael, > > On Thu, 14 Sep 2017, Michael J Gruber wrote: > >> test-lib determines whether a file-system supports FIFOs and needs to do >> special casing for CYGWIN and MINGW. This separates those system &

Re: [PATCH 2/3] merge-base: return fork-point outside reflog

2017-09-15 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 15.09.2017 04:48: > Michael J Gruber <g...@grubix.eu> writes: > >> In fact, per documentation "--fork-point" looks at the reflog in >> addition to doing the usual walk from the tip. The original design >> description i

Re: [PATCH 1/3] t6010: test actual test output

2017-09-15 Thread Michael J Gruber
Jeff King venit, vidit, dixit 14.09.2017 16:34: > On Thu, Sep 14, 2017 at 03:15:18PM +0200, Michael J Gruber wrote: > >> 4f21454b55 ("merge-base: handle --fork-point without reflog", >> 2016-10-12) introduced a fix for merge-base --fork-point without reflog >> a

Re: [PATCH v3] commit-template: change a message to be more intuitive

2017-09-15 Thread Michael J Gruber
Kaartic Sivaraam venit, vidit, dixit 15.09.2017 06:50: > It's not good to use the phrase 'do not touch' to convey the information > that the cut-line should not be modified or removed as it could possibly > be mis-interpreted by a person who doesn't know that the word 'touch' has > the meaning of

Re: [PATCH 00/20] Read `packed-refs` using mmap()

2017-09-14 Thread Michael Haggerty
On 09/14/2017 10:23 PM, Johannes Schindelin wrote: > On Wed, 13 Sep 2017, Michael Haggerty wrote: > >> * `mmap()` the whole file rather than `read()`ing it. > > On Windows, a memory-mapped file cannot be renamed. As a consequence, the > following tests fail on `pu`: >

Re: [PATCH 00/20] Read `packed-refs` using mmap()

2017-09-14 Thread Michael Haggerty
On 09/13/2017 07:15 PM, Michael Haggerty wrote: > [...] > * `mmap()` the whole file rather than `read()`ing it. > [...] Apparently this doesn't work on Windows, because the `snapshot` is keeping the `packed-refs` file open too long, so the new file can't be renamed on top of it. I didn'

[PATCH 2/2] test-lib: ulimit does not limit on CYGWIN and MINGW

2017-09-14 Thread Michael J Gruber
Jones <ram...@ramsayjones.plus.com> Reported-by: Adam Dinwoodie <a...@dinwoodie.org> Reported-by: Johannes Schindelin <johannes.schinde...@gmx.de> Signed-off-by: Michael J Gruber <g...@grubix.eu> --- This is independent of my series, but should best go before so that no

[PATCH 1/2] test-lib: group system specific FIFO tests by system

2017-09-14 Thread Michael J Gruber
-off-by: Michael J Gruber <g...@grubix.eu> --- t/test-lib.sh | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 5fbd8d4a90..b8a0b05102 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -994,6 +994,10 @@ case $u

[PATCH 0/3] merge-base --fork-point fixes

2017-09-14 Thread Michael J Gruber
merge-base --fork-point does not quite work as advertised when the reflog is empty or partial. This series brings it in line with the documentation and, hopefully, with the original intent. Michael J Gruber (3): t6010: test actual test output merge-base: return fork-point outside reflog

[PATCH 1/3] t6010: test actual test output

2017-09-14 Thread Michael J Gruber
merge-base output, not just its return code. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- t/t6010-merge-base.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh index 31db7b5f91..17fffd7998 100755 --- a/t/t6010-mer

[PATCH 3/3] merge-base: find fork-point outside partial reflog

2017-09-14 Thread Michael J Gruber
of refname is found in any case, independent of the state of the reflog. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- builtin/merge-base.c | 2 +- t/t6010-merge-base.sh | 8 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/builtin/merge-base.c b/builtin/merge-

[PATCH 2/3] merge-base: return fork-point outside reflog

2017-09-14 Thread Michael J Gruber
Jakob <jakob.ekelh...@fsw.at> Signed-off-by: Michael J Gruber <g...@grubix.eu> --- builtin/merge-base.c | 18 +++--- t/t6010-merge-base.sh | 8 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/builtin/merge-base.c b/builtin/merge-base.c index 6dbd167d3b

Re: [PATCH] test-lib: don't use ulimit in test prerequisites on cygwin

2017-09-14 Thread Michael J Gruber
e explicit feature-based test > like using "ulimit" to set a limit and then reading back the limit in > a separate process, but that feels like overkill. It's still not clear whether these limits would be in effect or just reported. Rather than checking uname -s in several places, I think we should just define ulimit to be false in one place, or rather set the prerequisite there. Michael

Re: merge-base not working as expected when base is ahead

2017-09-14 Thread Michael J Gruber
v2.0.0 is in the reflog because it's the checked out HEAD.) Removing the corresponding check gives the merge base for v1.0.0 that you expect, and git still passes all tests. But I'll look at the history of these lines before I submit a patch. Michael

Re: [PATCH v2] commit-template: change a message to be more intuitive

2017-09-14 Thread Michael J Gruber
Kaartic Sivaraam venit, vidit, dixit 13.09.2017 15:05: > It's not good to use the phrase 'do not touch' to convey the information > that the cut-line should not be modified or removed as it could possibly > be mis-interpreted by a person who doesn't know that the word 'touch' has > the meaning of

Investment

2017-09-13 Thread Michael Stewart
do get back to me as soon as you can. We will give you more details as we are interested to work with you by having you investing and managing the fund accordingly I wait to hear from you Yours sincerely, Michael Stewart

[PATCH 16/20] ref_store: implement `refs_peel_ref()` generically

2017-09-13 Thread Michael Haggerty
be done generically; it doesn't have to be implemented per-backend. So implement `refs_peel_ref()` in `refs.c` and remove the `peel_ref()` method from the refs API. This removes the last callers of a couple of functions, so delete them. More cleanup to come... Signed-off-by: Michael Haggerty <m

[PATCH 13/20] read_packed_refs(): ensure that references are ordered when read

2017-09-13 Thread Michael Haggerty
correctly sorted, include that trait when we write or rewrite a `packed-refs` file. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 222 ++ 1 file changed, 206 insertions(+), 16 deletions(-) diff --git a/refs/

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

2017-09-13 Thread Michael Haggerty
that this change slightly tightens up the parsing of the `parse-ref` 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 04/20] die_unterminated_line(), die_invalid_line(): new functions

2017-09-13 Thread Michael Haggerty
, and `die_invalid_line()` checks for an EOL itself, because these calling conventions will be convenient for future callers. (Efficiency is not a concern here because these functions are only ever called if the `packed-refs` file is corrupt.) Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/

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

2017-09-13 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

[PATCH 15/20] packed_read_raw_ref(): read the reference from the mmapped buffer

2017-09-13 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 18/20] ref_cache: remove support for storing peeled values

2017-09-13 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 19/20] mmapped_ref_iterator: inline into `packed_ref_iterator`

2017-09-13 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 d76051c7f5..a3f8a19b9b 100644 -

[PATCH 14/20] packed_ref_iterator_begin(): iterate using `mmapped_ref_iterator`

2017-09-13 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 2a36dd9afb..a018a6c8ad

[PATCH 17/20] packed_ref_store: get rid of the `ref_cache` entirely

2017-09-13 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 88242a49fe..9868a5c198

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

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

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

2017-09-13 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 12/20] packed_ref_cache: keep the `packed-refs` file open and mmapped

2017-09-13 Thread Michael Haggerty
As long as a `packed_ref_cache` object is in use, keep the corresponding `packed-refs` file open and mmapped. This is still pointless, because we only read the file immediately after instantiating the `packed_ref_cache`. But that will soon change. Signed-off-by: Michael Haggerty <m

[PATCH 05/20] read_packed_refs(): use mmap to read the `packed-refs` file

2017-09-13 Thread Michael Haggerty
It's still done in a pretty stupid way, involving more data copying than necessary. That will improve in future commits. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 42 -- 1 file changed, 32 insertions(

[PATCH 03/20] packed_ref_cache: add a backlink to the associated `packed_ref_store`

2017-09-13 Thread Michael Haggerty
It will prove convenient in upcoming patches. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index e411501871..a3d9

[PATCH 07/20] read_packed_refs(): make parsing of the header line more robust

2017-09-13 Thread Michael Haggerty
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 21 +++-- 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 141f02b9c8..a45e3ff92f 100644 --- a/refs/packed-backend.c +++ b/refs/

[PATCH 06/20] read_packed_refs(): only check for a header at the top of the file

2017-09-13 Thread Michael Haggerty
This tightens up the parsing a bit; previously, stray header-looking lines would have been processed. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 35 --- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git

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

2017-09-13 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 01/20] ref_iterator: keep track of whether the iterator output is ordered

2017-09-13 Thread Michael Haggerty
in `overlay_ref_iterator_begin()` to verify that its inputs are ordered. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c| 4 refs/files-backend.c | 16 +--- refs/iterator.c | 15 ++- refs/packed-backend.c | 2 +- refs/ref-cache.c

[PATCH 02/20] prefix_ref_iterator: break when we leave the prefix

2017-09-13 Thread Michael Haggerty
prefix, so we have to skip over any unwanted references before we get to the ones that we want. Signed-off-by: Jeff King <p...@peff.net> Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- Note that the implementation of `compare_prefix()` here is a bit different than Peff'

[PATCH 00/20] Read `packed-refs` using mmap()

2017-09-13 Thread Michael Haggerty
This branch applies on top of mh/packed-ref-transactions. It can also be obtained from my git fork [1] as branch `mmap-packed-refs`. Michael [1] https://github.com/mhagger/git Jeff King (1): prefix_ref_iterator: break when we leave the prefix Michael Haggerty (19): ref_iterator: keep track

Re: Unexpected pass for t6120-describe.sh on cygwin

2017-09-13 Thread Michael J Gruber
does not lie about the value either it's a simple prerequisite test (test ulimit present, if yes set and get and compare the stack size values). Michael

Re: [PATCH 4/4] packed refs: pass .git dir instead of packed-refs path to init_fn

2017-09-13 Thread Michael Haggerty
OTOH, the function certainly doesn't belong in the vtable slot for `init`, because a `packed_ref_store` can't serve as a repository's logical reference store. Did you have a particular reason for changing this now, aside from "consistency"? If not, feel free to drop this patch and I'll implement something more like what I have described above when I have time. Michael

Re: [PATCH 3/4] replace-objects: evaluate replacement refs without using the object store

2017-09-13 Thread Michael Haggerty
d that consumers of this API can handle broken references? Aside from missing values, broken references can have illegal names (though hopefully not unsafe in the sense of causing filesystem traversal outside of `$GITDIR`). Michael

Re: [PATCH 00/12] Clean up notes-related code around `load_subtree()`

2017-09-12 Thread Michael Haggerty
On Sun, Sep 10, 2017 at 9:39 AM, Jeff King <p...@peff.net> wrote: > On Sun, Sep 10, 2017 at 06:45:08AM +0200, Michael Haggerty wrote: > >> > So nothing to see here, but since I spent 20 minutes scratching my head >> > (and I know others look at Coverity output and ma

Re: Unexpected pass for t6120-describe.sh on cygwin

2017-09-10 Thread Michael J Gruber
Ramsay Jones venit, vidit, dixit 09.09.2017 15:13: > Hi Adam, > > I ran the test-suite on the 'pu' branch last night (simply because > that was what I had built at the time!), which resulted in a PASS, > but t6120 was showing a 'TODO passed' for #52. > > This is a test introduced by Michael's

Re: [PATCH v2 08/11] t1404: demonstrate two problems with reference transactions

2017-09-09 Thread Michael Haggerty
On 09/09/2017 01:17 PM, Jeff King wrote: > On Fri, Sep 08, 2017 at 03:51:50PM +0200, Michael Haggerty wrote: > [...] > So if we get what we want, we execute ":" which should be a successful > exit code. I think the `:` is superfluous even if we care about the exit code of t

Re: [PATCH 00/12] Clean up notes-related code around `load_subtree()`

2017-09-09 Thread Michael Haggerty
On 09/09/2017 12:31 PM, Jeff King wrote: > On Sat, Aug 26, 2017 at 10:28:00AM +0200, Michael Haggerty wrote: > >> It turns out that the comment is incorrect, but there was nevertheless >> plenty that could be cleaned up in the area: >> >> * Make macro `GIT_NIBBLE` sa

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

2017-09-09 Thread Michael Haggerty
erator is now used (Micheal was right > all along). I read over all of the refs-related changes in this patch series. Aside from the comments that I left, they look good to me. Thanks! Michael

Re: [PATCH v4 16/16] refs.c: reindent get_submodule_ref_store()

2017-09-09 Thread Michael Haggerty
trailing slashes */ > Now I'm confused. Is it still allowed to call `get_submodule_ref_store(NULL)`? If not, then the previous commit should probably do if (!submdule) BUG(...); Either way, it seems like it would be natural to combine this commit with the previous one. Michael

Re: [PATCH v4 15/16] refs.c: remove fallback-to-main-store code get_submodule_ref_store()

2017-09-09 Thread Michael Haggerty
ot;submodule" (path) argument must be a non-NULL, non-empty string. > > So, this "if NULL or empty string" code block should never ever > trigger. And it's wrong to fall back to the main ref store > anyway. Delete it. Nice! Thanks for the cleanup. Michael

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

2017-09-09 Thread Michael Haggerty
*/ > + return ITER_SELECT_0; I don't understand the point of the comment. If we should ignore common refs here, why not do it rather than commenting about it? Wouldn't it be really easy to implement? OTOH if it's not needed, then why the comment? > [...] Michael

Re: [PATCH v4 11/16] revision.c: --all adds HEAD from all worktrees

2017-09-09 Thread Michael Haggerty
tore(submodule); > } else > refs = get_main_ref_store(); Tiny nit: s/.e.g/e.g./ > [...] Michael

Re: [PATCH v4 10/16] refs: remove dead for_each_*_submodule()

2017-09-08 Thread Michael Haggerty
i-ref-iteration.txt | 7 ++ > refs.c| 33 > --- > refs.h| 10 > 3 files changed, 2 insertions(+), 48 deletions(-) What a lovely diffstat :-) Michael

Re: [PATCH v4 06/16] refs: move submodule slash stripping code to get_submodule_ref_store

2017-09-08 Thread Michael Haggerty
crete suggestion. Michael

[PATCH] load_subtree(): check that `prefix_len` is in the expected range

2017-09-08 Thread Michael Haggerty
This value, which is stashed in the last byte of an object_id hash, gets handed around a lot. So add a sanity check before using it in `load_subtree()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- This patch is an addendum to v1 of the mh/notes-cleanup patch series [1]. I

Re: [PATCH] refs: make sure we never pass NULL to hashcpy

2017-09-08 Thread Michael Haggerty
On 09/08/2017 02:46 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> I did just realize one thing: `ref_transaction_update()` takes `flags` >> as an argument and alters it using >> >>> flags |= (new_sha1 ? REF_H

[PATCH v2 07/11] files_initial_transaction_commit(): use a transaction for packed refs

2017-09-08 Thread Michael Haggerty
Use a `packed_ref_store` transaction in the implementation of `files_initial_transaction_commit()` rather than using internal features of the packed ref store. This further decouples `files_ref_store` from `packed_ref_store`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs

[PATCH v2 11/11] files_transaction_finish(): delete reflogs before references

2017-09-08 Thread Michael Haggerty
If the deletion steps unexpectedly fail, it is less bad to leave a reference without its reflog than it is to leave a reflog without its reference, since the latter is an invalid repository state. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.

[PATCH v2 08/11] t1404: demonstrate two problems with reference transactions

2017-09-08 Thread Michael Haggerty
. * If the `packed-refs` file is locked so long that `update-ref` fails to lock it, then the reference can be left permanently in the incorrect state described in the previous point. In a moment, both problems will be fixed. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- t/t1404-upda

[PATCH v2 06/11] prune_refs(): also free the linked list

2017-09-08 Thread Michael Haggerty
At least since v1.7, the elements of the `refs_to_prune` linked list have been leaked. Fix the leak by teaching `prune_refs()` to free the list elements as it processes them. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 14 ++ 1 file chang

[PATCH v2 10/11] packed-backend: rip out some now-unused code

2017-09-08 Thread Michael Haggerty
Now the outside world interacts with the packed ref store only via the generic refs API plus a few lock-related functions. This allows us to delete some functions that are no longer used, thereby completing the encapsulation of the packed ref store. Signed-off-by: Michael Haggerty <m

[PATCH v2 09/11] files_ref_store: use a transaction to update packed refs

2017-09-08 Thread Michael Haggerty
d-refs" lock until after the loose references have been finalized, thus preventing a simultaneous "pack-refs" process from packing the loose version of the reference in the time gap, which would otherwise defeat our attempt to delete it. Signed-off-by: Michael H

[PATCH v2 04/11] packed_delete_refs(): implement method

2017-09-08 Thread Michael Haggerty
Implement `packed_delete_refs()` using a reference transaction. This means that `files_delete_refs()` can use `refs_delete_refs()` instead of `repack_without_refs()` to delete any packed references, decreasing the coupling between the classes. Signed-off-by: Michael Haggerty <mhag...@alum.mit.

[PATCH v2 02/11] struct ref_transaction: add a place for backends to store data

2017-09-08 Thread Michael Haggerty
`packed_ref_store` is going to want to store some transaction-wide data, so make a place for it. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/refs-internal.h | 1 + 1 file changed, 1 insertion(+) diff --git a/refs/refs-internal.h b/refs/refs-internal.h index b02d

[PATCH v2 03/11] packed_ref_store: implement reference transactions

2017-09-08 Thread Michael Haggerty
Implement the methods needed to support reference transactions for the packed-refs backend. The new methods are not yet used. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 313 +- refs/packed-backend.h

[PATCH v2 05/11] files_pack_refs(): use a reference transaction to write packed refs

2017-09-08 Thread Michael Haggerty
Now that the packed reference store supports transactions, we can use a transaction to write the packed versions of references that we want to pack. This decreases the coupling between `files_ref_store` and `packed_ref_store`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs

[PATCH v2 01/11] packed-backend: don't adjust the reference count on lock/unlock

2017-09-08 Thread Michael Haggerty
to find the cache whose reference count it needs to decrement. This whole issue will soon become moot due to upcoming changes that avoid changing the in-memory cache as part of updating the packed-refs on disk, but this change makes that transition easier. Signed-off-by: Michael Haggerty <m

[PATCH v2 00/11] Implement transactions for the packed ref store

2017-09-08 Thread Michael Haggerty
packed-ref-transactions` from my GitHub repo [2]. Michael [1] https://public-inbox.org/git/cover.1503993268.git.mhag...@alum.mit.edu/ [2] https://github.com/mhagger/git Michael Haggerty (11): packed-backend: don't adjust the reference count on lock/unlock struct ref_transaction: add a place fo

Re: git diff doesn't quite work as documented?

2017-09-08 Thread Michael J Gruber
on language terms (which is okay, of course) and are unaware of the defined meanings of technical terms. Explaining them in every place where they are used simply does not scale. Maybe we should make more use of our glossary (extend it, enhance it, promote it) and somehow mark all technical terms as such when they are used (say, italics in HTML), or at least when the exact meaning is relevant like in the case above, and possibly link to the glossary (-item). Michael

Re: [PATCH 08/10] files_ref_store: use a transaction to update packed refs

2017-09-08 Thread Michael Haggerty
On 09/08/2017 09:38 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:32AM +0200, Michael Haggerty wrote: > >> First, the old code didn't obtain the `packed-refs` lock until >> `files_transaction_finish()`. This means that a failure to acquire the >> `packed-refs` lock

Re: [PATCH 0/4] Test name-rev with small stack

2017-09-08 Thread Michael J Gruber
Jeff King venit, vidit, dixit 07.09.2017 16:54: > On Thu, Sep 07, 2017 at 04:02:19PM +0200, Michael J Gruber wrote: > >> name-rev segfaults for me in emacs.git with the typical 8102 stack size. >> The reason is the recursive walk that name-rev uses. >> >> T

Re: [PATCH 07/10] t1404: demonstrate two problems with reference transactions

2017-09-08 Thread Michael Haggerty
On 09/08/2017 06:44 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> +git for-each-ref $prefix >actual && >> +grep "Unable to create $Q.*packed-refs.lock$Q: File exists" err && > > I added a squa

Re: [PATCH 06/10] files_initial_transaction_commit(): use a transaction for packed refs

2017-09-08 Thread Michael Haggerty
On 09/08/2017 09:27 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:30AM +0200, Michael Haggerty wrote: > >> Used a `packed_ref_store` transaction in the implementation of >> `files_initial_transaction_commit()` rather than using internal >> features of the packed

Re: [PATCH 01/10] packed-backend: don't adjust the reference count on lock/unlock

2017-09-08 Thread Michael Haggerty
On 09/08/2017 08:52 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:25AM +0200, Michael Haggerty wrote: > >> The old code incremented the packed ref cache reference count when >> acquiring the packed-refs lock, and decremented the count when >> releasing the lock. Thi

Re: [PATCH 02/10] struct ref_transaction: add a place for backends to store data

2017-09-08 Thread Michael Haggerty
On 09/08/2017 09:02 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 10:20:26AM +0200, Michael Haggerty wrote: > >> `packed_ref_store` is going to want to store some transaction-wide >> data, so make a place for it. > > That makes sense, although... > >> diff --

[PATCH 0/4] Test name-rev with small stack

2017-09-07 Thread Michael J Gruber
name-rev segfaults for me in emacs.git with the typical 8102 stack size. The reason is the recursive walk that name-rev uses. This series adds a test to mark this as known failure, after some clean-ups. Michael J Gruber (4): t7004: move limited stack prereq to test-lib t6120: test name-rev

[PATCH 1/4] t7004: move limited stack prereq to test-lib

2017-09-07 Thread Michael J Gruber
The lazy prerequisite ULIMIT_STACK_SIZE is used only in t7004 so far. Move it to test-lib.sh so that it can be used in other tests (which it will be in a follow-up commit). Signed-off-by: Michael J Gruber <g...@grubix.eu> --- t/t7004-tag.sh | 6 -- t/test-lib.sh | 6 ++ 2

[PATCH 2/4] t6120: test name-rev --all and --stdin

2017-09-07 Thread Michael J Gruber
name-rev is used in a few tests, but tested only in t6120 along with describe so far. Add tests for name-rev with --all and --stdin. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- t/t6120-describe.sh | 25 + 1 file changed, 25 insertions(+) diff --git a/t

[PATCH 4/4] t6120: test describe and name-rev with deep repos

2017-09-07 Thread Michael J Gruber
of other subtests the same. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- t/t6120-describe.sh | 31 +++ 1 file changed, 31 insertions(+) diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh index 1997ccde56..dd6dd9df9b 100755 --- a/t/t6120-describe.sh +++ b/t

[PATCH 3/4] t6120: clean up state after breaking repo

2017-09-07 Thread Michael J Gruber
t6120 breaks the repo state intentionally in the last tests. Clean up the breakage afterwards (and before adding more tests). Signed-off-by: Michael J Gruber <g...@grubix.eu> --- t/t6120-describe.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t6120-describe.sh b/t/t6120-descr

Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX

2017-09-07 Thread Michael J Gruber
Jeff King venit, vidit, dixit 06.09.2017 15:35: > On Wed, Sep 06, 2017 at 01:59:31PM +0200, Michael J Gruber wrote: > >> BTW, there's more fallout from those name-rev changes: In connection >> with that other thread about surprising describe results for emacs.git I >> n

Re: [PATCH] refs: make sure we never pass NULL to hashcpy

2017-09-07 Thread Michael Haggerty
ags &= ~REF_HAVE_NEW; > if (old_sha1) > flags |=REF_HAVE_OLD; > else > flags &= ~REF_HAVE_OLD; ? This might be a nice change to have anyway, to isolate `ref_transaction_update()` from mistakes by its callers. For that matter, one might want to be even more selective about what bits are allowed in the `flags` argument to `ref_transaction_update()`'s callers: > flags &= REF_ALLOWED_FLAGS; /* value would need to be determined */ Michael

Re: [PATCH] rev-parse: don't trim bisect refnames

2017-09-06 Thread Michael Haggerty
We can use the > same solution here (and piggy-back on its test). It looks good to me. Michael

Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX

2017-09-06 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 06.09.2017 05:35: > Michael J Gruber <g...@grubix.eu> writes: > >> Earlier, dddbad728c ("timestamp_t: a new data type for timestamps", >> 2017-04-26) changed several types to timestamp_t. >> >> 5589e87fd8 (&quo

Re: signing commits using gpg2

2017-09-05 Thread Michael J Gruber
r.smudge=egrep "^ *(email|name|signingkey) = " > commit.gpgsign=true > So, gpg2 works and gpg does not. This is typical for the way in which the gpg upgrade path is broken, and your distro installs gpg because it still relies on it. git sees two executables gpg and gpg2 and uses the first, so as to not migrate your secrete key store inadvertently. Short answer: Use git config --global gpg.program gpg2 to make git use gpg2 which apparantly is your working gnupg setup. Michael

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

2017-09-05 Thread Michael Haggerty
out getting rid of the extra `O(lg N)` search (and I agree with you that it doesn't matter in this case), I think the clean way to do it would be for `string_list` to expose a method like struct string_list_item *string_list_insert_at_index(struct string_list *list, size_t index, const char *string); and to use it, together with `string_list_find_insert_index()`, to avoid having to search twice. Michael

Re: [PATCH 07/10] t1404: demonstrate two problems with reference transactions

2017-08-30 Thread Michael Haggerty
On 08/30/2017 07:21 PM, Stefan Beller wrote: > On Tue, Aug 29, 2017 at 1:20 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >> [...] >> +test_expect_failure 'no bogus intermediate values during delete' ' >> + prefix=refs/slow-transaction &

[PATCH] name-rev: change ULONG_MAX to TIME_MAX

2017-08-30 Thread Michael J Gruber
Change the remaining constant to the one appropriate for the current type Signed-off-by: Michael J Gruber <g...@grubix.eu> --- builtin/name-rev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/name-rev.c b/builtin/name-rev.c index c41ea7c2a6..598da6c8bc 100644 --- a/b

Re: [PATCH] config: use a static lock_file struct

2017-08-30 Thread Michael Haggerty
e, `create_tempfile(tempfile, path)` and its friends could accept NULL as the first argument, in which case it would malloc a `struct tempfile` itself, and mark it as being owned by the tempfile module. Such objects would be freed when deactivated. But if the caller passes in a non-NULL `tempfile` argument, the old behavior would be retained. Michael

Re: [PATCH] config: use a static lock_file struct

2017-08-30 Thread Michael Haggerty
nter and add new items at the end of the list, and ensure that all significant callers remove locks in the *same* order that they were added. The second and third options might be easy or difficult (depend on how much current callers need to be changed) and certainly would be easy to break again as new callers are added in the future. > [...] Michael

Re: [PATCH] config: use a static lock_file struct

2017-08-29 Thread Michael Haggerty
On Wed, Aug 30, 2017 at 6:31 AM, Jeff King <p...@peff.net> wrote: > On Wed, Aug 30, 2017 at 05:25:18AM +0200, Michael Haggerty wrote: >> It was surprisingly hard trying to get that code to do the right thing, >> non-racily, in every error path. Since `remove_tempfiles()` can

Re: [PATCH] config: use a static lock_file struct

2017-08-29 Thread Michael Haggerty
om the list (via commit or rollback) before being freed. > > As far as I know anyway. This restriction dates back to the very early > days of the lockfile code and has been carried through the various > tempfile-cleanup refactorings over the years (mostly because each was > afraid to ma

Re: [PATCH 04/10] packed_delete_refs(): implement method

2017-08-29 Thread Michael Haggerty
On 08/29/2017 08:07 PM, Brandon Williams wrote: > On 08/29, Michael Haggerty wrote: >> [...] >> diff --git a/refs/packed-backend.c b/refs/packed-backend.c >> index d19b3bfba5..83a088118f 100644 >> --- a/refs/packed-backend.c >> +++ b/refs/packed-backend.c >

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

2017-08-29 Thread Michael Haggerty
v3 looks good to me. Thanks! Reviewed-by: Michael Haggerty <mhag...@alum.mit.edu> Michael

Re: git describe and "the smallest number of commits possible"

2017-08-29 Thread Michael J Gruber
describe cmt" clearly tries to calculate the number of commits in cmt that are not in t (for each candidate t), which ought to be "rev-list --count t..cmt", but fails spectacularly. I kind of suspect the priming of the tag depth on first encounter ("t->depth = seen_commits - 1;") to be the culprit because that depends on the way we walk, not the topology, but I wouldn't know how to do better. Michael

Re: [PATCH v2 2/2] refs/files-backend: fix memory leak in lock_ref_for_update

2017-08-29 Thread Michael Haggerty
ou like to add another patch to your series to fix this up? I'd do it myself, but it's a little bit awkward because the fix will conflict with your patch. Thanks, Michael

Re: [PATCH v2 1/2] refs/files-backend: add longer-scoped copy of string to list

2017-08-29 Thread Michael Haggerty
en cleared. > > Rearrange the handling of `referent`, so that we don't add it directly > to `affected_refnames`. Instead, first just check whether `referent` > exists in the string list, and later add `new_update->refname`. > > Helped-by: Michael Haggerty <mhag...@alum.mit

[PATCH 03/10] packed_ref_store: implement reference transactions

2017-08-29 Thread Michael Haggerty
Implement the methods needed to support reference transactions for the packed-refs backend. The new methods are not yet used. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/packed-backend.c | 313 +- refs/packed-backend.h

[PATCH 02/10] struct ref_transaction: add a place for backends to store data

2017-08-29 Thread Michael Haggerty
`packed_ref_store` is going to want to store some transaction-wide data, so make a place for it. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/refs-internal.h | 1 + 1 file changed, 1 insertion(+) diff --git a/refs/refs-internal.h b/refs/refs-internal.h index b02d

[PATCH 10/10] files_transaction_finish(): delete reflogs before references

2017-08-29 Thread Michael Haggerty
If the deletion steps unexpectedly fail, it is less bad to leave a reference without its reflog than it is to leave a reflog without its reference, since the latter is an invalid repository state. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.

[PATCH 09/10] packed-backend: rip out some now-unused code

2017-08-29 Thread Michael Haggerty
Now the outside world interacts with the packed ref store only via the generic refs API plus a few lock-related functions. This allows us to delete some functions that are no longer used, thereby completing the encapsulation of the packed ref store. Signed-off-by: Michael Haggerty <m

[PATCH 08/10] files_ref_store: use a transaction to update packed refs

2017-08-29 Thread Michael Haggerty
d-refs" lock until after the loose references have been finalized, thus preventing a simultaneous "pack-refs" process from packing the loose version of the reference in the time gap, which would otherwise defeat our attempt to delete it. Signed-off-by: Michael H

[PATCH 01/10] packed-backend: don't adjust the reference count on lock/unlock

2017-08-29 Thread Michael Haggerty
reference count it needs to decrement. This whole issue will soon become moot due to upcoming changes that avoid changing the in-memory cache as part of updating the packed-refs on disk, but this change makes that transition easier. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/

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