Re: [PATCH] revision: introduce prepare_revision_walk_extended()

On 16 December 2017 at 13:12, René Scharfe wrote: > prepare_revision_walk() allows callers to take ownership of the array of > pending objects by setting the rev_info flag "leak_pending" and copying > the object_array "pending". They use it to clear commit marks after > setup is

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

`test_expect_failure` to document that we currently respect the pager-configuration with `--edit-description`. The current behavior is buggy since the pager interferes with the editor and makes the end result completely broken. See also b3ee740c8 (t7006: add tests for how git tag paginates, 2017-08-02). S

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

h this commit reuses for git-branch.txt. This fixes the failing test added in the previous commit. Also adapt the test for whether `git branch --set-upstream-to` respects `pager.branch`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- Documentation/git-branch.txt | 6 ++ t/t700

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

escription` as it would have before the previous commit. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- Documentation/git-branch.txt | 2 +- t/t7006-pager.sh | 10 +- builtin/branch.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/D

Re: [PATCH 2/2] Triangular workflow

On 17 November 2017 at 17:07, Daniel Bensoussan wrote: > The documentation about triangular workflow was not clear enough. I think you would be able to `git rebase -i` these two patches into a single, perfect patch. ;-) Maybe in collaboration with Albertin? >

Re: [PATCH 1/2] Documentation about triangular workflow

On 17 November 2017 at 17:07, Daniel Bensoussan wrote: > +- If the maintainer accepts the changes, he merges them into the > + **UPSTREAM** repository. Personally, I would prefer "they" and "their" over "he" and "his". I'm not sure how universal this preference

Re: [PATCH] sequencer: reschedule pick if index can't be locked

On 16 November 2017 at 11:43, Phillip Wood <phillip.w...@talktalk.net> wrote: > On 15/11/17 18:44, Martin Ågren wrote: >> >> On 15 November 2017 at 11:41, Phillip Wood <phillip.w...@talktalk.net> >> wrote: >> >> From the commit message, I would have

Re: [PATCH] branch doc: remove --set-upstream from synopsis

On 16 November 2017 at 08:46, Todd Zullinger wrote: > Support for the --set-upstream option was removed in 52668846ea > (builtin/branch: stop supporting the "--set-upstream" option, > 2017-08-17), after a long deprecation period. > > Remove the option from the command synopsis for

Re: [PATCH] sequencer: reschedule pick if index can't be locked

On 15 November 2017 at 11:41, Phillip Wood wrote: > From: Phillip Wood > > Return an error instead of dying if the index cannot be locked in > do_recursive_merge() as if the commit cannot be picked it needs to be > rescheduled when

Re: [PATCH 1/2] merge: close the index lock when not writing the new index

On 11 November 2017 at 00:13, Joel Teichroeb wrote: > If the merge does not have anything to do, it does not unlock the index, > causing any further index operations to fail. Thus, always unlock the index > regardless of outcome. > if (clean < 0) >

Re: [PATCH v2 3/9] ref_transaction_update(): die on disallowed flags

On 5 November 2017 at 09:42, Michael Haggerty wrote: > Callers shouldn't be passing disallowed flags into > `ref_transaction_update()`. So instead of masking them off, treat it > as a bug if any are set. > > Signed-off-by: Michael Haggerty > --- >

Re: [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable}

On 6 November 2017 at 11:48, Jeff King <p...@peff.net> wrote: > On Sun, Nov 05, 2017 at 12:58:18PM +0100, Martin Ågren wrote: >> In particular, they do not teach `--paginate` to use the pager >> configured by `pager.foo.command`. It is already now possible to use >>

[PATCH v3 1/2] builtin/merge-base: free commit lists

're here and reuse the `revs`-variable instead. That matches several other users of `reduce_heads()`. The memory-leak that this hides will be addressed in the next commit. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v3: Like v2 but s/UNLEAK/free_commit_list/ and rebased onto maint

[PATCH v3 2/2] reduce_heads: fix memory leaks

leaks using `free_commit_list()`. While we're here, document `reduce_heads()` and mark it as `extern`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- v3: Like v2 but rebased onto maint. commit.h

[PATCH v2 2/2] reduce_heads: fix memory leaks

leaks using `free_commit_list()`. While we're here, document `reduce_heads()` and mark it as `extern`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- commit.h| 18 +- builtin/commit.c| 2 +- builtin/fmt-merge-msg.c | 2 +- builtin/merge-

[PATCH v2 1/2] builtin/merge-base: UNLEAK commit lists

`reduce_heads()`. The memory-leak that this hides will be addressed in the next commit. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- builtin/merge-base.c | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/builtin/merge-bas

[PATCH v2 0/2] Re: reduce_heads: fix memory leaks

with UNLEAK in them, I'm all ears. Martin [1] https://public-inbox.org/git/20171101090326.8091-1-martin.ag...@gmail.com/ Martin Ågren (2): builtin/merge-base: UNLEAK commit lists reduce_heads: fix memory leaks commit.h| 18 +- builtin/commit.c| 2

[PATCH v2 4/4] bisect: fix memory leak when returning best element

When `find_bisection()` returns a single list entry, it leaks the other entries. Move the to-be-returned item to the front and free the remainder. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- bisect.c | 6 +- 1 fil

[PATCH v2 3/4] bisect: fix off-by-one error in `best_bisection_sorted()`

d. Do not update `p` the very last round in the loop. This ensures that after the loop, `p->next` points to the remainder of the list, and we can set it to NULL. While we're here, free that remainder to fix a memory leak. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by:

[PATCH v2 2/4] bisect: fix memory leak in `find_bisection()`

`find_bisection()` rebuilds the commit list it is given by reversing it and skipping uninteresting commits. The uninteresting list entries are leaked. Free them to fix the leak. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- bisect.c | 4 +++- 1 file changed, 3 insertions

[PATCH v2 1/4] bisect: change calling-convention of `find_bisection()`

and has void return type. That should make it harder to misuse this function. While we're here, document this function. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- bisect.h | 12 +--- bisect.c | 16 +++- builtin/rev-list.c | 3 +-- 3 files c

[PATCH v2 0/4] bisect: assorted leak-fixes

ical to v1. Martin [1] https://public-inbox.org/git/4795556016c25e4a78241362547c5468877f808d.1509557518.git.martin.ag...@gmail.com/ Martin Ågren (4): bisect: change calling-convention of `find_bisection()` bisect: fix memory leak in `find_bisection()` bisect: fix off-by

[PATCH 2/4] pager: refactor `pager_command_config()`

es it easy to look for other remainders in the next patch. While at it, before assigning to `value`, free any old value we might already have picked up. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- pager.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/p

[PATCH 3/4] pager: introduce `pager.*.command` and `.enable`

that might have been configured with `pager.foo`, do the same for `pager.foo.command`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- Documentation/config.txt | 17 +++ Documentation/git-tag.txt | 3 +- Documentation/git.txt | 2 +- t/t7

[PATCH 4/4] pager: make `pager.foo.command` imply `.enable=true`

If `pager.foo.command` gets configured and there is no configuration (yet) saying whether we should page, act as if `pager.foo.enable=true`. This means that a lone `pager.foo` can always be written as a lone `pager.foo.command` or `pager.foo.enable`. Signed-off-by: Martin Ågren <martin

[PATCH 1/4] t7006: document that `pager.foo` can be partially preserved

foo` can be configured. Those commits mustn't change how `pager.foo` behaves, so add tests for these two cases. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- t/t7006-pager.sh | 18 ++ 1 file changed, 18 insertions(+) diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh

[PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable}

If someone wants to pick these up and bring them to completion, great! If not and if I or someone else feels confident about which way to go, then I can revisit these. Martin Martin Ågren (4): t7006: document that `pager.foo` can be partially preserved pager: refactor `pager_command_config()`

Re: [PATCH 1/3] bisect: fix memory leak and document `find_bisection()`

On 2 November 2017 at 05:54, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> `find_bisection()` rebuilds the commit list it is given by reversing it >> and skipping uninteresting commits. The uninteresting list entries a

Re: [PATCH] reduce_heads: fix memory leaks

On 2 November 2017 at 04:11, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> diff --git a/builtin/merge-base.c b/builtin/merge-base.c >> index 6dbd167d3..b1b7590c4 100644 >> --- a/builtin/merge-base.c >> ++

[PATCH v2] grep: take the read-lock when adding a submodule

thread is reading in the list through `read_sha1_file()`. Take the grep read-lock while adding the submodule. The lock is used to serialize uses of non-thread-safe parts of Git's API, including `read_sha1_file()`. Helped-by: Brandon Williams <bmw...@google.com> Signed-off-by: Martin

[PATCH 2/3] bisect: fix off-by-one error in `best_bisection_sorted()`

d. Do not update `p` the very last round in the loop. This ensures that after the loop, `p->next` points to the remainder of the list, and we can set it to NULL. While we're here, free that remainder to fix a memory leak. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- bisect.c

