[PATCH 3/1] config: let `config_store_data_clear()` handle `key`

2018-05-13 Thread Martin Ågren
pointer, but need to `xstrdup()` it. Signed-off-by: Martin Ågren --- config.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/config.c b/config.c index 2e3c6c94e9..963edacf10 100644 --- a/config.c +++ b/config.c @@ -2335,6 +2335,7 @@ struct config_store_data { void

[PATCH 2/1] config: let `config_store_data_clear()` handle `value_regex`

2018-05-13 Thread Martin Ågren
Instead of carefully clearing up `value_regex` in each code path, let `config_store_data_clear()` handle that. Signed-off-by: Martin Ågren --- I *think* that it should be ok to `regfree()` after `regcomp()` failed, but I'll need to look into that some more (and say something about it i

Re: [PATCH] config: free resources of `struct config_store_data`

2018-05-13 Thread Martin Ågren
On 13 May 2018 at 10:59, Eric Sunshine wrote: > On Sun, May 13, 2018 at 4:23 AM Martin Ågren wrote: > >> Introduce and use a small helper function `config_store_data_clear()` to >> plug these leaks. This should be safe. The memory tracked here is config >> parse

Re: [PATCH] config: free resources of `struct config_store_data`

2018-05-13 Thread Martin Ågren
On 13 May 2018 at 10:23, Martin Ågren wrote: > +void config_store_data_clear(struct config_store_data *store) I will do s/void/static void/ here...

Re: [PATCH v2 08/12] commit-graph: verify commit contents against odb

2018-05-15 Thread Martin Ågren
On 11 May 2018 at 23:15, Derrick Stolee wrote: I finally sat down today and familiarized myself with the commit-graph code a little. My biggest surprise was when I noticed that there is a hash checksum at the end of the commit-graph-file. That in combination with the tests where you flip some byt

[PATCH v2 0/3] unpack_trees_options: free messages when done

2018-05-16 Thread Martin Ågren
Hi Elijah On 16 May 2018 at 16:32, Elijah Newren wrote: > On Sat, Apr 28, 2018 at 4:32 AM, Martin Ågren wrote: >> As you noted elsewhere [1], Ben is also working in this area. I'd be >> perfectly happy to sit on these patches until both of your contributions >> come t

[PATCH v2 1/3] merge: setup `opts` later in `checkout_fast_forward()`

2018-05-16 Thread Martin Ågren
early returns. This patch is best viewed using something like this (note the tab!): --color-moved --anchored=" trees[nr_trees] = parse_tree_indirect" Signed-off-by: Martin Ågren --- merge.c | 34 ++ 1 file changed, 18 insertions(+), 16 deletions(-)

[PATCH v2 3/3] unpack_trees_options: free messages when done

2018-05-16 Thread Martin Ågren
()` and not any other members of the `struct unpack_trees_options`. Signed-off-by: Martin Ågren --- unpack-trees.h | 5 + builtin/checkout.c | 1 + merge-recursive.c | 1 + merge.c| 3 +++ unpack-trees.c | 11 +++ 5 files changed, 21 insertions(+) diff --git

[PATCH v2 2/3] merge-recursive: provide pair of `unpack_trees_{start,finish}()`

2018-05-16 Thread Martin Ågren
that, teaching `..._finish()` to free more memory. (So rather than moving the FIXME-comment, just drop it, since it will be addressed soon enough.) Also call `..._finish()` when `merge_trees()` returns early. Signed-off-by: Elijah Newren Signed-off-by: Martin Ågren --- merge-recursive.c | 29

Re: [PATCH v2 1/3] merge: setup `opts` later in `checkout_fast_forward()`

2018-05-16 Thread Martin Ågren
On 16 May 2018 at 18:41, Stefan Beller wrote: > On Wed, May 16, 2018 at 9:30 AM, Martin Ågren wrote: >> >> This patch is best viewed using something like this (note the tab!): >> --color-moved --anchored=" trees[nr_trees] = parse_tree_indirect" > > Heh

Re: [PATCH v2 09/12] fsck: verify commit-graph

2018-05-17 Thread Martin Ågren
On 11 May 2018 at 23:15, Derrick Stolee wrote: > If core.commitGraph is true, verify the contents of the commit-graph > during 'git fsck' using the 'git commit-graph verify' subcommand. Run > this check on all alternates, as well. > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh >

Re: [PATCH v2 10/12] commit-graph: add '--reachable' option

2018-05-17 Thread Martin Ågren
On 11 May 2018 at 23:15, Derrick Stolee wrote: > When writing commit-graph files, it can be convenient to ask for all > reachable commits (starting at the ref set) in the resulting file. This > is particularly helpful when writing to stdin is complicated, such as a > future integration with 'git g

Re: [PATCH v2 11/12] gc: automatically write commit-graph files

2018-05-17 Thread Martin Ågren
On 11 May 2018 at 23:15, Derrick Stolee wrote: > The commit-graph file is a very helpful feature for speeding up git > operations. In order to make it more useful, write the commit-graph file > by default during standard garbage collection operations. So does it really write by default... > Add

Re: [PATCH v2 3/3] unpack_trees_options: free messages when done

2018-05-17 Thread Martin Ågren
On 18 May 2018 at 00:10, Junio C Hamano wrote: > Martin Ågren writes: > >> The `opts` string array contains multiple copies of the same pointers. >> Be careful to only free each pointer once, then zeroize the whole array >> so that we do not leave any dangling poi

Re: error(?) in "man git-stash" regarding "--keep-index"

2018-05-18 Thread Martin Ågren
On 18 May 2018 at 11:37, Robert P. J. Day wrote: > > toward the bottom of "man git-stash", one reads part of an example: > > # ... hack hack hack ... > $ git add --patch foo# add just first part to the index > $ git stash push --keep-index# save all other changes to the sta

Re: error(?) in "man git-stash" regarding "--keep-index"

2018-05-18 Thread Martin Ågren
On 18 May 2018 at 12:51, Robert P. J. Day wrote: > On Fri, 18 May 2018, Martin Ågren wrote: > >> On 18 May 2018 at 11:37, Robert P. J. Day wrote: >> > >> > toward the bottom of "man git-stash", one reads part of an example: >> > >> &g

Re: error(?) in "man git-stash" regarding "--keep-index"

2018-05-18 Thread Martin Ågren
On 18 May 2018 at 17:43, Robert P. J. Day wrote: > On Fri, 18 May 2018, Sybille Peters wrote: > >> My 2c on this: >> >> 1) "If the --keep-index option is used, all changes already added to >>the index are left intact" (manpage git stash) >> >> That appears to be correct and clear > > yup, th

[PATCH v3 0/3] unpack_trees_options: free messages when done

2018-05-18 Thread Martin Ågren
Junio, I keep a separate string-list of strings to free. That should make things more future-proof. v2: https://public-inbox.org/git/cover.1526488122.git.martin.ag...@gmail.com/ Martin Elijah Newren (1): merge-recursive: provide pair of `unpack_trees_{start,finish}()` Martin Ågren (2): merge

[PATCH v3 1/3] merge: setup `opts` later in `checkout_fast_forward()`

2018-05-18 Thread Martin Ågren
early returns. This patch is best viewed using something like this (note the tab!): --color-moved --anchored=" trees[nr_trees] = parse_tree_indirect" Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- merge.c | 32 +--- 1 file changed, 17

[PATCH v3 2/3] merge-recursive: provide pair of `unpack_trees_{start,finish}()`

2018-05-18 Thread Martin Ågren
that, teaching `..._finish()` to free more memory. (So rather than moving the FIXME-comment, just drop it, since it will be addressed soon enough.) Also call `..._finish()` when `merge_trees()` returns early. Signed-off-by: Elijah Newren Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano

[PATCH v3 3/3] unpack_trees_options: free messages when done

2018-05-18 Thread Martin Ågren
any other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano Signed-off-by: Martin Ågren --- unpack-trees.h | 6 ++ builtin/checkout.c | 1 + merge-recursive.c | 1 + merge.c| 3 +++ unpack-trees.c | 23 +++ 5 files changed

Re: [PATCH v3 3/3] unpack_trees_options: free messages when done

2018-05-18 Thread Martin Ågren
On 19 May 2018 at 03:02, Jeff King wrote: > On Fri, May 18, 2018 at 03:30:44PM -0700, Elijah Newren wrote: > >> > would become: >> > >> > msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOUPTODATE_FILE] = >> > string_list_appendf(&opts->msgs_to_free, msg, cmd, cmd)->string; >> > >> > I don't kn

Re: [PATCH] Use OPT_SET_INT_F() for cmdline option specification

2018-05-20 Thread Martin Ågren
On 20 May 2018 at 10:12, Nguyễn Thái Ngọc Duy wrote: > The only thing these commands need is extra parseopt flags which can be > passed in by OPT_SET_INT_F() and it is a bit more compact than full > struct initialization. > diff --git a/archive.c b/archive.c > index 93ab175b0b..4fe7bec60c 100644

[PATCH v4 1/4] merge: setup `opts` later in `checkout_fast_forward()`

2018-05-20 Thread Martin Ågren
early returns. This patch is best viewed using something like this (note the tab!): --color-moved --anchored=" trees[nr_trees] = parse_tree_indirect" Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- merge.c | 34 ++ 1 file changed, 18

[PATCH v4 4/4] unpack_trees_options: free messages when done

2018-05-20 Thread Martin Ågren
any other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano Helped-by: Jeff King Signed-off-by: Martin Ågren --- unpack-trees.h | 6 ++ builtin/checkout.c | 1 + merge-recursive.c | 1 + merge.c| 3 +++ unpack-trees.c | 20

[PATCH v4 0/4] unpack_trees_options: free messages when done

2018-05-20 Thread Martin Ågren
This is v4 of my series for taking care of the memory allocated by `setup_unpack_trees_porcelain()`. As before, this is based on bp/merge-rename-config. On 19 May 2018 at 08:13, Martin Ågren wrote: > On 19 May 2018 at 03:02, Jeff King wrote: >> >>> > would becom

[PATCH v4 2/4] merge-recursive: provide pair of `unpack_trees_{start,finish}()`

2018-05-20 Thread Martin Ågren
that, teaching `..._finish()` to free more memory. (So rather than moving the FIXME-comment, just drop it, since it will be addressed soon enough.) Also call `..._finish()` when `merge_trees()` returns early. Signed-off-by: Elijah Newren Signed-off-by: Martin Ågren --- merge-recursive.c | 29

[PATCH v4 3/4] string-list: provide `string_list_appendf()`

2018-05-20 Thread Martin Ågren
unclear and leaking memory. With `strdup_strings = 1` on the other hand, we can easily add formatted strings without going through `string_list_append_nodup()` or playing with `strdup_strings`. Suggested-by: Jeff King Signed-off-by: Martin Ågren --- string-list.h | 9 + string-list.c | 13

[PATCH v2 1/3] config: free resources of `struct config_store_data`

2018-05-20 Thread Martin Ågren
the struct that are candidates for freeing in this new function (`key` and `value_regex`). Those are actually already being taken care of. The next couple of patches will move their freeing into the function we are adding here. Signed-off-by: Martin Ågren --- config.c | 9 + 1 file

[PATCH v2 0/3] config: free resources of `struct config_store_data`

2018-05-20 Thread Martin Ågren
On 14 May 2018 at 05:03, Eric Sunshine wrote: > On Sun, May 13, 2018 at 5:58 AM, Martin Ågren wrote: >> >> How about the following two patches as patches 2/3 and 3/3? I would also >> need to mention in the commit message of this patch (1/3) that the >> function will

[PATCH v2 3/3] config: let `config_store_data_clear()` handle `key`

2018-05-20 Thread Martin Ågren
pointer, but need to `xstrdup()` it. Signed-off-by: Martin Ågren --- config.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/config.c b/config.c index ac71f3f2e1..339a92235d 100644 --- a/config.c +++ b/config.c @@ -2335,6 +2335,7 @@ struct config_store_data { static

[PATCH v2 2/3] config: let `config_store_data_clear()` handle `value_regex`

2018-05-20 Thread Martin Ågren
: Eric Sunshine Signed-off-by: Martin Ågren --- config.c | 17 ++--- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/config.c b/config.c index b3282f7193..ac71f3f2e1 100644 --- a/config.c +++ b/config.c @@ -2335,6 +2335,11 @@ struct config_store_data { static void

[PATCH] regex: do not call `regfree()` if compilation fails

2018-05-20 Thread Martin Ågren
ons/regcomp.html [2] https://www.redhat.com/archives/libvir-list/2013-September/msg00262.html Researched-by: Eric Sunshine Signed-off-byi Martin Ågren --- On 14 May 2018 at 05:05, Eric Sunshine wrote: > My research (for instance [1,2]) seems to indicate that it would be > better to avoid reg

[PATCH v5 0/4] unpack_trees_options: free messages when done

2018-05-21 Thread Martin Ågren
-recursive: provide pair of `unpack_trees_{start,finish}()` Junio C Hamano (1): argv-array: return the pushed string from argv_push*() Martin Ågren (2): merge: setup `opts` later in `checkout_fast_forward()` unpack_trees_options: free messages when done argv-array.h | 4 ++-- unpa

[PATCH v5 1/4] merge: setup `opts` later in `checkout_fast_forward()`

2018-05-21 Thread Martin Ågren
early returns. This patch is best viewed using something like this (note the tab!): --color-moved --anchored=" trees[nr_trees] = parse_tree_indirect" Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- merge.c | 32 +--- 1 file changed, 17

[PATCH v5 4/4] unpack_trees_options: free messages when done

2018-05-21 Thread Martin Ågren
other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano Signed-off-by: Junio C Hamano Signed-off-by: Martin Ågren --- unpack-trees.h | 8 +++- builtin/checkout.c | 1 + merge-recursive.c | 1 + merge.c| 3 +++ unpack-trees.c | 17

[PATCH v5 2/4] merge-recursive: provide pair of `unpack_trees_{start,finish}()`

2018-05-21 Thread Martin Ågren
that, teaching `..._finish()` to free more memory. (So rather than moving the FIXME-comment, just drop it, since it will be addressed soon enough.) Also call `..._finish()` when `merge_trees()` returns early. Signed-off-by: Elijah Newren Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano

[PATCH v5 3/4] argv-array: return the pushed string from argv_push*()

2018-05-21 Thread Martin Ågren
e call, so we do not return any one of these strings from these two functions in order to reduce the chance to misuse the API. Signed-off-by: Junio C Hamano Signed-off-by: Martin Ågren --- argv-array.h | 4 ++-- argv-array.c | 6 -- 2 files changed, 6 insertions(+), 4 deletions(-) diff --g

Re: symbol string_list_appendf() unused

2018-05-22 Thread Martin Ågren
Hi Ramsay On 22 May 2018 at 02:08, Ramsay Jones wrote: > On 22/05/18 00:59, Junio C Hamano wrote: >> There is a reroll by Martin that ties all the loose ends. > > Ah, OK, sorry for the noise. No worry. Thanks for pointing out the unused function to me. I appreciate it. Martin

Re: [PATCH] regex: do not call `regfree()` if compilation fails

2018-05-22 Thread Martin Ågren
On 22 May 2018 at 04:20, Eric Sunshine wrote: > On Mon, May 21, 2018 at 2:43 PM, Stefan Beller wrote: >> On Sun, May 20, 2018 at 3:50 AM, Martin Ågren wrote: >>> It is apparently undefined behavior to call `regfree()` on a regex where >>> `regcomp()` failed. [...] &g

Re: [PATCH v5 0/4] unpack_trees_options: free messages when done

2018-05-22 Thread Martin Ågren
On 22 May 2018 at 04:54, Junio C Hamano wrote: > Junio C Hamano writes: >> Hmph, this unfortunately depends on 'next', which means we cannot >> merge it down to 'maint' later to fix these leaks. I guess it is >> not a huge deal, though. We've lived with these message leaks for >> quite some ti

[RFC PATCH 0/3] usage: prefix all lines in `vreportf()`, not just the first

2018-05-25 Thread Martin Ågren
sts, but since I'm running out of time anyway, I thought I'd post this as a heads-up of "I'm looking into this". I do wonder: If our tests rely on grepping for "warning" (for pretty good reasons), how many scripts out there do something similar (for maybe-not-s

[RFC PATCH 2/3] usage: prefix all lines in `vreportf()`, not just the first

2018-05-25 Thread Martin Ågren
users). Note that we need to adjust quite a few tests as a result of this change. All of these changes are because we obviously need to prefix more lines in various "expect"-files -- except for one instance of a trailing empty line that disappears with this commit (see t7609). This is a ve

[RFC PATCH 3/3] usage: translate the "error: "-prefix and others

2018-05-25 Thread Martin Ågren
ever appear anyway) so it makes sense to let the prefix be nontranslated, too. Signed-off-by: Martin Ågren --- This change breaks several tests under GETTEXT_POISON, e.g. t6030 which does this: git bisect skip > my_bisect_log.txt 2>&1 && grep "warning

[RFC PATCH 1/3] usage: extract `prefix_suffix_lines()` from `advise()`

2018-05-25 Thread Martin Ågren
t;warning: "-messages. In preparation for that, extract the code for printing the individual lines and expose it through git-compat-util.h. Signed-off-by: Martin Ågren --- I'm open for suggestions on the naming of `prefix_suffix_lines()`... git-compat-util.h | 8 advice.c

Re: [RFC PATCH 2/3] usage: prefix all lines in `vreportf()`, not just the first

2018-05-28 Thread Martin Ågren
On 28 May 2018 at 23:45, Junio C Hamano wrote: > Duy Nguyen writes: > +error: sub/added +error: sub/addedtoo +error: Please move or remove them before you switch branches. Aborting EOF >>> >>> This shows the typical effect of this series, which (I subjective

Re: [RFC PATCH 2/3] usage: prefix all lines in `vreportf()`, not just the first

2018-05-29 Thread Martin Ågren
On 29 May 2018 at 07:50, Junio C Hamano wrote: > Martin Ågren writes: > >>> - allow callers to align 1st prefix (e.g. "error: ") with the >>>leading indent for the second and subsequent lines by passing the >>>second prefix with appropriate

Re: [RFC PATCH 2/3] usage: prefix all lines in `vreportf()`, not just the first

2018-05-30 Thread Martin Ågren
On 29 May 2018 at 17:50, Duy Nguyen wrote: > On Tue, May 29, 2018 at 6:49 AM, Martin Ågren wrote: >> On 28 May 2018 at 23:45, Junio C Hamano wrote: >>> Duy Nguyen writes: >>> >>>>>> +error: sub/added >>>>>> +error: su

Re: [RFC PATCH 2/3] usage: prefix all lines in `vreportf()`, not just the first

2018-05-30 Thread Martin Ågren
On 29 May 2018 at 23:32, Jeff King wrote: > On Mon, May 28, 2018 at 06:25:18PM +0900, Junio C Hamano wrote: > >> This shows the typical effect of this series, which (I subjectively >> think) gives us a more pleasant end-user experience. > > Heh, that is one of the cases that I found most ugly when

Re: [RFC PATCH 1/3] usage: extract `prefix_suffix_lines()` from `advise()`

2018-05-30 Thread Martin Ågren
On 30 May 2018 at 08:00, Junio C Hamano wrote: > Junio C Hamano writes: > >> Jeff King writes: >> >>> But most importantly, it means we could eventually colorize errors, too, >>> where we are not allowed to allocate. >>> >>> So perhaps: >>> >>> void report_lines(FILE *out, >>>

Re: Unaligned accesses in sha1dc

2017-06-01 Thread Martin Ågren
On 1 June 2017 at 12:08, Andreas Schwab wrote: > On Jun 01 2017, Junio C Hamano wrote: > >> Depending on the model of "ARM" (or "SPARC") emulated with QEMU, and >> depending on the OS that runs on such an "ARM" or "SPARC", we may >> not see this---if the emulated OS has the "software unaligned-ac

Re: Unaligned accesses in sha1dc

2017-06-01 Thread Martin Ågren
On 1 June 2017 at 12:33, Ævar Arnfjörð Bjarmason wrote: > On Thu, Jun 1, 2017 at 12:26 PM, Martin Ågren wrote: >> On 1 June 2017 at 12:08, Andreas Schwab wrote: >>> Even if the architecture implements unaligned accesses in hardware, it >>> is still undefined behav

Re: Unaligned accesses in sha1dc

2017-06-01 Thread Martin Ågren
On 1 June 2017 at 13:53, Martin Ågren wrote: > On 1 June 2017 at 12:33, Ævar Arnfjörð Bjarmason wrote: >> On Thu, Jun 1, 2017 at 12:26 PM, Martin Ågren wrote: >>> On 1 June 2017 at 12:08, Andreas Schwab wrote: >>>> Even if the architecture implements unaligned ac

Re: Unaligned accesses in sha1dc

2017-06-02 Thread Martin Ågren
On 2 June 2017 at 10:51, Ævar Arnfjörð Bjarmason wrote: > On Fri, Jun 2, 2017 at 2:15 AM, Junio C Hamano wrote: >> Martin Ågren writes: >> >>> I looked into this some more. It turns out it is possible to trigger >>> undefined behavior on "next". Her

Re: Unaligned accesses in sha1dc

2017-06-02 Thread Martin Ågren
On 2 June 2017 at 21:32, Ævar Arnfjörð Bjarmason wrote: > On Fri, Jun 2, 2017 at 11:49 AM, Martin Ågren wrote: >> On 2 June 2017 at 10:51, Ævar Arnfjörð Bjarmason wrote: >>> On Fri, Jun 2, 2017 at 2:15 AM, Junio C Hamano wrote: >>>> Martin Ågren writes: >>&

Re: [PATCH 2/2] *.[ch] refactoring: make use of the freez() wrapper

2017-06-09 Thread Martin Ågren
On 9 June 2017 at 10:53, Ævar Arnfjörð Bjarmason wrote: > Replace occurrences of `free(p); p = NULL` with `freez(p)`. This > introduces no functional changes, but cuts the number of lines spent > on this cleanup in half. It's even better than that. ;) > 48 files changed, 97 insertions(+), 197 d

Re: [PATCH] use DIV_ROUND_UP

2017-07-09 Thread Martin Ågren
On 8 July 2017 at 12:35, René Scharfe wrote: > Convert code that divides and rounds up to use DIV_ROUND_UP to make the > intent clearer and reduce the number of magic constants. ... > diff --git a/sha1_name.c b/sha1_name.c > index e7f7b12ceb..8c513dbff6 100644 > --- a/sha1_name.c > +++ b/sha1_name

Re: [PATCH] use DIV_ROUND_UP

2017-07-09 Thread Martin Ågren
On 9 July 2017 at 16:01, René Scharfe wrote: > Am 09.07.2017 um 15:25 schrieb Martin Ågren: >> On 8 July 2017 at 12:35, René Scharfe wrote: >>> Convert code that divides and rounds up to use DIV_ROUND_UP to make the >>> intent clearer and reduce the number of magic

[PATCH 0/7] tag: more fine-grained pager-configuration

2017-07-10 Thread Martin Ågren
... That's definitely outside this series, but should not be prohibited by it... A review would be much appreciated. Comments on the way I structured the series would be just as welcome as ones on the final result. Martin [1] https://public-inbox.org/git/nrmbrl$hsk$1...@blaine.gmane.org/T/ Martin

[PATCH 1/7] api-builtin.txt: document SUPPORT_SUPER_PREFIX

2017-07-10 Thread Martin Ågren
able flags. Signed-off-by: Martin Ågren --- Documentation/technical/api-builtin.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/technical/api-builtin.txt b/Documentation/technical/api-builtin.txt index 22a39b929..60f442822 100644 --- a/Documentation/technical/api-builtin

[PATCH 3/7] git.c: provide setup_auto_pager()

2017-07-10 Thread Martin Ågren
-patterns or to define whether we split at the first or last '.', so it seems just as easy, and certainly more flexible, to handle the more general situation. Signed-off-by: Martin Ågren --- builtin.h | 14 ++ git.c | 28 2 files changed

[PATCH 2/7] git.c: let builtins opt for handling `pager.foo` themselves

2017-07-10 Thread Martin Ågren
applies to two code-paths -- one in run_builtin() and one in execv_dashed_external(). Document the flag in api-builtin.txt. Don't add any users of IGNORE_PAGER_CONFIG just yet, one will follow in a later patch. Suggested-by: Jeff King Signed-off-by: Martin Ågren --- Documentation/techn

[PATCH 4/7] t7006: add tests for how git tag paginates

2017-07-10 Thread Martin Ågren
ich case no editor is launched, but that is irrelevant, since we just want to see whether the pager is used or not. (If `git tag -am` ever learns to avoid the pager, these tests will need to be updated and two of them will fail.) Signed-off-by: Martin Ågren --- t/t7006-pager.sh | 40 +

[PATCH 7/7] tag: make git tag -l use pager by default

2017-07-10 Thread Martin Ågren
The previous patch introduced `pager.tag.list`. Its default was to use the value of `pager.tag` or, if that was also not set, to fall back to "off". Change that fallback value to "on". Let the default value for `pager.tag` remain at "off". Update documentation and

[PATCH 6/7] tag: make git tag -l consider new config `pager.tag.list`

2017-07-10 Thread Martin Ågren
n tree which carefully ensures that setup_auto_pager() is called precisely once. A greedy approach such as the one taken here works just fine. Noticed-by: Anatoly Borodin Suggested-by: Jeff King Signed-off-by: Martin Ågren --- Documentation/git-tag.txt | 4 builtin/tag.c

[PATCH 5/7] tag: handle `pager.tag`-configuration within the builtin

2017-07-10 Thread Martin Ågren
erroneous usages. Signed-off-by: Martin Ågren --- builtin/tag.c | 2 ++ git.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/tag.c b/builtin/tag.c index 01154ea8d..e0f129872 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -446,6 +446,8 @@ int cmd_ta

Re: [PATCH 1/7] api-builtin.txt: document SUPPORT_SUPER_PREFIX

2017-07-10 Thread Martin Ågren
On 11 July 2017 at 00:50, Brandon Williams wrote: > On 07/10, Martin Ågren wrote: >> Commit 74866d75 ("git: make super-prefix option", 2016-10-07) introduced >> SUPPORT_SUPER_PREFIX as a builtin flag without documenting it in >> api-builtin.txt. The next patch wil

Re: [PATCH 2/7] git.c: let builtins opt for handling `pager.foo` themselves

2017-07-11 Thread Martin Ågren
On 11 July 2017 at 12:24, Jeff King wrote: > On Mon, Jul 10, 2017 at 11:55:15PM +0200, Martin Ågren wrote: > >> To allow individual builtins to make more informed decisions about when >> to respect `pager.foo`, introduce a flag IGNORE_PAGER_CONFIG. If the flag >> is set,

Re: [PATCH 3/7] git.c: provide setup_auto_pager()

2017-07-11 Thread Martin Ågren
On 11 July 2017 at 12:37, Jeff King wrote: > On Mon, Jul 10, 2017 at 11:55:16PM +0200, Martin Ågren wrote: > >> +void setup_auto_pager(const char *cmd, int def) >> +{ >> + if (use_pager != -1) >> + return; > > I think you probably also want

Re: [PATCH 6/7] tag: make git tag -l consider new config `pager.tag.list`

2017-07-11 Thread Martin Ågren
On 11 July 2017 at 12:41, Jeff King wrote: > On Mon, Jul 10, 2017 at 11:55:19PM +0200, Martin Ågren wrote: > >> diff --git a/builtin/tag.c b/builtin/tag.c >> index e0f129872..e96ef7d70 100644 >> --- a/builtin/tag.c >> +++ b/builtin/tag.c >> @@ -446,6 +446,8

Re: [PATCH 0/7] tag: more fine-grained pager-configuration

2017-07-11 Thread Martin Ågren
On 11 July 2017 at 12:19, Jeff King wrote: > On Mon, Jul 10, 2017 at 11:55:13PM +0200, Martin Ågren wrote: > >> Using, e.g., `git -c pager.tag tag -a new-tag` results in errors such >> as "Vim: Warning: Output is not to a terminal" and a garbled terminal. >> A

Re: [PATCH 0/7] tag: more fine-grained pager-configuration

2017-07-12 Thread Martin Ågren
On 11 July 2017 at 00:42, Junio C Hamano wrote: > Martin Ågren writes: >> I am not moving all builtins to handling the pager on their own, >> but instead introduce a flag IGNORE_PAGER_CONFIG and use it only >> with the tag builtin. That means there's another flag

Re: [PATCH 3/7] worktree move: new command

2018-02-06 Thread Martin Ågren
SANITIZE=leak? My t2038 passed, so I went ahead with the full test >> suite and saw so many failures. I did see in your original mails that >> you focused on t and t0001 only.. > > Yeah, I did those two scripts to try to prove to myself that the > approach was good. But I ha

[PATCH 1/3] t7006: add tests for how git config paginates

2018-02-11 Thread Martin Ågren
after those. Signed-off-by: Martin Ågren --- t/t7006-pager.sh | 42 +++--- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index f5f46a95b4..5a7b757c94 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.

[PATCH 3/3] config: change default of `pager.config` to "on"

2018-02-11 Thread Martin Ågren
git config --edit` as it would have before the previous commit. Signed-off-by: Martin Ågren --- Documentation/git-config.txt | 2 +- t/t7006-pager.sh | 12 ++-- builtin/config.c | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/git-

[PATCH 2/3] config: respect `pager.config` in list/get-mode only

2018-02-11 Thread Martin Ågren
at we page all of --get[-*] and --list, than to divide the (current and future) getters into "pages" and "doesn't". This fixes the failing test added in the previous commit. Also adapt the test for whether `git config foo.bar bar` respects `pager.config`. Signed-off-by: M

Re: [PATCH 3/7] worktree move: new command

2018-02-12 Thread Martin Ågren
On 12 February 2018 at 10:56, Duy Nguyen wrote: > On Wed, Feb 7, 2018 at 3:05 AM, Martin Ågren wrote: >> On 6 February 2018 at 03:13, Jeff King wrote: >>> On Mon, Feb 05, 2018 at 08:28:10PM +0700, Duy Nguyen wrote: >>>> I learned SANITIZE=leak today! It not o

Re: [PATCH 1/3] t7006: add tests for how git config paginates

2018-02-12 Thread Martin Ågren
On 12 February 2018 at 23:17, Junio C Hamano wrote: > Martin Ågren writes: > >> +test_expect_success TTY 'git config respects pager.config when setting' ' >> + rm -f paginated.out && >> + test_terminal git -c pager.config config foo.bar ba

Re: [PATCH 2/3] config: respect `pager.config` in list/get-mode only

2018-02-13 Thread Martin Ågren
On 13 February 2018 at 11:25, Duy Nguyen wrote: > On Sun, Feb 11, 2018 at 11:40 PM, Martin Ågren wrote: >> Similar to de121ffe5 (tag: respect `pager.tag` in list-mode only, >> 2017-08-02), use the DELAY_PAGER_CONFIG-mechanism to only respect >> `pager.config` when we ar

[PATCH] t/known-leaky: add list of known-leaky test scripts

2018-02-14 Thread Martin Ågren
arily test our C code, but all of them might trigger leaks, in which case they would belong in this list. Suggested-by: Nguyễn Thái Ngọc Duy Helped-by: Jeff King Signed-off-by: Martin Ågren --- t/known-leaky | 539 ++ 1 file changed, 539

Re: [PATCH] t/known-leaky: add list of known-leaky test scripts

2018-02-20 Thread Martin Ågren
On 19 February 2018 at 22:29, Jeff King wrote: > On Wed, Feb 14, 2018 at 10:56:37PM +0100, Martin Ågren wrote: > >> Here's what a list of known leaks might look like. It feels a bit >> awkward to post a known-incomplete list (I don't run all tests). Duy >> offere

