[PATCH v2 5/5] lock_file: move static locks into functions

that. After this commit, the remaining occurrences of "static struct lock_file" are locks that are used from within different functions. That is, they need to remain static. (Short of more intrusive changes like passing around pointers to non-static locks.) Signed-off-by: Martin Ågren

[PATCH v2 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

the temp- and lockfile-machinery would keep a pointer into the struct. But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.) Reviewed-by: Stefan Beller <sbel...@google.com> Signed-off-by: Martin Ågren <martin.ag...@

[PATCH v2 4/5] lock_file: make function-local locks non-static

()` or `exit()` or by returning from a `cmd_foo()`. As pointed out by Jeff King, it would be bad if someone held on to a `struct lock_file *` for some reason. After some grepping, I agree with his findings: no-one appears to be doing that. Signed-off-by: Martin Ågren <martin.ag...@gmail.

Re: [PATCH v2 03/12] commit-graph: test that 'verify' finds corruption

On 11 May 2018 at 23:15, Derrick Stolee wrote: > +test_expect_success 'detect bad signature' ' > + cd "$TRASH_DIRECTORY/full" && I was a bit surprised at the "cd outside subshell", but then realized that this file already does that. It will only be a problem if

Re: [PATCH v2 01/12] commit-graph: add 'verify' subcommand

On 11 May 2018 at 23:15, Derrick Stolee wrote: > graph_name = get_commit_graph_filename(opts.obj_dir); > graph = load_commit_graph_one(graph_name); > + FREE_AND_NULL(graph_name); > > if (!graph) > die("graph file %s does not

Re: [PATCH v2 02/12] commit-graph: verify file header information

On 11 May 2018 at 23:15, Derrick Stolee wrote: > During a run of 'git commit-graph verify', list the issues with the > header information in the commit-graph file. Some of this information > is inferred from the loaded 'struct commit_graph'. Some header > information is

Re: [PATCH v2 04/12] commit-graph: parse commit from chosen graph

> -int parse_commit_in_graph(struct commit *item) > +int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item) I think this function should be static.

Re: [PATCH v2 07/12] commit-graph: load a root tree from specific graph

On 11 May 2018 at 23:15, Derrick Stolee wrote: > -struct tree *get_commit_tree_in_graph(const struct commit *c) > +static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g, > +const struct commit *c) > { >

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

On 11 May 2018 at 23:15, Derrick Stolee wrote: > When running 'git commit-graph verify', compare the contents of the > commits that are loaded from the commit-graph file with commits that are > loaded directly from the object database. This includes checking the > root tree

Re: [PATCH v2 06/12] commit: force commit to parse from object database

On 11 May 2018 at 23:15, Derrick Stolee wrote: > -int parse_commit_gently(struct commit *item, int quiet_on_missing) > +int parse_commit_internal(struct commit *item, int quiet_on_missing, int > use_commit_graph) > { > enum object_type type; > void

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

pointer, but need to `xstrdup()` it. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- 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 @@

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

Instead of carefully clearing up `value_regex` in each code path, let `config_store_data_clear()` handle that. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- I *think* that it should be ok to `regfree()` after `regcomp()` failed, but I'll need to look into that some more (a

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

On 13 May 2018 at 10:59, Eric Sunshine <sunsh...@sunshineco.com> wrote: > On Sun, May 13, 2018 at 4:23 AM Martin Ågren <martin.ag...@gmail.com> wrote: > >> Introduce and use a small helper function `config_store_data_clear()` to >> plug these leaks. This should be

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

is config parser events. Once the users (`git_config_set_multivar_in_file_gently()` and `git_config_copy_or_rename_section_in_file()` at the moment) are done, no-one should be holding on to a pointer into this memory. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- config.c | 9 ++

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

On 13 May 2018 at 10:23, Martin Ågren <martin.ag...@gmail.com> wrote: > +void config_store_data_clear(struct config_store_data *store) I will do s/void/static void/ here...

Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch

On 11 May 2018 at 19:23, Derrick Stolee wrote: > Martin's initial test cases are wonderful. I've adapted them to test the > other conditions in the verify_commit_graph() method and caught some > interesting behavior in the process. I'm preparing v2 so we can investigate > the

Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch

On 10 May 2018 at 21:22, Stefan Beller <sbel...@google.com> wrote: > On Thu, May 10, 2018 at 12:05 PM, Martin Ågren <martin.ag...@gmail.com> wrote: >> I hope to find time to do some more hands-on testing of this to see that >> errors actually do get caught. &g

Re: [PATCH 01/28] t/test-lib: add an SHA1 prerequisite

On 8 May 2018 at 01:30, brian m. carlson <sand...@crustytoothpaste.net> wrote: > On Mon, May 07, 2018 at 12:10:39PM +0200, Martin Ågren wrote: >> On 7 May 2018 at 01:17, brian m. carlson <sand...@crustytoothpaste.net> >> wrote: >> > Add an SHA1 prerequi

Re: [PATCH 06/28] t0000: annotate with SHA1 prerequisite

On 8 May 2018 at 01:40, brian m. carlson <sand...@crustytoothpaste.net> wrote: > On Mon, May 07, 2018 at 12:24:13PM +0200, Martin Ågren wrote: >> This could be more centralized at the top of the file, more likely to be >> noticed during a `make test` and easier to adapt

Re: [PATCH 2/2] Documentation: render revisions correctly under Asciidoctor

On 8 May 2018 at 03:13, brian m. carlson <sand...@crustytoothpaste.net> wrote: > On Mon, May 07, 2018 at 06:11:43AM +0200, Martin Ågren wrote: >> Excellent. These two patches fix my original problem and seem like the >> obviously correct approach (in hindsight ;-) ). I w

[PATCH] refs: handle null-oid for pseudorefs

ng errors, let's use it and signal an error to the caller instead of dying hard. Reported-by: Rafael Ascensão <rafa.al...@gmail.com> Helped-by: Rafael Ascensão <rafa.al...@gmail.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- (David's twopensource-address bounced, so I'm t

[PATCH 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

here. But everything is prepared for gently propagating the error, so let's do that instead. There is similar dead code in `delete_pseudoref()`, but let's save that for the next patch. While at it, make the lock non-static. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- refs

[PATCH 4/5] lock_file: make function-local locks non-static

These `struct lock_file`s are local to their respective functions and we can drop their staticness. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- apply.c| 2 +- builtin/describe.c | 2 +- builtin/difftool.c | 2 +- builtin/gc.c | 2 +- builtin/m

[PATCH 5/5] lock_file: move static locks into functions

rom within different functions. That is, they need to remain static. (Short of more intrusive changes like passing around pointers to non-static locks.) Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- t/helper/test-scrap-cache-tree.c | 4 ++-- builtin/add.c| 3 +

[PATCH 1/5] t/helper/test-write-cache: clean up lock-handling

` into the function and drop the staticness. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- t/helper/test-write-cache.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/t/helper/test-write-cache.c b/t/helper/test-write-cache.c index 017d