[PATCH 3/3] bisect: fix memory leak when returning best element

When `find_bisection()` returns a single list entry, it leaks the other entries. Move the to-be-returned item to the front and free the remainder. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- bisect.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bise

[PATCH 1/3] bisect: fix memory leak and document `find_bisection()`

that the original list should not be examined by the caller. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- bisect.c | 4 +++- bisect.h | 7 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/bisect.c b/bisect.c index 96beeb5d1..f9de4f2e8 100644 --- a/bisect.c +++ b/bi

[PATCH] reduce_heads: fix memory leaks

leaks using `free_commit_list()`. While we're here, document `reduce_heads()` and mark it as `extern`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- builtin/commit.c| 2 +- builtin/fmt-merge-msg.c | 2 +- builtin/merge-base.c| 6 +- builtin/merge.c

Re: [PATCH 5/7] refs: tidy up and adjust visibility of the `ref_update` flags

On 28 October 2017 at 11:49, Michael Haggerty wrote: > The constants used for `ref_update::flags` were rather disorganized: > * The documentation wasn't very consistent and partly still referred > to sha1s rather than oids. > @@ -478,22 +462,23 @@ struct ref_transaction

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: [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 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] merge-recursive: check GIT_MERGE_VERBOSITY only once

On 24 October 2017 at 18:45, Eric Sunshine wrote: > On Tue, Oct 24, 2017 at 12:28 PM, Stefan Beller wrote: >> On Tue, Oct 24, 2017 at 8:27 AM, Andrey Okoshkin >> wrote: >>> Add check of 'GIT_MERGE_VERBOSITY' environment

Re: [PATCH/RFC] grep: turn off threading when recursing submodules

On 19 October 2017 at 21:34, Jeff King wrote: > On Thu, Oct 19, 2017 at 12:26:18PM -0700, Brandon Williams wrote: > >> One alternative to turning off threading would be to employ proper >> locking (like I failed to do) by wrapping the call the >> 'add_to_alternates_memory()' in

[PATCH/RFC] grep: turn off threading when recursing submodules

threading. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- This is a simple, stupid solution. I'm posting this in patch-form so that I can do one better than just mailing about the race and waving my hands. Maybe threading is common enough that reverting could be a better ap

Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

ression by turning off internal handling of -h for repository setup, > and option parsing, as well as the corresponding test in t0012. > > Reported-by: Thomas Rikl <tr...@online.de> > Analyzed-by: Martin Ågren <martin.ag...@gmail.com> > Signed-off-by: Rene Scharfe <

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

On 17 October 2017 at 17:39, René Scharfe wrote: > Stop advertising -h as the short equivalent of --heads, because it's > used for showing a short help text for almost all other git commands. > Since the ba5f28bf79 (ls-remote: use parse-options api) it has only > been working when

Re: Bug: git ls-remote -h / --head results differ in output

On 15 October 2017 at 12:02, Thomas Rikl wrote: > Example: > > tom1 ~/emacs/spacemacs/.emacs.d $ export LANG=en_US.utf8 > > tom1 ~/emacs/spacemacs/.emacs.d $ git ls-remote -h > usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=] > [-q | --quiet]

Re: [PATCH] column: show auto columns when pager is active

On 11 October 2017 at 20:36, Kevin Daudt <m...@ikke.info> wrote: > On Wed, Oct 11, 2017 at 08:12:35PM +0200, Martin Ågren wrote: >> On 11 October 2017 at 19:23, Kevin Daudt <m...@ikke.info> wrote: >> I wonder if it's useful to set COLUMNS a bit lower so that this has t

