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

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

2018-05-21 Thread Martin Ågren
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 2/4] merge-recursive: provide pair of `unpack_trees_{start,finish}()`

2018-05-21 Thread Martin Ågren
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 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 <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 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 <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com

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

2018-05-21 Thread Martin Ågren
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] regex: do not call `regfree()` if compilation fails

2018-05-20 Thread Martin Ågren
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

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

2018-05-20 Thread Martin Ågren
-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 v2 1/3] config: free resources of `struct config_store_data`

2018-05-20 Thread Martin Ågren
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`

2018-05-20 Thread Martin Ågren
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 v4 2/4] merge-recursive: provide pair of `unpack_trees_{start,finish}()`

2018-05-20 Thread Martin Ågren
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()`

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

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

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

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

2018-05-20 Thread Martin Ågren
()` 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.

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

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

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

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

2018-05-18 Thread Martin Ågren
()` 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}()`

2018-05-18 Thread Martin Ågren
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 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 <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com

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

2018-05-18 Thread Martin Ågren
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"

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

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 <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"

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

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

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

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

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 <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="

[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 <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}()`

2018-05-16 Thread Martin Ågren
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()`

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

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

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

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 <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] config: free resources of `struct config_store_data`

2018-05-13 Thread Martin Ågren
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 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 <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`

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

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

2018-05-13 Thread Martin Ågren
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 v2 08/12] commit-graph: verify commit contents against odb

2018-05-12 Thread Martin Ågren
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 07/12] commit-graph: load a root tree from specific graph

2018-05-12 Thread Martin Ågren
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 06/12] commit: force commit to parse from object database

2018-05-12 Thread Martin Ågren
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

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

2018-05-12 Thread Martin Ågren
> -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 03/12] commit-graph: test that 'verify' finds corruption

2018-05-12 Thread Martin Ågren
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 02/12] commit-graph: verify file header information

2018-05-12 Thread Martin Ågren
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 01/12] commit-graph: add 'verify' subcommand

2018-05-12 Thread Martin Ågren
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 00/12] Integrate commit-graph into fsck, gc, and fetch

2018-05-11 Thread Martin Ågren
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

2018-05-10 Thread Martin Ågren
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

[PATCH v2 1/3] refs.c: refer to "object ID", not "sha1", in error messages

2018-05-10 Thread Martin Ågren
g the message, we do not continue, but actually drop the lock and return -1 without deleting the pseudoref. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- We could make error-reporting more consistent in general in this file, but I'd rather not lose track of the original goal of

[PATCH v2 0/3] refs: handle zero oid for pseudorefs

2018-05-10 Thread Martin Ågren
On 7 May 2018 at 12:05, Martin Ågren <martin.ag...@gmail.com> wrote: > On 7 May 2018 at 09:39, Michael Haggerty <mhag...@alum.mit.edu> wrote: >> Thanks for the patch. This looks good to me. But it it seems that the >> test coverage related to pseudorefs is st

[PATCH v2 2/3] t1400: add tests around adding/deleting pseudorefs

2018-05-10 Thread Martin Ågren
ling tests will be fixed in the next commit. It is only natural to test deletion as well. Test deletion without an old OID, with a correct one and with an incorrect one. Suggested-by: Michael Haggerty <mhag...@alum.mit.edu> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- t/t1

[PATCH v2 3/3] refs: handle zero oid for pseudorefs

2018-05-10 Thread Martin Ågren
ation and fixes both failing tests from the previous commit. Since we have a `strbuf err` for collecting 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

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

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 19:34, Derrick Stolee wrote: > Commits 01-07 focus on the 'git commit-graph verify' subcommand. These > are ready for full, rigorous review. I don't know about "full" and "rigorous", but I tried to offer my thoughts. A lingering feeling I have is that

Re: [PATCH 04/12] commit-graph: verify fanout and lookup table

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 19:34, Derrick Stolee wrote: > While running 'git commit-graph verify', verify that the object IDs > are listed in lexicographic order and that the fanout table correctly > navigates into that list of object IDs. > > Signed-off-by: Derrick Stolee

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

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 19:34, 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 01/12] commit-graph: add 'verify' subcommand

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 19:34, Derrick Stolee wrote: > In case the commit-graph file becomes corrupt, we need a way to > verify its contents match the object database. In the manner of s/verify its/verify that its/ might read better. > 'git fsck' we will implement a 'git

