Re: [PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules

2017-05-17 Thread Junio C Hamano
Stefan Beller writes: > diff --git a/builtin/fetch.c b/builtin/fetch.c > index 4ef7a08afc..510ef1c9de 100644 > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -1344,7 +1344,7 @@ int cmd_fetch(int argc, const char **argv, const char > *prefix) >

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

2017-05-17 Thread Jeff King
On Thu, May 18, 2017 at 01:57:45PM +0900, Junio C Hamano wrote: > >> + if (!f) { > >> + if (errno == ENOENT) { > >> + /* > >> + * This is OK; it just means that no > >> + * "packed-refs" file has been written yet, > >> +

[PATCH] t5400: avoid concurrent writes into a trace file

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 08:41:01PM +0200, Johannes Sixt wrote: > Am 17.05.2017 um 07:44 schrieb Jeff King: > > I wonder if there's a way we could convince the trace for the two > > programs to go to separate locations. We don't care about receive-pack's > > trace at all. So maybe: > > This

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

2017-05-17 Thread Junio C Hamano
Jeff King writes: > 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 didn't >> exist. But it could be that there is some

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 > 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 don't need a sign, so we use

Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 08:15:03PM +0200, Johannes Sixt wrote: > Am 17.05.2017 um 16:26 schrieb Ben Peart: > > On 5/16/2017 3:13 PM, Johannes Sixt wrote: > > > Am 16.05.2017 um 19:17 schrieb Ben Peart: > > > > OK, now I'm confused as to the best path for adding a get_be64. This > > > > one is

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 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 up pointing the `refname` field of the

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

2017-05-17 Thread Junio C Hamano
Michael Haggerty 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 up pointing the `refname` field of the iterator past the NUL > of the actual reference name

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

2017-05-17 Thread Junio C Hamano
Stefan Beller writes: > On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty > wrote: >> In particular, make it clear that they make copies of the sha1 >> arguments. > > A couple weeks ago we had plans on getting rid of SHA1 in > "the near future" IIRC.

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

2017-05-17 Thread Junio C Hamano
Michael Haggerty 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 test temporary directory cannot be cleaned up, which > means that subsequent attempts to run

Re: [PATCHv2 11/20] diff.c: convert emit_rewrite_lines to use emit_line_*

2017-05-17 Thread Junio C Hamano
Stefan Beller writes: > In a later patch, I want to propose an option to detect > moved lines in a diff, which cannot be done in a one-pass over > the diff. Instead we need to go over the whole diff twice, > because we cannot detect the first line of the two corresponding >

Re: [PATCHv2 12/20] submodule.c: convert show_submodule_summary to use emit_line_fmt

2017-05-17 Thread Junio C Hamano
Stefan Beller writes: >>> +static void show_submodule_header(struct diff_options *o, const char *path, >>> struct object_id *one, struct object_id *two, >>> unsigned dirty_submodule, const char *meta, >>> const char *reset, >> ... >>

Re: t5545: reduced test coverage

2017-05-17 Thread Junio C Hamano
Ramsay Jones writes: > I would probably have simply split the test file into two, but ... > Hmph, that's a thought. > ... this looks good to me. (tested on Linux and cygwin). Thanks. -- >8 -- Subject: [PATCH] test: allow skipping the remainder Because TAP

Re: [PATCH v3 8/8] t7061: status --ignored now searches untracked dirs

2017-05-17 Thread Junio C Hamano
Samuel Lijin writes: > Signed-off-by: Samuel Lijin > --- This was fixed at "hide untracked" patch, so squash these changes in to that commit and add something like This fixes known breakages in t7061 documented earlier in the series. at the end of

Re: [PATCH v3 7/8] t7300: clean -d now skips untracked dirs containing ignored files

2017-05-17 Thread Junio C Hamano
This and 8/8 needs to be part of the code change that fixes them. Otherwise, running tests after applying only 1-6/8 would give you: Test Summary Report --- t7061-wtstatus-ignore.sh (Wstat: 0 Tests: 21 Failed: 0) TODO passed: 1-2 t7300-clean.sh

Re: [PATCH v3 6/8] clean: teach clean -d to skip dirs containing ignored files

2017-05-17 Thread Junio C Hamano
Samuel Lijin writes: > @@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char > *prefix) > > fill_directory(, ); > > - for (i = 0; i < dir.nr; i++) { > + for (k = i = 0; i < dir.nr; i++) { > struct dir_entry *ent =

Re: [PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

2017-05-17 Thread Manish Goregaokar
Hm, my invocation of git-send-email keeps getting the threading wrong. Is there a recommended set of arguments to the command? Thanks, -Manish Goregaokar On Wed, May 17, 2017 at 6:42 PM, wrote: > From: Manish Goregaokar > > To ensure that `git

[PATCH v2 2/2] reachable: Add HEADs of all worktrees to reachability analysis

2017-05-17 Thread manishearth
From: Manish Goregaokar * reachable.c: mark_reachable_objects: Include other worktrees Signed-off-by: Manish Goregaokar --- reachable.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/reachable.c b/reachable.c index d0199ca..439708e

[PATCH v2 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

2017-05-17 Thread manishearth
From: Manish Goregaokar To ensure that `git prune` does not remove refs checked out in other worktrees, we need to include these HEADs in the set of roots. This adds the iteration function necessary to do this. Signed-off-by: Manish Goregaokar ---

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

2017-05-17 Thread Manish Goregaokar
> Just as an fyi, its usually fine to send out a path RFC Ah, this is helpful! Yes, I was still trying to get the tests to run, so consider this WIP. I have since gotten them to run and found one failure which I fixed (didn't null-check `commit`). Waiting for them to finish again, will send new

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 10:57:34AM -0700, Stefan Beller wrote: > On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty > wrote: > > 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. > > This

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 05:17:17PM -0700, Brandon Williams wrote: > > This made me wonder how we handle the locking for ref_stores besides the > > main one (e.g., for submodules). The lockfile structs have to remain > > valid for the length of the program. Previously those stores could have > >

HI

2017-05-17 Thread NT
Greetings, My name is Neil Trotter the current winner of 108 million Pounds on the Euro million Jackpot Draw for 2014, and i bring to you perfect good news for such a perfect timing as this. I know this might be surprising for you to have received this at this stage But because of my last years

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

2017-05-17 Thread Brandon Williams
On 05/17, Stefan Beller wrote: > On Wed, May 17, 2017 at 6:17 AM, 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

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

2017-05-17 Thread Brandon Williams
On 05/17, 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 > > track of whether it is locked, embed the `lock_file`

Re: [PATCH/RFC 0/3] Use sha1collisiondetection as a submodule

2017-05-17 Thread Brandon Williams
On 05/17, Stefan Beller wrote: > On Wed, May 17, 2017 at 4:38 AM, Ævar Arnfjörð Bjarmason > wrote: > > On Wed, May 17, 2017 at 9:09 AM, Junio C Hamano wrote: > >> Ævar Arnfjörð Bjarmason writes: > >> > >>> On Wed, May 17, 2017 at 7:39 AM,

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

2017-05-17 Thread Brandon Williams
On 05/17, Manish Goregaokar wrote: > Oh, btw, refs.c needs an #include "worktree.h" to work; I didn't get a > chance to test this after rebasing onto the maint branch. > > (There's also another fix it needs to have no warnings, but that's not > going to affect building). I have this fixed

Re: [PATCH] tests: add an optional test to test git-annex

2017-05-17 Thread Brandon Williams
On 05/17, Stefan Beller wrote: > On Wed, May 17, 2017 at 12:33 PM, Ævar Arnfjörð Bjarmason > wrote: > > On Wed, May 17, 2017 at 12:19 PM, Junio C Hamano wrote: > >> Ævar Arnfjörð Bjarmason writes: > >> > Well, it is one thing to place

Re: [PATCH] tests: add an optional test to test git-annex

2017-05-17 Thread Brandon Williams
On 05/17, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > > >> Well, it is one thing to place git-annex under CI to make sure its > >> latest and greatest works together well with our latest and greatest > >> (and it may be something we want to see happen), but

Re: [PATCH] tests: add an optional test to test git-annex

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 12:33 PM, Ævar Arnfjörð Bjarmason wrote: > On Wed, May 17, 2017 at 12:19 PM, Junio C Hamano wrote: >> Ævar Arnfjörð Bjarmason writes: >> Well, it is one thing to place git-annex under CI to make sure its

Re: [PATCH 1/2] refs: Add for_each_worktree_ref for iterating over all worktree HEADs

2017-05-17 Thread Manish Goregaokar
Oh, btw, refs.c needs an #include "worktree.h" to work; I didn't get a chance to test this after rebasing onto the maint branch. (There's also another fix it needs to have no warnings, but that's not going to affect building). I have this fixed locally, but I'll wait for the rest of the review

[PATCH 0/3] Add option to recurse into submodules

2017-05-17 Thread Stefan Beller
In a submodule heavy workflow it becomes tedious to pass in --recurse-submodules all the time, so make an option for it. Thanks, Stefan Stefan Beller (3): submodule.c: add has_submodules to check if we have any submodules submodule test invocation: only pass additional arguments Introduce

[PATCH 1/3] submodule.c: add has_submodules to check if we have any submodules

2017-05-17 Thread Stefan Beller
When submodules are involved, it often slows down the process, as most submodule related handling is either done via a child process or by iterating over the index finding all gitlinks. For most commands that may interact with submodules, we need have a quick check if we do have any submodules at

[PATCH 3/3] Introduce submodule.recurse option

2017-05-17 Thread Stefan Beller
Any command that understands the boolean --recurse-submodule=[true/false] can have its default changed to true, by setting the submodule.recurse option. git-push takes a --recurse-submodule argument but it is not boolean, hence it is not included (yet?). Signed-off-by: Stefan Beller

[PATCH 2/3] submodule test invocation: only pass additional arguments

2017-05-17 Thread Stefan Beller
In a later patch we want to test a command without the '--recurse-submodules' given, but instead we'd give a '-c . To enable that we'll just pass the minimum required to the submodule testing, such that we can construct the command with the option easier. Signed-off-by: Stefan Beller

Re: Diff topic branch + working copy changes?

2017-05-17 Thread Samuel Lijin
On Wed, May 17, 2017 at 9:39 AM, Robert Dailey wrote: > > Would be nice in the future to have another revision specification > like @{wc} for "HEAD + working copy". I guess this technically isn't a > revision, but something along those lines. Or maybe just an >

Re: [PATCHv2 11/20] diff.c: convert emit_rewrite_lines to use emit_line_*

2017-05-17 Thread Stefan Beller
On Tue, May 16, 2017 at 10:03 PM, Junio C Hamano wrote: > Stefan Beller writes: > The reason why we can lose the LF immediately after the incomplete > line we found in the above loop is because the updated emit_line_0() > adds LF if its input is an

Re: [PATCHv2 12/20] submodule.c: convert show_submodule_summary to use emit_line_fmt

2017-05-17 Thread Stefan Beller
On Tue, May 16, 2017 at 10:19 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> In a later patch, I want to propose an option to detect >> moved lines in a diff, which cannot be done in a one-pass over >> the diff. Instead we need to go over the whole

Re: t5545: reduced test coverage

2017-05-17 Thread Ramsay Jones
On 17/05/17 05:23, Junio C Hamano wrote: > Junio C Hamano writes: > >> Junio C Hamano writes: >> >>> It appears to me that only a few tests in the entire script wants to >>> work with HTTP server, so perhaps moving them to the end, together >>> with the

Re: [PATCH/RFC 0/3] Use sha1collisiondetection as a submodule

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 12:45 PM, Ævar Arnfjörð Bjarmason wrote: > On Wed, May 17, 2017 at 8:52 PM, Stefan Beller wrote: >> On Wed, May 17, 2017 at 4:38 AM, Ævar Arnfjörð Bjarmason >> wrote: >>> On Wed, May 17, 2017 at 9:09 AM, Junio C

Hallo

2017-05-17 Thread hi
Hallo www . rbgraceful .com Es ist sehr heiß heute Gute Neuigkeiten für dich 19.9euro für Ray Ban Sonnenbrillen von US, freies Verschiffen, 2 Tage, zum an Ihr Haus anzukommen 웃음과 기쁨이 넘치는 2016년 병신년이 되시기 바랍니다.

Re: [PATCH/RFC 0/3] Use sha1collisiondetection as a submodule

2017-05-17 Thread Ævar Arnfjörð Bjarmason
On Wed, May 17, 2017 at 8:52 PM, Stefan Beller wrote: > On Wed, May 17, 2017 at 4:38 AM, Ævar Arnfjörð Bjarmason > wrote: >> On Wed, May 17, 2017 at 9:09 AM, Junio C Hamano wrote: >>> Ævar Arnfjörð Bjarmason writes: >>>

Re: [PATCH] tests: add an optional test to test git-annex

2017-05-17 Thread Ævar Arnfjörð Bjarmason
On Wed, May 17, 2017 at 12:19 PM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >>> Well, it is one thing to place git-annex under CI to make sure its >>> latest and greatest works together well with our latest and greatest >>> (and it may be

Re: [PATCH/RFC 0/3] Use sha1collisiondetection as a submodule

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 4:38 AM, Ævar Arnfjörð Bjarmason wrote: > On Wed, May 17, 2017 at 9:09 AM, Junio C Hamano wrote: >> Ævar Arnfjörð Bjarmason writes: >> >>> On Wed, May 17, 2017 at 7:39 AM, Junio C Hamano wrote:

Re: t5400 failure on Windows

2017-05-17 Thread Johannes Sixt
Am 17.05.2017 um 07:44 schrieb Jeff King: > I wonder if there's a way we could convince the trace for the two > programs to go to separate locations. We don't care about receive-pack's > trace at all. So maybe: This works. Below it is with a commit message. I'm unsure about the sign-off

Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

2017-05-17 Thread Johannes Sixt
Am 17.05.2017 um 16:26 schrieb Ben Peart: On 5/16/2017 3:13 PM, Johannes Sixt wrote: Am 16.05.2017 um 19:17 schrieb Ben Peart: OK, now I'm confused as to the best path for adding a get_be64. This one is trivial: #define get_be64(p)ntohll(*(uint64_t *)(p)) I cringe when I see a cast

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

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 6:42 AM, Jeff King wrote: > On Wed, May 17, 2017 at 02:05:23PM +0200, Michael Haggerty wrote: > >> This patch series is the next leg on a journey towards reading >> `packed-refs` using `mmap()`, the most interesting aspect of which is >> that we will often

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

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty wrote: > 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. This change will work in a world where all Git implementations obey a lock. If there

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

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty wrote: > Break the function `ref_transaction_commit()` into two functions, > `ref_transaction_prepare()` and `ref_transaction_finish()`, with a > third function, `ref_transaction_abort()`, that can be used to abort > the

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

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty wrote: > Extract function from `files_transaction_commit()`. It will soon have > another caller. This sounds odd to me. Maybe it is missing words? of s/function/the functionality to cleanup/ Code makes sense to me. Stefan

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

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 6:17 AM, 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 >> track of whether it is

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

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty wrote: > In particular, make it clear that they make copies of the sha1 > arguments. A couple weeks ago we had plans on getting rid of SHA1 in "the near future" IIRC. Would it make sense to not go down the SHA1 road further

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

2017-05-17 Thread Stefan Beller
On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty 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 don't need a sign, so we use that extra bit to be able to process transactions up to twice

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; >> + >> +/* >> + * Iff the packed-refs file

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 didn't >> exist. But it could be that there is

Re: [PATCH] sha1dc: fix issues with a big endian platform

2017-05-17 Thread Johannes Schindelin
Hi Junio, On Wed, 17 May 2017, Junio C Hamano wrote: > diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c > index 35e9dd5bf4..ae25318c47 100644 > --- a/sha1dc/sha1.c > +++ b/sha1dc/sha1.c > @@ -20,7 +20,7 @@ > */ > #if (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \ >

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 >> track of whether it is locked, embed the

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 05:01:41PM +0200, Michael Haggerty wrote: > 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

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 interface. Let's add them now

Re: [PATCH v6 04/11] run-command: use the async-signal-safe execv instead of execvp

2017-05-17 Thread Brandon Williams
On 05/16, Jeff King wrote: > On Wed, May 17, 2017 at 11:15:43AM +0900, Junio C Hamano wrote: > > > > + if (errno == ENOEXEC) > > > + execv(argv.argv[0], (char *const *) argv.argv); > > > > "/bin/sh" tries to run "/usr/bin/git" that was not executable (well, > > the one in

Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

2017-05-17 Thread Ben Peart
On 5/16/2017 3:13 PM, Johannes Sixt wrote: Am 16.05.2017 um 19:17 schrieb Ben Peart: OK, now I'm confused as to the best path for adding a get_be64. This one is trivial: #define get_be64(p)ntohll(*(uint64_t *)(p)) I cringe when I see a cast like this. Unless you can guarantee that p

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 04:11:15PM +0200, Michael Haggerty wrote: > > I suspect it's undefined behavior according to the standard, though I'd > > guess in practice it would be fine. But if I'm understanding it > > correctly, this is the same check as: > > > > if (strlen(iter->iter0->refname)

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 prefix comparisons. So set it

[ANNOUNCE] Git Rev News edition 27

2017-05-17 Thread Christian Couder
Hi everyone, The 27th edition of Git Rev News is now published: https://git.github.io/rev_news/2017/05/17/edition-27/ Thanks a lot to all the contributors and helpers! Enjoy, Christian, Thomas, Jakub and Markus.

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 >> @@ -292,7 +292,19 @@ static int

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 01:14:34PM +0200, Michael Haggerty wrote: > While working on reference code, I was running `git branch` under > `strace`, when I noticed that `$GIT_DIR/HEAD` was being `lstat()`ed and > `read()` 121 times. This is in a repository with 114 branches, so > probably it is

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 test temporary directory

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: >> >> * `if

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 02:05:23PM +0200, Michael Haggerty wrote: > This patch series is the next leg on a journey towards reading > `packed-refs` using `mmap()`, the most interesting aspect of which is > that we will often be able to avoid having to read the whole > `packed-refs` file if we only

Re: Diff topic branch + working copy changes?

2017-05-17 Thread Robert Dailey
On Wed, May 17, 2017 at 8:39 AM, Robert Dailey wrote: > Thanks Junio, I forgot about merge-base. I'll create some aliases for now: > > # Diff Branch > db = "!f() { : git diff ; git diff $(git merge-base @{upstream} > HEAD) ; }; f" > > # Diff Tool Branch >

Re: Diff topic branch + working copy changes?

2017-05-17 Thread Robert Dailey
On Tue, May 16, 2017 at 9:47 PM, Junio C Hamano wrote: > Robert Dailey writes: > >> So for a topic branch based on master, I can diff ONLY my changes on >> the topic branch by doing this simple command: >> >> $ git diff origin/master... >> >> However,

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 02:05:45PM +0200, Michael Haggerty wrote: > From: Jeff King This patch did originate with me, but I know you had to fix several things to integrate it in your series. So I'll review it anyway, and give you full blame for any bugs. :) > When we are

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 02:05:43PM +0200, Michael Haggerty wrote: > 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. Good catch. This should definitely go at the outer-most layer of the

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

2017-05-17 Thread Jeff King
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 didn't > exist. But it could be that there is some other error opening the > file (e.g.,

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

2017-05-17 Thread Jeff King
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_transaction_commit(struct ref_store > *ref_store, > if (transaction->state !=

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

2017-05-17 Thread Jeff King
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 > track of whether it is locked, embed the `lock_file` instance directly > in the `files_ref_store`

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

2017-05-17 Thread Jeff King
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; > + > + /* > + * Iff the packed-refs file associated with this instance is > + * currently

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

2017-05-17 Thread Jeff King
On Wed, May 17, 2017 at 02:05:31PM +0200, Michael Haggerty wrote: > It will soon prove useful. Very mysterious... -Peff

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

2017-05-17 Thread Jeff King
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 interface. Let's add them now before the interface > becomes truly polymorphic and

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

2017-05-17 Thread Jeff King
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 prefix comparisons. So set it to the empty string. Hmm. So givne a refname

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

2017-05-17 Thread Jeff King
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 > @@ -292,7 +292,19 @@ static int prefix_ref_iterator_advance(struct > ref_iterator

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

2017-05-17 Thread Jeff King
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 test temporary directory cannot be cleaned up, which > means that

[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 --- refs.c | 17 + refs/files-backend.c | 17 - refs/refs-internal.h | 8 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/refs.c

[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 --- 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 --- refs/files-backend.c | 12 refs/ref-cache.c | 6 +- refs/ref-cache.h | 3 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git

[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 --- 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 inline int

[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 --- refs.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index ffc9bd0be5..68a0872562

[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 --- refs.c | 5 + refs/files-backend.c | 11 --- 2 files

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

2017-05-17 Thread Michael Haggerty
From: Jeff King When we are matching refnames "as path" against a pattern, then we know that the beginning of any refname that can match the pattern has to match the part of the pattern up to the first glob character. For example, if the pattern is `refs/heads/foo*bar`, then it

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

2017-05-17 Thread Michael Haggerty
When iterating over references, reference priming is used to make sure that loose references are read into the ref-cache before packed references, to avoid races. It used to be that the prefix passed to reference iterators almost always ended in `/`, for example `refs/heads/`. In that case, the

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

2017-05-17 Thread Michael Haggerty
Signed-off-by: Michael Haggerty --- 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, const char

[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 --- 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 @@ struct

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

2017-05-17 Thread Michael Haggerty
Instead of using a global `lock_file` instance for the main "packed-refs" file and using a pointer in `files_ref_store` to keep track of whether it is locked, embed the `lock_file` instance directly in the `files_ref_store` struct and use the new `is_lock_file_locked()` function to keep track of

[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 --- refs/files-backend.c | 42 -- 1 file changed, 28

[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 Haggerty

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

2017-05-17 Thread Michael Haggerty
Break the function `ref_transaction_commit()` into two functions, `ref_transaction_prepare()` and `ref_transaction_finish()`, with a third function, `ref_transaction_abort()`, that can be used to abort the transaction. Break up the `ref_store` methods similarly. This split will make it easier to

[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 --- refs.c | 8 +++- 1 file changed, 7

[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 --- refs/files-backend.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git

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

2017-05-17 Thread Michael Haggerty
The old code ignored any errors encountered when trying to fopen the "packed-refs" file, treating all such failures as if the file didn't exist. But it could be that there is some other error opening the file (e.g., permissions problems), and we don't want to silently ignore such problems. So

  1   2   >