Re: [PATCH v4 5/8] interpret-trailers: add --parse convenience option

On 16 August 2017 at 10:20, Jeff King <p...@peff.net> wrote: > On Tue, Aug 15, 2017 at 01:26:53PM +0200, Martin Ågren wrote: > >> > This command reads some patches or commit messages from either the >> > - arguments or the standard input if no is specified. T

[PATCH v2 0/4] Some ThreadSanitizer-results

feel like I have any idea which of them is better. 2) hashmap_add, which I could try my hands on if Jeff doesn't beat me to it -- his proposed change should fix it and I doubt I could come up with anything "better", considering he knows the code. Martin Martin Ågren (4): conver

[PATCH v2 3/4] strbuf_setlen: don't write to strbuf_slopbuf

1510528 bytes large (another 0.1% increase). Suggested-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: no "ifdef TSAN"; moved check from strbuf_reset into strbuf_setlen strbuf.h | 5 - 1 file changed, 4 insertions(+), 1 de

[PATCH v2 2/4] pack-objects: take lock before accessing `remaining`

y again. We could tweak the contract so that the lock should be taken before calling find_deltas, but let's defer that until someone can actually show that "unlock+lock" has a measurable negative impact. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: unchanged from v1

[PATCH v2 1/4] convert: always initialize attr_action in convert_attrs

no problem right now. But convert_attrs is obviously trying not to rely on such an implementation-detail of another component. Make the initialization of attr_action after the if-else. Remove the earlier assignments. Suggested-by: Torsten Bögershausen <tbo...@web.de> Signed-off-by: Martin Ågre

[PATCH v2 4/4] ThreadSanitizer: add suppressions

ile is used by setting the environment variable TSAN_OPTIONS to, e.g., "suppressions=$(pwd)/.tsan-suppressions". Observe that relative paths such as ".tsan-suppressions" might not work. Signed-off-by: Martin Ågren <martin.ag...@gmail.com> --- v2: added NEEDSWORK; reworded

Re: [PATCH v2 2/2] refs/files-backend: fix memory leak in lock_ref_for_update

On 29 August 2017 at 10:39, Michael Haggerty <mhag...@alum.mit.edu> wrote: > On 08/28/2017 10:32 PM, Martin Ågren wrote: >> After the previous patch, none of the functions we call hold on to >> `referent.buf`, so we can safely release the string buffer before >> retu

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

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

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

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

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

[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

[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

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

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

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

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

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

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

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] 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: git update-ref fails to create reference. (bug)

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 v3 0/7] Some doc-fixes

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: [PATCH v4 7/7] contrib/git-jump/git-jump: jump to match column in addition to line

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

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

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

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 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

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 4/5] lock_file: make function-local locks non-static

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/2] Documentation: render revisions correctly under Asciidoctor

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: [PATCH 01/28] t/test-lib: add an SHA1 prerequisite

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 10/28] t: skip pack tests if not using SHA-1

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] refs: handle null-oid for pseudorefs

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 06/28] t0000: annotate with SHA1 prerequisite

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 4/5] lock_file: make function-local locks non-static

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 3/5] refs.c: drop dead code checking lock status in `delete_pseudoref()`

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 4/5] lock_file: make function-local locks non-static

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 v5 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'

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 v2 2/6] doc: align 'diff --no-index' in text with synopsis

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

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

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 1/6] doc: improve formatting in githooks.txt

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

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 2/2] unpack_trees_options: free messages when done

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: Regression in patch add?

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 v4 6/6] get_short_oid: document & warn if we ignore the type selector

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?

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 v2 0/5] getting rid of most "static struct lock_file"s

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

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

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: [PATCH 02/12] commit-graph: verify file header information

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 04/12] commit-graph: verify fanout and lookup table

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 00/12] Integrate commit-graph into fsck, gc, and fetch

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

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

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

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

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

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 1/3] refs.c: refer to "object ID", not "sha1", in error messages

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 1/5] t/helper/test-write-cache: clean up lock-handling

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

-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

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

<    1   2   3   4   5   6   >