Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check

2018-12-05 Thread René Scharfe
Am 05.12.2018 um 09:15 schrieb Jeff King: > On Wed, Dec 05, 2018 at 01:51:36AM -0500, Jeff King wrote: > >>> This >>> function is easily converted to struct object_id, though, as its single >>> caller can pass one on -- this makes the copy unnecessary. >> >> If you mean modifying

Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check

2018-12-04 Thread René Scharfe
Am 05.12.2018 um 05:46 schrieb Jeff King: > On Tue, Dec 04, 2018 at 10:45:13PM +0100, René Scharfe wrote: > >>> The comment in the context there is warning callers to remember to load >>> the cache first. Now that we have individual caches, might it make sense >>&

Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check

2018-12-04 Thread René Scharfe
Am 03.12.2018 um 23:04 schrieb Jeff King: > On Sun, Dec 02, 2018 at 11:52:50AM +0100, René Scharfe wrote: > >>> And for mu.git, a ~20k object repo: >>> >>> Test origin/master >>> peff/jk/loos

Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check

2018-12-02 Thread René Scharfe
Am 13.11.2018 um 11:02 schrieb Ævar Arnfjörð Bjarmason: > > On Mon, Nov 12 2018, Ævar Arnfjörð Bjarmason wrote: > >> On Mon, Nov 12 2018, Ævar Arnfjörð Bjarmason wrote: >> >>> I get: >>> >>> Test origin/master >>> peff/jk/loose-cache

Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check

