[PATCH 4/3] refs: pass NULL to refs_resolve_refdup() if hash is not needed

2017-10-01 Thread René Scharfe
This gets us rid of a write-only variable. Signed-off-by: Rene Scharfe --- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index fec77744b4..ed00ddab1a 100644 --- a/refs/files-backend.c +++

[PATCH 5/3] refs: pass NULL to resolve_refdup() if hash is not needed

2017-10-01 Thread René Scharfe
This allows us to get rid of several write-only variables. Signed-off-by: Rene Scharfe --- builtin/checkout.c | 3 +-- builtin/receive-pack.c | 3 +-- ref-filter.c | 7 ++- reflog-walk.c | 6 ++ transport.c| 3 +-- wt-status.c

Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers

2017-10-01 Thread Junio C Hamano
Taylor Blau writes: > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh > index 2a9fcf713..2bd0c5da7 100755 > --- a/t/t6300-for-each-ref.sh > +++ b/t/t6300-for-each-ref.sh > @@ -597,6 +597,9 @@ Acked-by: A U Thor > > EOF > > +unfold () {

Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers

2017-10-01 Thread Junio C Hamano
Jeff King writes: > This patch didn't apply for me on top of the others. I get: > > Applying: ref-filter.c: use trailer_opts to format trailers > error: patch failed: ref-filter.c:178 > error: ref-filter.c: patch does not apply > Patch failed at 0004 ref-filter.c: use

Re: "man git-checkout", man page is inconsistent between SYNOPSIS and details

2017-10-01 Thread Robert P. J. Day
On Sun, 1 Oct 2017, Junio C Hamano wrote: > "Robert P. J. Day" writes: > > > it's simply a matter of the forms not matching between the SYNOPSIS > > and the DESCRIPTION sections. am i making sense? > > I think the whole thing is wrong and the root cause is because the >

Re: [PATCH 5/3] refs: pass NULL to resolve_refdup() if hash is not needed

2017-10-01 Thread Junio C Hamano
René Scharfe writes: > This allows us to get rid of several write-only variables. > > Signed-off-by: Rene Scharfe > --- > builtin/checkout.c | 3 +-- > builtin/receive-pack.c | 3 +-- > ref-filter.c | 7 ++- > reflog-walk.c | 6 ++ >

Re: [PATCH v8 00/12] Fast git status via a file system watcher

2017-10-01 Thread Junio C Hamano
Ben Peart writes: > I had accumulated the same set of changes with one addition of removing > a duplicate "the" from a comment in the fsmonitor.h file: > > diff --git a/fsmonitor.h b/fsmonitor.h > index 8eb6163455..0de644e01a 100644 > --- a/fsmonitor.h > +++

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

2017-10-01 Thread Martin Ågren
There is no longer any need to allocate and leak a `struct lock_file`. The previous patch addressed an instance where we needed a minor tweak alongside the trivial changes. Deal with the remaining instances where we allocate and leak a struct within a single function. Change them to have the

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

2017-10-01 Thread Martin Ågren
A recent series allowed `struct lock_file`s to be freed [1], so I wanted to get rid of the "simple" leaks of this kind. I found a couple of lock-related cleanups along the way and it resulted in this series. It feels a bit scary at eleven patches -- especially as this is about locking -- but I

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

2017-10-01 Thread Martin Ågren
There is no longer any need to allocate and leak a `struct lock_file`. Initialize it on the stack instead. Instead of setting `lock = NULL` to signal that we have already rolled back, and that we should not do any more work, check with `is_lock_file_locked()`. Since we take the lock with

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

2017-10-01 Thread Martin Ågren
If `do_write_index(..., struct tempfile *, ...)` fails to close the temporary file, it deletes it. This resets the pointer to NULL, but only the pointer which is local to `do_write_index()`, not the pointer that the caller holds. If the caller ever dereferences its pointer, we have undefined

[PATCH 11/11] read-cache: roll back lock on error with `COMMIT_LOCK`

2017-10-01 Thread Martin Ågren
When `write_locked_index(..., lock, COMMIT_LOCK)` returns, the lockfile might have been neither committed nor rolled back. This happens if the call to `do_write_index()` early in `do_write_locked_index()` fails, or if `write_shared_index()` fails. Some callers obviously do not expect the lock to

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

2017-10-01 Thread Martin Ågren
Similar to a previous patch, we do not need to use `newfd` to signal that we have a lockfile to clean up. We can just unconditionally call `rollback_lock_file`. If we do not hold the lock, it will be a no-op. Where we check `newfd` to decide whether we need to take the lock, we can instead use

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

2017-10-01 Thread Martin Ågren
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 --- cache.h | 16 1 file changed, 16 insertions(+) diff --git a/cache.h b/cache.h index

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

2017-10-01 Thread Martin Ågren
`write_locked_index()` takes two flags: `COMMIT_LOCK` and `CLOSE_LOCK`. At most one is allowed. But it is also possible to use no flag, i.e., `0`. But when `write_locked_index()` calls `do_write_index()`, the temporary file, a.k.a. the lockfile, will be closed. So passing `0` is effectively the

Re: will git rebase has side effect

2017-10-01 Thread Kevin Daudt
Forgot to cc the mailing list. On Sun, Oct 01, 2017 at 09:23:23PM +0800, Yubin Ruan wrote: > Suppose that I have such a history of commit locally: > > A --> B --> C --> D > > If I then add a few more commits locally > > A --> B --> C --> D --> E --> F --> G > > And then I do a rebase and

[PATCH 2/2] commit: use strbuf_add() to add a buffer to a strbuf

2017-10-01 Thread René Scharfe
This is shorter, easier and makes the intent clearer. Patch generated with Coccinelle and contrib/coccinelle/strbuf.cocci. Signed-off-by: Rene Scharfe --- builtin/commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/commit.c b/builtin/commit.c

Re: will git rebase has side effect

2017-10-01 Thread Yubin Ruan
2017-10-01 22:17 GMT+08:00 Kevin Daudt : > Forgot to cc the mailing list. > > On Sun, Oct 01, 2017 at 09:23:23PM +0800, Yubin Ruan wrote: >> Suppose that I have such a history of commit locally: >> >> A --> B --> C --> D >> >> If I then add a few more commits locally >> >> A --> B

Re: "man git-stash" explanation of "--include-untracked" and "--all" seems ambiguous

2017-10-01 Thread Thomas Gummerer
On 09/29, Robert P. J. Day wrote: > > from the man page: > > "If the --include-untracked option is used, all untracked files are > also stashed and then cleaned up with git clean, leaving the working > directory in a very clean state. If the --all option is used instead >

Re: "man git-stash" explanation of "--include-untracked" and "--all" seems ambiguous

2017-10-01 Thread Robert P. J. Day
On Sun, 1 Oct 2017, Thomas Gummerer wrote: > On 09/29, Robert P. J. Day wrote: > > > > from the man page: > > > > "If the --include-untracked option is used, all untracked files > > are also stashed and then cleaned up with git clean, leaving the > > working directory in a very clean state. If

[PATCH] path: use strbuf_add_real_path()

2017-10-01 Thread René Scharfe
Avoid a string copy to a static buffer by using strbuf_add_real_path() instead of combining strbuf_addstr() and real_path(). Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci. Signed-off-by: Rene Scharfe --- path.c | 2 +- 1 file changed, 1 insertion(+), 1

[PATCH] use strbuf_addstr() for adding strings to strbufs

2017-10-01 Thread René Scharfe
Use strbuf_addstr() instead of strbuf_addf() for adding strings. That's simpler and makes the intent clearer. Patch generated by Coccinelle and contrib/coccinelle/strbuf.cocci; adjusted indentation in refs/packed-backend.c manually. Signed-off-by: Rene Scharfe ---

[PATCH] tag: avoid NULL pointer arithmetic

2017-10-01 Thread René Scharfe
lookup_blob() etc. can return NULL if the referenced object isn't of the expected type. In theory it's wrong to reference the object member in that case. In practice it's OK because it's located at offset 0 for all types, so the pointer arithmetic (NULL + 0) is optimized out by the compiler.

[PATCH] repository: use FREE_AND_NULL

2017-10-01 Thread René Scharfe
Use the macro FREE_AND_NULL to release allocated objects and clear their pointers. This is shorter and documents the intent better by combining the two related operations into one. Patch generated with Coccinelle and contrib/coccinelle/free.cocci. Signed-off-by: Rene Scharfe ---

[PATCH v4 1/6] pretty.c: delimit "%(trailers)" arguments with ","

2017-10-01 Thread Taylor Blau
In preparation for adding consistent "%(trailers)" atom options to `git-for-each-ref(1)`'s "--format" argument, change "%(trailers)" in pretty.c to separate sub-arguments with a ",", instead of a ":". Multiple sub-arguments are given either as "%(trailers:unfold,only)" or

Re: what is git's position on "classic" mac -only end of lines?

2017-10-01 Thread Bryan Turner
On Sun, Oct 1, 2017 at 10:52 AM, Robert P. J. Day wrote: > > sorry for more pedantic nitpickery, but i'm trying to write a > section on how to properly process mixtures of EOLs in git, and when i > read "man git-config", everything seems to refer to Mac OS X and macOS >

[PATCH] graph: use strbuf_addchars() to add spaces

2017-10-01 Thread René Scharfe
strbuf_addf() can be used to add a specific number of space characters by using the format "%*s" with an empty string and specifying the desired width. Use strbuf_addchars() instead as it's shorter, makes the intent clearer and is a bit more efficient. Signed-off-by: Rene Scharfe

[PATCH 1/2] coccinelle: remove parentheses that become unnecessary

2017-10-01 Thread René Scharfe
Transformations that hide multiplications can end up with an pair of parentheses that is no longer needed. E.g. with a rule like this: @@ expression E; @@ - E * 2 + double(E) ... we might get a patch like this: - x = (a + b) * 2; + x = double((a + b)); Add a pair of

[PATCH v4 5/6] ref-filter.c: use trailer_opts to format trailers

2017-10-01 Thread Taylor Blau
Fill trailer_opts with "unfold" and "only" to match the sub-arguments given to the "%(trailers)" atom. Then, let's use the filled trailer_opts instance with 'format_trailers_from_commit' in order to format trailers in the desired manner. Signed-off-by: Taylor Blau ---

[PATCH v4 4/6] doc: use modern "`"-style code fencing

2017-10-01 Thread Taylor Blau
"'"- (single-quote) styled code fencing is no longer considered modern within the "Documentation/" subtree. In preparation for adding additional information to this section of git-for-each-ref(1)'s documentation, update old-style code fencing to use "`"-style fencing instead. Signed-off-by:

[PATCH v4 2/6] t6300: refactor %(trailers) tests

2017-10-01 Thread Taylor Blau
We currently have one test for %(trailers) in `git-for-each-ref(1)`, through "%(contents:trailers)". In preparation for more, let's add a few things: - Move the commit creation step to its own test so that it can be re-used. - Add a non-trailer to the commit's trailers to test that

Re: [PATCH v4 0/6] Support %(trailers) arguments in for-each-ref(1)

2017-10-01 Thread Taylor Blau
Hi, Attached is the fourth revision of my patch-set "Support %(trailers) arguments in for-each-ref(1)". It includes the following changes since v3: * Teach unfold() to unfold multiple lines. * Rebase patches to apply on top of master. Thanks in advance, and thanks for all of your help

[PATCH v4 3/6] doc: 'trailers' is the preferred way to format trailers

2017-10-01 Thread Taylor Blau
The documentation makes reference to 'contents:trailers' as an example to dig the trailers out of a commit. 'trailers' is an unmentioned alternative, which is treated as an alias of 'contents:trailers'. Since 'trailers' is easier to type, prefer that as the designated way to dig out trailers

[PATCH v4 6/6] ref-filter.c: parse trailers arguments with %(contents) atom

2017-10-01 Thread Taylor Blau
The %(contents) atom takes a contents "field" as its argument. Since "trailers" is one of those fields, extend contents_atom_parser to parse "trailers"'s arguments when used through "%(contents)", like: %(contents:trailers:unfold,only) A caveat: trailers_atom_parser expects NULL when no

what is git's position on "classic" mac -only end of lines?

2017-10-01 Thread Robert P. J. Day
sorry for more pedantic nitpickery, but i'm trying to write a section on how to properly process mixtures of EOLs in git, and when i read "man git-config", everything seems to refer to Mac OS X and macOS (and linux, of course) using for EOL, with very little mention of what one does if faced

Re: [PATCH v2] Add a comment to .clang-format about the meaning of the file

2017-10-01 Thread Junio C Hamano
Stephan Beyer writes: > Having a .clang-format file in a project can be understood in a way that code > has to be in the style defined by the .clang-format file, i.e., you just have > to run clang-format over all code and you are set. This is not the case in the > Git project,

Re: what is git's position on "classic" mac -only end of lines?

2017-10-01 Thread Johannes Sixt
Am 01.10.2017 um 21:29 schrieb Bryan Turner: On Sun, Oct 1, 2017 at 10:52 AM, Robert P. J. Day wrote: sorry for more pedantic nitpickery, but i'm trying to write a section on how to properly process mixtures of EOLs in git, and when i read "man git-config",

[PATCH 2/2] run-command: use ALLOC_ARRAY

2017-10-01 Thread René Scharfe
Use the macro ALLOC_ARRAY to allocate an array. This is shorter and eaasier, as it automatically infers the size of elements. Patch generated with Coccinelle and contrib/coccinelle/array.cocci. Signeg-off-by: Rene Scharfe --- run-command.c | 2 +- 1 file changed, 1 insertion(+),

Re: will git rebase has side effect

2017-10-01 Thread Kevin Daudt
On Mon, Oct 02, 2017 at 12:06:38AM +0800, Yubin Ruan wrote: > 2017-10-01 22:17 GMT+08:00 Kevin Daudt : > > Forgot to cc the mailing list. > > > > On Sun, Oct 01, 2017 at 09:23:23PM +0800, Yubin Ruan wrote: > >> Suppose that I have such a history of commit locally: > >> > >> A --> B

[PATCH] builtin/: add UNLEAKs

2017-10-01 Thread Martin Ågren
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 --- builtin/checkout.c

will git rebase has side effect

2017-10-01 Thread Yubin Ruan
Suppose that I have such a history of commit locally: A --> B --> C --> D If I then add a few more commits locally A --> B --> C --> D --> E --> F --> G And then I do a rebase and squash F and G into one single commit H. What side effect will this rebase have? How will this affect "git push

[PATCH 1/2] coccinelle: add rules to use strbuf_add() for adding buffers to strbufs

2017-10-01 Thread René Scharfe
strbuf_addf() can be used with "%.*s" to add a buffer of a specific size to a strbuf, but the resulting code is longer, may require casting the length to int and is less efficient than simply calling strbuf_add(). Add Coccinelle rules for transforming calls of the former type to the latter.

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

2017-10-01 Thread Martin Ågren
On 25 September 2017 at 18:08, Jeff King 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 wrote > the script below to try to give some

[PATCH v2] Add a comment to .clang-format about the meaning of the file

2017-10-01 Thread Stephan Beyer
Having a .clang-format file in a project can be understood in a way that code has to be in the style defined by the .clang-format file, i.e., you just have to run clang-format over all code and you are set. This is not the case in the Git project, which is now reflected by a comment in the

Donation for you!!

2017-10-01 Thread Carlos Slim Helu
GREETINGS, My name is Carlos Slim Helu, A philanthropist the CEO and Chairman of the Carlos Slim Helu Foundation Charitable Foundation, one of the largest private foundations in the world. I believe strongly in‘giving while living’ I had one idea that never changed in my mind — that you

[PATCH 06/11] apply: move lockfile into `apply_state`

2017-10-01 Thread Martin Ågren
We have two users of `struct apply_state` and the related functionality in apply.c. Each user sets up its `apply_state` by handing over a pointer to its static `lock_file`. (Before 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we could never free lockfiles, so making them

[PATCH 05/11] cache-tree: simplify locking logic

2017-10-01 Thread Martin Ågren
After we have taken the lock using `LOCK_DIE_ON_ERROR`, we know that `newfd` is non-negative. So when we check for exactly that property before calling `write_locked_index()`, the outcome is guaranteed. If we write and commit successfully, we set `newfd = -1`, so that we can later avoid calling

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

2017-10-01 Thread Martin Ågren
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 --- lockfile.h | 4 ++-- 1 file

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

2017-10-01 Thread Martin Ågren
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

Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom

2017-10-01 Thread Junio C Hamano
Taylor Blau writes: > A caveat: trailers_atom_parser expects NULL when no arguments are given > (see: `parse_ref_filter_atom`). This is because string_list_split (given > a maxsplit of -1) returns a 1-ary string_list* containing the given > string if the delimiter could not be

Re: [PATCH v4 1/6] pretty.c: delimit "%(trailers)" arguments with ","

2017-10-01 Thread Jeff King
On Mon, Oct 02, 2017 at 09:11:50AM +0900, Junio C Hamano wrote: > > diff --git a/pretty.c b/pretty.c > > index 94eab5c89..eec128bc1 100644 > > --- a/pretty.c > > +++ b/pretty.c > > @@ -1056,6 +1056,25 @@ static size_t parse_padding_placeholder(struct > > strbuf *sb, > > return 0; > > } > >

Re: [PATCH v6 0/7] Support %(trailers) arguments in for-each-ref(1)

2017-10-01 Thread Taylor Blau
Hi, Attached is the sixth revision of my patch-set "Support %(trailers) arguments in for-each-ref(1)". In includes the following changes since v5: * Added an additional patch to change t4205 to harden `unfold()` against multi-line trailer folding. * Added a missing parameter call in

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

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 04:56:03PM +0200, Martin Ågren wrote: > There is no longer any need to allocate and leak a `struct lock_file`. > The previous patch addressed an instance where we needed a minor tweak > alongside the trivial changes. > > Deal with the remaining instances where we allocate

Re: [PATCH v4 4/6] doc: use modern "`"-style code fencing

2017-10-01 Thread Taylor Blau
On Mon, Oct 02, 2017 at 08:55:53AM +0900, Junio C Hamano wrote: > Taylor Blau writes: > > > "'"- (single-quote) styled code fencing is no longer considered modern > > within the "Documentation/" subtree. > > > > In preparation for adding additional information to this section

Re: [PATCH v4 1/6] pretty.c: delimit "%(trailers)" arguments with ","

2017-10-01 Thread Junio C Hamano
Taylor Blau writes: > In preparation for adding consistent "%(trailers)" atom options to > `git-for-each-ref(1)`'s "--format" argument, change "%(trailers)" in > pretty.c to separate sub-arguments with a ",", instead of a ":". > > Multiple sub-arguments are given either as

Re: [PATCH v4 2/6] t6300: refactor %(trailers) tests

2017-10-01 Thread Junio C Hamano
Looks good to me, thanks.

Re: [PATCH v4 5/6] ref-filter.c: use trailer_opts to format trailers

2017-10-01 Thread Junio C Hamano
Taylor Blau writes: > +test_expect_success '%(trailers) rejects unknown trailers arguments' ' > + cat >expect <<-EOF && > + fatal: unknown %(trailers) argument: unsupported > + EOF > + test_must_fail git for-each-ref --format="%(trailers:unsupported)" > 2>actual

Re: [PATCH v6 3/3] submodule: port submodule subcommand 'status' from shell to C

2017-10-01 Thread Junio C Hamano
Prathamesh Chavan writes: > > #define CB_OPT_QUIET (1<<0) > +#define CB_OPT_CACHED(1<<1) > +#define CB_OPT_RECURSIVE (1<<2) Same comments on both naming and formatting. > @@ -245,6 +250,53 @@ static char *get_submodule_displaypath(const char

Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 06:00:25PM +0900, Junio C Hamano wrote: > Taylor Blau writes: > > > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh > > index 2a9fcf713..2bd0c5da7 100755 > > --- a/t/t6300-for-each-ref.sh > > +++ b/t/t6300-for-each-ref.sh > > @@ -597,6

Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 05:33:04PM -0700, Taylor Blau wrote: > The %(contents) atom takes a contents "field" as its argument. Since > "trailers" is one of those fields, extend contents_atom_parser to parse > "trailers"'s arguments when used through "%(contents)", like: > >

Re: [PATCH 05/11] cache-tree: simplify locking logic

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 04:56:06PM +0200, Martin Ågren wrote: > After we have taken the lock using `LOCK_DIE_ON_ERROR`, we know that > `newfd` is non-negative. So when we check for exactly that property > before calling `write_locked_index()`, the outcome is guaranteed. > > If we write and

Re: what is git's position on "classic" mac -only end of lines?

2017-10-01 Thread Robert P. J. Day
On Sun, 1 Oct 2017, Johannes Sixt wrote: > Am 01.10.2017 um 21:29 schrieb Bryan Turner: > > On Sun, Oct 1, 2017 at 10:52 AM, Robert P. J. Day > > wrote: > > > > > >sorry for more pedantic nitpickery, but i'm trying to write a > > > section on how to properly process

Re: "man git-stash" explanation of "--include-untracked" and "--all" seems ambiguous

2017-10-01 Thread Junio C Hamano
Thomas Gummerer writes: > This is fine when --include-untracked is specified first, as --all > implies --include-untracked, but I guess the behaviour could be a bit > surprising if --all is specified first and --include-untracked later > on the command line. > > Changing

Re: [PATCH v4 4/6] doc: use modern "`"-style code fencing

2017-10-01 Thread Junio C Hamano
Taylor Blau writes: > "'"- (single-quote) styled code fencing is no longer considered modern > within the "Documentation/" subtree. > > In preparation for adding additional information to this section of > git-for-each-ref(1)'s documentation, update old-style code fencing to >

Re: [PATCH v5 0/6] Support %(trailers) arguments in for-each-ref(1)

2017-10-01 Thread Taylor Blau
Hi, Attached is the fifth revision of my patch-set "Support %(trailers) arguments in for-each-ref(1)". It includes the following changes since v4: * Clarified "ref-filter.c: parse trailers arguments with %(contents) atom" to include reasoning for passing NULL as "" empty string in

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

2017-10-01 Thread Martin Ågren
On 2 October 2017 at 05:49, Junio C Hamano wrote: > Martin Ågren writes: > >> ... Instead, require that one of the >> flags is set. Adjust documentation and the assert we already have for >> checking that we don't have too many flags. Add a macro

Re: [PATCH] graph: use strbuf_addchars() to add spaces

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 04:45:45PM +0200, René Scharfe wrote: > strbuf_addf() can be used to add a specific number of space characters > by using the format "%*s" with an empty string and specifying the > desired width. Use strbuf_addchars() instead as it's shorter, makes the > intent clearer

Re: [PATCH 4/5] ref-filter.c: use trailer_opts to format trailers

2017-10-01 Thread Taylor Blau
On Mon, Oct 02, 2017 at 01:05:07AM -0400, Jeff King wrote: > On Sun, Oct 01, 2017 at 06:00:25PM +0900, Junio C Hamano wrote: > > > Taylor Blau writes: > > > > > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh > > > index 2a9fcf713..2bd0c5da7 100755 > > > ---

Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom

2017-10-01 Thread Taylor Blau
On Mon, Oct 02, 2017 at 01:03:51AM -0400, Jeff King wrote: > On Sun, Oct 01, 2017 at 05:33:04PM -0700, Taylor Blau wrote: > > > The %(contents) atom takes a contents "field" as its argument. Since > > "trailers" is one of those fields, extend contents_atom_parser to parse > > "trailers"'s

Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 10:12:16PM -0700, Taylor Blau wrote: > > Doh, that string_list behavior is what I was missing in my earlier > > comments. I agree this is probably the best way of doing it. I'm tempted > > to say that parse_ref_filter_atom() should do a similar thing. Right now > > we've

Re: [PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom

2017-10-01 Thread Taylor Blau
On Mon, Oct 02, 2017 at 01:51:11PM +0900, Junio C Hamano wrote: > Taylor Blau writes: > > > @@ -212,9 +212,10 @@ static void contents_atom_parser(const struct > > ref_format *format, struct used_at > > atom->u.contents.option = C_SIG; > > else if (!strcmp(arg,

Re: [PATCH 03/11] lockfile: fix documentation on `close_lock_file_gently()`

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 04:56:04PM +0200, Martin Ågren wrote: > 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. Oops, thanks for

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

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 04:56:08PM +0200, Martin Ågren wrote: > Similar to a previous patch, we do not need to use `newfd` to signal > that we have a lockfile to clean up. We can just unconditionally call > `rollback_lock_file`. If we do not hold the lock, it will be a no-op. > > Where we check

[PATCH 0/1] ref-filter.c: pass empty-string as NULL to atom parsers

2017-10-01 Thread Taylor Blau
Hi, Attached is a one-long patch series to un-distinguish between atoms without sub-arguments ("%(refname)") and atoms with empty sub-argument lists ("%(refname:)"). This addresses a user-experience issue that Peff points out: > Doh, that string_list behavior is what I was missing in my earlier

Re: "man git-checkout", man page is inconsistent between SYNOPSIS and details

2017-10-01 Thread Junio C Hamano
"Robert P. J. Day" writes: > ok, i'm going to have to digest all that; pretty sure someone else > will need to change the man page to clarify the apparent inconsistency > i was referring to: > > SYNOPSIS >git checkout [-p|--patch] [] [--] [...] > DESCRIPTION >

[PATCH v5 2/6] t6300: refactor %(trailers) tests

2017-10-01 Thread Taylor Blau
We currently have one test for %(trailers) in `git-for-each-ref(1)`, through "%(contents:trailers)". In preparation for more, let's add a few things: - Move the commit creation step to its own test so that it can be re-used. - Add a non-trailer to the commit's trailers to test that

[PATCH v5 1/6] pretty.c: delimit "%(trailers)" arguments with ","

2017-10-01 Thread Taylor Blau
In preparation for adding consistent "%(trailers)" atom options to `git-for-each-ref(1)`'s "--format" argument, change "%(trailers)" in pretty.c to separate sub-arguments with a ",", instead of a ":". Multiple sub-arguments are given either as "%(trailers:unfold,only)" or

[PATCH v5 3/6] doc: 'trailers' is the preferred way to format trailers

2017-10-01 Thread Taylor Blau
The documentation makes reference to 'contents:trailers' as an example to dig the trailers out of a commit. 'trailers' is an unmentioned alternative, which is treated as an alias of 'contents:trailers'. Since 'trailers' is easier to type, prefer that as the designated way to dig out trailers

[PATCH v5 5/6] ref-filter.c: use trailer_opts to format trailers

2017-10-01 Thread Taylor Blau
Fill trailer_opts with "unfold" and "only" to match the sub-arguments given to the "%(trailers)" atom. Then, let's use the filled trailer_opts instance with 'format_trailers_from_commit' in order to format trailers in the desired manner. Signed-off-by: Taylor Blau ---

[PATCH v5 6/6] ref-filter.c: parse trailers arguments with %(contents) atom

2017-10-01 Thread Taylor Blau
The %(contents) atom takes a contents "field" as its argument. Since "trailers" is one of those fields, extend contents_atom_parser to parse "trailers"'s arguments when used through "%(contents)", like: %(contents:trailers:unfold,only) A caveat: trailers_atom_parser expects NULL when no

[PATCH v5 4/6] doc: use modern "`"-style code quoting

2017-10-01 Thread Taylor Blau
"'"- (single-quote) styled code quoting is no longer considered modern within the "Documentation/" subtree. In preparation for adding additional information to this section of git-for-each-ref(1)'s documentation, update old-style code quoting to use "`"-style quoting instead. Signed-off-by:

Re: [PATCH v6 2/3] submodule--helper: introduce for_each_listed_submodule()

2017-10-01 Thread Junio C Hamano
Prathamesh Chavan writes: > Introduce function for_each_listed_submodule() and replace a loop > in module_init() with a call to it. > > The new function will also be used in other parts of the > system in later patches. > > Mentored-by: Christian Couder

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

2017-10-01 Thread Martin Ågren
On 2 October 2017 at 05:37, Junio C Hamano wrote: > Martin Ågren 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 > order that requires us to leak

Re: [PATCH] tag: avoid NULL pointer arithmetic

2017-10-01 Thread Junio C Hamano
René Scharfe writes: > lookup_blob() etc. can return NULL if the referenced object isn't of the > expected type. In theory it's wrong to reference the object member in > that case. In practice it's OK because it's located at offset 0 for all > types, so the pointer arithmetic

Re: [PATCH] tag: avoid NULL pointer arithmetic

2017-10-01 Thread Jeff King
On Sun, Oct 01, 2017 at 04:45:13PM +0200, René Scharfe wrote: > lookup_blob() etc. can return NULL if the referenced object isn't of the > expected type. In theory it's wrong to reference the object member in > that case. In practice it's OK because it's located at offset 0 for all > types, so

[PATCH 18/24] Convert remaining callers of resolve_gitlink_ref to object_id

2017-10-01 Thread brian m. carlson
Signed-off-by: brian m. carlson --- diff-lib.c | 4 ++-- dir.c | 8 read-cache.c | 6 +++--- unpack-trees.c | 8 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 4e0980caa8..af4f1b7865

[PATCH 04/24] refs: convert update_ref and refs_update_ref to use struct object_id

2017-10-01 Thread brian m. carlson
Convert update_ref, refs_update_ref, and write_pseudoref to use struct object_id. Update the existing callers as well. Remove update_ref_oid, as it is no longer needed. Signed-off-by: brian m. carlson --- bisect.c | 6 -- builtin/am.c

[PATCH 13/24] builtin/pack-objects: convert to struct object_id

2017-10-01 Thread brian m. carlson
This is one of the last unconverted callers to peel_ref. While we're fixing that, convert the rest of the file, since it will need to be converted at some point anyway. Signed-off-by: brian m. carlson --- builtin/pack-objects.c | 131

[PATCH 15/24] refs: convert read_ref_at to struct object_id

2017-10-01 Thread brian m. carlson
Convert the callers and internals, including struct read_ref_at_cb, of read_ref_at to use struct object_id. Signed-off-by: brian m. carlson --- builtin/show-branch.c | 5 ++--- refs.c| 34 +- refs.h|

[PATCH 07/24] refs: convert resolve_refdup and refs_resolve_refdup to struct object_id

2017-10-01 Thread brian m. carlson
All of the callers already pass the hash member of struct object_id, so update them to pass a pointer to the struct directly, This transformation was done with an update to declaration and definition and the following semantic patch: @@ expression E1, E2, E3, E4; @@ - resolve_refdup(E1, E2,

[PATCH 24/24] refs/files-backend: convert static functions to object_id

2017-10-01 Thread brian m. carlson
Convert several static functions to take pointers to struct object_id. Change the relevant parameters to write_packed_entry to be const, as we don't modify them. Rename lock_ref_sha1_basic to lock_ref_oid_basic to reflect its new argument. Signed-off-by: brian m. carlson

[PATCH 23/24] refs: convert read_raw_ref backends to struct object_id

2017-10-01 Thread brian m. carlson
Convert the unsigned char * parameter to struct object_id * for files_read_raw_ref and packed_read_raw-ref. Update the documentation. Switch from using get_sha1_hex and a hard-coded 40 to using parse_oid_hex. Signed-off-by: brian m. carlson --- refs.c

[PATCH 08/24] refs: convert read_ref and read_ref_full to object_id

2017-10-01 Thread brian m. carlson
All but two of the call sites already had parameters using the hash parameter of struct object_id, so convert them to take a pointer to the struct directly. Also convert refs_read_refs_full, the underlying implementation. Signed-off-by: brian m. carlson ---

[PATCH 14/24] refs: convert peel_ref to struct object_id

2017-10-01 Thread brian m. carlson
Convert peel_ref (and its corresponding backend) to struct object_id. This transformation was done with an update to the declaration, definition, and test helper and the following semantic patch: @@ expression E1, E2; @@ - peel_ref(E1, E2.hash) + peel_ref(E1, ) @@ expression E1, E2; @@ -

[PATCH 10/24] builtin/reflog: convert remaining unsigned char uses to object_id

2017-10-01 Thread brian m. carlson
Convert the remaining uses of unsigned char [20] to struct object_id. This conversion is needed for dwim_log. Signed-off-by: brian m. carlson --- builtin/reflog.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git

[PATCH 09/24] refs: convert dwim_ref and expand_ref to struct object_id

2017-10-01 Thread brian m. carlson
All of the callers of these functions just pass the hash member of a struct object_id, so convert them to use a pointer to struct object_id directly. Signed-off-by: brian m. carlson --- archive.c | 2 +- branch.c | 2 +-

[PATCH 12/24] pack-bitmap: convert traverse_bitmap_commit_list to object_id

2017-10-01 Thread brian m. carlson
Convert traverse_bitmap_commit_list and the callbacks it takes to use a pointer to struct object_id. Signed-off-by: brian m. carlson --- builtin/pack-objects.c | 8 builtin/rev-list.c | 4 ++-- pack-bitmap.c | 8 pack-bitmap.h

[PATCH 11/24] refs: convert dwim_log to struct object_id

2017-10-01 Thread brian m. carlson
Signed-off-by: brian m. carlson --- builtin/reflog.c | 4 ++-- reflog-walk.c| 2 +- refs.c | 8 refs.h | 2 +- sha1_name.c | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/builtin/reflog.c

[PATCH 17/24] sha1_file: convert index_path and index_fd to struct object_id

2017-10-01 Thread brian m. carlson
Convert these two functions and the functions that underlie them to take pointers to struct object_id. This is a prerequisite to convert resolve_gitlink_ref. Fix a stray tab in the middle of the index_mem call in index_pipe by converting it to a space. Signed-off-by: brian m. carlson

[PATCH 03/24] refs: convert delete_ref and refs_delete_ref to struct object_id

2017-10-01 Thread brian m. carlson
Convert delete_ref and refs_delete_ref to take a pointer to struct object_id. Update the documentation accordingly, including referring to null_oid in lowercase, as it is not a #define constant. Signed-off-by: brian m. carlson --- builtin/branch.c | 2 +-

  1   2   >