[PATCH v2 2/3] config: respect `pager.config` in list/get-mode only

2018-02-21 Thread Martin Ågren
ecedent that we will never be able to enforce throughout the codebase and that will just open a can of worms. This fixes the failing test added in the previous commit. Also adapt the test for whether `git config foo.bar bar` and `git config --get foo.bar` respects `pager.config`. Signed-

[PATCH v2 0/3] Re: t7006: add tests for how git config paginates

2018-02-21 Thread Martin Ågren
to "success". I have not done anything about that, except to try and motivate the choice better in the commit message of the second patch. Martin Martin Ågren (3): t7006: add tests for how git config paginates config: respect `pager.config` in list/get-mode only config: change

[PATCH v2 1/3] t7006: add tests for how git config paginates

2018-02-21 Thread Martin Ågren
k it, or place new tests like these next to it, but let's instead make the tests for `git config` as similar as possible to the ones for `git tag` and `git branch`, and place them after those. Signed-off-by: Martin Ågren --- t/t7006-pager.sh | 49 ++

[PATCH v2 3/3] config: change default of `pager.config` to "on"

2018-02-21 Thread Martin Ågren
git config --edit` as it would have before the previous commit. Signed-off-by: Martin Ågren --- Documentation/git-config.txt | 1 + t/t7006-pager.sh | 12 ++-- builtin/config.c | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/git-co

Re: [PATCH] t/known-leaky: add list of known-leaky test scripts

2018-02-26 Thread Martin Ågren
On 25 February 2018 at 04:48, Kaartic Sivaraam wrote: > On Wednesday 21 February 2018 02:14 AM, Martin Ågren wrote: >> To sum up: I probably won't be looking into Travis-ing such a blacklist >> in the near future. >> > > Just thinking out loud, how about having a

[PATCH 0/5] roll back locks in various code paths

2018-02-27 Thread Martin Ågren
which are already rolled back. I've based this on maint. There's a conflict on pu, with c7d4394111 (sequencer: avoid using errno clobbered by rollback_lock_file(), 2018-02-11). The conflict resolution would be to take my version for the "could not lock HEAD"-hunk. Martin Mart

[PATCH 1/5] sequencer: make lockfiles non-static

2018-02-27 Thread Martin Ågren
After 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can have lockfiles on the stack. One of these functions fails to always roll back the lock. That will be fixed in the next commit. Signed-off-by: Martin Ågren --- sequencer.c | 10 +- 1 file changed, 5

[PATCH 4/5] merge: always roll back lock in `checkout_fast_forward()`

2018-02-27 Thread Martin Ågren
it to do so. Signed-off-by: Martin Ågren --- merge.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/merge.c b/merge.c index 195b578700..f06a4773d4 100644 --- a/merge.c +++ b/merge.c @@ -113,17 +113,23 @@ int checkout_fast_forward(const struct object_id *head

[PATCH 5/5] sequencer: do not roll back lockfile unnecessarily

2018-02-27 Thread Martin Ågren
recurring error message. Signed-off-by: Martin Ågren --- sequencer.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/sequencer.c b/sequencer.c index e6bac4692a..0e6d04e4ce 100644 --- a/sequencer.c +++ b/sequencer.c @@ -303,10 +303,8 @@ static int write_mes

[PATCH 2/5] sequencer: always roll back lock in `do_recursive_merge()`

2018-02-27 Thread Martin Ågren
If we return early, we forget to roll back the lockfile. Do so. Signed-off-by: Martin Ågren --- sequencer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 90807c4559..e6bac4692a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -465,8 +465,10

[PATCH 3/5] merge-recursive: always roll back lock in `merge_recursive_generic()`

2018-02-27 Thread Martin Ågren
If we return early, we forget to roll back the lockfile. Do so. Signed-off-by: Martin Ågren --- merge-recursive.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/merge-recursive.c b/merge-recursive.c index cc5fa0a949..7345125691 100644 --- a/merge-recursive.c +++ b/merge

Re: [PATCH v5 01/12] sequencer: avoid using errno clobbered by rollback_lock_file()

2018-02-27 Thread Martin Ågren
On 26 February 2018 at 22:29, Johannes Schindelin wrote: > As pointed out in a review of the `--recreate-merges` patch series, > `rollback_lock_file()` clobbers errno. Therefore, we have to report the > error message that uses errno before calling said function. > > Signed-off-by: Johannes Schinde

Re: [PATCH 2/5] sequencer: always roll back lock in `do_recursive_merge()`

2018-02-27 Thread Martin Ågren
On 27 February 2018 at 22:44, Jeff King wrote: > I want to note one thing that confused me while reviewing. While looking > to see if there were other returns, I noticed that the lines right near > the end of your context are funny: > > if (active_cache_changed && > write_loc

Re: [PATCH 3/5] merge-recursive: always roll back lock in `merge_recursive_generic()`

2018-02-28 Thread Martin Ågren
On 27 February 2018 at 22:30, Martin Ågren wrote: > If we return early, we forget to roll back the lockfile. Do so. > > Signed-off-by: Martin Ågren > --- > merge-recursive.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/merge-recursive.c b/m

[PATCH v2 1/5] sequencer: make lockfiles non-static

2018-02-28 Thread Martin Ågren
After 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can have lockfiles on the stack. One of these functions fails to always roll back the lock. That will be fixed in the next commit. Signed-off-by: Martin Ågren --- sequencer.c | 10 +- 1 file changed, 5

[PATCH v2 2/5] sequencer: always roll back lock in `do_recursive_merge()`

2018-02-28 Thread Martin Ågren
If we return early, we forget to roll back the lockfile. Do so. Signed-off-by: Martin Ågren --- sequencer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 90807c4559..e6bac4692a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -465,8 +465,10

[PATCH v2 0/5] roll back locks in various code paths

2018-02-28 Thread Martin Ågren
and which fooled me when I prepared v1. Martin Martin Ågren (5): sequencer: make lockfiles non-static sequencer: always roll back lock in `do_recursive_merge()` merge-recursive: always roll back lock in `merge_recursive_generic()` merge: always roll back lock in `checkout_fast_forward()`

[PATCH v2 5/5] sequencer: do not roll back lockfile unnecessarily

2018-02-28 Thread Martin Ågren
recurring error message. Signed-off-by: Martin Ågren --- sequencer.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/sequencer.c b/sequencer.c index e6bac4692a..0e6d04e4ce 100644 --- a/sequencer.c +++ b/sequencer.c @@ -303,10 +303,8 @@ static int write_mes

[PATCH v2 4/5] merge: always roll back lock in `checkout_fast_forward()`

2018-02-28 Thread Martin Ågren
it to do so. Signed-off-by: Martin Ågren --- merge.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/merge.c b/merge.c index 195b578700..f06a4773d4 100644 --- a/merge.c +++ b/merge.c @@ -113,17 +113,23 @@ int checkout_fast_forward(const struct object_id *head

<    1   2   3   4   5   6   7   8   >