[PATCH 3/5] refs.c: drop dead code checking lock status in `delete_pseudoref()`

improved error-handling can be added later. While at it, make the lock non-static and reduce its scope. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- refs.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index 8c50b8b139..7abd30dfe1

[PATCH 0/5] getting rid of most "static struct lock_file"s

it should be a clear warning that the lock is being used by more than one function. Martin Martin Ågren (5): t/helper/test-write-cache: clean up lock-handling refs.c: do not die if locking fails in `write_pseudoref()` refs.c: drop dead code checking lock status in `delete_pseudoref()` lock

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

On 18 May 2018 at 00:10, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> 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

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

On 18 May 2018 at 12:51, Robert P. J. Day <rpj...@crashcourse.ca> wrote: > On Fri, 18 May 2018, Martin Ågren wrote: > >> On 18 May 2018 at 11:37, Robert P. J. Day <rpj...@crashcourse.ca> wrote: >> > >> > toward the bottom of "man git-stash",

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

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

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

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(>msgs_to_free, msg, cmd, cmd)->string; >> > >> >

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

later commit will expand on 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 <new...@gmail.com> Signed-of

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

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 <p...@peff.net> Signed-off-by: Martin Ågren <martin.ag...@gmail.com>

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

of 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 <martin.ag...@gmail.com> --- config

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