Re: [PATCH] column: show auto columns when pager is active

On 11 October 2017 at 19:23, Kevin Daudt wrote: > finalize_colopts in column.c only checks whether the output is a TTY to > determine if columns should be enabled with columns set to auto. Also check > if the pager is active. Maybe you could say something about the difficulties

Re: [PATCH] submodule: avoid sentence-lego in translated string

On 10 October 2017 at 04:35, Changwoo Ryu wrote: > 2017-10-10 10:26 GMT+09:00 Junio C Hamano : >> Jean-Noël AVILA writes: >> >>> On Monday, 9 October 2017, 09:47:26 CEST Stefan Beller wrote: >>> I always assumed that translators are

Re: [RFC] column: show auto columns when pager is active

On 10 October 2017 at 16:29, Jeff King wrote: > On Tue, Oct 10, 2017 at 10:10:19AM -0400, Jeff King wrote: > > it will randomly succeed or fail, depending on whether sed manages to > read the input before the stdin terminal is closed. > > I'm not sure of an easy way to fix

Re: [RFC] column: show auto columns when pager is active

On 10 October 2017 at 16:01, Jeff King <p...@peff.net> wrote: > On Tue, Oct 10, 2017 at 12:30:49PM +0200, Martin Ågren wrote: >> On 9 October 2017 at 23:45, Kevin Daudt <m...@ikke.info> wrote: >> > Since ff1e72483 (tag: change default of `pager.tag` to "on&quo

Re: [RFC] column: show auto columns when pager is active

On 9 October 2017 at 23:45, Kevin Daudt wrote: > When columns are set to automatic for git tag and the output is > paginated by git, the output is a single column instead of multiple > columns. > > Standard behaviour in git is to honor auto values when the pager is > active, which

Re: [PATCH] submodule: avoid sentence-lego in translated string

On 9 October 2017 at 03:30, Junio C Hamano <gits...@pobox.com> wrote: > On Mon, Oct 9, 2017 at 9:51 AM, brian m. carlson > <sand...@crustytoothpaste.net> wrote: >> On Sun, Oct 08, 2017 at 10:32:35AM +0100, Philip Oakley wrote: >>> From: "Martin Ågren

Re: [PATCH v3 03/10] protocol: introduce protocol extention mechanisms

On 6 October 2017 at 11:40, Junio C Hamano wrote: > Simon Ruderich writes: > >> Did you consider Stefan Beller's suggestion regarding a >> (white)list of allowed versions? >> >> On Mon, Sep 18, 2017 at 01:06:59PM -0700, Stefan Beller wrote: >>> Thinking

Re: [PATCH v3 03/10] protocol: introduce protocol extention mechanisms

On 6 October 2017 at 22:27, Stefan Beller wrote: >> I might be naive in thinking that protocol.version could be removed or >> redefined at our discretion just because it's marked as "experimental". > > Well the redefinition might very well occur, when we now say "set to v1 >

Re: "git rm" seems to do recursive removal even without "-r"

On 8 October 2017 at 13:56, Robert P. J. Day wrote: > but as i asked in my earlier post, if i wanted to remove *all* files > with names of "Makefile*", why can't i use: > > $ git rm 'Makefile*' > > just as i used: > > $ git rm '*.c' > > are those not both acceptable

Re: [PATCH] submodule: avoid sentence-lego in translated string

On 8 October 2017 at 11:32, Philip Oakley <philipoak...@iee.org> wrote: > From: "Martin Ågren" <martin.ag...@gmail.com> > >> We currently build an error message like "entry is a %s, not a commit", >> where the placeholder will be replaced with

[PATCH] submodule: avoid sentence-lego in translated string

and pick one of three different error messages. We might still want a `default` and maybe more tests. So go for this simpler approach instead. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- I browsed the diff of the .pot and found an addition that looked a bit translat

[PATCH v3 07/12] apply: move lockfile into `apply_state`

in the next patch.) Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- apply.c | 14 +- apply.h | 5 ++--- builtin/am.c| 3 +-- builtin/apply.c | 4 +--- 4 files changed, 9 insertions(+),

[PATCH v3 10/12] read-cache: drop explicit `CLOSE_LOCK`-flag