2018-11-27 Thread René Scharfe
Am 12.11.2018 um 15:54 schrieb Jeff King: > diff --git a/sha1-file.c b/sha1-file.c > index 4aae716a37..e53da0b701 100644 > --- a/sha1-file.c > +++ b/sha1-file.c > @@ -921,6 +921,24 @@ static int open_sha1_file(struct repository *r, > return -1; > } > > +static int quick_has_loose(struct

Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long

2018-11-19 Thread René Scharfe
Am 19.11.2018 um 06:33 schrieb Torsten Bögershausen: > The archive-tar.c is actually a good example, why a step-by-step update > is not ideal (the code would not work any more on Win64). > > If we look here: > > static int stream_blocked(const struct object_id *oid) > { > struct

Re: [PATCH 8/9] sha1-file: use loose object cache for quick existence check

2018-11-14 Thread René Scharfe
Am 13.11.2018 um 11:02 schrieb Ævar Arnfjörð Bjarmason: > So here's the same test not against NFS, but the local ext4 fs (CO7; > Linux 3.10) for sha1collisiondetection.git: > > Test origin/master > peff/jk/loose-cache

Re: [PATCH 9/9] fetch-pack: drop custom loose object cache

2018-11-12 Thread René Scharfe
Am 12.11.2018 um 20:32 schrieb Ævar Arnfjörð Bjarmason: > > On Mon, Nov 12 2018, René Scharfe wrote: >> This removes the only user of OBJECT_INFO_IGNORE_LOOSE. #leftoverbits > > With this series applied there's still a use of it left in > oid_object_info_exten

Re: [PATCH 7/9] object-store: provide helpers for loose_objects_cache

2018-11-12 Thread René Scharfe
Am 12.11.2018 um 15:50 schrieb Jeff King: > --- a/sha1-file.c > +++ b/sha1-file.c > @@ -2125,6 +2125,32 @@ int for_each_loose_object(each_loose_object_fn cb, > void *data, > return 0; > } > > +static int append_loose_object(const struct object_id *oid, const char *path, > +

Re: [PATCH 9/9] fetch-pack: drop custom loose object cache

2018-11-12 Thread René Scharfe
Am 12.11.2018 um 15:55 schrieb Jeff King: > Commit 024aa4696c (fetch-pack.c: use oidset to check existence of loose > object, 2018-03-14) added a cache to avoid calling stat() for a bunch of > loose objects we don't have. > > Now that OBJECT_INFO_QUICK handles this caching itself, we can drop the

Re: [PATCH] khash: silence -Wunused-function

2018-10-23 Thread René Scharfe
Am 23.10.2018 um 13:34 schrieb Carlo Marcelo Arenas Belón: > after 36da893114 ("config.mak.dev: enable -Wunused-function", 2018-10-18) > macro generated code should use a similar solution than commit-slab to > silence the compiler. With Clang 6 and GCC 8 on Debian I don't get any warnings on

Re: [PATCH] commit-reach: fix sorting commits by generation

2018-10-22 Thread René Scharfe
Am 22.10.2018 um 23:10 schrieb Thomas Gummerer: > compare_commit_by_gen is used to sort a list of pointers to 'struct > commit'. The comparison function for qsort is called with pointers to > the objects it needs to compare, so when sorting a list of 'struct > commit *', the arguments are of type

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-10-15 Thread René Scharfe
Am 15.10.2018 um 17:31 schrieb Derrick Stolee: > On 10/14/2018 10:29 AM, René Scharfe wrote: >> diff --git a/git-compat-util.h b/git-compat-util.h >> index 5f2e90932f..491230fc57 100644 >> --- a/git-compat-util.h >> +++ b/git-compat-util.h >> @@ -1066,6 +1066,21

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-10-14 Thread René Scharfe
Am 05.10.2018 um 21:42 schrieb Jeff King: > On Fri, Oct 05, 2018 at 09:36:28PM +0200, René Scharfe wrote: > >> Am 05.10.2018 um 21:08 schrieb Jeff King: >>> On Fri, Oct 05, 2018 at 08:48:27PM +0200, René Scharfe wrote: >>>> +#defin

Re: [PATCH v3] coccicheck: process every source file at once

2018-10-06 Thread René Scharfe
Am 05.10.2018 um 21:00 schrieb Jeff King: > On Fri, Oct 05, 2018 at 08:50:50PM +0200, SZEDER Gábor wrote: > >> On Fri, Oct 05, 2018 at 12:59:01PM -0400, Jeff King wrote: >>> On Fri, Oct 05, 2018 at 04:53:35PM +, Keller, Jacob E wrote: >>> > Are we OK with saying 1.3-1.8GB is necessary to

Re: [PATCH v3 2/5] fetch-pack: load tip_oids eagerly iff needed

2018-10-05 Thread René Scharfe
Am 05.10.2018 um 22:27 schrieb Jeff King: > On Fri, Oct 05, 2018 at 10:13:34PM +0200, René Scharfe wrote: > >>>> -{ >>>> - /* >>>> - * Note that this only looks at the ref lists the first time it's >>>> - * called. This works out in f

Re: [PATCH v3 2/5] fetch-pack: load tip_oids eagerly iff needed

2018-10-05 Thread René Scharfe
Am 05.10.2018 um 00:11 schrieb René Scharfe: > Am 04.10.2018 um 23:38 schrieb Jonathan Tan: >> I don't think the concerns are truly separated - the first loop quoted >> needs to know that in the second loop, tip_oids is accessed only if >> there is at least one unmatched ref

Re: [PATCH v3 2/5] fetch-pack: load tip_oids eagerly iff needed

2018-10-05 Thread René Scharfe
Am 05.10.2018 um 00:07 schrieb Jeff King: > On Thu, Oct 04, 2018 at 05:09:39PM +0200, René Scharfe wrote: > >> tip_oids_contain() lazily loads refs into an oidset at its first call. >> It abuses the internal (sub)member .map.tablesize of that oidset to >> check if

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-10-05 Thread René Scharfe
Am 05.10.2018 um 21:08 schrieb Jeff King: > On Fri, Oct 05, 2018 at 08:48:27PM +0200, René Scharfe wrote: >> +#define DEFINE_SORT(name, type, compare)\ >> +static int compare##_void(const void *one,

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-10-05 Thread René Scharfe
Am 05.10.2018 um 18:51 schrieb Jeff King: > On Fri, Oct 05, 2018 at 12:59:02AM +0200, René Scharfe wrote: > >> We could also do something like this to reduce the amount of manual >> casting, but do we want to? (Macro at the bottom, three semi-random >> examples at th

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-10-04 Thread René Scharfe
Am 01.10.2018 um 22:37 schrieb René Scharfe: > Am 01.10.2018 um 21:26 schrieb Derrick Stolee: >> On 10/1/2018 3:16 PM, René Scharfe wrote: >>> Am 28.06.2018 um 14:31 schrieb Derrick Stolee via GitGitGadget: >>>> diff --git a/commit-reach.c b/commit-reach.c >>

Re: [PATCH v3 2/5] fetch-pack: load tip_oids eagerly iff needed

2018-10-04 Thread René Scharfe
Am 04.10.2018 um 23:38 schrieb Jonathan Tan: >> Determine if the oidset needs to be populated upfront and then do that >> instead. This duplicates a loop, but simplifies the existing one by >> separating concerns between the two. > > [snip] > >> +if (strict) { >> +for (i = 0; i

[PATCH 5/5] oidset: uninline oidset_init()

2018-10-04 Thread René Scharfe
There is no need to inline oidset_init(), as it's typically only called twice in the lifetime of an oidset (once at the beginning and at the end by oidset_clear()) and kh_resize_* is quite big, so move its definition to oidset.c. Document it while we're at it. Signed-off-by: Rene Scharfe ---

[PATCH v3 4/5] oidset: use khash

2018-10-04 Thread René Scharfe
Reimplement oidset using khash.h in order to reduce its memory footprint and make it faster. Performance of a command that mainly checks for duplicate objects using an oidset, with master and Clang 6.0.1: $ cmd="./git-cat-file --batch-all-objects --unordered --buffer

[PATCH v3 3/5] khash: factor out kh_release_*

2018-10-04 Thread René Scharfe
Add a function for releasing the khash-internal allocations, but not the khash structure itself. It can be used with on-stack khash structs. Signed-off-by: Rene Scharfe --- 1 tab = 4 spaces here. khash.h | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/khash.h

[PATCH v3 2/5] fetch-pack: load tip_oids eagerly iff needed

2018-10-04 Thread René Scharfe
tip_oids_contain() lazily loads refs into an oidset at its first call. It abuses the internal (sub)member .map.tablesize of that oidset to check if it has done that already. Determine if the oidset needs to be populated upfront and then do that instead. This duplicates a loop, but simplifies the

[PATCH v3 1/5] fetch-pack: factor out is_unmatched_ref()

2018-10-04 Thread René Scharfe
Move the code to determine if a request is unmatched to its own little helper. This allows us to reuse it in a subsequent patch. Signed-off-by: Rene Scharfe --- fetch-pack.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c

Re: [PATCH v2 2/2] oidset: use khash

2018-10-04 Thread René Scharfe
Am 04.10.2018 um 08:48 schrieb Jeff King: > On Thu, Oct 04, 2018 at 07:56:44AM +0200, René Scharfe wrote: > >>> As the comment above notes, I think we're really looking at the case >>> where this gets populated on the first call, but not subsequent ones. It >&g

[PATCH v3 0/5] oidset: use khash

2018-10-04 Thread René Scharfe
Two new patches to avoid using oidset internals in fetch-pack: fetch-pack: factor out is_unmatched_ref() fetch-pack: load tip_oids eagerly iff needed Unchanged patch: khash: factor out kh_release_* Unchanged, except it doesn't touch fetch-pack anymore: oidset: use khash A new patch,

Re: [PATCH v2 2/2] oidset: use khash

2018-10-03 Thread René Scharfe
Am 03.10.2018 um 21:40 schrieb Jeff King: > On Wed, Oct 03, 2018 at 03:16:39PM +0200, René Scharfe wrote: >> diff --git a/fetch-pack.c b/fetch-pack.c >> index 75047a4b2a..a839315726 100644 >> --- a/fetch-pack.c >> +++ b/fetch-pack.c >> @@ -536,7 +536,7 @@ static i

[PATCH v2 2/2] oidset: use khash

2018-10-03 Thread René Scharfe
Reimplement oidset using khash.h in order to reduce its memory footprint and make it faster. Performance of a command that mainly checks for duplicate objects using an oidset, with master and Clang 6.0.1: $ cmd="./git-cat-file --batch-all-objects --unordered --buffer

[PATCH v2 1/2] khash: factor out kh_release_*

2018-10-03 Thread René Scharfe
Add a function for releasing the khash-internal allocations, but not the khash structure itself. It can be used with on-stack khash structs. Signed-off-by: Rene Scharfe --- 1 tab = 4 spaces here. khash.h | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/khash.h

[PATCH v2 0/2] oidset: use khash

2018-10-03 Thread René Scharfe
Continue the discussion of speeding up oidset started in [1] here in its own thread. [1] https://public-inbox.org/git/20180811172350.ga2...@sigill.intra.peff.net/ The first patch does a mild refactoring to support khash structures on the stack, and the second one converts oidset to khash.

[PATCH] sequencer: use return value of oidset_insert()

2018-10-03 Thread René Scharfe
oidset_insert() returns 1 if the object ID is already in the set and doesn't add it again, or 0 if it hadn't been present. Make use of that fact instead of checking with an extra oidset_contains() call. Signed-off-by: Rene Scharfe --- sequencer.c | 4 +--- 1 file changed, 1 insertion(+), 3

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-10-02 Thread René Scharfe
Am 01.10.2018 um 22:26 schrieb Jeff King: > On Mon, Oct 01, 2018 at 09:15:53PM +0200, René Scharfe wrote: > The reason hashmap.c was added was to avoid open addressing. ;) Because efficient removal of elements is easier to implement with chaining, according to 6a364ced49 (add a has

Re: What's cooking in git.git (Sep 2018, #04; Thu, 20)

2018-10-01 Thread René Scharfe
Am 01.10.2018 um 22:12 schrieb Jeff King: > On Mon, Oct 01, 2018 at 09:16:07PM +0200, René Scharfe wrote: > >> Am 21.09.2018 um 07:22 schrieb Junio C Hamano: >>> * cc/delta-islands (2018-08-16) 7 commits >> [...] >>> * jk/pack-delta-reuse-with-bitmap (2018-08-

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-10-01 Thread René Scharfe
Am 01.10.2018 um 21:26 schrieb Derrick Stolee: > On 10/1/2018 3:16 PM, René Scharfe wrote: >> Am 28.06.2018 um 14:31 schrieb Derrick Stolee via GitGitGadget: >>> diff --git a/commit-reach.c b/commit-reach.c >>> index c58e50fbb..ac132c8e4 100644 >>> --- a/com

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-10-01 Thread René Scharfe
Am 28.08.2018 um 01:03 schrieb Jeff King: > On Sun, Aug 26, 2018 at 01:37:41PM +0200, René Scharfe wrote: >> So it seems worth it. > > Hmm, that really does. Which is a shame, because I hoped that one day we > could get rid of the nasty macro-infestation that is khash.h. But it

Re: What's cooking in git.git (Sep 2018, #04; Thu, 20)

2018-10-01 Thread René Scharfe
Am 21.09.2018 um 07:22 schrieb Junio C Hamano: > * cc/delta-islands (2018-08-16) 7 commits > (merged to 'next' on 2018-08-27 at cf3d7bd93f) > + pack-objects: move 'layer' into 'struct packing_data' > + pack-objects: move tree_depth into 'struct packing_data' > + t5320: tests for delta islands

Re: [PATCH] grep: provide a noop --recursive option

2018-10-01 Thread René Scharfe
Am 29.09.2018 um 19:11 schrieb Junio C Hamano: > I however do not mind if we added "--recursive" with matching > "--no-recursive", and > > - made "--recursive" the default (obviously) > > - made "--no-recursive" a synonym to setting the recursion limit >to "never recurse" > > - and made

Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear

2018-10-01 Thread René Scharfe
Am 28.06.2018 um 14:31 schrieb Derrick Stolee via GitGitGadget: > diff --git a/commit-reach.c b/commit-reach.c > index c58e50fbb..ac132c8e4 100644 > --- a/commit-reach.c > +++ b/commit-reach.c > @@ -513,65 +513,88 @@ int commit_contains(struct ref_filter *filter, struct > commit *commit, >

Re: [PATCH v4 8/8] fsck: support comments & empty lines in skipList

2018-08-28 Thread René Scharfe
Am 28.08.2018 um 11:52 schrieb Ævar Arnfjörð Bjarmason: > It's annoying not to be able to put comments and empty lines in the > skipList, when e.g. keeping a big central list of commits to skip in > /etc/gitconfig, which was my motivation for 1362df0d41 ("fetch: > implement fetch.fsck.*",

Re: [PATCH v3 6/7] fsck: use oidset for skiplist

2018-08-28 Thread René Scharfe
Am 27.08.2018 um 22:15 schrieb Ævar Arnfjörð Bjarmason: > > On Mon, Aug 27 2018, Ævar Arnfjörð Bjarmason wrote: > >> From: René Scharfe >> >> Object IDs to skip are stored in a shared static oid_array. Lookups do >> a binary search on the sorted array.

Re: [PATCH v2 2/2] fsck: use oidset for skiplist

2018-08-27 Thread René Scharfe
Am 27.08.2018 um 09:37 schrieb Ævar Arnfjörð Bjarmason: > > On Sat, Aug 25 2018, René Scharfe wrote: > >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index 2fa65b7516..80ab570579 100644 >> --- a/Documentation/config.txt >> +++ b/Docum

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-26 Thread René Scharfe
Am 14.08.2018 um 03:58 schrieb Jeff King: > Your suggestion can be implemented using khash (my patch below). > >> Before: >> Benchmark #1: ./git-cat-file --batch-all-objects --buffer --unordered >> --batch-check='%(objectname)' >> >> Time (mean ± σ): 269.5 ms ± 26.7 ms[User: 247.7 ms,

[PATCH] mailinfo: support format=flowed

2018-08-25 Thread René Scharfe
Add best-effort support for patches sent using format=flowed (RFC 3676). Remove leading spaces ("unstuff"), remove soft line breaks (indicated by space + newline), but leave the signature separator (dash dash space newline) alone. Warn in git am when encountering a format=flowed patch, because

[PATCH v2 2/2] fsck: use oidset for skiplist

2018-08-25 Thread René Scharfe
Object IDs to skip are stored in a shared static oid_array. Lookups do a binary search on the sorted array. The code checks if the object IDs are already in the correct order while loading and skips sorting in that case. Lookups are done before reporting a (non-fatal) corruption and before

[PATCH v2 1/2] fsck: use strbuf_getline() to read skiplist file

2018-08-25 Thread René Scharfe
buffer is unlikely to contain a NUL character, so printing its contents using %s in a die() format is unsafe (detected with ASan). Use an idiomatic strbuf_getline() loop instead, which ensures the buffer is always NUL-terminated, supports CRLF files as well, accepts files without a newline after

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-25 Thread René Scharfe
Am 11.08.2018 um 18:54 schrieb Ævar Arnfjörð Bjarmason: > > On Sat, Aug 11 2018, René Scharfe wrote: > >> Object IDs to skip are stored in a shared static oid_array. Lookups do >> a binary search on the sorted array. The code checks if the object IDs >> are already

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-25 Thread René Scharfe
Am 11.08.2018 um 22:48 schrieb Ramsay Jones: > On 11/08/18 16:47, René Scharfe wrote: >> @@ -34,12 +36,12 @@ struct fsck_options { >> fsck_error error_func; >> unsigned strict:1; >> int *msg_type; >> -    struct oid_array *skiplist; >> +   

[PATCH] remote: improve argument help for add --mirror

2018-08-19 Thread René Scharfe
Group the possible values using a pair of parentheses and don't mark them for translation, as they are literal strings that have to be used as-is in any locale. Signed-off-by: Rene Scharfe --- builtin/remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/remote.c

[PATCH] parseopt: group literal string alternatives in argument help

2018-08-19 Thread René Scharfe
This formally clarifies that the "--option=" part is the same for all alternatives. Signed-off-by: Rene Scharfe --- builtin/pull.c | 2 +- builtin/push.c | 4 ++-- builtin/send-pack.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/pull.c

[PATCH] checkout-index: improve argument help for --stage

2018-08-19 Thread René Scharfe
Spell out all alternatives and avoid using a numerical range operator, as it is not mentioned in CodingGuidelines and the resulting string is still concise. Wrap them in parentheses to document clearly that the "--stage=" part is common among them. Signed-off-by: Rene Scharfe ---

Re: [PATCH 3/4] cat-file: use a single strbuf for all output

2018-08-14 Thread René Scharfe
rn makes it easy to see where we need to be > releasing the string in the caller (right after the loop > which uses it in each case). > > Based-on-a-patch-by: René Scharfe > Signed-off-by: Jeff King > --- > It also made it easy to see that without the prior patch, > we'd h

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-13 Thread René Scharfe
Am 13.08.2018 um 23:07 schrieb Junio C Hamano: > René Scharfe writes: > >> the mailing list [1], nor on the web interface [2]. The latter shows >> extra spaces on the context lines of the first hunk, though, which I >> can't see anywhere else. All the lines l

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-13 Thread René Scharfe
Am 13.08.2018 um 20:43 schrieb Junio C Hamano: René Scharfe writes: @@ -182,19 +181,10 @@ static int fsck_msg_type(enum fsck_msg_id msg_id, static void init_skiplist(struct fsck_options *options, const char *path) { - static struct oid_array skiplist = OID_ARRAY_INIT; - int

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-13 Thread René Scharfe
Am 11.08.2018 um 22:59 schrieb René Scharfe: If the current oidset implementation is so bad, why not replace it with one based on oid_array? ;-) Intuitively I'd try a hashmap with no payload and open addressing via sha1hash(), which should reduce memory allocations quite a bit -- no need

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-13 Thread René Scharfe
Am 11.08.2018 um 19:23 schrieb Jeff King: On Sat, Aug 11, 2018 at 01:02:48PM -0400, Jeff King wrote: - we could probably improve the speed of oidset. Two things I notice about its implementation: - it has to malloc for each entry, which I suspect is the main bottleneck.

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-11 Thread René Scharfe
Am 11.08.2018 um 19:23 schrieb Jeff King: On Sat, Aug 11, 2018 at 01:02:48PM -0400, Jeff King wrote: - we could probably improve the speed of oidset. Two things I notice about its implementation: Before any optimizations, my best-of-five timing for: git cat-file

Re: [PATCH 1/2] fsck: use strbuf_getline() to read skiplist file

2018-08-11 Thread René Scharfe
Am 11.08.2018 um 18:48 schrieb Jeff King: And one I'm not sure about: - a read() error will now be quietly ignored; I guess we'd have to check ferror(fp) to cover this. I'm not sure if it matters. I'm not sure, either. It would catch media errors or file system corruption, right?

[PATCH 2/2] fsck: use oidset for skiplist

2018-08-11 Thread René Scharfe
Object IDs to skip are stored in a shared static oid_array. Lookups do a binary search on the sorted array. The code checks if the object IDs are already in the correct order while loading and skips sorting in that case. Simplify the code by using an oidset instead. Memory usage is a bit

[PATCH 1/2] fsck: use strbuf_getline() to read skiplist file

2018-08-11 Thread René Scharfe
The char array named "buffer" is unlikely to contain a NUL character, so printing its contents using %s in a die() format is unsafe. Clang's ASan reports running over the end of buffer in the recently added skiplist tests in t5504-fetch-receive-strict.sh as a result. Use an idiomatic

Re: Re* [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 22:36 schrieb Junio C Hamano: > Ævar Arnfjörð Bjarmason writes: > >> Thanks, FWIW that's fine by me, and also if you want to drop this "fake" >> patch of mine and replace it with something René came up with (I have >> not yet read his 1-6 patches submitted on this topic, so

Re: [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 22:01 schrieb Junio C Hamano: > René Scharfe writes: > >> Am 02.08.2018 um 18:54 schrieb Jeff King: >>> PS I actually would have made the rule simply "does it begin with a >>> '<'", which seems simpler still. If people acci

[PATCH 6/6] parse-options: automatically infer PARSE_OPT_LITERAL_ARGHELP

2018-08-02 Thread René Scharfe
Parseopt wraps argument help strings in a pair of angular brackets by default, to tell users that they need to replace it with an actual value. This is useful in most cases, because most option arguments are indeed single values of a certain type. The option PARSE_OPT_LITERAL_ARGHELP needs to be

[PATCH 5/6] shortlog: correct option help for -w

2018-08-02 Thread René Scharfe
Wrap the placeholders in the option help string for -w in pairs of angular brackets to document that users need to replace them with actual numbers. Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding another pair. Signed-off-by: Rene Scharfe --- builtin/shortlog.c | 6

[PATCH 4/6] send-pack: specify --force-with-lease argument help explicitly

2018-08-02 Thread René Scharfe
Wrap each part of the argument help string in angular brackets to show that users need to replace them with actual values. Do that explicitly to balance the pairs nicely in the code and avoid confusing casual readers. Add the flag PARSE_OPT_LITERAL_ARGHELP to keep parseopt from adding another

[PATCH 3/6] pack-objects: specify --index-version argument help explicitly

2018-08-02 Thread René Scharfe
Wrap both placeholders in the argument help string in angular brackets to signal that users needs replace them with some actual value. Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding another pair. Signed-off-by: Rene Scharfe --- builtin/pack-objects.c | 4 ++-- 1 file

[PATCH 2/6] difftool: remove angular brackets from argument help

2018-08-02 Thread René Scharfe
Parseopt wraps arguments in a pair of angular brackets by default, signifying that the user needs to replace it with a value of the documented type. Remove the pairs from the option definitions to duplication and confusion. Signed-off-by: Rene Scharfe --- builtin/difftool.c | 4 ++-- 1 file

[PATCH 1/6] add, update-index: fix --chmod argument help

2018-08-02 Thread René Scharfe
Don't translate the argument specification for --chmod; "+x" and "-x" are the literal strings that the commands accept. Separate alternatives using a pipe character instead of a slash, for consistency. Use the flag PARSE_OPT_LITERAL_ARGHELP to prevent parseopt from adding a pair of angular

Re: [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 18:54 schrieb Jeff King: > PS I actually would have made the rule simply "does it begin with a > '<'", which seems simpler still. If people accidentally write " forgetting to close their brackets, that is a bug under both the > old and new behavior (just with slightly

Re: [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 17:31 schrieb Junio C Hamano: > René Scharfe writes: >> diff --git a/parse-options.c b/parse-options.c >> index 7db84227ab..fadfc6a833 100644 >> --- a/parse-options.c >> +++ b/parse-options.c >> @@ -660,7 +660,8 @@ int parse_options(int argc,

Re: Re* [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
> the end result would balance out, by somebody who forgot the > presence of PARSE_OPT_LITERAL_ARGHELP, which is the escape hatch > mechanism designed to help such a case. We just should use the > official escape hatch instead. > > Helped-by: René Scharfe I didn't do anything

Re: [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 17:06 schrieb René Scharfe: > According to its manpage the option should rather be shown like this: > > --force-with-lease[=[:]] > > ... to indicate that all three forms are valid: > > --force-with-lease > --force-with-lease=some_ref &g

Re: [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 16:21 schrieb Ævar Arnfjörð Bjarmason: > > On Thu, Aug 02 2018, René Scharfe wrote: > >> Am 02.08.2018 um 00:31 schrieb Ævar Arnfjörð Bjarmason: >>> But looking at this again it looks like this whole thing should just be >>> replaced by: >>

Re: [PoC] coccinelle: make Coccinelle-related make targets more fine-grained

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 13:55 schrieb SZEDER Gábor: > Let's add a bit of Makefile metaprogramming to generate finer-grained > make targets applying one semantic patch to only a single source file, > and specify these as dependencies of the targets applying one semantic > patch to all source files. This

Re: [PATCH] push: comment on a funny unbalanced option help

2018-08-02 Thread René Scharfe
Am 02.08.2018 um 00:31 schrieb Ævar Arnfjörð Bjarmason: > But looking at this again it looks like this whole thing should just be > replaced by: > > diff --git a/builtin/push.c b/builtin/push.c > index 9cd8e8cd56..b8fa15c101 100644 > --- a/builtin/push.c > +++ b/builtin/push.c

Re: [PATCH] remote: clear string_list after use in mv()

2018-08-02 Thread René Scharfe
Thank you for the review! Am 02.08.2018 um 04:56 schrieb Jonathan Nieder: > René Scharfe wrote: > >> Subject: remote: clear string_list after use in mv() > > This subject line doesn't fully reflect the goodness of this change. > How about something like: > > r

[PATCH] remote: clear string_list after use in mv()

2018-08-01 Thread René Scharfe
Switch to the _DUP variant of string_list for remote_branches to allow string_list_clear() to release the allocated memory at the end, and actually call that function. Free the util pointer as well; it is allocated in read_remote_branches(). NB: This string_list is empty until

Re: git merge -s subtree seems to be broken.

2018-07-31 Thread René Scharfe
Am 31.07.2018 um 17:50 schrieb Jeff King: > On Tue, Jul 31, 2018 at 11:03:17AM -0400, George Shammas wrote: > >> Bisecting around, this might be the commit that introduced the breakage. >> >> https://github.com/git/git/commit/d8febde >> >> I really hope that it hasn't been broken for 5 years and

Re: git merge -s subtree seems to be broken.

2018-07-31 Thread René Scharfe
Am 31.07.2018 um 23:06 schrieb Junio C Hamano: > Jeff King writes: > >> On Tue, Jul 31, 2018 at 01:23:04PM -0400, Jeff King wrote: >> ... >> So here it is fixed, and with a commit message. I'm not happy to omit a >> regression test, but I actually couldn't come up with a minimal one that >>

Re: Proposed approaches to supporting HTTP remotes in "git archive"

2018-07-29 Thread René Scharfe
Am 28.07.2018 um 00:32 schrieb Junio C Hamano: > Josh Steadmon writes: > >> # Supporting HTTP remotes in "git archive" >> >> We would like to allow remote archiving from HTTP servers. There are a >> few possible implementations to be discussed: >> >> ## Shallow clone to temporary repo >> >> This

Re: [PATCH 0/5] Misc Coccinelle-related improvements

2018-07-23 Thread René Scharfe
Am 23.07.2018 um 15:50 schrieb SZEDER Gábor: > Just a couple of minor Coccinelle-related improvements: > >- The first two patches are small cleanups. > >- The last three could make life perhaps just a tad bit easier for > devs running 'make coccicheck'. > > > SZEDER Gábor (5): >

Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'

2018-06-19 Thread René Scharfe
Am 19.06.2018 um 21:11 schrieb Jeff King: > On Tue, Jun 19, 2018 at 08:50:16PM +0200, René Scharfe wrote: > >> Negation causes the whole non-matching line to match, with --column >> reporting 1 or nothing in such a case, right? Or I think doing the >> same when the oper

Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'

2018-06-19 Thread René Scharfe
Am 19.06.2018 um 19:44 schrieb Taylor Blau: > diff --git a/grep.c b/grep.c > index f3329d82ed..a09935d8c5 100644 > --- a/grep.c > +++ b/grep.c > @@ -1257,8 +1257,8 @@ static int match_one_pattern(struct grep_pat *p, char > *bol, char *eol, > return hit; > } > > -static int

Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'

2018-06-19 Thread René Scharfe
Am 19.06.2018 um 19:48 schrieb Jeff King: > On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote: > >>> The key thing about this iteration is that it doesn't regress >>> performance, because we always short-circuit where we used to. The other >>> obvious

Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'

2018-06-19 Thread René Scharfe
Am 19.06.2018 um 19:44 schrieb Taylor Blau: > On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote: >> Am 19.06.2018 um 18:35 schrieb Jeff King: >>> On Mon, Jun 18, 2018 at 06:43:01PM -0500, Taylor Blau wrote: >> We could add an optimizer pass to reduce the number o

Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'

2018-06-19 Thread René Scharfe
Am 19.06.2018 um 18:35 schrieb Jeff King: > On Mon, Jun 18, 2018 at 06:43:01PM -0500, Taylor Blau wrote: >> The notable case that it does _not_ cover is matching the following >> line: >> >>a ... b >> >> with the following expression >> >>git grep --column -e b --or -e a >> >> This will

Re: [PATCH 24/30] merge-recursive: Add computation of collisions due to dir rename & merging

2018-06-10 Thread René Scharfe
Am 10.06.2018 um 12:56 schrieb René Scharfe: > Am 10.11.2017 um 20:05 schrieb Elijah Newren: >> +static struct dir_rename_entry *check_dir_renamed(const char *path, >> + struct hashmap *dir_renames) { >> +char temp[PATH_MA

Re: [PATCH 24/30] merge-recursive: Add computation of collisions due to dir rename & merging

2018-06-10 Thread René Scharfe
Am 10.11.2017 um 20:05 schrieb Elijah Newren: > +static struct dir_rename_entry *check_dir_renamed(const char *path, > + struct hashmap *dir_renames) { > + char temp[PATH_MAX]; > + char *end; > + struct dir_rename_entry *entry; > + > +

Re: [PATCH 2/2] builtin/blame: highlight recently changed lines

2018-06-09 Thread René Scharfe
Am 17.04.2018 um 23:30 schrieb Stefan Beller: > +static void parse_color_fields(const char *s) > +{ > + struct string_list l = STRING_LIST_INIT_DUP; > + struct string_list_item *item; > + enum { EXPECT_DATE, EXPECT_COLOR } next = EXPECT_COLOR; > + > + colorfield_nr = 0; > + > +

Re: [PATCH v8 1/2] json_writer: new routines to create data in JSON format

2018-06-08 Thread René Scharfe
Am 07.06.2018 um 16:12 schrieb g...@jeffhostetler.com: > Makefile| 2 + > json-writer.c | 419 > json-writer.h | 113 + > t/helper/test-json-writer.c | 572 >

Re: [PATCH v8 1/2] json_writer: new routines to create data in JSON format

2018-06-08 Thread René Scharfe
Am 07.06.2018 um 16:12 schrieb g...@jeffhostetler.com: > From: Jeff Hostetler > +/* > + * Add comma if we have already seen a member at this level. > + */ > +static inline void maybe_add_comma(struct json_writer *jw) > +{ > + if (!jw->open_stack.len) > + return; This is

Re: [PATCH v8 1/2] json_writer: new routines to create data in JSON format

2018-06-08 Thread René Scharfe
le encode such strings) in the future. > > The initial use for the json-writer routines is for generating telemetry > data for executed Git commands. Later, we may want to use them in other > commands, such as status. > > Helped-by: René Scharfe > Helped-by:

Re: [PATCH 3/5] query_fsmonitor: use xsnprintf for formatting integers

2018-05-19 Thread René Scharfe
Am 19.05.2018 um 03:57 schrieb Jeff King: > These formatted integers should always fit into their > 64-byte buffers. Let's use xsnprintf() to add an assertion > that this is the case, which makes auditing for other > unchecked snprintfs() easier. How about this instead? -- >8 -- Subject: [PATCH]

Re: [PATCH] fast-export: avoid NULL pointer arithmetic

2018-05-15 Thread René Scharfe
Am 14.05.2018 um 03:37 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> Storing integer values in pointers is a trick that seems to have worked >> so far for fast-export. A portable way to avoid that trick without >> requiring more memory would

Re: [PATCH] fast-export: avoid NULL pointer arithmetic

2018-05-12 Thread René Scharfe
Am 12.05.2018 um 10:45 schrieb René Scharfe: > Or we could roll our own custom hash map, as I mused in an earlier post. > That would duplicate quite a bit of code; are there reusable pieces > hidden within that could be extracted into common functions? At least it would allow us to

Re: [PATCH v14 4/4] ls-remote: create '--sort' option

2018-05-12 Thread René Scharfe
Am 09.04.2018 um 03:42 schrieb Harald Nordgren: > diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh > index 02106c922..83cd35c39 100755 > --- a/t/t5512-ls-remote.sh > +++ b/t/t5512-ls-remote.sh > @@ -170,14 +206,18 @@ test_expect_success 'overrides work between mixed >

Re: [PATCH] fast-export: avoid NULL pointer arithmetic

2018-05-12 Thread René Scharfe
Am 11.05.2018 um 19:42 schrieb Jeff King: > On Fri, May 11, 2018 at 03:34:19PM +0200, Duy Nguyen wrote: > >> On Fri, May 11, 2018 at 03:11:46PM +0200, Duy Nguyen wrote: >>> Back to fast-export, can we just allocate a new int on heap and point >>> it there? Allocating small pieces becomes quite

Re: [PATCH] fast-export: avoid NULL pointer arithmetic

2018-05-10 Thread René Scharfe
Am 10.05.2018 um 12:51 schrieb Junio C Hamano: > René Scharfe <l@web.de> writes: > >> The standard says about uintptr_t that "any valid pointer to void can >> be converted to this type, then converted back to pointer to void, and >> the result will compare

  1   2   3   4   5   6   7   8   9   10   >