On 14 May 2018 at 05:03, Eric Sunshine <sunsh...@sunshineco.com> wrote: > On Sun, May 13, 2018 at 5:58 AM, Martin Ågren <martin.ag...@gmail.com> wrote: >> >> How about the following two patches as patches 2/3 and 3/3? I would also >> need to mention in the

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

pointer, but need to `xstrdup()` it. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- 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 @@

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

-by: Eric Sunshine <sunsh...@sunshineco.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- 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 @@ -233

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

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 <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com

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

()` and not any other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano <gits...@pobox.com> Helped-by: Jeff King <p...@peff.net> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- unpack-trees.h | 6 ++ builtin/checkout.c | 1 + merge-recursive.

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

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 <martin.ag...@gmail.com> wrote: > On 19 May 2018 at 03:02, Jeff King <p...@peff.net> wrote:

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

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

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

p.html [2] https://www.redhat.com/archives/libvir-list/2013-September/msg00262.html Researched-by: Eric Sunshine <sunsh...@sunshineco.com> Signed-off-byi Martin Ågren <martin.ag...@gmail.com> --- On 14 May 2018 at 05:05, Eric Sunshine <sunsh...@sunshineco.com> wrote: > My rese

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

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

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

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

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

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

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

()` and not any other members of the `struct unpack_trees_options`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- unpack-trees.h | 5 + builtin/checkout.c | 1 + merge-recursive.c | 1 + merge.c| 3 +++ unpack-trees.c | 11 +++ 5 files chang

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

e next commit will expand on 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 <new...@gmail.com> Signed-of

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

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 <martin.ag...@gmail.com> --- merge.c | 34 ++ 1 file changed, 18 i

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

Hi Elijah On 16 May 2018 at 16:32, Elijah Newren <new...@gmail.com> wrote: > On Sat, Apr 28, 2018 at 4:32 AM, Martin Ågren <martin.ag...@gmail.com> wrote: >> As you noted elsewhere [1], Ben is also working in this area. I'd be >> perfectly happy to sit on these

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

later commit will expand on 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 <new...@gmail.com> Signed-of

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

h a single 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 <gits...@pobox.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- argv-array.h | 4 ++-- argv-array.c

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

other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- unpack-trees.h | 8 +++- builtin/checkout.c | 1 + merge-recursive.

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

n Elijah Newren (1): merge-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

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

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 <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com

Re: symbol string_list_appendf() unused

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

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

On 22 May 2018 at 04:20, Eric Sunshine <sunsh...@sunshineco.com> wrote: > On Mon, May 21, 2018 at 2:43 PM, Stefan Beller <sbel...@google.com> wrote: >> On Sun, May 20, 2018 at 3:50 AM, Martin Ågren <martin.ag...@gmail.com> wrote: >>> It is apparently

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

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

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

'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-so-good reasons, but still)? Do we wan

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

uture 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

appear anyway) so it makes sense to let the prefix be nontranslated, too. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- This change breaks several tests under GETTEXT_POISON, e.g. t6030 which does this: git bisect skip > my_bisect_log.txt 2>&1 &&

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

"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 <martin.ag...@gmail.com> --- I'm open for suggestions on the naming of `prefix_suffix_lines()`... git-compat-util.h |

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

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 <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com

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