ining flag, `COMMIT_LOCK`. This means we neither have nor suggest that we have a mode to write the index and leave the file open. Whatever extra contents we might eventually want to write, we should probably write it from within `write_locked_index()` itself anyway. Signed-off-by: Martin Ågren <martin.ag

[PATCH v3 08/12] apply: remove `newfd` from `struct apply_state`

`is_lock_file_locked()`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- apply.c | 17 ++--- apply.h | 3 +-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/apply.c b/apply.c index 5a6ca10a7..d676

[PATCH v3 12/12] read_cache: roll back lock in `update_index_if_able()`

in cache.h. If `write_locked_index(..., COMMIT_LOCK)` fails, it will roll back the lock for us (see the previous commit). Noticed-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- cache.h

[PATCH v3 09/12] cache.h: document `write_locked_index()`

The next patches will tweak the behavior of this function. Document it in order to establish a basis for those patches. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- cache.h | 16 1 file changed, 16 inserti

[PATCH v3 01/12] sha1_file: do not leak `lock_file`

lso future-proves the code by making it obvious that we intend to take exactly one of these paths. Improved-by: Jeff King <p...@peff.net> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- sha1_file.c | 19 --

[PATCH v3 02/12] treewide: prefer lockfiles on the stack

the `struct lock_file` on the stack instead. These instances were identified by running `git grep "^\s*struct lock_file\s*\*"`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- builtin/am.c | 24 +++---

[PATCH v3 06/12] cache-tree: simplify locking logic