Re: Regression in patch add?

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 15:16, Oliver Joseph Ash wrote: > (Apologies, I accidentally sent this as a reply to the original post, instead > of your email. I'm new to this!) (No worries.) ;-) >> does your test involve unusual file systems, funny characters in filenames, >> ..?

Re: [PATCH v4 6/6] get_short_oid: document & warn if we ignore the type selector

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 14:43, Ævar Arnfjörð Bjarmason wrote: > The SHA1 prefix 06fa currently matches no blobs in git.git. When > disambiguating short SHA1s we've been quietly ignoring the user's type > selector as a fallback mechanism, this was intentionally added in > 1ffa26c461

Re: Regression in patch add?

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 12:41, Oliver Joseph Ash wrote: > I just ran into a similar problem: > https://stackoverflow.com/questions/50258565/git-editing-hunks-fails-when-file-has-other-hunks > > I can reproduce on 2.17.0. The issue doesn't occur on 2.16.2, however. > > Is this a

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

2018-05-10 Thread Martin Ågren
On 10 May 2018 at 08:01, Junio C Hamano wrote: > Jeff King writes: > >> I don't think it's worth re-rolling, but one thing to think about for >> future cleanups: you split the patches by touched area, not by >> functionality. So the first three patches have a

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

2018-05-09 Thread Martin Ågren
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()`

2018-05-09 Thread Martin Ågren
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

2018-05-09 Thread Martin Ågren
()` 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.

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

2018-05-09 Thread Martin Ågren
on the stack.) 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 017dc30380..8837717d36 100644 --- a/t/helper/test

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

2018-05-09 Thread Martin Ågren
-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.) Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- refs.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index 8c50b8b139..c006004bcd 100644 --- a/refs.c

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

2018-05-09 Thread Martin Ågren
direction. Thanks all for the valuable feedback on v1. I could have saved everyone some trouble by writing better commit messages from the start, and probably also by using `--thread` when formatting the patches... Martin Martin Ågren (5): t/helper/test-write-cache: clean up lock-handling refs.c: do

Re: [PATCH v5 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'

2018-05-09 Thread Martin Ågren
On 9 May 2018 at 12:41, Phillip Wood wrote: > On 09/05/18 03:13, Taylor Blau wrote: >> >> +--column:: >> + Prefix the 1-indexed byte-offset of the first match on non-context >> lines. This >> + option is incompatible with '--invert-match', and extended >>

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

2018-05-09 Thread Martin Ågren
On 9 May 2018 at 18:19, Duy Nguyen wrote: > On Tue, May 8, 2018 at 8:18 PM, Jeff King wrote: >> It should be totally safe. If you look at "struct lock_file", it is now >> simply a pointer to a tempfile allocated on the heap (in fact, I thought >> about getting

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

2018-05-08 Thread Martin Ågren
On 8 May 2018 at 20:10, Jeff King <p...@peff.net> wrote: > On Sun, May 06, 2018 at 04:10:29PM +0200, Martin Ågren wrote: >> Unlike in the previous patch, this function is not prepared for >> indicating errors via a `strbuf err`, so let's just drop the dead code. >> Any

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

2018-05-08 Thread Martin Ågren
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

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

2018-05-08 Thread Martin Ågren
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 01/28] t/test-lib: add an SHA1 prerequisite

2018-05-08 Thread Martin Ågren
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 4/5] lock_file: make function-local locks non-static

2018-05-07 Thread Martin Ågren
On 7 May 2018 at 17:24, Duy Nguyen <pclo...@gmail.com> wrote: > On Sun, May 6, 2018 at 9:32 PM, Martin Ågren <martin.ag...@gmail.com> wrote: >> On 6 May 2018 at 19:42, Duy Nguyen <pclo...@gmail.com> wrote: >> Thank you Duy for your comments. How about I wr

Re: [PATCH 10/28] t: skip pack tests if not using SHA-1

2018-05-07 Thread Martin Ågren
On 7 May 2018 at 01:17, brian m. carlson wrote: > These tests rely on creating packs with specially named objects which > are necessarily dependent on the hash used. Skip these tests if we're > not using SHA-1. > > Signed-off-by: brian m. carlson

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

2018-05-07 Thread Martin Ågren
On 7 May 2018 at 01:17, brian m. carlson wrote: > Since this is a core test that tests basic functionality, annotate the > assertions that have dependencies on SHA-1 with the appropriate > prerequisite. > > Signed-off-by: brian m. carlson

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

2018-05-07 Thread Martin Ågren
On 7 May 2018 at 01:17, brian m. carlson wrote: > Add an SHA1 prerequisite to annotate both of these types of tests and > disable them when we're using a different hash. In the future, we can > create versions of these tests which handle both SHA-1 and NewHash.

Re: [PATCH] refs: handle null-oid for pseudorefs

2018-05-07 Thread Martin Ågren
On 7 May 2018 at 09:39, Michael Haggerty wrote: > Thanks for the patch. This looks good to me. But it it seems that the > test coverage related to pseudorefs is still not great. Ideally, all of > the following combinations should be tested: > > Pre-update value |

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

2018-05-06 Thread Martin Ågren
On 6 May 2018 at 22:42, brian m. carlson wrote: > When creating a literal block from an indented block without any sort of > delimiters, Asciidoctor strips off all leading whitespace, resulting in > a misrendered chart. Use an explicit literal block to indicate to >

Re: [bug] Multiline value should error if the next line is section

2018-05-06 Thread Martin Ågren
Hi Shulhan Thank you for your report. I'm abbreviating a bit: On 6 May 2018 at 21:03, Shulhan wrote: > [alias] > tree = --no-pager log --graph \ > -n 20 \ > [user] > name = Shulhan > > (2) Run `git config -f git.config -l` > > The command print

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