()` and not any other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- unpack-trees.h | 6 ++ builtin/checkout.c | 1 + merge-recursive.c | 1 + merge.c| 3 +++ unpack-tree

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

e next commit will expand on 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 <new...@gmail.com> Signed-of

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

by 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

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

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

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

On 16 May 2018 at 18:41, Stefan Beller <sbel...@google.com> wrote: > On Wed, May 16, 2018 at 9:30 AM, Martin Ågren <martin.ag...@gmail.com> wrote: >> >> This patch is best viewed using something like this (note the tab!): >> --color-moved --anchored="

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

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

Re: [PATCH 09/20] abbrev tests: test for "git-log" behavior

On 9 June 2018 at 11:56, Ævar Arnfjörð Bjarmason wrote: > > On Sat, Jun 09 2018, Martin Ågren wrote: > >> On 9 June 2018 at 00:41, Ævar Arnfjörð Bjarmason wrote: >>> The "log" family of commands does its own parsing for --abbrev in >>> revision.c, s

Re: [PATCH 17/20] abbrev: unify the handling of empty values

On 9 June 2018 at 00:41, Ævar Arnfjörð Bjarmason wrote: > For no good reason the --abbrev= command-line option was less strict > than the core.abbrev config option, which came down to the latter > using git_config_int() which rejects an empty string, but the rest of > the parsing using strtoul()

Re: [PATCH 20/20] abbrev: add a core.validateAbbrev setting

On 9 June 2018 at 00:41, Ævar Arnfjörð Bjarmason wrote: > Instead of trying really hard to find an unambiguous SHA-1 we can with > core.validateAbbrev=false, and preferably combined with the new > support for relative core.abbrev values (such as +4) unconditionally > print a short SHA-1 without

Re: [PATCH 17/20] abbrev: unify the handling of empty values

On 9 June 2018 at 16:24, Martin Ågren wrote: > On 9 June 2018 at 00:41, Ævar Arnfjörð Bjarmason wrote: >> For no good reason the --abbrev= command-line option was less strict >> than the core.abbrev config option, which came down to the latter >> using git_config_int() w

Re: [PATCH 19/20] abbrev: support relative abbrev values

On 9 June 2018 at 00:41, Ævar Arnfjörð Bjarmason wrote: > diff --git a/Documentation/config.txt b/Documentation/config.txt > index ab641bf5a9..abf07be7b6 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -919,6 +919,12 @@ core.abbrev:: > in your repository,

Re: [PATCH] fsck: avoid looking at NULL blob->object

On 9 June 2018 at 10:32, Jeff King wrote: > Except it _does_ do one non-trivial thing, which is call the > report() function, which wants us to pass a pointer to a > "struct object". Which we don't have (we have only a "struct > object_id"). So we erroneously passed the NULL object, which

Re: [PATCH 09/20] abbrev tests: test for "git-log" behavior

On 9 June 2018 at 00:41, Ævar Arnfjörð Bjarmason wrote: > The "log" family of commands does its own parsing for --abbrev in > revision.c, so having dedicated tests for it makes sense. > +for i in $(test_seq 4 40) I've just been skimming so might have missed something, but I see several

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

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: [RFC PATCH 2/3] usage: prefix all lines in `vreportf()`, not just the first

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

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

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

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: [PATCH v2 05/36] refspec: convert valid_fetch_refspec to use parse_refspec

Hi Brandon, On 17 May 2018 at 00:57, Brandon Williams wrote: > Convert 'valid_fetch_refspec()' to use the new 'parse_refspec()' > function to only parse a single refspec and eliminate an allocation. > > Signed-off-by: Brandon Williams > --- > refspec.c | 17 - > refspec.h | 3

[PATCH] refspec: initalize `refspec_item` in `valid_fetch_refspec()`

by calling `valid_fetch_refspec(":*")`. Zero the struct, as is done in other users of `struct refspec_item`. Signed-off-by: Martin Ågren --- I found some time to look into this. It does not seem to be a user-visible bug, so not particularly critical. refspec.c | 5 - 1 file changed, 4 inser

Re: [PATCH] refspec: initalize `refspec_item` in `valid_fetch_refspec()`

On 4 June 2018 at 23:55, Ævar Arnfjörð Bjarmason wrote: > I think this makes more sense instead of this fix: [...] > -void refspec_item_init(struct refspec_item *item, const char *refspec, int > fetch) > +int refspec_item_init(struct refspec_item *item, const char *refspec, int > fetch) > { >

Re: [PATCH 0/3] refspec: refactor & fix free() behavior

into two different patches to make it clear that > all callers of refpsec_item_init relying on dieing. Me too. >> Martin Ågren (1): >> refspec: initalize `refspec_item` in `valid_fetch_refspec()` I was a bit surprised at first that this wasn't a "while at it" in the second pat

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

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

[PATCH] revisions.txt: expand tabs to spaces in diagram

The diagram renders fine in AsciiDoc before and after this patch. Asciidoctor, on the other hand, ignores the tabs entirely, which results in different indentation for different lines. The graph illustration earlier in the document already uses spaces instead of a tab. Signed-off-by: Martin Ågren

Re: [PATCH] revisions.txt: expand tabs to spaces in diagram

On 2 May 2018 at 06:50, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> The diagram renders fine in AsciiDoc before and after this patch. >> Asciidoctor, on the other hand, ignores the tabs entirely, which resul

Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C

mmer than `FREE_AND_NULL` and has a slightly larger potential for misuse. Document that `FREE_AND_NULL` should be preferred. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- argv-array.c | 3 +-- git-compat-util.h | 8 submodule.c | 3 +-- 3 files changed, 10 insertion

Re: [PATCH v16 Part II 5/8] bisect--helper: `bisect_next_check` shell function in C

On 27 October 2017 at 17:06, Pranit Bauva wrote: > + /* > +* have bad (or new) but not good (or old). We could bisect > +* although this is less optimum. > +*/ > + fprintf(stderr, _("Warning:

Re: [PATCH v16 Part II 6/8] bisect--helper: `get_terms` & `bisect_terms` shell function in C

On 27 October 2017 at 17:06, Pranit Bauva wrote: > + for (i = 0; i < argc; i++) { > + if (!strcmp(argv[i], "--term-good")) > + printf(_("%s\n"), terms->term_good); > + else if (!strcmp(argv[i], "--term-bad")) > +

Re: [BUG] v2.16.0-rc0 seg faults when git bisect skip

On 6 January 2018 at 15:27, Yasushi SHOJI wrote: > best_bisection_sorted() seems to do > > - get the commit list along with the number of elements in the list > - walk the list one by one to check whether a element have TREESAME or not > - if TREESAME, skip > - if

Re: [BUG] v2.16.0-rc0 seg faults when git bisect skip

d and bad. But we had no tests for this behavior. Test the behavior when we mark a commit first bad, then good, but also when we are already in the bad state, where `git bisect skip` should notice it. This test would have caught the segfault which was recently fixed in 2e9fdc795c (bisect: fi

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

t; approach was good. But I haven't really pushed it any further. > > Martin Ågren (cc'd) did some follow-up work, but I think we still have a > long way to go. Agreed. :-) > My hope is that people who are interested in > leak-checking their new code can run some specific script

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

On 12 February 2018 at 10:56, Duy Nguyen <pclo...@gmail.com> wrote: > On Wed, Feb 7, 2018 at 3:05 AM, Martin Ågren <martin.ag...@gmail.com> wrote: >> On 6 February 2018 at 03:13, Jeff King <p...@peff.net> wrote: >>> On Mon, Feb 05, 2018 at 08:28:10PM +07

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

On 12 February 2018 at 23:17, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> +test_expect_success TTY 'git config respects pager.config when setting' ' >> + rm -f paginated.out && >> + test_ter

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

e. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- 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/

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

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: Martin

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

git config --edit` as it would have before the previous commit. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- Documentation/git-config.txt | 2 +- t/t7006-pager.sh | 12 ++-- builtin/config.c | 2 +- 3 files changed, 8 insertions(+), 8 deletion

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

On 13 February 2018 at 11:25, Duy Nguyen <pclo...@gmail.com> wrote: > On Sun, Feb 11, 2018 at 11:40 PM, Martin Ågren <martin.ag...@gmail.com> wrote: >> Similar to de121ffe5 (tag: respect `pager.tag` in list-mode only, >> 2017-08-02), use the DELAY_PAGER_CONFI

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

trigger leaks, in which case they would belong in this list. Suggested-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com> Helped-by: Jeff King <p...@peff.net> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- t/known-leaky | 539 +

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

nt 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-off-by: Martin Ågren

<    1   2   3   4   5   6   >