` so that we have one variable less to reason about. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- cache-tree.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cache-tree.c b/cache-tree.c ind

[PATCH v3 05/12] checkout-index: simplify locking logic

-allocate tempfiles on heap, 2017-09-05), we can have lockfiles on the stack. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- builtin/checkout-index.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/buil

[PATCH v3 11/12] read-cache: leave lock in right state in `write_locked_index()`

are inconsistent. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v3: Maybe the commit message wasn't too long, but it was loong. For example, some of it just duplicated stuff from the previous patch. builtin/difftool.c | 1 - cache.h| 4 merge.c| 4 +---

[PATCH v3 04/12] tempfile: fix documentation on `delete_tempfile()`

The function has always been documented as returning 0 or -1. It is in fact `void`. Correct that. As part of the rearrangements we lose the mention that `delete_tempfile()` might set `errno`. Because there is no return value, the user can't really know whether it did anyway. Signed-off-by: Martin

[PATCH v3 00/12] Re: various lockfile-leaks and -fixes

On 6 October 2017 at 21:44, Martin Ågren <martin.ag...@gmail.com> wrote: > Ok, thanks. I've got a rerolled series running through the final checks > right now. I did end up making this log message a bit more succinct. This is v3 of this series. All patches are identical to ma/lo

[PATCH v3 03/12] lockfile: fix documentation on `close_lock_file_gently()`

Commit 83a3069a3 (lockfile: do not rollback lock on failed close, 2017-09-05) forgot to update the documentation by the function definition to reflect that the lock is not rolled back in case closing fails. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano

Re: [PATCH v2 11/12] read-cache: leave lock in right state in `write_locked_index()`

On 6 October 2017 at 14:02, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> On 6 October 2017 at 04:01, Junio C Hamano <gits...@pobox.com> wrote: >>> Martin Ågren <martin.ag...@gmail.com> writes: &

Re: [PATCH v3 03/10] protocol: introduce protocol extention mechanisms

On 6 October 2017 at 14:09, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> Maybe I'm missing something Git-specific, but isn't the only thing that >> needs to be done now, to document/specify that 1) the client s

Re: [PATCH v3 03/10] protocol: introduce protocol extention mechanisms

On 6 October 2017 at 11:40, Junio C Hamano wrote: > Simon Ruderich writes: > >> Did you consider Stefan Beller's suggestion regarding a >> (white)list of allowed versions? >> >> On Mon, Sep 18, 2017 at 01:06:59PM -0700, Stefan Beller wrote: >>> Thinking

Re: [PATCH v2 11/12] read-cache: leave lock in right state in `write_locked_index()`

On 6 October 2017 at 04:01, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> v2: Except for the slightly different documentation in cache.h, this is >> a squash of the last two patches of v1. I hope the commit message is &

Re: [PATCH v2 10/12] read-cache: drop explicit `CLOSE_LOCK`-flag

On 6 October 2017 at 03:39, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> diff --git a/read-cache.c b/read-cache.c >> index 65f4fe837..1c917eba9 100644 >> --- a/read-cache.c >> +++ b/read-cac

[PATCH v2 07/12] apply: move lockfile into `apply_state`

in the next patch.) Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- v2: Identical. apply.c | 14 +- apply.h | 5 ++--- builtin/am.c| 3 +-- builtin/apply.c | 4 +--- 4 files changed, 9 inser

[PATCH v2 08/12] apply: remove `newfd` from `struct apply_state`

`is_lock_file_locked()`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- v2: Identical. apply.c | 17 ++--- apply.h | 3 +-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/apply.c b/apply.c ind

[PATCH v2 09/12] cache.h: document `write_locked_index()`

The next patches will tweak the behavior of this function. Document it in order to establish a basis for those patches. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: Tweaked to go better with the changed approach in the later patches. cache.h | 16 1 file c

[PATCH v2 04/12] tempfile: fix documentation on `delete_tempfile()`

The function has always been documented as returning 0 or -1. It is in fact `void`. Correct that. As part of the rearrangements we lose the mention that `delete_tempfile()` might set `errno`. Because there is no return value, the user can't really know whether it did anyway. Signed-off-by: Martin

[PATCH v2 11/12] read-cache: leave lock in right state in `write_locked_index()`

he: close index.lock in do_write_index, 2017-04-26). Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: Except for the slightly different documentation in cache.h, this is a squash of the last two patches of v1. I hope the commit message is better. builtin/difftool.c | 1 - cache.h

[PATCH v2 12/12] read_cache: roll back lock in `update_index_if_able()`

in cache.h. If `write_locked_index(..., COMMIT_LOCK)` fails, it will roll back the lock for us (see the previous commit). Noticed-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: New patch. cache.h | 4 read-cache.c | 5 ++--- 2 f

[PATCH v2 10/12] read-cache: drop explicit `CLOSE_LOCK`-flag

ining flag, `COMMIT_LOCK`. This means we neither have nor suggest that we have a mode to write the index and leave the file open. Whatever extra contents we might eventually want to write, we should probably write it from within `write_locked_index()` itself anyway. Signed-off-by: Martin Ågren <martin.ag

[PATCH v2 00/12] Re: various lockfile-leaks and -fixes

On 3 October 2017 at 08:21, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: >> On 2 October 2017 at 08:30, Junio C Hamano <gits...@pobox.com> wrote: >> >> Thanks both of you for your comments. Based on t

[PATCH v2 01/12] sha1_file: do not leak `lock_file`

lso future-proves the code by making it obvious that we intend to take exactly one of these paths. Improved-by: Jeff King <p...@peff.net> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: Moved the rollback to the end to have an obvious if-else instead of retaining the origi

[PATCH v2 02/12] treewide: prefer lockfiles on the stack

the `struct lock_file` on the stack instead. These instances were identified by running `git grep "^\s*struct lock_file\s*\*"`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- v2: Identical. b

[PATCH v2 06/12] cache-tree: simplify locking logic

` so that we have one variable less to reason about. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano <gits...@pobox.com> --- v2: Identical. cache-tree.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cache-tree.c b/

[PATCH v2 05/12] checkout-index: simplify locking logic

-allocate tempfiles on heap, 2017-09-05), we can have lockfiles on the stack. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: New patch. builtin/checkout-index.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/builtin/checkout-index.c b/builtin/checkout-i

[PATCH v2 03/12] lockfile: fix documentation on `close_lock_file_gently()`

Commit 83a3069a3 (lockfile: do not rollback lock on failed close, 2017-09-05) forgot to update the documentation by the function definition to reflect that the lock is not rolled back in case closing fails. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> Signed-off-by: Junio C Hamano

Re: What's cooking in git.git (Sep 2017, #06; Fri, 29)

On 29 September 2017 at 06:34, Junio C Hamano wrote: > > * sd/branch-copy (2017-09-24) 4 commits > (merged to 'next' on 2017-09-28 at a6eceefa02) > + branch: fix "copy" to never touch HEAD > + branch: add a --copy (-c) option to go with --move (-m) > + branch: add test for