2018-05-06 Thread Martin Ågren
On 6 May 2018 at 19:42, Duy Nguyen <pclo...@gmail.com> wrote: > On Sun, May 6, 2018 at 7:26 PM, Duy Nguyen <pclo...@gmail.com> wrote: >> On Sun, May 6, 2018 at 4:10 PM, Martin Ågren <martin.ag...@gmail.com> wrote: >>> These `struct lock_file`s are local to their

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

2018-05-06 Thread Martin Ågren
On 6 May 2018 at 17:48, David Turner <nova...@novalis.org> wrote: > On Sun, 2018-05-06 at 16:10 +0200, Martin Ågren wrote: >> While at it, make the lock non-static. > Re making the lock static, I wonder about the following case: > > if (read_ref(pseudoref, _old_oid))

Re: [PATCH v4 7/7] contrib/git-jump/git-jump: jump to match column in addition to line

2018-05-06 Thread Martin Ågren
On 5 May 2018 at 04:43, Taylor Blau wrote: > Take advantage of 'git-grep(1)''s new option, '--column' in order to > teach Peff's 'git-jump' script how to jump to the correct column for any > given match. > > 'git-grep(1)''s output is in the correct format for Vim's jump list,

Re: [PATCH v2 07/18] branch-diff: indent the diffs just like tbdiff

2018-05-06 Thread Martin Ågren
On 4 May 2018 at 17:34, Johannes Schindelin wrote: > @@ -353,6 +358,7 @@ static void output(struct string_list *a, struct > string_list *b, > int cmd_branch_diff(int argc, const char **argv, const char *prefix) > { > struct diff_options diffopt = { NULL }; >

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

2018-05-06 Thread Martin Ågren
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

2018-05-06 Thread Martin Ågren
` 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()`

2018-05-06 Thread Martin Ågren
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 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

2018-05-06 Thread Martin Ågren
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

2018-05-06 Thread Martin Ågren
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 0/5] getting rid of most "static struct lock_file"s

2018-05-06 Thread Martin Ågren
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

[PATCH] refs: handle null-oid for pseudorefs

2018-05-06 Thread Martin Ågren
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

Re: [PATCH v3 0/7] Some doc-fixes

2018-05-04 Thread Martin Ågren
On 3 May 2018 at 20:48, Andreas Heiduk wrote: > Changes since the last reroll: > > - Better commit comment for "doc: align 'diff --no-index' in text and > synopsis" > This includes Martin's `s/with/and/` comment. > - Eric's typo fix in "doc: add note about shell quoting to

Re: git update-ref fails to create reference. (bug)

2018-05-04 Thread Martin Ågren
Hi Rafael, On 4 May 2018 at 18:28, Rafael Ascensão wrote: > While trying to create a pseudo reference named REF pointing to the > empty tree iff it doesn't exist, I stumbled on the following: > > I assume both are valid ways to create such reference: > a) $ echo -e

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

2018-05-02 Thread Martin Ågren
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

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

2018-04-30 Thread Martin Ågren
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 2/2] unpack_trees_options: free messages when done

2018-04-28 Thread Martin Ågren
From: Elijah Newren Hi Elijah, [Since this is leaving the topic of rename-detection in favour of leak-plugging, I'm shortening the cc-list a bit.] > So, instead, I'd like to see something like the below > (built on top of my series): Thanks a lot. I now have the below patch

Re: [PATCH v2 1/6] doc: improve formatting in githooks.txt

2018-04-27 Thread Martin Ågren
On 27 April 2018 at 19:04, Andreas Heiduk <ashei...@gmail.com> wrote: > Typeset commands and similar things with as `git foo` instead of > 'git foo' or 'git-foo' and add linkgit to the commands which run > the hooks. > > Signed-off-by: Andreas Heiduk <ashei...@gmail.co

Re: [PATCH v2 2/6] doc: align 'diff --no-index' in text with synopsis

2018-04-27 Thread Martin Ågren
On 27 April 2018 at 20:28, Andreas Heiduk <ashei...@gmail.com> wrote: > Am 27.04.2018 um 19:18 schrieb Martin Ågren: >> On 27 April 2018 at 19:04, Andreas Heiduk <ashei...@gmail.com> wrote: >>> The two '' parameters are not optional but the option >>> '-

Re: [PATCH v2 2/6] doc: align 'diff --no-index' in text with synopsis

2018-04-27 Thread Martin Ågren
On 27 April 2018 at 20:40, Andreas Heiduk wrote: > Am 27.04.2018 um 19:33 schrieb Eric Sunshine: >> On Fri, Apr 27, 2018 at 1:04 PM, Andreas Heiduk wrote: >>> The two '' parameters are not optional but the option >>> '--no-index' is. Also move the

Re: [PATCH v2 2/6] doc: align 'diff --no-index' in text with synopsis

2018-04-27 Thread Martin Ågren
k <ashei...@gmail.com> > Reviewed-by: Martin Ågren <martin.ag...@gmail.com> Strictly speaking, my Reviewed-by was on another patch. I do find this one better though thanks to Junio's suggestion (except the mismatch with the commit message). Thanks for continuing with this series. Martin

<    1   2   3   4   5   6   >