Re: [PATCH] builtin/: add UNLEAKs

On 2 October 2017 at 08:25, Jeff King <p...@peff.net> wrote: > On Sun, Oct 01, 2017 at 07:42:08PM +0200, Martin Ågren wrote: > >> Add some UNLEAKs where we are about to return from `cmd_*`. UNLEAK the >> variables in the same order as we've declared them. While addressing &

Re: [PATCH 00/11] various lockfile-leaks and -fixes

On 2 October 2017 at 08:30, Junio C Hamano wrote: > Jeff King writes: > >> I've read through each and with the exception of patches 10 and 11, they >> all look good to me. >> >> For 10 and 11, I think you've convinced me that the current behavior is >> buggy. I

Re: [PATCH 09/11] read-cache: require flags for `write_locked_index()`

On 2 October 2017 at 06:14, Martin Ågren <martin.ag...@gmail.com> wrote: > On 2 October 2017 at 05:49, Junio C Hamano <gits...@pobox.com> wrote: >> Martin Ågren <martin.ag...@gmail.com> writes: >> >>> ... Instead, require that one of the >>> f

Re: [PATCH 01/11] sha1_file: do not leak `lock_file`

On 2 October 2017 at 07:26, Jeff King <p...@peff.net> wrote: > On Sun, Oct 01, 2017 at 04:56:02PM +0200, Martin Ågren wrote: > > The original code is actually a bit confusing/unsafe, as we set the > "found" flag early and rollback here: > >> @@ -481,17 +481,15

Re: [PATCH 09/11] read-cache: require flags for `write_locked_index()`

On 2 October 2017 at 05:49, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > >> ... Instead, require that one of the >> flags is set. Adjust documentation and the assert we already have for >> checking that we do

Re: [PATCH 02/11] treewide: prefer lockfiles on the stack

On 2 October 2017 at 05:37, Junio C Hamano <gits...@pobox.com> wrote: > Martin Ågren <martin.ag...@gmail.com> writes: > > This is not a show-stopper for this patch series, but just something > I noticed, something that used to be unavoidable in the old world > ord

[PATCH] builtin/: add UNLEAKs

Add some UNLEAKs where we are about to return from `cmd_*`. UNLEAK the variables in the same order as we've declared them. While addressing `msg` in builtin/tag.c, convert the existing `strbuf_release()` calls as well. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- builtin/chec

Re: [PATCH v2 0/6] reroll ma/plugleaks; more `object_array`-fixes

On 25 September 2017 at 18:08, Jeff King <p...@peff.net> wrote: > On Sun, Sep 24, 2017 at 09:59:28PM +0200, Martin Ågren wrote: > >> > I'm not sure of the best way to count things. >> > > But at least on the topic of "how many unique leaks are there", I w

[PATCH 08/11] cache.h: document `write_locked_index()`

The next patches will tweak the behavior of this function. Document it in order to establish a basis for those patches. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- cache.h | 16 1 file changed, 16 insertions(+) diff --git a/cache.h b/cache.h index ea6

[PATCH 07/11] apply: remove `newfd` from `struct apply_state`

`is_lock_file_locked()`. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- apply.c | 17 ++--- apply.h | 3 +-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/apply.c b/apply.c index 5a6ca10a7..d676debd5 100644 --- a/apply.c +++ b/apply.c @@ -79,7 +79,6

[PATCH 09/11] read-cache: require flags for `write_locked_index()`

t we don't have too many flags. Add a macro `HAS_SINGLE_BIT` (inspired by `HAS_MULTI_BITS`) to simplify this check and similar checks in the future. When (if) we need "write the index and leave the lockfile open", we can revisit this. Signed-off-by: Martin Ågren <martin.ag...@

[PATCH 10/11] read-cache: don't leave dangling pointer in `do_write_index()`

26). Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- cache.h | 2 ++ read-cache.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cache.h b/cache.h index 32aa8afdf..4d09da792 100644 --- a/cache.h +++ b/cache.h @@ -617,6 +617,8 @@ extern int read_index_unmer

<    1   2   3   4   5   6   >