[PATCH v2 0/2] branch --list: print useful info whilst interactive rebasing a detached HEAD

[Kaartic Sivaraam]

Ok, I seem to have forgotten the cover letter. So, here's one.

The changes from v1 are as follows:

* Changes to the commit message of 1/2 to fix some errors

* Code changes to 1/2 to address the comments from v1

* Patch 2/2 is new. It's adds tests for the issue that 1/2 tries to fix.
  It's written by Eric with a little fix by me to make it work with
  GETTEXT_POISON.

An interdiff for 1/2:

diff --git a/ref-filter.c b/ref-filter.c
index a4c917c96..db2baedfe 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1309,18 +1309,14 @@ char *get_head_description(void)
memset(, 0, sizeof(state));
wt_status_get_state(, 1);
if (state.rebase_in_progress ||
-   state.rebase_interactive_in_progress)
-   {
-   const char *rebasing = NULL;
-   if (state.branch != NULL)
-   rebasing = state.branch;
+   state.rebase_interactive_in_progress) {
+   if (state.branch)
+   strbuf_addf(, _("(no branch, rebasing %s)"),
+   state.branch);
else
-   rebasing = state.detached_from;
-
-   strbuf_addf(, _("(no branch, rebasing %s)"),
-   rebasing);
-   }
-   else if (state.bisect_in_progress)
+   strbuf_addf(, _("(no branch, rebasing
detached HEAD %s)"),
+   state.detached_from);
+   } else if (state.bisect_in_progress)
strbuf_addf(, _("(no branch, bisect started on %s)"),
state.branch);
else if (state.detached_from) {



Eric Sunshine (1):
  t3200: verify "branch --list" sanity when rebasing from detached HEAD

Kaartic Sivaraam (1):
  branch --list: print useful info whilst interactive rebasing a
detached HEAD

 ref-filter.c  | 12 
 t/t3200-branch.sh | 24 
 2 files changed, 32 insertions(+), 4 deletions(-)



signature.asc
Description: OpenPGP digital signature

[PATCH v2 1/2] branch --list: print useful info whilst interactive rebasing a detached HEAD

[Kaartic Sivaraam]

When rebasing interactively (rebase -i), "git branch --list" prints
a line indicating the current branch being rebased. This works well
when the interactive rebase is initiated when a local branch is
checked out.

This doesn't play well when the rebase is initiated on a detached
HEAD. When "git branch --list" tries to print information related
to the interactive rebase in this case it tries to print the name
of a branch using an uninitialized variable and thus tries to
print a "null pointer string". As a consequence, it does not provide
useful information while also inducing undefined behaviour.

So, print the point from which the rebase was started when interactive
rebasing a detached HEAD.

Signed-off-by: Kaartic Sivaraam 
---
 ref-filter.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index f9e25aea7..db2baedfe 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1309,10 +1309,14 @@ char *get_head_description(void)
memset(, 0, sizeof(state));
wt_status_get_state(, 1);
if (state.rebase_in_progress ||
-   state.rebase_interactive_in_progress)
-   strbuf_addf(, _("(no branch, rebasing %s)"),
-   state.branch);
-   else if (state.bisect_in_progress)
+   state.rebase_interactive_in_progress) {
+   if (state.branch)
+   strbuf_addf(, _("(no branch, rebasing %s)"),
+   state.branch);
+   else
+   strbuf_addf(, _("(no branch, rebasing detached 
HEAD %s)"),
+   state.detached_from);
+   } else if (state.bisect_in_progress)
strbuf_addf(, _("(no branch, bisect started on %s)"),
state.branch);
else if (state.detached_from) {
-- 
2.17.0.rc0.231.g781580f06

[PATCH v2 2/2] t3200: verify "branch --list" sanity when rebasing from detached HEAD

[Kaartic Sivaraam]

From: Eric Sunshine 

"git branch --list" shows an in-progress rebase as:

  * (no branch, rebasing )
master
...

However, if the rebase is started from a detached HEAD, then there is no
, and it would attempt to print a NULL pointer. The previous
commit fixed this problem, so add a test to verify that the output is
sane in this situation.

Note that the "detached HEAD" test case might actually fail in some cases
as the actual output of "git branch --list" might contain remote branch
names which is not considered by the test case as it is rare to happen
in the test environment.

Signed-off-by: Eric Sunshine 
Signed-off-by: Kaartic Sivaraam 
---
 t/t3200-branch.sh | 24 
 1 file changed, 24 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 503a88d02..738b5eb22 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -6,6 +6,7 @@
 test_description='git branch assorted tests'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
 
 test_expect_success 'prepare a trivial repository' '
echo Hello >A &&
@@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with 
--no-merged' '
test_must_fail git branch --merged HEAD --no-merged HEAD
 '
 
+test_expect_success '--list during rebase' '
+   test_when_finished "reset_rebase" &&
+   git checkout master &&
+   FAKE_LINES="1 edit 2" &&
+   export FAKE_LINES &&
+   set_fake_editor &&
+   git rebase -i HEAD~2 &&
+   git branch --list >actual &&
+   test_i18ngrep "rebasing master" actual
+'
+
+test_expect_success '--list during rebase from detached HEAD' '
+   test_when_finished "reset_rebase && git checkout master" &&
+   git checkout HEAD^0 &&
+   oid=$(git rev-parse --short HEAD) &&
+   FAKE_LINES="1 edit 2" &&
+   export FAKE_LINES &&
+   set_fake_editor &&
+   git rebase -i HEAD~2 &&
+   git branch --list >actual &&
+   test_i18ngrep "rebasing detached HEAD $oid" actual
+'
+
 test_expect_success 'tracking with unexpected .fetch refspec' '
rm -rf a b c d &&
git init a &&
-- 
2.17.0.rc0.231.g781580f06

[PATCH v6] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote, based on the one from
for-each-ref. This e.g. allows ref names to be sorted by version
semantics, so that v1.2 is sorted before v1.10.

Signed-off-by: Harald Nordgren 
---

Notes:
Moving 'int i' declaration outside of 'for' loop init

 Documentation/git-ls-remote.txt | 12 +++-
 builtin/ls-remote.c | 27 +--
 t/t5512-ls-remote.sh| 41 -
 3 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..17fae7218 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,16 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..113a2fd7d 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
int show_symref_target = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
+   int i;
 
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
for ( ; ref; ref = ref->next) {
+   struct ref_array_item *item;
if (!check_ref_type(ref, flags))
continue;
if (!tail_match(pattern, ref->name))
continue;
+
+   FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
+   item->symref = ref->symref;
+   item->objectname = ref->old_oid;
+
+   REALLOC_ARRAY(array.items, array.nr + 1);
+   array.items[array.nr++] = item;
+   }
+
+   if (sorting)
+   ref_array_sort(sorting, );
+
+   for (i = 0; i < array.nr; i++) {
+   const struct ref_array_item *ref = array.items[i];
if (show_symref_target && ref->symref)
-   printf("ref: %s\t%s\n", ref->symref, ref->name);
-   printf("%s\t%s\n", oid_to_hex(>old_oid), ref->name);
+   printf("ref: %s\t%s\n", ref->symref, ref->refname);
+   printf("%s\t%s\n", oid_to_hex(>objectname), ref->refname);
status = 0; /* we found something */
}
return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 02106c922..66370cd88 

Re: [PATCH 7/7] diff.c: add --color-moved-ignore-space-delta option

[Jonathan Tan]

On Mon,  2 Apr 2018 15:48:54 -0700
Stefan Beller  wrote:

> +struct ws_delta {
> + int deltachars;
> + char firstchar;
> +};

I'll just make some overall design comments.

Shouldn't this be a string of characters (or a char* and len) and
whether it was added or removed? If you're only checking the first
character, this would not work if the other characters were different.

> @@ -717,10 +752,20 @@ static int moved_entry_cmp(const void 
> *hashmap_cmp_fn_data,
>   const struct diff_options *diffopt = hashmap_cmp_fn_data;
>   const struct moved_entry *a = entry;
>   const struct moved_entry *b = entry_or_key;
> + unsigned flags = diffopt->color_moved & XDF_WHITESPACE_FLAGS;
> +
> + if (diffopt->color_moved & COLOR_MOVED_DELTA_WHITESPACES)
> + /*
> +  * As there is not specific white space config given,
> +  * we'd need to check for a new block, so ignore all
> +  * white space. The setup of the white space
> +  * configuration for the next block is done else where
> +  */
> + flags |= XDF_IGNORE_WHITESPACE;
>  
>   return !xdiff_compare_lines(a->es->line, a->es->len,
>   b->es->line, b->es->len,
> - diffopt->color_moved & 
> XDF_WHITESPACE_FLAGS);
> + flags);
>  }

I think we should just prohibit combining this with any of the
whitespace ignoring flags except for the space-at-eol one. They seem to
contradict anyway.

> +test_expect_success 'compare whitespace delta across moved blocks' '
> +
> + git reset --hard &&
> + q_to_tab <<-\EOF >text.txt &&
> + QIndented
> + QText
> + Qacross
> + Qfive
> + Qlines
> + QBut!
> + Qthis
> + QQone
> + Qline
> + QQdid
> + Qnot
> + QQadjust
> + EOF

Do we need 5 lines? I thought 2 would suffice. (It's the ALNUM_COUNT
that matters, as far as I know.) This makes it hard to see that the
"But!" line is the one that counts.

> +test_expect_success 'compare whitespace delta across moved blocks with 
> multiple indentation levels' '
> + # alternative: "python programmers would love the move detection, too"
> +
> + git reset --hard &&
> + q_to_tab <<-\EOF >test.py &&
> + class test:
> + Qdef f(x):
> + QQ"""
> + QQA simple python function
> + QQthat returns the square of a number
> + QQAlthough it may not be pythonic
> + QQ"""
> + QQdef g(x):
> + QQQ"""
> + QQQNested function that returns the same number
> + QQQWe just multiply by 1.0
> + QQQso we can write a comment about it
> + QQQas we need longer blocks
> + QQQ"""
> + QQQreturn 1.0 * x
> + QQ# Another comment for f(x)
> + QQ# also spanning multiple lines
> + QQ# to make a block
> + QQreturn g(x) * g(x)
> + Qdef h(x):
> + QQ# Another function unrelated to the previous
> + QQ# but building a block,
> + QQ# long enough to call it a block
> + QQreturn x * 1.0
> + EOF

If the objective it just to show that the functions f and g are treated
as one unit despite their lines being of multiple indentation levels,
the test file could be much shorter.

Re: [PATCH 6/7] diff.c: decouple white space treatment for move detection from generic option

[Jonathan Tan]

On Mon,  2 Apr 2018 15:48:53 -0700
Stefan Beller  wrote:

> In the original implementation of the move detection logic we assumed that
> the choice for ignoring white space changes is the same for the move
> detection as it is for the generic diff.  It turns out this is wrong.

I don't think we can say that this is wrong - just that we decided that
it would be useful to allow different whitespace handling methods when
calculating the diff and when detecting moves.

> There are a couple of things where the user wants to input their
> decision into the diff machinery:
> 
> * Whether or not to ignore white space for the general diff detection.
>   This is covered with the -w, -b options already.
> * Whether the move detection ought to pay attention to white spaces
>   in general. And if so, how are white spaces handled for the block
>   detection.
> 
> One example would be reviewing a current patch under discussion, that moves
> code around.  In that case, you may want to have the general diff machinery
> not ignore the white spaces (i.e. exact matching), as that will show you
> the diff as the patch sent. The code moved however may have another
> indentation level, such that you want to ignore white spaces for the move
> detection. The code may be in python, such that spaces matter for program
> flow, though. That is why you'd want each block to have the same change
> in white space.
> 
> This patch decouples white space treatment in the general diff machinery
> from the white space treatment in the move detection.
> 
> This adds all the the options for ignoring white space that the regular
> diff machinery has to the move detection, such that we can cover all the
> cases that were introduced in 7a55427094 (Merge branch
> 'jk/diff-color-moved-fix', 2017-11-06).

I would just write the above as follows:

  Allow the user to specify that whitespace should be ignored
  differently during detection of moved lines than during generation of
  added and removed lines. This is done by providing analogs to the
  --ignore-space-at-eol, -b, and -w options (namely,
  --color-moved-[no-]ignore-space-at-eol ) that affect
  only the color of the output, and making the existing
  --ignore-space-at-eol, -b, and -w options no longer affect the color
  of the output.

(And if the above is not clear enough that this is a
backwards-incompatible change, we might need to call it out.)

> The example from above (different lines in the diff with -w for the regular
> diff) is covered in a test. Convert the existing tests to be more explicit
> on their choice of white space behavior in the move detection as the tests
> should not fail if the default is changed.

This statement is confusing to me - of course the tests should fail,
since we changed the defaults. I think it is sufficient to just mention
that whitespace handling has to be explicitly specified for the move
detection part.

> +--color-moved-[no-]ignore-all-space::
> + Ignore whitespace when comparing lines when performing the move
> + detection for --color-moved.  This ignores differences even if
> + one line has whitespace where the other line has none.
> +--color-moved-[no-]ignore-space-change::
> + Ignore changes in amount of whitespace when performing the move
> + detection for --color-moved.  This ignores whitespace
> + at line end, and considers all other sequences of one or
> + more whitespace characters to be equivalent.
> +--color-moved-[no-]ignore-space-at-eol::
> + Ignore changes in whitespace at EOL when performing the move
> + detection for --color-moved.

Optional: I would reorder this to be in the same order as the analogous
options (--ignore-space-at-eol first, then -b, then -w).

> @@ -720,7 +720,7 @@ static int moved_entry_cmp(const void 
> *hashmap_cmp_fn_data,
>  
>   return !xdiff_compare_lines(a->es->line, a->es->len,
>   b->es->line, b->es->len,
> - diffopt->xdl_opts);
> + diffopt->color_moved & 
> XDF_WHITESPACE_FLAGS);
>  }
>  
>  static struct moved_entry *prepare_entry(struct diff_options *o,
> @@ -728,8 +728,9 @@ static struct moved_entry *prepare_entry(struct 
> diff_options *o,
>  {
>   struct moved_entry *ret = xmalloc(sizeof(*ret));
>   struct emitted_diff_symbol *l = >emitted_symbols->buf[line_no];
> + unsigned flags = o->color_moved & XDF_WHITESPACE_FLAGS;
>  
> - ret->ent.hash = xdiff_hash_string(l->line, l->len, o->xdl_opts);
> + ret->ent.hash = xdiff_hash_string(l->line, l->len, flags);
>   ret->es = l;
>   ret->next_line = NULL;

These 2 changes looked suspicious to me at first, but then I saw that
moved_entry_cmp() and prepare_entry() are only used during move
detection.

> @@ -1419,7 +1422,10 @@ test_expect_success 'move detection ignoring 
> whitespace ' '
>   EOF
>   test_cmp expected actual &&
>  
> - git diff HEAD --no-renames -w 

Re: [RFC PATCH 0/7] Moved code detection: ignore space on uniform indentation

[Jacob Keller]

On Mon, Apr 2, 2018 at 4:47 PM, Jonathan Tan  wrote:
> On Mon,  2 Apr 2018 15:48:47 -0700
> Stefan Beller  wrote:
>
>> This is a re-attempt of [1], which allows the moved code detection to
>> ignore blanks in various modes.
>>
>> patches 1-5 are refactoring, patch 6 adds all existing white space options
>> of regular diff to the move detection. (I am unsure about this patch,
>> as I presume we want to keep the option space at a minimum if possible).
>
> My preference is to not do this until a need has been demonstrated, but
> this sounds like it could be useful one day. I'll review the patches
> from the viewpoint that we do want this feature.
>
>> The fun is in the last patch, which allows white space sensitive
>> languages to trust the move detection, too. Each block that is marked as
>> moved will have the same delta in {in-, de-}dentation.
>> I would think this mode might be a reasonable default eventually.
>
> This sounds like a good idea. "Trust" is probably too strong a word, but
> I can see this being useful even in non-whitespace-sensitive languages
> with nested blocks (like C).

The ability to detect moved code despite whitespace changes would be
good, even while showing diffs with the whitespace intact.

We may not need *all* the options available, but allowing the internal
settings to enable this but have the user-visible controls be limited
should be totally fine.

Thanks,
Jake

Re: [PATCH 5/7] diff.c: refactor internal representation for coloring moved code

[Jonathan Tan]

On Mon,  2 Apr 2018 15:48:52 -0700
Stefan Beller  wrote:

> At the time the move coloring was implemented we thought an enum of modes
> is the best to configure this feature.  However as we want to tack on new
> features, the enum would grow exponentially.
> 
> Refactor the code such that features are enabled via bits. Currently we can
> * activate the move detection,
> * enable the block detection on top, and
> * enable the dimming inside a block, though this could be done without
>   block detection as well (mode "plain, dimmed")

Firstly, patches 1-4 are obviously correct.

As for this patch, I don't think that using flags is the right way to do
this. We are not under any size pressure for struct diff_options, and
the additional options that we plan to add (color-moved-whitespace-flags
and ignore-space-delta) can easily be additional fields instead.

Re: [RFC PATCH 0/7] Moved code detection: ignore space on uniform indentation

[Jonathan Tan]

On Mon,  2 Apr 2018 15:48:47 -0700
Stefan Beller  wrote:

> This is a re-attempt of [1], which allows the moved code detection to
> ignore blanks in various modes.
> 
> patches 1-5 are refactoring, patch 6 adds all existing white space options
> of regular diff to the move detection. (I am unsure about this patch,
> as I presume we want to keep the option space at a minimum if possible).

My preference is to not do this until a need has been demonstrated, but
this sounds like it could be useful one day. I'll review the patches
from the viewpoint that we do want this feature.

> The fun is in the last patch, which allows white space sensitive
> languages to trust the move detection, too. Each block that is marked as
> moved will have the same delta in {in-, de-}dentation.
> I would think this mode might be a reasonable default eventually.

This sounds like a good idea. "Trust" is probably too strong a word, but
I can see this being useful even in non-whitespace-sensitive languages
with nested blocks (like C).

RE: [ANNOUNCE] Git v2.17.0

[Randall S. Becker]

On April 2, 2018 7:20 PM, Junio C Hamano wrote:
> To: Stefan Beller 
> Cc: Randall S. Becker ; git 
> Subject: Re: [ANNOUNCE] Git v2.17.0
> 
> Stefan Beller  writes:
> 
> > Patch at
> > https://public-
> inbox.org/git/010f01d38a9e$a5c4f290$f14ed7b0$@nexbridge
> > .com/
> 
> Thanks for a pointer.  I think it was left behind and got forgotten while
> waiting for a reroll and tying the loose ends.
> 
> I'll go offline for most of the rest of the week.  It would be wonderful
if Git
> 2.17 turns out to be flawless, but that is not a realistic expectation.
Wishing
> for the second best, I'd very much appreciate it if people worked hard to
find
> and fix regressions and collect materials for its first maintenance
release
> 2.17.1 ;-)

No worries. We're running the test suite now (NonStop Platform) and assuming
all is good, it goes into our prod environment this week.

Cheers,
Randall

[PATCH] send-email: fix docs regarding storing password with git credential

[Michal Nazarewicz]

First of all, ‘git credential fill’ does not store credentials
but is used to *read* them.  The command which adds credentials
to the helper’s store is ‘git credential approve’.

Second of all, git-send-email will include port number in host
parameter when getting the password so it has to be set when
storing the password as well.

Apply the two above to fix the Gmail example in git-send-email
documentation.

Signed-off-by: Michał Nazarewicz 
---
 Documentation/git-send-email.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 71ef97ba9..172c7b344 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -477,13 +477,12 @@ 
https://security.google.com/settings/security/apppasswords to setup an
 app-specific password.  Once setup, you can store it with the credentials
 helper:
 
-   $ git credential fill
+   $ git credential approve
protocol=smtp
-   host=smtp.gmail.com
+   host=smtp.gmail.com:587
username=youn...@gmail.com
password=app-password
 
-
 Once your commits are ready to be sent to the mailing list, run the
 following commands:
 
-- 
2.16.2

Re: [ANNOUNCE] Git v2.17.0

[Junio C Hamano]

Stefan Beller  writes:

> Patch at 
> https://public-inbox.org/git/010f01d38a9e$a5c4f290$f14ed7b0$@nexbridge.com/

Thanks for a pointer.  I think it was left behind and got forgotten
while waiting for a reroll and tying the loose ends.

I'll go offline for most of the rest of the week.  It would be
wonderful if Git 2.17 turns out to be flawless, but that is not a
realistic expectation.  Wishing for the second best, I'd very much
appreciate it if people worked hard to find and fix regressions and
collect materials for its first maintenance release 2.17.1 ;-)

Thanks.

Re: [PATCH v5] ls-remote: create '--sort' option

[Eric Sunshine]

On Mon, Apr 2, 2018 at 6:53 PM, Eric Sunshine  wrote:
> On Mon, Apr 2, 2018 at 6:11 PM, Harald Nordgren
>  wrote:
>> Create a '--sort' option for ls-remote, based on the one from
>> for-each-ref. This e.g. allows ref names to be sorted by version
>> semantics, so that v1.2 is sorted before v1.10.
>>
>> Signed-off-by: Harald Nordgren 
>> ---
>
> Please take pity on reviewers and use this space below the "---" line
> following your sign-off to explain what changed since the previous
> attempt and to provide a link to the previous round(s), like this[1].
> Thanks.
>
> [1]: https://www.youtube.com/watch?v=r_8ydghbGSg

Or, like this if we want to be really accurate[1].

[1]: 
https://public-inbox.org/git/20180402210523.83293-1-haraldnordg...@gmail.com/

Re: [PATCH v5] ls-remote: create '--sort' option

[Eric Sunshine]

On Mon, Apr 2, 2018 at 6:11 PM, Harald Nordgren
 wrote:
> Create a '--sort' option for ls-remote, based on the one from
> for-each-ref. This e.g. allows ref names to be sorted by version
> semantics, so that v1.2 is sorted before v1.10.
>
> Signed-off-by: Harald Nordgren 
> ---

Please take pity on reviewers and use this space below the "---" line
following your sign-off to explain what changed since the previous
attempt and to provide a link to the previous round(s), like this[1].
Thanks.

[1]: https://www.youtube.com/watch?v=r_8ydghbGSg

>  Documentation/git-ls-remote.txt | 12 +++-
>  builtin/ls-remote.c | 26 --
>  t/t5512-ls-remote.sh| 41 
> -
>  3 files changed, 75 insertions(+), 4 deletions(-)

[PATCH 1/7] xdiff/xdiff.h: remove unused flags

[Stefan Beller]

These flags were there since the beginning (3443546f6e (Use a *real*
built-in diff generator, 2006-03-24), but were never used. Remove them.

Signed-off-by: Stefan Beller 
---
 xdiff/xdiff.h | 8 
 1 file changed, 8 deletions(-)

diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index c1937a2911..2356da5f78 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -52,14 +52,6 @@ extern "C" {
 #define XDL_EMIT_FUNCNAMES (1 << 0)
 #define XDL_EMIT_FUNCCONTEXT (1 << 2)
 
-#define XDL_MMB_READONLY (1 << 0)
-
-#define XDL_MMF_ATOMIC (1 << 0)
-
-#define XDL_BDOP_INS 1
-#define XDL_BDOP_CPY 2
-#define XDL_BDOP_INSB 3
-
 /* merge simplification levels */
 #define XDL_MERGE_MINIMAL 0
 #define XDL_MERGE_EAGER 1
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 7/7] diff.c: add --color-moved-ignore-space-delta option

[Stefan Beller]

This marks moved code still as blocks when their indentation level
changes uniformly.

Signed-off-by: Stefan Beller 
---
 diff.c |  93 --
 diff.h |   1 +
 t/t4015-diff-whitespace.sh | 191 +
 3 files changed, 278 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index 5fe2930dca..9f969be588 100644
--- a/diff.c
+++ b/diff.c
@@ -709,6 +709,41 @@ struct moved_entry {
struct moved_entry *next_line;
 };
 
+struct ws_delta {
+   int deltachars;
+   char firstchar;
+};
+
+static void compute_ws_delta(const struct emitted_diff_symbol *a,
+const struct emitted_diff_symbol *b,
+struct ws_delta *out)
+{
+   int i;
+   const struct emitted_diff_symbol *longer = a->len > b->len ? a : b;
+
+
+   out->deltachars = 0;
+   out->firstchar = 0;
+
+   if (longer->len > 0 && isspace(longer->line[0]))
+   out->firstchar = longer->line[0];
+   else
+   return;
+
+   for (i = 0; i < a->len; i++)
+   if (a->line[i] == out->firstchar)
+   out->deltachars ++;
+
+   for (i = 0; i < b->len; i++)
+   if (b->line[i] == out->firstchar)
+   out->deltachars --;
+}
+
+static int compare_ws_delta(const struct ws_delta *a, const struct ws_delta *b)
+{
+   return  a->firstchar == b->firstchar && a->deltachars == b->deltachars;
+}
+
 static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
   const void *entry,
   const void *entry_or_key,
@@ -717,10 +752,20 @@ static int moved_entry_cmp(const void 
*hashmap_cmp_fn_data,
const struct diff_options *diffopt = hashmap_cmp_fn_data;
const struct moved_entry *a = entry;
const struct moved_entry *b = entry_or_key;
+   unsigned flags = diffopt->color_moved & XDF_WHITESPACE_FLAGS;
+
+   if (diffopt->color_moved & COLOR_MOVED_DELTA_WHITESPACES)
+   /*
+* As there is not specific white space config given,
+* we'd need to check for a new block, so ignore all
+* white space. The setup of the white space
+* configuration for the next block is done else where
+*/
+   flags |= XDF_IGNORE_WHITESPACE;
 
return !xdiff_compare_lines(a->es->line, a->es->len,
b->es->line, b->es->len,
-   diffopt->color_moved & 
XDF_WHITESPACE_FLAGS);
+   flags);
 }
 
 static struct moved_entry *prepare_entry(struct diff_options *o,
@@ -770,7 +815,8 @@ static void add_lines_to_move_detection(struct diff_options 
*o,
 }
 
 static int shrink_potential_moved_blocks(struct moved_entry **pmb,
-int pmb_nr)
+int pmb_nr,
+struct ws_delta **wsd)
 {
int lp, rp;
 
@@ -786,6 +832,8 @@ static int shrink_potential_moved_blocks(struct moved_entry 
**pmb,
 
if (lp < pmb_nr && rp > -1 && lp < rp) {
pmb[lp] = pmb[rp];
+   if (*wsd)
+   (*wsd)[lp] = (*wsd)[rp];
pmb[rp] = NULL;
rp--;
lp++;
@@ -835,8 +883,11 @@ static void mark_color_as_moved(struct diff_options *o,
 {
struct moved_entry **pmb = NULL; /* potentially moved blocks */
int pmb_nr = 0, pmb_alloc = 0;
-   int n, flipped_block = 1, block_length = 0;
 
+   struct ws_delta *wsd = NULL; /* white space deltas between pmb */
+   int wsd_alloc = 0;
+
+   int n, flipped_block = 1, block_length = 0;
 
for (n = 0; n < o->emitted_symbols->nr; n++) {
struct hashmap *hm = NULL;
@@ -879,14 +930,30 @@ static void mark_color_as_moved(struct diff_options *o,
struct moved_entry *p = pmb[i];
struct moved_entry *pnext = (p && p->next_line) ?
p->next_line : NULL;
-   if (pnext && !hm->cmpfn(o, pnext, match, NULL)) {
-   pmb[i] = p->next_line;
+
+   if (o->color_moved & COLOR_MOVED_DELTA_WHITESPACES) {
+   struct ws_delta out;
+
+   if (pnext)
+   compute_ws_delta(l, pnext->es, );
+   if (pnext &&
+   !hm->cmpfn(o, pnext, match, NULL) &&
+   compare_ws_delta(, [i])) {
+   pmb[i] = p->next_line;
+   /* wsd[i] is the same */
+   } else {
+ 

[PATCH 2/7] xdiff/xdiffi.c: remove unneeded function declarations

[Stefan Beller]

There is no need to forward-declare these functions, as they are used
after their implementation only.

Signed-off-by: Stefan Beller 
---
 xdiff/xdiffi.c | 17 -
 1 file changed, 17 deletions(-)

diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 0de1ef463b..3e8aff92bc 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -22,34 +22,17 @@
 
 #include "xinclude.h"
 
-
-
 #define XDL_MAX_COST_MIN 256
 #define XDL_HEUR_MIN_COST 256
 #define XDL_LINE_MAX (long)((1UL << (CHAR_BIT * sizeof(long) - 1)) - 1)
 #define XDL_SNAKE_CNT 20
 #define XDL_K_HEUR 4
 
-
-
 typedef struct s_xdpsplit {
long i1, i2;
int min_lo, min_hi;
 } xdpsplit_t;
 
-
-
-
-static long xdl_split(unsigned long const *ha1, long off1, long lim1,
- unsigned long const *ha2, long off2, long lim2,
- long *kvdf, long *kvdb, int need_min, xdpsplit_t *spl,
- xdalgoenv_t *xenv);
-static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long 
chg1, long chg2);
-
-
-
-
-
 /*
  * See "An O(ND) Difference Algorithm and its Variations", by Eugene Myers.
  * Basically considers a "box" (off1, off2, lim1, lim2) and scan from both
-- 
2.17.0.484.g0c8726318c-goog

[PATCH 6/7] diff.c: decouple white space treatment for move detection from generic option

[Stefan Beller]

In the original implementation of the move detection logic we assumed that
the choice for ignoring white space changes is the same for the move
detection as it is for the generic diff.  It turns out this is wrong.

There are a couple of things where the user wants to input their
decision into the diff machinery:

* Whether or not to ignore white space for the general diff detection.
  This is covered with the -w, -b options already.
* Whether the move detection ought to pay attention to white spaces
  in general. And if so, how are white spaces handled for the block
  detection.

One example would be reviewing a current patch under discussion, that moves
code around.  In that case, you may want to have the general diff machinery
not ignore the white spaces (i.e. exact matching), as that will show you
the diff as the patch sent. The code moved however may have another
indentation level, such that you want to ignore white spaces for the move
detection. The code may be in python, such that spaces matter for program
flow, though. That is why you'd want each block to have the same change
in white space.

This patch decouples white space treatment in the general diff machinery
from the white space treatment in the move detection.

This adds all the the options for ignoring white space that the regular
diff machinery has to the move detection, such that we can cover all the
cases that were introduced in 7a55427094 (Merge branch
'jk/diff-color-moved-fix', 2017-11-06).

The example from above (different lines in the diff with -w for the regular
diff) is covered in a test. Convert the existing tests to be more explicit
on their choice of white space behavior in the move detection as the tests
should not fail if the default is changed.

Signed-off-by: Stefan Beller 
---
 Documentation/diff-options.txt |  13 +++
 diff.c |  17 +++-
 t/t4015-diff-whitespace.sh | 150 +++--
 3 files changed, 171 insertions(+), 9 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index e3a44f03cd..4ca09c1977 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -288,6 +288,19 @@ dimmed_zebra::
blocks are considered interesting, the rest is uninteresting.
 --
 
+--color-moved-[no-]ignore-all-space::
+   Ignore whitespace when comparing lines when performing the move
+   detection for --color-moved.  This ignores differences even if
+   one line has whitespace where the other line has none.
+--color-moved-[no-]ignore-space-change::
+   Ignore changes in amount of whitespace when performing the move
+   detection for --color-moved.  This ignores whitespace
+   at line end, and considers all other sequences of one or
+   more whitespace characters to be equivalent.
+--color-moved-[no-]ignore-space-at-eol::
+   Ignore changes in whitespace at EOL when performing the move
+   detection for --color-moved.
+
 --word-diff[=]::
Show a word diff, using the  to delimit changed words.
By default, words are delimited by whitespace; see
diff --git a/diff.c b/diff.c
index 2052a43f7a..5fe2930dca 100644
--- a/diff.c
+++ b/diff.c
@@ -720,7 +720,7 @@ static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
 
return !xdiff_compare_lines(a->es->line, a->es->len,
b->es->line, b->es->len,
-   diffopt->xdl_opts);
+   diffopt->color_moved & 
XDF_WHITESPACE_FLAGS);
 }
 
 static struct moved_entry *prepare_entry(struct diff_options *o,
@@ -728,8 +728,9 @@ static struct moved_entry *prepare_entry(struct 
diff_options *o,
 {
struct moved_entry *ret = xmalloc(sizeof(*ret));
struct emitted_diff_symbol *l = >emitted_symbols->buf[line_no];
+   unsigned flags = o->color_moved & XDF_WHITESPACE_FLAGS;
 
-   ret->ent.hash = xdiff_hash_string(l->line, l->len, o->xdl_opts);
+   ret->ent.hash = xdiff_hash_string(l->line, l->len, flags);
ret->es = l;
ret->next_line = NULL;
 
@@ -4638,6 +4639,18 @@ int diff_opt_parse(struct diff_options *options,
DIFF_XDL_SET(options, IGNORE_CR_AT_EOL);
else if (!strcmp(arg, "--ignore-blank-lines"))
DIFF_XDL_SET(options, IGNORE_BLANK_LINES);
+   else if (!strcmp(arg, "--color-moved-no-ignore-all-space"))
+   options->color_moved &= ~XDF_IGNORE_WHITESPACE;
+   else if (!strcmp(arg, "--color-moved-no-ignore-space-change"))
+   options->color_moved &= ~XDF_IGNORE_WHITESPACE_CHANGE;
+   else if (!strcmp(arg, "--color-moved-no-ignore-space-at-eol"))
+   options->color_moved &= ~XDF_IGNORE_WHITESPACE_AT_EOL;
+   else if (!strcmp(arg, "--color-moved-ignore-all-space"))
+   options->color_moved |= XDF_IGNORE_WHITESPACE;
+   else if (!strcmp(arg, "--color-moved-ignore-space-change"))
+

[PATCH 5/7] diff.c: refactor internal representation for coloring moved code

[Stefan Beller]

At the time the move coloring was implemented we thought an enum of modes
is the best to configure this feature.  However as we want to tack on new
features, the enum would grow exponentially.

Refactor the code such that features are enabled via bits. Currently we can
* activate the move detection,
* enable the block detection on top, and
* enable the dimming inside a block, though this could be done without
  block detection as well (mode "plain, dimmed")

Choose the flags to not be at bit position 2,3,4 as the next patch
will occupy these.

Signed-off-by: Stefan Beller 
---
 diff.c | 27 ++-
 diff.h | 17 ++---
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/diff.c b/diff.c
index 879b8a5d9d..2052a43f7a 100644
--- a/diff.c
+++ b/diff.c
@@ -260,7 +260,7 @@ static int parse_color_moved(const char *arg)
 {
switch (git_parse_maybe_bool(arg)) {
case 0:
-   return COLOR_MOVED_NO;
+   return 0;
case 1:
return COLOR_MOVED_DEFAULT;
default:
@@ -268,15 +268,17 @@ static int parse_color_moved(const char *arg)
}
 
if (!strcmp(arg, "no"))
-   return COLOR_MOVED_NO;
+   return 0;
else if (!strcmp(arg, "plain"))
-   return COLOR_MOVED_PLAIN;
+   return COLOR_MOVED_ENABLED;
else if (!strcmp(arg, "zebra"))
-   return COLOR_MOVED_ZEBRA;
+   return COLOR_MOVED_ENABLED | COLOR_MOVED_FIND_BLOCKS;
else if (!strcmp(arg, "default"))
return COLOR_MOVED_DEFAULT;
else if (!strcmp(arg, "dimmed_zebra"))
-   return COLOR_MOVED_ZEBRA_DIM;
+   return COLOR_MOVED_ENABLED |
+  COLOR_MOVED_FIND_BLOCKS |
+  COLOR_MOVED_DIMMED_BLOCKS;
else
return error(_("color moved setting must be one of 'no', 
'default', 'zebra', 'dimmed_zebra', 'plain'"));
 }
@@ -794,7 +796,7 @@ static int shrink_potential_moved_blocks(struct moved_entry 
**pmb,
 }
 
 /*
- * If o->color_moved is COLOR_MOVED_PLAIN, this function does nothing.
+ * If o->color_moved is not set to find blocks, this function does nothing.
  *
  * Otherwise, if the last block has fewer alphanumeric characters than
  * COLOR_MOVED_MIN_ALNUM_COUNT, unset DIFF_SYMBOL_MOVED_LINE on all lines in
@@ -809,7 +811,7 @@ static int shrink_potential_moved_blocks(struct moved_entry 
**pmb,
 static void adjust_last_block(struct diff_options *o, int n, int block_length)
 {
int i, alnum_count = 0;
-   if (o->color_moved == COLOR_MOVED_PLAIN)
+   if (!(o->color_moved & COLOR_MOVED_FIND_BLOCKS))
return;
for (i = 1; i < block_length + 1; i++) {
const char *c = o->emitted_symbols->buf[n - i].line;
@@ -868,7 +870,7 @@ static void mark_color_as_moved(struct diff_options *o,
 
l->flags |= DIFF_SYMBOL_MOVED_LINE;
 
-   if (o->color_moved == COLOR_MOVED_PLAIN)
+   if (!(o->color_moved & COLOR_MOVED_FIND_BLOCKS))
continue;
 
/* Check any potential block runs, advance each or nullify */
@@ -4697,12 +4699,11 @@ int diff_opt_parse(struct diff_options *options,
else if (!strcmp(arg, "--no-color"))
options->use_color = 0;
else if (!strcmp(arg, "--color-moved")) {
-   if (diff_color_moved_default)
-   options->color_moved = diff_color_moved_default;
-   if (options->color_moved == COLOR_MOVED_NO)
+   options->color_moved = diff_color_moved_default;
+   if (!options->color_moved)
options->color_moved = COLOR_MOVED_DEFAULT;
} else if (!strcmp(arg, "--no-color-moved"))
-   options->color_moved = COLOR_MOVED_NO;
+   options->color_moved = 0;
else if (skip_prefix(arg, "--color-moved=", )) {
int cm = parse_color_moved(arg);
if (cm < 0)
@@ -5543,7 +5544,7 @@ static void diff_flush_patch_all_file_pairs(struct 
diff_options *o)
 
add_lines_to_move_detection(o, _lines, _lines);
mark_color_as_moved(o, _lines, _lines);
-   if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
+   if (o->color_moved & COLOR_MOVED_DIMMED_BLOCKS)
dim_moved_lines(o);
 
hashmap_free(_lines, 0);
diff --git a/diff.h b/diff.h
index d29560f822..9542017986 100644
--- a/diff.h
+++ b/diff.h
@@ -205,13 +205,16 @@ struct diff_options {
int diff_path_counter;
 
struct emitted_diff_symbols *emitted_symbols;
-   enum {
-   COLOR_MOVED_NO = 0,
-   COLOR_MOVED_PLAIN = 1,
-   COLOR_MOVED_ZEBRA = 2,
-   COLOR_MOVED_ZEBRA_DIM = 3,
-   } color_moved;
-   #define 

[PATCH 4/7] diff.c: adjust hash function signature to match hashmap expectation

[Stefan Beller]

This makes the follow up patch easier.

Signed-off-by: Stefan Beller 
---
 diff.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index 52f6e02130..879b8a5d9d 100644
--- a/diff.c
+++ b/diff.c
@@ -707,11 +707,15 @@ struct moved_entry {
struct moved_entry *next_line;
 };
 
-static int moved_entry_cmp(const struct diff_options *diffopt,
-  const struct moved_entry *a,
-  const struct moved_entry *b,
+static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
+  const void *entry,
+  const void *entry_or_key,
   const void *keydata)
 {
+   const struct diff_options *diffopt = hashmap_cmp_fn_data;
+   const struct moved_entry *a = entry;
+   const struct moved_entry *b = entry_or_key;
+
return !xdiff_compare_lines(a->es->line, a->es->len,
b->es->line, b->es->len,
diffopt->xdl_opts);
@@ -5534,10 +5538,8 @@ static void diff_flush_patch_all_file_pairs(struct 
diff_options *o)
if (o->color_moved) {
struct hashmap add_lines, del_lines;
 
-   hashmap_init(_lines,
-(hashmap_cmp_fn)moved_entry_cmp, o, 0);
-   hashmap_init(_lines,
-(hashmap_cmp_fn)moved_entry_cmp, o, 0);
+   hashmap_init(_lines, moved_entry_cmp, o, 0);
+   hashmap_init(_lines, moved_entry_cmp, o, 0);
 
add_lines_to_move_detection(o, _lines, _lines);
mark_color_as_moved(o, _lines, _lines);
-- 
2.17.0.484.g0c8726318c-goog

[RFC PATCH 0/7] Moved code detection: ignore space on uniform indentation

[Stefan Beller]

This is a re-attempt of [1], which allows the moved code detection to
ignore blanks in various modes.

patches 1-5 are refactoring, patch 6 adds all existing white space options
of regular diff to the move detection. (I am unsure about this patch,
as I presume we want to keep the option space at a minimum if possible).

The fun is in the last patch, which allows white space sensitive
languages to trust the move detection, too. Each block that is marked as
moved will have the same delta in {in-, de-}dentation.
I would think this mode might be a reasonable default eventually.

Thanks,
Stefan

[1] https://public-inbox.org/git/20171025224620.27657-1-sbel...@google.com/

Stefan Beller (7):
  xdiff/xdiff.h: remove unused flags
  xdiff/xdiffi.c: remove unneeded function declarations
  diff.c: do not pass diff options as keydata to hashmap
  diff.c: adjust hash function signature to match hashmap expectation
  diff.c: refactor internal representation for coloring moved code
  diff.c: decouple white space treatment for move detection from generic
option
  diff.c: add --color-moved-ignore-space-delta option

 Documentation/diff-options.txt |  13 ++
 diff.c | 155 ---
 diff.h |  18 +-
 t/t4015-diff-whitespace.sh | 341 -
 xdiff/xdiff.h  |   8 -
 xdiff/xdiffi.c |  17 --
 6 files changed, 483 insertions(+), 69 deletions(-)

-- 
2.17.0.484.g0c8726318c-goog

[PATCH 3/7] diff.c: do not pass diff options as keydata to hashmap

[Stefan Beller]

The diff options are passed to the compare function as
'hashmap_cmp_fn_data', which are given when the hashmaps
are initialized.

A later patch will make use of the keydata to signal
different settings for comparision.

Signed-off-by: Stefan Beller 
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 4c59f5f5d3..52f6e02130 100644
--- a/diff.c
+++ b/diff.c
@@ -842,13 +842,13 @@ static void mark_color_as_moved(struct diff_options *o,
case DIFF_SYMBOL_PLUS:
hm = del_lines;
key = prepare_entry(o, n);
-   match = hashmap_get(hm, key, o);
+   match = hashmap_get(hm, key, NULL);
free(key);
break;
case DIFF_SYMBOL_MINUS:
hm = add_lines;
key = prepare_entry(o, n);
-   match = hashmap_get(hm, key, o);
+   match = hashmap_get(hm, key, NULL);
free(key);
break;
default:
-- 
2.17.0.484.g0c8726318c-goog

[PATCH v5] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote, based on the one from
for-each-ref. This e.g. allows ref names to be sorted by version
semantics, so that v1.2 is sorted before v1.10.

Signed-off-by: Harald Nordgren 
---
 Documentation/git-ls-remote.txt | 12 +++-
 builtin/ls-remote.c | 26 --
 t/t5512-ls-remote.sh| 41 -
 3 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..17fae7218 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,16 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..7e2d820c4 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -47,6 +48,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +63,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +73,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -104,13 +111,28 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
for ( ; ref; ref = ref->next) {
+   struct ref_array_item *item;
if (!check_ref_type(ref, flags))
continue;
if (!tail_match(pattern, ref->name))
continue;
+
+   FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
+   item->symref = ref->symref;
+   item->objectname = ref->old_oid;
+
+   REALLOC_ARRAY(array.items, array.nr + 1);
+   array.items[array.nr++] = item;
+   }
+
+   if (sorting)
+   ref_array_sort(sorting, );
+
+   for (int i = 0; i < array.nr; i++) {
+   const struct ref_array_item *ref = array.items[i];
if (show_symref_target && ref->symref)
-   printf("ref: %s\t%s\n", ref->symref, ref->name);
-   printf("%s\t%s\n", oid_to_hex(>old_oid), ref->name);
+   printf("ref: %s\t%s\n", ref->symref, ref->refname);
+   printf("%s\t%s\n", oid_to_hex(>objectname), ref->refname);
status = 0; /* we found something */
}
return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 02106c922..66370cd88 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -10,6 +10,9 @@ test_expect_success setup '
test_tick &&
git commit -m initial &&
git tag mark &&
+   git 

Re: [PATCH] send-email: report host and port separately when calling git credential

[Junio C Hamano]

Michal Nazarewicz  writes:

> When git-send-email uses git-credential to get SMTP password, it will
> communicate SMTP host and port (if both are provided) as a single entry
> ‘host=:’.  This trips the ‘git-credential-store’ helper
> which expects those values as separate keys (‘host’ and ‘port’).
>
> Send the values as separate pieces of information so things work
> smoothly.
>
> Signed-off-by: Michał Nazarewicz 
> ---
>  git-send-email.perl | 11 ++-
>  1 file changed, 2 insertions(+), 9 deletions(-)

"git help credential" mentions protocol, host, path, username and
password (and also url which is a short-hand for setting protocol
and host), but not "port".  And common sense tells me, when a system
allows setting host but not port, that it would expect host:port to
be given when the service is running a non-standard port, so from
that point of view, I suspect that the current code is working as
expected.  In fact, credential.h, which defines the API, does not
have any "port" field, either, so I am not sure how this is expected
to change anything without touching the back-end that talks over the
pipe via _credential_run-->credential_write callchain.

Now, it is a separate matter if it were a better design if the
credential API had 'host' and 'port' defined as separate keys to the
authentication information.  Such an alternative design would have
made certain things harder while some other things easier (e.g. "use
this credential to the host no matter what port the service runs"
may be easier to implement if 'host' and 'port' are separate).


> diff --git a/git-send-email.perl b/git-send-email.perl
> index 2fa7818ca..2a9f89a58 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1229,14 +1229,6 @@ sub maildomain {
>   return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
>  }
>  
> -sub smtp_host_string {
> - if (defined $smtp_server_port) {
> - return "$smtp_server:$smtp_server_port";
> - } else {
> - return $smtp_server;
> - }
> -}
> -
>  # Returns 1 if authentication succeeded or was not necessary
>  # (smtp_user was not specified), and 0 otherwise.
>  
> @@ -1263,7 +1255,8 @@ sub smtp_auth_maybe {
>   # reject credentials.
>   $auth = Git::credential({
>   'protocol' => 'smtp',
> - 'host' => smtp_host_string(),
> + 'host' => $smtp_server,
> + 'port' => $smtp_server_port,
>   'username' => $smtp_authuser,
>   # if there's no password, "git credential fill" will
>   # give us one, otherwise it'll just pass this one.

Re: [PATCH v7 08/14] commit-graph: implement git commit-graph read

[Junio C Hamano]

Derrick Stolee  writes:

> From: Derrick Stolee 
> ...
> +static int graph_read(int argc, const char **argv)
> +{
> + struct commit_graph *graph = 0;

The previous round said NULL above, not 0, and NULL is the better
way to spell it, I would think.

Re: [PATCH] ls-remote: create '--sort' option

[Jeff King]

On Mon, Apr 02, 2018 at 10:07:36PM +0200, Harald Nordgren wrote:

> @@ -108,9 +115,25 @@ int cmd_ls_remote(int argc, const char **argv, const 
> char *prefix)
>   continue;
>   if (!tail_match(pattern, ref->name))
>   continue;
> +
> + struct ref_array_item *item;

We don't allow mixed declarations and code; this line needs to go at the
top of the block.

> + FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
> + item->symref = ref->symref;
> + item->objectname = ref->old_oid;
> +
> + REALLOC_ARRAY(array.items, array.nr + 1);
> + array.items[array.nr++] = item;
> + }

This will grow by one each time, which ends up doing quadratic work in
the number of items (although it can often be less if the realloc is
able to just extend the block). It should probably use ALLOC_GROW()
to move it more aggressively.

Interestingly, the ref-filter code itself seems to have the same
problem, but I couldn't measure any speedup. So either my analysis is
totally wrong, or we end up reusing the block most of the time in
practice.

> +
> + if (sorting) {
> + ref_array_sort(sorting, );
> + }

Minor style nit: we usually omit the braces for a one-liner block.

The ref-filter code lets you sort on any arbitrary field. So you could
do:

  git ls-remote --sort=committerdate

What should that do?

With your patch, I think we'll just get something like:

  fatal: missing object 468165c1d8a442994a825f3684528361727cd8c0 for HEAD

which is perhaps the best we can do (it might even work if you've
recently fetched from the other side).

-Peff

Re: [PATCH v6 2/6] reset: introduce show-new-head-line option

[Thomas Gummerer]

On 04/02, Junio C Hamano wrote:
> This is completely offtopic tangent, but I wonder how hidden-bool or
> hidden options[] element in general interacts with the recent
> addition of helping command line completion.  Are we already doing
> the right thing?

I had a quick look at this, and it looks like we're doing the right
thing here.  The following snipped from the 'show_gitcomp' function in
parse-options.c:


>   for (; opts->type != OPTION_END; opts++) {
>   const char *suffix = "";
>
>   if (!opts->long_name)
>   continue;
>   if (opts->flags & (PARSE_OPT_HIDDEN | PARSE_OPT_NOCOMPLETE))
>   continue;

So if the PARSE_OPT_HIDDEN flag is given, we skip printing the option,
which seems like the right thing to do.

Re: [PATCH] ls-remote: create '--sort' option

[Harald Nordgren]

I shortened the commit message. Since we already have this feature for
other sub-commands I guess I don't have to explain what version
semantics are.

Have to say I'm a bit confused about 'git send-mail'. This time I tried

git send-email --subject-prefix="PATCH v4"
--in-reply-to='20180402005248.52418-1-haraldnordg...@gmail.com'
--compose -1

...and the supplied 'git@vger.kernel.org' on the prompt. hopefully
everyone received the latest patches. However, at least what it looks
like from Gmail / Google Inbox, the discussion is now split into two
threads. Hopefully the patches still make sense though.

I will post replies to the original email chain after this message.

On Mon, Apr 2, 2018 at 10:12 PM, Junio C Hamano  wrote:
> Harald Nordgren  writes:
>
>> Subject: Re: [PATCH] ls-remote: create '--sort' option
>
> It would be more helpful to mark as [PATCH v2] a rerolled patch like
> this.
>
>> Create a '--sort' option for ls-remote. This is useful e.g. for the Go
>> repository after the release of version 1.10, where by default v1.10 is
>> sorted before v1.2. See:
>>
>>   $ git ls-remote -t https://go.googlesource.com/go
>
> Does this sample command line also need updating for the refined
> design?
>
>>   ...
>>   205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1
>>   d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1
>>   1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2
>>   bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10
>>   ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1
>> ...
>> + git ls-remote --sort="-version:refname" --tags self >actual &&
>> + test_cmp expect actual
>> +...
>> + git ls-remote --sort="-refname" --tags self >actual &&
>
> These both look sensible (also the variant without the dash prefix
> which I omitted from the quote).
>
>
>

Re: [PATCH v6 2/6] reset: introduce show-new-head-line option

[Thomas Gummerer]

On 04/02, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > +test_expect_success 'reset --no-show-new-head-line suppresses "HEAD is now 
> > at" output' '
> > +   git reset --hard --no-show-new-head-line HEAD >actual &&
> > +   ! grep "HEAD is now at"  > +'
> 
> As builtin/reset.c::print_new_head_line() does this:
> 
>   printf(_("HEAD is now at %s"), hex);
> 
> this needs to use "test_i18ngrep !" instead, no?

Ah yes you're right.  Thanks for catching this.

[PATCH v4] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote, based on the one from
for-each-ref. This e.g. allows ref names to be sorted by version
semantics, so that v1.2 is sorted before v1.10.

Signed-off-by: Harald Nordgren 
---
 Documentation/git-ls-remote.txt | 12 +++-
 builtin/ls-remote.c | 27 +--
 t/t5512-ls-remote.sh| 41 -
 3 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..17fae7218 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,16 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..5521c72f4 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -47,6 +48,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +63,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +73,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -108,9 +115,25 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
continue;
if (!tail_match(pattern, ref->name))
continue;
+
+   struct ref_array_item *item;
+   FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
+   item->symref = ref->symref;
+   item->objectname = ref->old_oid;
+
+   REALLOC_ARRAY(array.items, array.nr + 1);
+   array.items[array.nr++] = item;
+   }
+
+   if (sorting) {
+   ref_array_sort(sorting, );
+   }
+
+   for (int i = 0; i < array.nr; i++) {
+   const struct ref_array_item *ref = array.items[i];
if (show_symref_target && ref->symref)
-   printf("ref: %s\t%s\n", ref->symref, ref->name);
-   printf("%s\t%s\n", oid_to_hex(>old_oid), ref->name);
+   printf("ref: %s\t%s\n", ref->symref, ref->refname);
+   printf("%s\t%s\n", oid_to_hex(>objectname), ref->refname);
status = 0; /* we found something */
}
return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 02106c922..66370cd88 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -10,6 +10,9 @@ test_expect_success setup '
test_tick &&
git commit -m initial &&
git tag mark &&
+   git tag mark1.1 &&
+   git tag mark1.2 &&
+   git tag mark1.10 &&
git show-ref --tags -d | sed -e "s/ /   /" >expected.tag &&
(
echo 

Re: [PATCH v6 2/6] reset: introduce show-new-head-line option

[Thomas Gummerer]

On 04/02, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > Introduce a new --show-new-head-line command line option, that
> > determines whether the "HEAD is now at ..." message is printed or not.
> > It is enabled by default to preserve the current behaviour.
> >
> > It will be used in a subsequent commit to disable printing the "HEAD is
> > now at ..." line in 'git worktree add', so it can be replaced with a
> > custom one, that explains the behaviour better.
> >
> > We cannot just pass the --quite flag from 'git worktree add', as that
> > would also hide progress output when checking out large working trees,
> > which is undesirable.
> >
> > As this option is only for internal use, which we probably don't want
> > to advertise to our users, at least until there's a need for it, make
> > it a hidden flag.
> 
> As a temporary hack, adding something like this may be OK, but in
> the longer run, I think we should (at least) consider refactoring
> "reset --hard" codepath to a freestanding and silent helper function
> so that both cmd_reset() and add_worktree() can call it and report
> the outcome in their own phrasing.

I agree that refactoring this would have been nicer.  The biggest
obstacle there is that the 'git reset' needs to run with a different
"GIT_DIR" and "GIT_WORK_TREE", which is not easy to accomplish in the
current code.

It does seem like something that would be much easier once we have
'struct repository' being passed through everywhere.  I'd rather wait
a bit more for the 'struct repository' series to land, which will
hopefully make the refactoring easier (although I didn't have time to
follow the series closely).

> And I support the decision not to advertise this as a new feature or
> an option by implementing it as hidden-bool.
> 
> This is completely offtopic tangent, but I wonder how hidden-bool or
> hidden options[] element in general interacts with the recent
> addition of helping command line completion.  Are we already doing
> the right thing?

Re: git stash push -u deletes untracked files

[Thomas Gummerer]

On 04/02, Hosam Aly Mahmoud wrote:
> Hi,
> 
> Using Git 2.16.3 on MacOS 10.13.3, running `git stash push
> --include-untracked` deletes the untracked files specified in its
> arguments and creates an empty stash commit.
> 
> In the example below, I create a repository with a single file and a
> single commit. Then I create two untracked files and push one of them
> to the stash. Although I get an error, an empty stash commit is
> generated and the specified file is deleted.

Thanks for your report.  This has been reported first in
https://public-inbox.org/git/349f9369-b799-4f7b-bda1-33bcbd7ea...@syntevo.com/,
and fixed there.  The fix wasn't merged to 'master' yet, but it is in
the 'next' branch.

Now that 2.17 was released, I'd expect this to be merged to 'master'
at some point.  If you'd like to help the fix along, it would be
awesome if you could test the current 'next' branch, and report back
whether or not the fix works for you.

> ```
> $ git init . && touch README && git commit -m "README"
> $ touch my-file my-other-file
> $ git status
> On branch master
> Untracked files:
> my-file
> my-other-file
> 
> nothing added to commit but untracked files present
> $ git stash push -u my-file
> Saved working directory and index state WIP on master: e89afc6 README
> fatal: pathspec 'my-file' did not match any files
> error: unrecognized input
> $ git status
> On branch master
> Untracked files:
> my-other-file
> 
> nothing added to commit but untracked files present
> $ git stash list
> stash@{0}: WIP on master: e89afc6 README
> $ git stash show
> $
> $ ls
> READMEmy-other-file
> ```
> 
> I tested this using git built from the latest commit on master at the
> time of writing (c2a499e6c31ed613a606ffdeb5bb74ab41e9a586) and got the
> same results. Could you please check it out?
> 
> 
> Thank you,
> 
> Hosam Aly

[PATCH v7 08/14] commit-graph: implement git commit-graph read

[Derrick Stolee]

From: Derrick Stolee 

Teach git-commit-graph to read commit graph files and summarize their contents.

Use the read subcommand to verify the contents of a commit graph file in the
tests.

Signed-off-by: Derrick Stolee 
---
 Documentation/git-commit-graph.txt |  12 +++
 builtin/commit-graph.c |  56 
 commit-graph.c | 137 -
 commit-graph.h |  23 +
 t/t5318-commit-graph.sh|  32 +--
 5 files changed, 254 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index 47996e8f89..8aad8303f5 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -9,6 +9,7 @@ git-commit-graph - Write and verify Git commit graph files
 SYNOPSIS
 
 [verse]
+'git commit-graph read' [--object-dir ]
 'git commit-graph write'  [--object-dir ]
 
 
@@ -35,6 +36,11 @@ COMMANDS
 Write a commit graph file based on the commits found in packfiles.
 Includes all commits from the existing commit graph file.
 
+'read'::
+
+Read a graph file given by the commit-graph file and output basic
+details about the graph file. Used for debugging purposes.
+
 
 EXAMPLES
 
@@ -45,6 +51,12 @@ EXAMPLES
 $ git commit-graph write
 
 
+* Read basic information from the commit-graph file.
++
+
+$ git commit-graph read
+
+
 
 GIT
 ---
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 26b6360289..e3f67401fb 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -7,10 +7,16 @@
 
 static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--object-dir ]"),
+   N_("git commit-graph read [--object-dir ]"),
N_("git commit-graph write [--object-dir ]"),
NULL
 };
 
+static const char * const builtin_commit_graph_read_usage[] = {
+   N_("git commit-graph read [--object-dir ]"),
+   NULL
+};
+
 static const char * const builtin_commit_graph_write_usage[] = {
N_("git commit-graph write [--object-dir ]"),
NULL
@@ -20,6 +26,54 @@ static struct opts_commit_graph {
const char *obj_dir;
 } opts;
 
+static int graph_read(int argc, const char **argv)
+{
+   struct commit_graph *graph = 0;
+   char *graph_name;
+
+   static struct option builtin_commit_graph_read_options[] = {
+   OPT_STRING(0, "object-dir", _dir,
+   N_("dir"),
+   N_("The object directory to store the graph")),
+   OPT_END(),
+   };
+
+   argc = parse_options(argc, argv, NULL,
+builtin_commit_graph_read_options,
+builtin_commit_graph_read_usage, 0);
+
+   if (!opts.obj_dir)
+   opts.obj_dir = get_object_directory();
+
+   graph_name = get_commit_graph_filename(opts.obj_dir);
+   graph = load_commit_graph_one(graph_name);
+
+   if (!graph)
+   die("graph file %s does not exist", graph_name);
+   FREE_AND_NULL(graph_name);
+
+   printf("header: %08x %d %d %d %d\n",
+   ntohl(*(uint32_t*)graph->data),
+   *(unsigned char*)(graph->data + 4),
+   *(unsigned char*)(graph->data + 5),
+   *(unsigned char*)(graph->data + 6),
+   *(unsigned char*)(graph->data + 7));
+   printf("num_commits: %u\n", graph->num_commits);
+   printf("chunks:");
+
+   if (graph->chunk_oid_fanout)
+   printf(" oid_fanout");
+   if (graph->chunk_oid_lookup)
+   printf(" oid_lookup");
+   if (graph->chunk_commit_data)
+   printf(" commit_metadata");
+   if (graph->chunk_large_edges)
+   printf(" large_edges");
+   printf("\n");
+
+   return 0;
+}
+
 static int graph_write(int argc, const char **argv)
 {
static struct option builtin_commit_graph_write_options[] = {
@@ -60,6 +114,8 @@ int cmd_commit_graph(int argc, const char **argv, const char 
*prefix)
 PARSE_OPT_STOP_AT_NON_OPTION);
 
if (argc > 0) {
+   if (!strcmp(argv[0], "read"))
+   return graph_read(argc, argv);
if (!strcmp(argv[0], "write"))
return graph_write(argc, argv);
}
diff --git a/commit-graph.c b/commit-graph.c
index f3f7c4f189..b1bd3a892d 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -39,11 +39,146 @@
GRAPH_OID_LEN + 8)
 
 
-static char *get_commit_graph_filename(const char *obj_dir)
+char *get_commit_graph_filename(const char *obj_dir)
 {
return xstrfmt("%s/info/commit-graph", obj_dir);
 }
 
+static struct commit_graph *alloc_commit_graph(void)
+{
+   struct commit_graph *g = 

[PATCH v7 05/14] commit-graph: create git-commit-graph builtin

[Derrick Stolee]

From: Derrick Stolee 

Teach git the 'commit-graph' builtin that will be used for writing and
reading packed graph files. The current implementation is mostly
empty, except for an '--object-dir' option.

Signed-off-by: Derrick Stolee 
---
 .gitignore |  1 +
 Documentation/git-commit-graph.txt | 10 +++
 Makefile   |  1 +
 builtin.h  |  1 +
 builtin/commit-graph.c | 36 ++
 command-list.txt   |  1 +
 contrib/completion/git-completion.bash |  2 ++
 git.c  |  1 +
 8 files changed, 53 insertions(+)
 create mode 100644 Documentation/git-commit-graph.txt
 create mode 100644 builtin/commit-graph.c

diff --git a/.gitignore b/.gitignore
index 833ef3b0b7..e82f90184d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,6 +34,7 @@
 /git-clone
 /git-column
 /git-commit
+/git-commit-graph
 /git-commit-tree
 /git-config
 /git-count-objects
diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
new file mode 100644
index 00..f3b34622a8
--- /dev/null
+++ b/Documentation/git-commit-graph.txt
@@ -0,0 +1,10 @@
+git-commit-graph(1)
+===
+
+NAME
+
+git-commit-graph - Write and verify Git commit graph files
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index a1d8775adb..a59b62bed1 100644
--- a/Makefile
+++ b/Makefile
@@ -952,6 +952,7 @@ BUILTIN_OBJS += builtin/clone.o
 BUILTIN_OBJS += builtin/column.o
 BUILTIN_OBJS += builtin/commit-tree.o
 BUILTIN_OBJS += builtin/commit.o
+BUILTIN_OBJS += builtin/commit-graph.o
 BUILTIN_OBJS += builtin/config.o
 BUILTIN_OBJS += builtin/count-objects.o
 BUILTIN_OBJS += builtin/credential.o
diff --git a/builtin.h b/builtin.h
index 42378f3aa4..079855b6d4 100644
--- a/builtin.h
+++ b/builtin.h
@@ -149,6 +149,7 @@ extern int cmd_clone(int argc, const char **argv, const 
char *prefix);
 extern int cmd_clean(int argc, const char **argv, const char *prefix);
 extern int cmd_column(int argc, const char **argv, const char *prefix);
 extern int cmd_commit(int argc, const char **argv, const char *prefix);
+extern int cmd_commit_graph(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_config(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
new file mode 100644
index 00..b466ecd781
--- /dev/null
+++ b/builtin/commit-graph.c
@@ -0,0 +1,36 @@
+#include "builtin.h"
+#include "config.h"
+#include "parse-options.h"
+
+static char const * const builtin_commit_graph_usage[] = {
+   N_("git commit-graph [--object-dir ]"),
+   NULL
+};
+
+static struct opts_commit_graph {
+   const char *obj_dir;
+} opts;
+
+
+int cmd_commit_graph(int argc, const char **argv, const char *prefix)
+{
+   static struct option builtin_commit_graph_options[] = {
+   OPT_STRING(0, "object-dir", _dir,
+   N_("dir"),
+   N_("The object directory to store the graph")),
+   OPT_END(),
+   };
+
+   if (argc == 2 && !strcmp(argv[1], "-h"))
+   usage_with_options(builtin_commit_graph_usage,
+  builtin_commit_graph_options);
+
+   git_config(git_default_config, NULL);
+   argc = parse_options(argc, argv, prefix,
+builtin_commit_graph_options,
+builtin_commit_graph_usage,
+PARSE_OPT_STOP_AT_NON_OPTION);
+
+   usage_with_options(builtin_commit_graph_usage,
+  builtin_commit_graph_options);
+}
diff --git a/command-list.txt b/command-list.txt
index a1fad28fd8..835c5890be 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -34,6 +34,7 @@ git-clean   mainporcelain
 git-clone   mainporcelain   init
 git-column  purehelpers
 git-commit  mainporcelain   history
+git-commit-graphplumbingmanipulators
 git-commit-tree plumbingmanipulators
 git-config  ancillarymanipulators
 git-count-objects   ancillaryinterrogators
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index b09c8a2362..6726daaf69 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -878,6 +878,7 @@ __git_list_porcelain_commands ()
check-ref-format) : plumbing;;
checkout-index)   : plumbing;;
column)   : internal helper;;
+   

[PATCH v7 07/14] commit-graph: implement git-commit-graph write

[Derrick Stolee]

From: Derrick Stolee 

Teach git-commit-graph to write graph files. Create new test script to verify
this command succeeds without failure.

Signed-off-by: Derrick Stolee 
---
 Documentation/git-commit-graph.txt |  41 ++
 builtin/commit-graph.c |  33 
 t/t5318-commit-graph.sh| 124 +
 3 files changed, 198 insertions(+)
 create mode 100755 t/t5318-commit-graph.sh

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index f3b34622a8..47996e8f89 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -5,6 +5,47 @@ NAME
 
 git-commit-graph - Write and verify Git commit graph files
 
+
+SYNOPSIS
+
+[verse]
+'git commit-graph write'  [--object-dir ]
+
+
+DESCRIPTION
+---
+
+Manage the serialized commit graph file.
+
+
+OPTIONS
+---
+--object-dir::
+   Use given directory for the location of packfiles and commit graph
+   file. This parameter exists to specify the location of an alternate
+   that only has the objects directory, not a full .git directory. The
+   commit graph file is expected to be at /info/commit-graph and
+   the packfiles are expected to be in /pack.
+
+
+COMMANDS
+
+'write'::
+
+Write a commit graph file based on the commits found in packfiles.
+Includes all commits from the existing commit graph file.
+
+
+EXAMPLES
+
+
+* Write a commit graph file for the packed commits in your local .git folder.
++
+
+$ git commit-graph write
+
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index b466ecd781..26b6360289 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -1,9 +1,18 @@
 #include "builtin.h"
 #include "config.h"
+#include "dir.h"
+#include "lockfile.h"
 #include "parse-options.h"
+#include "commit-graph.h"
 
 static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--object-dir ]"),
+   N_("git commit-graph write [--object-dir ]"),
+   NULL
+};
+
+static const char * const builtin_commit_graph_write_usage[] = {
+   N_("git commit-graph write [--object-dir ]"),
NULL
 };
 
@@ -11,6 +20,25 @@ static struct opts_commit_graph {
const char *obj_dir;
 } opts;
 
+static int graph_write(int argc, const char **argv)
+{
+   static struct option builtin_commit_graph_write_options[] = {
+   OPT_STRING(0, "object-dir", _dir,
+   N_("dir"),
+   N_("The object directory to store the graph")),
+   OPT_END(),
+   };
+
+   argc = parse_options(argc, argv, NULL,
+builtin_commit_graph_write_options,
+builtin_commit_graph_write_usage, 0);
+
+   if (!opts.obj_dir)
+   opts.obj_dir = get_object_directory();
+
+   write_commit_graph(opts.obj_dir);
+   return 0;
+}
 
 int cmd_commit_graph(int argc, const char **argv, const char *prefix)
 {
@@ -31,6 +59,11 @@ int cmd_commit_graph(int argc, const char **argv, const char 
*prefix)
 builtin_commit_graph_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
 
+   if (argc > 0) {
+   if (!strcmp(argv[0], "write"))
+   return graph_write(argc, argv);
+   }
+
usage_with_options(builtin_commit_graph_usage,
   builtin_commit_graph_options);
 }
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
new file mode 100755
index 00..d7b635bd68
--- /dev/null
+++ b/t/t5318-commit-graph.sh
@@ -0,0 +1,124 @@
+#!/bin/sh
+
+test_description='commit graph'
+. ./test-lib.sh
+
+test_expect_success 'setup full repo' '
+   mkdir full &&
+   cd "$TRASH_DIRECTORY/full" &&
+   git init &&
+   objdir=".git/objects"
+'
+
+test_expect_success 'write graph with no packs' '
+   cd "$TRASH_DIRECTORY/full" &&
+   git commit-graph write --object-dir . &&
+   test_path_is_file info/commit-graph
+'
+
+test_expect_success 'create commits and repack' '
+   cd "$TRASH_DIRECTORY/full" &&
+   for i in $(test_seq 3)
+   do
+   test_commit $i &&
+   git branch commits/$i
+   done &&
+   git repack
+'
+
+test_expect_success 'write graph' '
+   cd "$TRASH_DIRECTORY/full" &&
+   graph1=$(git commit-graph write) &&
+   test_path_is_file $objdir/info/commit-graph
+'
+
+test_expect_success 'Add more commits' '
+   cd "$TRASH_DIRECTORY/full" &&
+   git reset --hard commits/1 &&
+   for i in $(test_seq 4 5)
+   do
+   test_commit $i &&
+   git branch commits/$i
+   done &&
+   git reset --hard commits/2 &&
+   for i in $(test_seq 6 7)
+ 

[PATCH v7 12/14] commit-graph: read only from specific pack-indexes

[Derrick Stolee]

From: Derrick Stolee 

Teach git-commit-graph to inspect the objects only in a certain list
of pack-indexes within the given pack directory. This allows updating
the commit graph iteratively.

Signed-off-by: Derrick Stolee 
---
 Documentation/git-commit-graph.txt | 11 +-
 builtin/commit-graph.c | 33 +++---
 commit-graph.c | 26 +--
 commit-graph.h |  4 +++-
 packfile.c |  4 ++--
 packfile.h |  2 ++
 t/t5318-commit-graph.sh| 10 +
 7 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index 8aad8303f5..8143cc3f07 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -34,7 +34,9 @@ COMMANDS
 'write'::
 
 Write a commit graph file based on the commits found in packfiles.
-Includes all commits from the existing commit graph file.
++
+With the `--stdin-packs` option, generate the new commit graph by
+walking objects only in the specified pack-indexes.
 
 'read'::
 
@@ -51,6 +53,13 @@ EXAMPLES
 $ git commit-graph write
 
 
+* Write a graph file, extending the current graph file using commits
+* in .
++
+
+$ echo  | git commit-graph write --stdin-packs
+
+
 * Read basic information from the commit-graph file.
 +
 
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index e3f67401fb..9f83c872e9 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -8,7 +8,7 @@
 static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--object-dir ]"),
N_("git commit-graph read [--object-dir ]"),
-   N_("git commit-graph write [--object-dir ]"),
+   N_("git commit-graph write [--object-dir ] [--stdin-packs]"),
NULL
 };
 
@@ -18,12 +18,13 @@ static const char * const builtin_commit_graph_read_usage[] 
= {
 };
 
 static const char * const builtin_commit_graph_write_usage[] = {
-   N_("git commit-graph write [--object-dir ]"),
+   N_("git commit-graph write [--object-dir ] [--stdin-packs]"),
NULL
 };
 
 static struct opts_commit_graph {
const char *obj_dir;
+   int stdin_packs;
 } opts;
 
 static int graph_read(int argc, const char **argv)
@@ -76,10 +77,18 @@ static int graph_read(int argc, const char **argv)
 
 static int graph_write(int argc, const char **argv)
 {
+   const char **pack_indexes = NULL;
+   int packs_nr = 0;
+   const char **lines = NULL;
+   int lines_nr = 0;
+   int lines_alloc = 0;
+
static struct option builtin_commit_graph_write_options[] = {
OPT_STRING(0, "object-dir", _dir,
N_("dir"),
N_("The object directory to store the graph")),
+   OPT_BOOL(0, "stdin-packs", _packs,
+   N_("scan pack-indexes listed by stdin for commits")),
OPT_END(),
};
 
@@ -90,7 +99,25 @@ static int graph_write(int argc, const char **argv)
if (!opts.obj_dir)
opts.obj_dir = get_object_directory();
 
-   write_commit_graph(opts.obj_dir);
+   if (opts.stdin_packs) {
+   struct strbuf buf = STRBUF_INIT;
+   lines_nr = 0;
+   lines_alloc = 128;
+   ALLOC_ARRAY(lines, lines_alloc);
+
+   while (strbuf_getline(, stdin) != EOF) {
+   ALLOC_GROW(lines, lines_nr + 1, lines_alloc);
+   lines[lines_nr++] = strbuf_detach(, NULL);
+   }
+
+   pack_indexes = lines;
+   packs_nr = lines_nr;
+   }
+
+   write_commit_graph(opts.obj_dir,
+  pack_indexes,
+  packs_nr);
+
return 0;
 }
 
diff --git a/commit-graph.c b/commit-graph.c
index 983454785e..fa19b83a8e 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -549,7 +549,9 @@ static void close_reachable(struct packed_oid_list *oids)
}
 }
 
-void write_commit_graph(const char *obj_dir)
+void write_commit_graph(const char *obj_dir,
+   const char **pack_indexes,
+   int nr_packs)
 {
struct packed_oid_list oids;
struct packed_commit_list commits;
@@ -571,7 +573,27 @@ void write_commit_graph(const char *obj_dir)
oids.alloc = 1024;
ALLOC_ARRAY(oids.list, oids.alloc);
 
-   for_each_packed_object(add_packed_commits, , 0);
+   if (pack_indexes) {
+   struct strbuf packname = STRBUF_INIT;
+   int dirlen;
+   strbuf_addf(, "%s/pack/", obj_dir);
+   dirlen = packname.len;
+   

[PATCH v7 14/14] commit-graph: implement "--additive" option

[Derrick Stolee]

From: Derrick Stolee 

Teach git-commit-graph to add all commits from the existing
commit-graph file to the file about to be written. This should be
used when adding new commits without performing garbage collection.

Signed-off-by: Derrick Stolee 
---
 Documentation/git-commit-graph.txt | 10 ++
 builtin/commit-graph.c | 10 +++---
 commit-graph.c | 17 -
 commit-graph.h |  3 ++-
 t/t5318-commit-graph.sh| 10 ++
 5 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index 442ac243e6..4c97b555cc 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -43,6 +43,9 @@ With the `--stdin-commits` option, generate the new commit 
graph by
 walking commits starting at the commits specified in stdin as a list
 of OIDs in hex, one OID per line. (Cannot be combined with
 --stdin-packs.)
++
+With the `--append` option, include all commits that are present in the
+existing commit-graph file.
 
 'read'::
 
@@ -72,6 +75,13 @@ $ echo  | git commit-graph write --stdin-packs
 $ git show-ref -s | git commit-graph write --stdin-commits
 
 
+* Write a graph file containing all commits in the current
+* commit-graph file along with those reachable from HEAD.
++
+
+$ git rev-parse HEAD | git commit-graph write --stdin-commits --append
+
+
 * Read basic information from the commit-graph file.
 +
 
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index f5fc717b8f..41c4f76caf 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -8,7 +8,7 @@
 static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--object-dir ]"),
N_("git commit-graph read [--object-dir ]"),
-   N_("git commit-graph write [--object-dir ] 
[--stdin-packs|--stdin-commits]"),
+   N_("git commit-graph write [--object-dir ] [--append] 
[--stdin-packs|--stdin-commits]"),
NULL
 };
 
@@ -18,7 +18,7 @@ static const char * const builtin_commit_graph_read_usage[] = 
{
 };
 
 static const char * const builtin_commit_graph_write_usage[] = {
-   N_("git commit-graph write [--object-dir ] 
[--stdin-packs|--stdin-commits]"),
+   N_("git commit-graph write [--object-dir ] [--append] 
[--stdin-packs|--stdin-commits]"),
NULL
 };
 
@@ -26,6 +26,7 @@ static struct opts_commit_graph {
const char *obj_dir;
int stdin_packs;
int stdin_commits;
+   int append;
 } opts;
 
 static int graph_read(int argc, const char **argv)
@@ -94,6 +95,8 @@ static int graph_write(int argc, const char **argv)
N_("scan pack-indexes listed by stdin for commits")),
OPT_BOOL(0, "stdin-commits", _commits,
N_("start walk at commits listed by stdin")),
+   OPT_BOOL(0, "append", ,
+   N_("include all commits already in the commit-graph 
file")),
OPT_END(),
};
 
@@ -131,7 +134,8 @@ static int graph_write(int argc, const char **argv)
   pack_indexes,
   packs_nr,
   commit_hex,
-  commits_nr);
+  commits_nr,
+  opts.append);
 
return 0;
 }
diff --git a/commit-graph.c b/commit-graph.c
index 253bc2213a..1fc63d541b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -553,7 +553,8 @@ void write_commit_graph(const char *obj_dir,
const char **pack_indexes,
int nr_packs,
const char **commit_hex,
-   int nr_commits)
+   int nr_commits,
+   int append)
 {
struct packed_oid_list oids;
struct packed_commit_list commits;
@@ -571,10 +572,24 @@ void write_commit_graph(const char *obj_dir,
oids.nr = 0;
oids.alloc = approximate_object_count() / 4;
 
+   if (append) {
+   prepare_commit_graph_one(obj_dir);
+   if (commit_graph)
+   oids.alloc += commit_graph->num_commits;
+   }
+
if (oids.alloc < 1024)
oids.alloc = 1024;
ALLOC_ARRAY(oids.list, oids.alloc);
 
+   if (append && commit_graph) {
+   for (i = 0; i < commit_graph->num_commits; i++) {
+   const unsigned char *hash = 
commit_graph->chunk_oid_lookup +
+   commit_graph->hash_len * i;
+   hashcpy(oids.list[oids.nr++].hash, hash);
+   }
+   }
+
if (pack_indexes) {
struct strbuf 

[PATCH v7 09/14] commit-graph: add core.commitGraph setting

[Derrick Stolee]

From: Derrick Stolee 

The commit graph feature is controlled by the new core.commitGraph config
setting. This defaults to 0, so the feature is opt-in.

The intention of core.commitGraph is that a user can always stop checking
for or parsing commit graph files if core.commitGraph=0.

Signed-off-by: Derrick Stolee 
---
 Documentation/config.txt | 4 
 cache.h  | 1 +
 config.c | 5 +
 environment.c| 1 +
 4 files changed, 11 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4e0cff87f6..e5c7013fb0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -898,6 +898,10 @@ core.notesRef::
 This setting defaults to "refs/notes/commits", and it can be overridden by
 the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
+core.commitGraph::
+   Enable git commit graph feature. Allows reading from the
+   commit-graph file.
+
 core.sparseCheckout::
Enable "sparse checkout" feature. See section "Sparse checkout" in
linkgit:git-read-tree[1] for more information.
diff --git a/cache.h b/cache.h
index a61b2d3f0d..8bdbcbbbf7 100644
--- a/cache.h
+++ b/cache.h
@@ -805,6 +805,7 @@ extern char *git_replace_ref_base;
 
 extern int fsync_object_files;
 extern int core_preload_index;
+extern int core_commit_graph;
 extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
 extern int protect_hfs;
diff --git a/config.c b/config.c
index b0c20e6cb8..25ee4a676c 100644
--- a/config.c
+++ b/config.c
@@ -1226,6 +1226,11 @@ static int git_default_core_config(const char *var, 
const char *value)
return 0;
}
 
+   if (!strcmp(var, "core.commitgraph")) {
+   core_commit_graph = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "core.sparsecheckout")) {
core_apply_sparse_checkout = git_config_bool(var, value);
return 0;
diff --git a/environment.c b/environment.c
index d6dd64662c..8853e2f0dd 100644
--- a/environment.c
+++ b/environment.c
@@ -62,6 +62,7 @@ enum push_default_type push_default = 
PUSH_DEFAULT_UNSPECIFIED;
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 char *notes_ref_name;
 int grafts_replace_parents = 1;
+int core_commit_graph;
 int core_apply_sparse_checkout;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
-- 
2.17.0.14.gba1221a8ce

[PATCH v7 10/14] commit-graph: close under reachability

[Derrick Stolee]

From: Derrick Stolee 

Teach write_commit_graph() to walk all parents from the commits
discovered in packfiles. This prevents gaps given by loose objects or
previously-missed packfiles.

Also automatically add commits from the existing graph file, if it
exists.

Signed-off-by: Derrick Stolee 
---
 commit-graph.c | 45 +
 1 file changed, 45 insertions(+)

diff --git a/commit-graph.c b/commit-graph.c
index b1bd3a892d..ea29c5c2d8 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -367,6 +367,50 @@ static int add_packed_commits(const struct object_id *oid,
return 0;
 }
 
+static void add_missing_parents(struct packed_oid_list *oids, struct commit 
*commit)
+{
+   struct commit_list *parent;
+   for (parent = commit->parents; parent; parent = parent->next) {
+   if (!(parent->item->object.flags & UNINTERESTING)) {
+   ALLOC_GROW(oids->list, oids->nr + 1, oids->alloc);
+   oidcpy(>list[oids->nr], 
&(parent->item->object.oid));
+   oids->nr++;
+   parent->item->object.flags |= UNINTERESTING;
+   }
+   }
+}
+
+static void close_reachable(struct packed_oid_list *oids)
+{
+   int i;
+   struct commit *commit;
+
+   for (i = 0; i < oids->nr; i++) {
+   commit = lookup_commit(>list[i]);
+   if (commit)
+   commit->object.flags |= UNINTERESTING;
+   }
+
+   /*
+* As this loop runs, oids->nr may grow, but not more
+* than the number of missing commits in the reachable
+* closure.
+*/
+   for (i = 0; i < oids->nr; i++) {
+   commit = lookup_commit(>list[i]);
+
+   if (commit && !parse_commit(commit))
+   add_missing_parents(oids, commit);
+   }
+
+   for (i = 0; i < oids->nr; i++) {
+   commit = lookup_commit(>list[i]);
+
+   if (commit)
+   commit->object.flags &= ~UNINTERESTING;
+   }
+}
+
 void write_commit_graph(const char *obj_dir)
 {
struct packed_oid_list oids;
@@ -390,6 +434,7 @@ void write_commit_graph(const char *obj_dir)
ALLOC_ARRAY(oids.list, oids.alloc);
 
for_each_packed_object(add_packed_commits, , 0);
+   close_reachable();
 
QSORT(oids.list, oids.nr, commit_compare);
 
-- 
2.17.0.14.gba1221a8ce

[PATCH v7 02/14] csum-file: refactor finalize_hashfile() method

[Derrick Stolee]

From: Derrick Stolee 

If we want to use a hashfile on the temporary file for a lockfile, then
we need finalize_hashfile() to fully write the trailing hash but also keep
the file descriptor open.

Do this by adding a new CSUM_HASH_IN_STREAM flag along with a functional
change that checks this flag before writing the checksum to the stream.
This differs from previous behavior since it would be written if either
CSUM_CLOSE or CSUM_FSYNC is provided.

Signed-off-by: Derrick Stolee 
---
 builtin/pack-objects.c | 4 ++--
 bulk-checkin.c | 2 +-
 csum-file.c| 8 
 csum-file.h| 5 +++--
 pack-bitmap-write.c| 2 +-
 pack-write.c   | 5 +++--
 6 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ab3e80ee49..b09bbf4f4c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -837,9 +837,9 @@ static void write_pack_file(void)
 * If so, rewrite it like in fast-import
 */
if (pack_to_stdout) {
-   finalize_hashfile(f, oid.hash, CSUM_CLOSE);
+   finalize_hashfile(f, oid.hash, CSUM_HASH_IN_STREAM | 
CSUM_CLOSE);
} else if (nr_written == nr_remaining) {
-   finalize_hashfile(f, oid.hash, CSUM_FSYNC);
+   finalize_hashfile(f, oid.hash, CSUM_HASH_IN_STREAM | 
CSUM_FSYNC | CSUM_CLOSE);
} else {
int fd = finalize_hashfile(f, oid.hash, 0);
fixup_pack_header_footer(fd, oid.hash, pack_tmp_name,
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 227cc9f3b1..70b14fdf41 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -35,7 +35,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
unlink(state->pack_tmp_name);
goto clear_exit;
} else if (state->nr_written == 1) {
-   finalize_hashfile(state->f, oid.hash, CSUM_FSYNC);
+   finalize_hashfile(state->f, oid.hash, CSUM_HASH_IN_STREAM | 
CSUM_FSYNC | CSUM_CLOSE);
} else {
int fd = finalize_hashfile(state->f, oid.hash, 0);
fixup_pack_header_footer(fd, oid.hash, state->pack_tmp_name,
diff --git a/csum-file.c b/csum-file.c
index e6c95a6915..53ce37f7ca 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -61,11 +61,11 @@ int finalize_hashfile(struct hashfile *f, unsigned char 
*result, unsigned int fl
the_hash_algo->final_fn(f->buffer, >ctx);
if (result)
hashcpy(result, f->buffer);
-   if (flags & (CSUM_CLOSE | CSUM_FSYNC)) {
-   /* write checksum and close fd */
+   if (flags & CSUM_HASH_IN_STREAM)
flush(f, f->buffer, the_hash_algo->rawsz);
-   if (flags & CSUM_FSYNC)
-   fsync_or_die(f->fd, f->name);
+   if (flags & CSUM_FSYNC)
+   fsync_or_die(f->fd, f->name);
+   if (flags & CSUM_CLOSE) {
if (close(f->fd))
die_errno("%s: sha1 file error on close", f->name);
fd = 0;
diff --git a/csum-file.h b/csum-file.h
index 9ba87f0a6c..c5a2e335e7 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -27,8 +27,9 @@ extern void hashfile_checkpoint(struct hashfile *, struct 
hashfile_checkpoint *)
 extern int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *);
 
 /* finalize_hashfile flags */
-#define CSUM_CLOSE 1
-#define CSUM_FSYNC 2
+#define CSUM_CLOSE 1
+#define CSUM_FSYNC 2
+#define CSUM_HASH_IN_STREAM4
 
 extern struct hashfile *hashfd(int fd, const char *name);
 extern struct hashfile *hashfd_check(const char *name);
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 662b44f97d..db4c832428 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -535,7 +535,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
if (options & BITMAP_OPT_HASH_CACHE)
write_hash_cache(f, index, index_nr);
 
-   finalize_hashfile(f, NULL, CSUM_FSYNC);
+   finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC | 
CSUM_CLOSE);
 
if (adjust_shared_perm(tmp_file.buf))
die_errno("unable to make temporary bitmap file readable");
diff --git a/pack-write.c b/pack-write.c
index 044f427392..a9d46bc03f 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -170,8 +170,9 @@ const char *write_idx_file(const char *index_name, struct 
pack_idx_entry **objec
}
 
hashwrite(f, sha1, the_hash_algo->rawsz);
-   finalize_hashfile(f, NULL, ((opts->flags & WRITE_IDX_VERIFY)
-   ? CSUM_CLOSE : CSUM_FSYNC));
+   finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_CLOSE |
+   ((opts->flags & WRITE_IDX_VERIFY)
+   ? 0 : CSUM_FSYNC));
return 

[PATCH v7 04/14] graph: add commit graph design document

[Derrick Stolee]

From: Derrick Stolee 

Add Documentation/technical/commit-graph.txt with details of the planned
commit graph feature, including future plans.

Signed-off-by: Derrick Stolee 
---
 Documentation/technical/commit-graph.txt | 163 +++
 1 file changed, 163 insertions(+)
 create mode 100644 Documentation/technical/commit-graph.txt

diff --git a/Documentation/technical/commit-graph.txt 
b/Documentation/technical/commit-graph.txt
new file mode 100644
index 00..0550c6d0dc
--- /dev/null
+++ b/Documentation/technical/commit-graph.txt
@@ -0,0 +1,163 @@
+Git Commit Graph Design Notes
+=
+
+Git walks the commit graph for many reasons, including:
+
+1. Listing and filtering commit history.
+2. Computing merge bases.
+
+These operations can become slow as the commit count grows. The merge
+base calculation shows up in many user-facing commands, such as 'merge-base'
+or 'status' and can take minutes to compute depending on history shape.
+
+There are two main costs here:
+
+1. Decompressing and parsing commits.
+2. Walking the entire graph to satisfy topological order constraints.
+
+The commit graph file is a supplemental data structure that accelerates
+commit graph walks. If a user downgrades or disables the 'core.commitGraph'
+config setting, then the existing ODB is sufficient. The file is stored
+as "commit-graph" either in the .git/objects/info directory or in the info
+directory of an alternate.
+
+The commit graph file stores the commit graph structure along with some
+extra metadata to speed up graph walks. By listing commit OIDs in lexi-
+cographic order, we can identify an integer position for each commit and
+refer to the parents of a commit using those integer positions. We use
+binary search to find initial commits and then use the integer positions
+for fast lookups during the walk.
+
+A consumer may load the following info for a commit from the graph:
+
+1. The commit OID.
+2. The list of parents, along with their integer position.
+3. The commit date.
+4. The root tree OID.
+5. The generation number (see definition below).
+
+Values 1-4 satisfy the requirements of parse_commit_gently().
+
+Define the "generation number" of a commit recursively as follows:
+
+ * A commit with no parents (a root commit) has generation number one.
+
+ * A commit with at least one parent has generation number one more than
+   the largest generation number among its parents.
+
+Equivalently, the generation number of a commit A is one more than the
+length of a longest path from A to a root commit. The recursive definition
+is easier to use for computation and observing the following property:
+
+If A and B are commits with generation numbers N and M, respectively,
+and N <= M, then A cannot reach B. That is, we know without searching
+that B is not an ancestor of A because it is further from a root commit
+than A.
+
+Conversely, when checking if A is an ancestor of B, then we only need
+to walk commits until all commits on the walk boundary have generation
+number at most N. If we walk commits using a priority queue seeded by
+generation numbers, then we always expand the boundary commit with highest
+generation number and can easily detect the stopping condition.
+
+This property can be used to significantly reduce the time it takes to
+walk commits and determine topological relationships. Without generation
+numbers, the general heuristic is the following:
+
+If A and B are commits with commit time X and Y, respectively, and
+X < Y, then A _probably_ cannot reach B.
+
+This heuristic is currently used whenever the computation is allowed to
+violate topological relationships due to clock skew (such as "git log"
+with default order), but is not used when the topological order is
+required (such as merge base calculations, "git log --graph").
+
+In practice, we expect some commits to be created recently and not stored
+in the commit graph. We can treat these commits as having "infinite"
+generation number and walk until reaching commits with known generation
+number.
+
+Design Details
+--
+
+- The commit graph file is stored in a file named 'commit-graph' in the
+  .git/objects/info directory. This could be stored in the info directory
+  of an alternate.
+
+- The core.commitGraph config setting must be on to consume graph files.
+
+- The file format includes parameters for the object ID hash function,
+  so a future change of hash algorithm does not require a change in format.
+
+Future Work
+---
+
+- The commit graph feature currently does not honor commit grafts. This can
+  be remedied by duplicating or refactoring the current graft logic.
+
+- The 'commit-graph' subcommand does not have a "verify" mode that is
+  necessary for integration with fsck.
+
+- The file format includes room for precomputed generation numbers. These
+  are not currently computed, so 

[PATCH v7 03/14] commit-graph: add format document

[Derrick Stolee]

From: Derrick Stolee 

Add document specifying the binary format for commit graphs. This
format allows for:

* New versions.
* New hash functions and hash lengths.
* Optional extensions.

Basic header information is followed by a binary table of contents
into "chunks" that include:

* An ordered list of commit object IDs.
* A 256-entry fanout into that list of OIDs.
* A list of metadata for the commits.
* A list of "large edges" to enable octopus merges.

The format automatically includes two parent positions for every
commit. This favors speed over space, since using only one position
per commit would cause an extra level of indirection for every merge
commit. (Octopus merges suffer from this indirection, but they are
very rare.)

Signed-off-by: Derrick Stolee 
---
 .../technical/commit-graph-format.txt | 97 +++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/technical/commit-graph-format.txt

diff --git a/Documentation/technical/commit-graph-format.txt 
b/Documentation/technical/commit-graph-format.txt
new file mode 100644
index 00..ad6af8105c
--- /dev/null
+++ b/Documentation/technical/commit-graph-format.txt
@@ -0,0 +1,97 @@
+Git commit graph format
+===
+
+The Git commit graph stores a list of commit OIDs and some associated
+metadata, including:
+
+- The generation number of the commit. Commits with no parents have
+  generation number 1; commits with parents have generation number
+  one more than the maximum generation number of its parents. We
+  reserve zero as special, and can be used to mark a generation
+  number invalid or as "not computed".
+
+- The root tree OID.
+
+- The commit date.
+
+- The parents of the commit, stored using positional references within
+  the graph file.
+
+These positional references are stored as unsigned 32-bit integers
+corresponding to the array position withing the list of commit OIDs. We
+use the most-significant bit for special purposes, so we can store at most
+(1 << 31) - 1 (around 2 billion) commits.
+
+== Commit graph files have the following format:
+
+In order to allow extensions that add extra data to the graph, we organize
+the body into "chunks" and provide a binary lookup table at the beginning
+of the body. The header includes certain values, such as number of chunks
+and hash type.
+
+All 4-byte numbers are in network order.
+
+HEADER:
+
+  4-byte signature:
+  The signature is: {'C', 'G', 'P', 'H'}
+
+  1-byte version number:
+  Currently, the only valid version is 1.
+
+  1-byte Hash Version (1 = SHA-1)
+  We infer the hash length (H) from this value.
+
+  1-byte number (C) of "chunks"
+
+  1-byte (reserved for later use)
+ Current clients should ignore this value.
+
+CHUNK LOOKUP:
+
+  (C + 1) * 12 bytes listing the table of contents for the chunks:
+  First 4 bytes describe the chunk id. Value 0 is a terminating label.
+  Other 8 bytes provide the byte-offset in current file for chunk to
+  start. (Chunks are ordered contiguously in the file, so you can infer
+  the length using the next chunk position if necessary.) Each chunk
+  ID appears at most once.
+
+  The remaining data in the body is described one chunk at a time, and
+  these chunks may be given in any order. Chunks are required unless
+  otherwise specified.
+
+CHUNK DATA:
+
+  OID Fanout (ID: {'O', 'I', 'D', 'F'}) (256 * 4 bytes)
+  The ith entry, F[i], stores the number of OIDs with first
+  byte at most i. Thus F[255] stores the total
+  number of commits (N).
+
+  OID Lookup (ID: {'O', 'I', 'D', 'L'}) (N * H bytes)
+  The OIDs for all commits in the graph, sorted in ascending order.
+
+  Commit Data (ID: {'C', 'G', 'E', 'T' }) (N * (H + 16) bytes)
+* The first H bytes are for the OID of the root tree.
+* The next 8 bytes are for the positions of the first two parents
+  of the ith commit. Stores value 0x if no parent in that
+  position. If there are more than two parents, the second value
+  has its most-significant bit on and the other bits store an array
+  position into the Large Edge List chunk.
+* The next 8 bytes store the generation number of the commit and
+  the commit time in seconds since EPOCH. The generation number
+  uses the higher 30 bits of the first 4 bytes, while the commit
+  time uses the 32 bits of the second 4 bytes, along with the lowest
+  2 bits of the lowest byte, storing the 33rd and 34th bit of the
+  commit time.
+
+  Large Edge List (ID: {'E', 'D', 'G', 'E'}) [Optional]
+  This list of 4-byte values store the second through nth parents for
+  all octopus merges. The second parent value in the commit data stores
+  an array position within this list along with the most-significant bit
+  on. Starting at that array position, iterate through this list of commit
+  positions for the parents until reaching a 

Re: [PATCH v6 2/6] reset: introduce show-new-head-line option

[Junio C Hamano]

Thomas Gummerer  writes:

> +test_expect_success 'reset --no-show-new-head-line suppresses "HEAD is now 
> at" output' '
> + git reset --hard --no-show-new-head-line HEAD >actual &&
> + ! grep "HEAD is now at"  +'

As builtin/reset.c::print_new_head_line() does this:

printf(_("HEAD is now at %s"), hex);

this needs to use "test_i18ngrep !" instead, no?

[PATCH v7 13/14] commit-graph: build graph from starting commits

[Derrick Stolee]

From: Derrick Stolee 

Teach git-commit-graph to read commits from stdin when the
--stdin-commits flag is specified. Commits reachable from these
commits are added to the graph. This is a much faster way to construct
the graph than inspecting all packed objects, but is restricted to
known tips.

For the Linux repository, 700,000+ commits were added to the graph
file starting from 'master' in 7-9 seconds, depending on the number
of packfiles in the repo (1, 24, or 120).

Signed-off-by: Derrick Stolee 
---
 Documentation/git-commit-graph.txt | 14 +-
 builtin/commit-graph.c | 27 +--
 commit-graph.c | 27 +--
 commit-graph.h |  4 +++-
 t/t5318-commit-graph.sh| 13 +
 5 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index 8143cc3f07..442ac243e6 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -36,7 +36,13 @@ COMMANDS
 Write a commit graph file based on the commits found in packfiles.
 +
 With the `--stdin-packs` option, generate the new commit graph by
-walking objects only in the specified pack-indexes.
+walking objects only in the specified pack-indexes. (Cannot be combined
+with --stdin-commits.)
++
+With the `--stdin-commits` option, generate the new commit graph by
+walking commits starting at the commits specified in stdin as a list
+of OIDs in hex, one OID per line. (Cannot be combined with
+--stdin-packs.)
 
 'read'::
 
@@ -60,6 +66,12 @@ $ git commit-graph write
 $ echo  | git commit-graph write --stdin-packs
 
 
+* Write a graph file containing all reachable commits.
++
+
+$ git show-ref -s | git commit-graph write --stdin-commits
+
+
 * Read basic information from the commit-graph file.
 +
 
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 9f83c872e9..f5fc717b8f 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -8,7 +8,7 @@
 static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--object-dir ]"),
N_("git commit-graph read [--object-dir ]"),
-   N_("git commit-graph write [--object-dir ] [--stdin-packs]"),
+   N_("git commit-graph write [--object-dir ] 
[--stdin-packs|--stdin-commits]"),
NULL
 };
 
@@ -18,13 +18,14 @@ static const char * const builtin_commit_graph_read_usage[] 
= {
 };
 
 static const char * const builtin_commit_graph_write_usage[] = {
-   N_("git commit-graph write [--object-dir ] [--stdin-packs]"),
+   N_("git commit-graph write [--object-dir ] 
[--stdin-packs|--stdin-commits]"),
NULL
 };
 
 static struct opts_commit_graph {
const char *obj_dir;
int stdin_packs;
+   int stdin_commits;
 } opts;
 
 static int graph_read(int argc, const char **argv)
@@ -79,6 +80,8 @@ static int graph_write(int argc, const char **argv)
 {
const char **pack_indexes = NULL;
int packs_nr = 0;
+   const char **commit_hex = NULL;
+   int commits_nr = 0;
const char **lines = NULL;
int lines_nr = 0;
int lines_alloc = 0;
@@ -89,6 +92,8 @@ static int graph_write(int argc, const char **argv)
N_("The object directory to store the graph")),
OPT_BOOL(0, "stdin-packs", _packs,
N_("scan pack-indexes listed by stdin for commits")),
+   OPT_BOOL(0, "stdin-commits", _commits,
+   N_("start walk at commits listed by stdin")),
OPT_END(),
};
 
@@ -96,10 +101,12 @@ static int graph_write(int argc, const char **argv)
 builtin_commit_graph_write_options,
 builtin_commit_graph_write_usage, 0);
 
+   if (opts.stdin_packs && opts.stdin_commits)
+   die(_("cannot use both --stdin-commits and --stdin-packs"));
if (!opts.obj_dir)
opts.obj_dir = get_object_directory();
 
-   if (opts.stdin_packs) {
+   if (opts.stdin_packs || opts.stdin_commits) {
struct strbuf buf = STRBUF_INIT;
lines_nr = 0;
lines_alloc = 128;
@@ -110,13 +117,21 @@ static int graph_write(int argc, const char **argv)
lines[lines_nr++] = strbuf_detach(, NULL);
}
 
-   pack_indexes = lines;
-   packs_nr = lines_nr;
+   if (opts.stdin_packs) {
+   pack_indexes = lines;
+   packs_nr = lines_nr;
+   }
+   if (opts.stdin_commits) {
+   commit_hex = lines;
+   

[PATCH v7 11/14] commit: integrate commit graph with commit parsing

[Derrick Stolee]

From: Derrick Stolee 

Teach Git to inspect a commit graph file to supply the contents of a
struct commit when calling parse_commit_gently(). This implementation
satisfies all post-conditions on the struct commit, including loading
parents, the root tree, and the commit date.

If core.commitGraph is false, then do not check graph files.

In test script t5318-commit-graph.sh, add output-matching conditions on
read-only graph operations.

By loading commits from the graph instead of parsing commit buffers, we
save a lot of time on long commit walks. Here are some performance
results for a copy of the Linux repository where 'master' has 678,653
reachable commits and is behind 'origin/master' by 59,929 commits.

| Command  | Before | After  | Rel % |
|--|||---|
| log --oneline --topo-order -1000 |  8.31s |  0.94s | -88%  |
| branch -vv   |  1.02s |  0.14s | -86%  |
| rev-list --all   |  5.89s |  1.07s | -81%  |
| rev-list --all --objects | 66.15s | 58.45s | -11%  |

Signed-off-by: Derrick Stolee 
---
 alloc.c |   1 +
 commit-graph.c  | 141 +++-
 commit-graph.h  |  12 
 commit.c|   3 +
 commit.h|   3 +
 t/t5318-commit-graph.sh |  47 +-
 6 files changed, 205 insertions(+), 2 deletions(-)

diff --git a/alloc.c b/alloc.c
index 12afadfacd..cf4f8b61e1 100644
--- a/alloc.c
+++ b/alloc.c
@@ -93,6 +93,7 @@ void *alloc_commit_node(void)
struct commit *c = alloc_node(_state, sizeof(struct commit));
c->object.type = OBJ_COMMIT;
c->index = alloc_commit_index();
+   c->graph_pos = COMMIT_NOT_FROM_GRAPH;
return c;
 }
 
diff --git a/commit-graph.c b/commit-graph.c
index ea29c5c2d8..983454785e 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -38,7 +38,6 @@
 #define GRAPH_MIN_SIZE (5 * GRAPH_CHUNKLOOKUP_WIDTH + GRAPH_FANOUT_SIZE + \
GRAPH_OID_LEN + 8)
 
-
 char *get_commit_graph_filename(const char *obj_dir)
 {
return xstrfmt("%s/info/commit-graph", obj_dir);
@@ -179,6 +178,145 @@ struct commit_graph *load_commit_graph_one(const char 
*graph_file)
exit(1);
 }
 
+/* global storage */
+struct commit_graph *commit_graph = NULL;
+
+static void prepare_commit_graph_one(const char *obj_dir)
+{
+   char *graph_name;
+
+   if (commit_graph)
+   return;
+
+   graph_name = get_commit_graph_filename(obj_dir);
+   commit_graph = load_commit_graph_one(graph_name);
+
+   FREE_AND_NULL(graph_name);
+}
+
+static int prepare_commit_graph_run_once = 0;
+static void prepare_commit_graph(void)
+{
+   struct alternate_object_database *alt;
+   char *obj_dir;
+
+   if (prepare_commit_graph_run_once)
+   return;
+   prepare_commit_graph_run_once = 1;
+
+   obj_dir = get_object_directory();
+   prepare_commit_graph_one(obj_dir);
+   prepare_alt_odb();
+   for (alt = alt_odb_list; !commit_graph && alt; alt = alt->next)
+   prepare_commit_graph_one(alt->path);
+}
+
+static void close_commit_graph(void)
+{
+   if (!commit_graph)
+   return;
+
+   if (commit_graph->graph_fd >= 0) {
+   munmap((void *)commit_graph->data, commit_graph->data_len);
+   commit_graph->data = NULL;
+   close(commit_graph->graph_fd);
+   }
+
+   FREE_AND_NULL(commit_graph);
+}
+
+static int bsearch_graph(struct commit_graph *g, struct object_id *oid, 
uint32_t *pos)
+{
+   return bsearch_hash(oid->hash, g->chunk_oid_fanout,
+   g->chunk_oid_lookup, g->hash_len, pos);
+}
+
+static struct commit_list **insert_parent_or_die(struct commit_graph *g,
+uint64_t pos,
+struct commit_list **pptr)
+{
+   struct commit *c;
+   struct object_id oid;
+   hashcpy(oid.hash, g->chunk_oid_lookup + g->hash_len * pos);
+   c = lookup_commit();
+   if (!c)
+   die("could not find commit %s", oid_to_hex());
+   c->graph_pos = pos;
+   return _list_insert(c, pptr)->next;
+}
+
+static int fill_commit_in_graph(struct commit *item, struct commit_graph *g, 
uint32_t pos)
+{
+   struct object_id oid;
+   uint32_t edge_value;
+   uint32_t *parent_data_ptr;
+   uint64_t date_low, date_high;
+   struct commit_list **pptr;
+   const unsigned char *commit_data = g->chunk_commit_data + (g->hash_len 
+ 16) * pos;
+
+   item->object.parsed = 1;
+   item->graph_pos = pos;
+
+   hashcpy(oid.hash, commit_data);
+   item->tree = lookup_tree();
+
+   date_high = get_be32(commit_data + g->hash_len + 8) & 0x3;
+   date_low = get_be32(commit_data + g->hash_len + 12);
+   item->date = 

[PATCH v7 01/14] csum-file: rename hashclose() to finalize_hashfile()

[Derrick Stolee]

From: Derrick Stolee 

The hashclose() method behaves very differently depending on the flags
parameter. In particular, the file descriptor is not always closed.

Perform a simple rename of "hashclose()" to "finalize_hashfile()" in
preparation for functional changes.

Signed-off-by: Derrick Stolee 
---
 builtin/index-pack.c   | 2 +-
 builtin/pack-objects.c | 6 +++---
 bulk-checkin.c | 4 ++--
 csum-file.c| 2 +-
 csum-file.h| 4 ++--
 fast-import.c  | 2 +-
 pack-bitmap-write.c| 2 +-
 pack-write.c   | 4 ++--
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index bda84a92ef..8bcf280e0b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1270,7 +1270,7 @@ static void conclude_pack(int fix_thin_pack, const char 
*curr_pack, unsigned cha
nr_objects - nr_objects_initial);
stop_progress_msg(, msg.buf);
strbuf_release();
-   hashclose(f, tail_hash, 0);
+   finalize_hashfile(f, tail_hash, 0);
hashcpy(read_hash, pack_hash);
fixup_pack_header_footer(output_fd, pack_hash,
 curr_pack, nr_objects,
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e9d3cfb9e3..ab3e80ee49 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -837,11 +837,11 @@ static void write_pack_file(void)
 * If so, rewrite it like in fast-import
 */
if (pack_to_stdout) {
-   hashclose(f, oid.hash, CSUM_CLOSE);
+   finalize_hashfile(f, oid.hash, CSUM_CLOSE);
} else if (nr_written == nr_remaining) {
-   hashclose(f, oid.hash, CSUM_FSYNC);
+   finalize_hashfile(f, oid.hash, CSUM_FSYNC);
} else {
-   int fd = hashclose(f, oid.hash, 0);
+   int fd = finalize_hashfile(f, oid.hash, 0);
fixup_pack_header_footer(fd, oid.hash, pack_tmp_name,
 nr_written, oid.hash, offset);
close(fd);
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 9d87eac07b..227cc9f3b1 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -35,9 +35,9 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
unlink(state->pack_tmp_name);
goto clear_exit;
} else if (state->nr_written == 1) {
-   hashclose(state->f, oid.hash, CSUM_FSYNC);
+   finalize_hashfile(state->f, oid.hash, CSUM_FSYNC);
} else {
-   int fd = hashclose(state->f, oid.hash, 0);
+   int fd = finalize_hashfile(state->f, oid.hash, 0);
fixup_pack_header_footer(fd, oid.hash, state->pack_tmp_name,
 state->nr_written, oid.hash,
 state->offset);
diff --git a/csum-file.c b/csum-file.c
index 5eda7fb6af..e6c95a6915 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -53,7 +53,7 @@ void hashflush(struct hashfile *f)
}
 }
 
-int hashclose(struct hashfile *f, unsigned char *result, unsigned int flags)
+int finalize_hashfile(struct hashfile *f, unsigned char *result, unsigned int 
flags)
 {
int fd;
 
diff --git a/csum-file.h b/csum-file.h
index 992e5c0141..9ba87f0a6c 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -26,14 +26,14 @@ struct hashfile_checkpoint {
 extern void hashfile_checkpoint(struct hashfile *, struct hashfile_checkpoint 
*);
 extern int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *);
 
-/* hashclose flags */
+/* finalize_hashfile flags */
 #define CSUM_CLOSE 1
 #define CSUM_FSYNC 2
 
 extern struct hashfile *hashfd(int fd, const char *name);
 extern struct hashfile *hashfd_check(const char *name);
 extern struct hashfile *hashfd_throughput(int fd, const char *name, struct 
progress *tp);
-extern int hashclose(struct hashfile *, unsigned char *, unsigned int);
+extern int finalize_hashfile(struct hashfile *, unsigned char *, unsigned int);
 extern void hashwrite(struct hashfile *, const void *, unsigned int);
 extern void hashflush(struct hashfile *f);
 extern void crc32_begin(struct hashfile *);
diff --git a/fast-import.c b/fast-import.c
index b5db5d20b1..6d96f55d9d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1016,7 +1016,7 @@ static void end_packfile(void)
struct tag *t;
 
close_pack_windows(pack_data);
-   hashclose(pack_file, cur_pack_oid.hash, 0);
+   finalize_hashfile(pack_file, cur_pack_oid.hash, 0);
fixup_pack_header_footer(pack_data->pack_fd, pack_data->sha1,
pack_data->pack_name, object_count,
  

[PATCH v7 00/14] Serialized Git Commit Graph

[Derrick Stolee]

This patch has only a few changes since v6:

* Fixed whitespace issues using 'git rebase --whitespace=fix'

* The --stdin-packs docs now refer to "pack-indexes" insead of "packs"

* Modified description of --object-dir option to warn use is rare

* Replaced '--additive' with '--append'

* In "commit-graph: close under reachability" I greatly simplified
  the check that every reachable commit is included. While running
  tests I noticed that the revision walk machinery could not keep up
  with a very large queue created when combined with the '--append'
  option that added all commits from the existing file as starting
  points for the walk. The new algorithm simply appends missing commits
  to the end of the list, which are then iterated to ensure their
  parents are in the list.

I have a few patch series prepared that provide further performance
improvments following this patch.

-- >8 --

This patch contains a way to serialize the commit graph.

The current implementation defines a new file format to store the graph
structure (parent relationships) and basic commit metadata (commit date,
root tree OID) in order to prevent parsing raw commits while performing
basic graph walks. For example, we do not need to parse the full commit
when performing these walks:

* 'git log --topo-order -1000' walks all reachable commits to avoid
  incorrect topological orders, but only needs the commit message for
  the top 1000 commits.

* 'git merge-base  ' may walk many commits to find the correct
  boundary between the commits reachable from A and those reachable
  from B. No commit messages are needed.

* 'git branch -vv' checks ahead/behind status for all local branches
  compared to their upstream remote branches. This is essentially as
  hard as computing merge bases for each.

The current patch speeds up these calculations by injecting a check in
parse_commit_gently() to check if there is a graph file and using that
to provide the required metadata to the struct commit.

The file format has room to store generation numbers, which will be
provided as a patch after this framework is merged. Generation numbers
are referenced by the design document but not implemented in order to
make the current patch focus on the graph construction process. Once
that is stable, it will be easier to add generation numbers and make
graph walks aware of generation numbers one-by-one.

By loading commits from the graph instead of parsing commit buffers, we
save a lot of time on long commit walks. Here are some performance
results for a copy of the Linux repository where 'master' has 678,653
reachable commits and is behind 'origin/master' by 59,929 commits.

| Command  | Before | After  | Rel % |
|--|||---|
| log --oneline --topo-order -1000 |  8.31s |  0.94s | -88%  |
| branch -vv   |  1.02s |  0.14s | -86%  |
| rev-list --all   |  5.89s |  1.07s | -81%  |
| rev-list --all --objects | 66.15s | 58.45s | -11%  |

To test this yourself, run the following on your repo:

  git config core.commitGraph true
  git show-ref -s | git commit-graph write --stdin-commits

The second command writes a commit graph file containing every commit
reachable from your refs. Now, all git commands that walk commits will
check your graph first before consulting the ODB. You can run your own
performance comparisons by toggling the 'core.commitGraph' setting.

[1] https://github.com/derrickstolee/git/pull/2
A GitHub pull request containing the latest version of this patch.

Derrick Stolee (14):
  csum-file: rename hashclose() to finalize_hashfile()
  csum-file: refactor finalize_hashfile() method
  commit-graph: add format document
  graph: add commit graph design document
  commit-graph: create git-commit-graph builtin
  commit-graph: implement write_commit_graph()
  commit-graph: implement git-commit-graph write
  commit-graph: implement git commit-graph read
  commit-graph: add core.commitGraph setting
  commit-graph: close under reachability
  commit: integrate commit graph with commit parsing
  commit-graph: read only from specific pack-indexes
  commit-graph: build graph from starting commits
  commit-graph: implement "--additive" option

 .gitignore|   1 +
 Documentation/config.txt  |   4 +
 Documentation/git-commit-graph.txt|  94 +++
 .../technical/commit-graph-format.txt |  97 +++
 Documentation/technical/commit-graph.txt  | 163 
 Makefile  |   2 +
 alloc.c   |   1 +
 builtin.h |   1 +
 builtin/commit-graph.c| 171 
 builtin/index-pack.c  |   2 +-
 builtin/pack-objects.c|   6 +-
 bulk-checkin.c|   4 +-
 cache.h 

[PATCH v7 06/14] commit-graph: implement write_commit_graph()

[Derrick Stolee]

From: Derrick Stolee 

Teach Git to write a commit graph file by checking all packed objects
to see if they are commits, then store the file in the given object
directory.

Helped-by: Jeff King 
Signed-off-by: Derrick Stolee 
---
 Makefile   |   1 +
 commit-graph.c | 359 +
 commit-graph.h |   6 +
 3 files changed, 366 insertions(+)
 create mode 100644 commit-graph.c
 create mode 100644 commit-graph.h

diff --git a/Makefile b/Makefile
index a59b62bed1..26a23257e9 100644
--- a/Makefile
+++ b/Makefile
@@ -777,6 +777,7 @@ LIB_OBJS += color.o
 LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
 LIB_OBJS += commit.o
+LIB_OBJS += commit-graph.o
 LIB_OBJS += compat/obstack.o
 LIB_OBJS += compat/terminal.o
 LIB_OBJS += config.o
diff --git a/commit-graph.c b/commit-graph.c
new file mode 100644
index 00..f3f7c4f189
--- /dev/null
+++ b/commit-graph.c
@@ -0,0 +1,359 @@
+#include "cache.h"
+#include "config.h"
+#include "git-compat-util.h"
+#include "lockfile.h"
+#include "pack.h"
+#include "packfile.h"
+#include "commit.h"
+#include "object.h"
+#include "revision.h"
+#include "sha1-lookup.h"
+#include "commit-graph.h"
+
+#define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
+#define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
+#define GRAPH_CHUNKID_OIDLOOKUP 0x4f49444c /* "OIDL" */
+#define GRAPH_CHUNKID_DATA 0x43444154 /* "CDAT" */
+#define GRAPH_CHUNKID_LARGEEDGES 0x45444745 /* "EDGE" */
+
+#define GRAPH_DATA_WIDTH 36
+
+#define GRAPH_VERSION_1 0x1
+#define GRAPH_VERSION GRAPH_VERSION_1
+
+#define GRAPH_OID_VERSION_SHA1 1
+#define GRAPH_OID_LEN_SHA1 GIT_SHA1_RAWSZ
+#define GRAPH_OID_VERSION GRAPH_OID_VERSION_SHA1
+#define GRAPH_OID_LEN GRAPH_OID_LEN_SHA1
+
+#define GRAPH_OCTOPUS_EDGES_NEEDED 0x8000
+#define GRAPH_PARENT_MISSING 0x7fff
+#define GRAPH_EDGE_LAST_MASK 0x7fff
+#define GRAPH_PARENT_NONE 0x7000
+
+#define GRAPH_LAST_EDGE 0x8000
+
+#define GRAPH_FANOUT_SIZE (4 * 256)
+#define GRAPH_CHUNKLOOKUP_WIDTH 12
+#define GRAPH_MIN_SIZE (5 * GRAPH_CHUNKLOOKUP_WIDTH + GRAPH_FANOUT_SIZE + \
+   GRAPH_OID_LEN + 8)
+
+
+static char *get_commit_graph_filename(const char *obj_dir)
+{
+   return xstrfmt("%s/info/commit-graph", obj_dir);
+}
+
+static void write_graph_chunk_fanout(struct hashfile *f,
+struct commit **commits,
+int nr_commits)
+{
+   int i, count = 0;
+   struct commit **list = commits;
+
+   /*
+* Write the first-level table (the list is sorted,
+* but we use a 256-entry lookup to be able to avoid
+* having to do eight extra binary search iterations).
+*/
+   for (i = 0; i < 256; i++) {
+   while (count < nr_commits) {
+   if ((*list)->object.oid.hash[0] != i)
+   break;
+   count++;
+   list++;
+   }
+
+   hashwrite_be32(f, count);
+   }
+}
+
+static void write_graph_chunk_oids(struct hashfile *f, int hash_len,
+  struct commit **commits, int nr_commits)
+{
+   struct commit **list = commits;
+   int count;
+   for (count = 0; count < nr_commits; count++, list++)
+   hashwrite(f, (*list)->object.oid.hash, (int)hash_len);
+}
+
+static const unsigned char *commit_to_sha1(size_t index, void *table)
+{
+   struct commit **commits = table;
+   return commits[index]->object.oid.hash;
+}
+
+static void write_graph_chunk_data(struct hashfile *f, int hash_len,
+  struct commit **commits, int nr_commits)
+{
+   struct commit **list = commits;
+   struct commit **last = commits + nr_commits;
+   uint32_t num_extra_edges = 0;
+
+   while (list < last) {
+   struct commit_list *parent;
+   int edge_value;
+   uint32_t packedDate[2];
+
+   parse_commit(*list);
+   hashwrite(f, (*list)->tree->object.oid.hash, hash_len);
+
+   parent = (*list)->parents;
+
+   if (!parent)
+   edge_value = GRAPH_PARENT_NONE;
+   else {
+   edge_value = sha1_pos(parent->item->object.oid.hash,
+ commits,
+ nr_commits,
+ commit_to_sha1);
+
+   if (edge_value < 0)
+   edge_value = GRAPH_PARENT_MISSING;
+   }
+
+   hashwrite_be32(f, edge_value);
+
+   if (parent)
+   parent = parent->next;
+
+   if (!parent)
+   edge_value = GRAPH_PARENT_NONE;
+   else if (parent->next)
+   edge_value = 

Re: [PATCH v6 2/6] reset: introduce show-new-head-line option

[Junio C Hamano]

Thomas Gummerer  writes:

> Introduce a new --show-new-head-line command line option, that
> determines whether the "HEAD is now at ..." message is printed or not.
> It is enabled by default to preserve the current behaviour.
>
> It will be used in a subsequent commit to disable printing the "HEAD is
> now at ..." line in 'git worktree add', so it can be replaced with a
> custom one, that explains the behaviour better.
>
> We cannot just pass the --quite flag from 'git worktree add', as that
> would also hide progress output when checking out large working trees,
> which is undesirable.
>
> As this option is only for internal use, which we probably don't want
> to advertise to our users, at least until there's a need for it, make
> it a hidden flag.

As a temporary hack, adding something like this may be OK, but in
the longer run, I think we should (at least) consider refactoring
"reset --hard" codepath to a freestanding and silent helper function
so that both cmd_reset() and add_worktree() can call it and report
the outcome in their own phrasing.

And I support the decision not to advertise this as a new feature or
an option by implementing it as hidden-bool.

This is completely offtopic tangent, but I wonder how hidden-bool or
hidden options[] element in general interacts with the recent
addition of helping command line completion.  Are we already doing
the right thing?

Re: [PATCH] ls-remote: create '--sort' option

[Junio C Hamano]

Harald Nordgren  writes:

> Subject: Re: [PATCH] ls-remote: create '--sort' option

It would be more helpful to mark as [PATCH v2] a rerolled patch like
this.

> Create a '--sort' option for ls-remote. This is useful e.g. for the Go
> repository after the release of version 1.10, where by default v1.10 is
> sorted before v1.2. See:
>
>   $ git ls-remote -t https://go.googlesource.com/go

Does this sample command line also need updating for the refined
design?

>   ...
>   205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1
>   d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1
>   1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2
>   bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10
>   ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1
> ...
> + git ls-remote --sort="-version:refname" --tags self >actual &&
> + test_cmp expect actual
> +...
> + git ls-remote --sort="-refname" --tags self >actual &&

These both look sensible (also the variant without the dash prefix
which I omitted from the quote).

RE: [ANNOUNCE] Git v2.17.0

[Randall S. Becker]

On April 2, 2018 4:02 PM, Stefan Beller found my change:
> On Mon, Apr 2, 2018 at 12:57 PM, Randall S. Becker
>  wrote:
> > On April 2, 2018 3:34 PM, Junio C Hamano wrote:
> >> The latest feature release Git v2.17.0 is now available at the usual
> >> places.  It is comprised of 516 non-merge commits since v2.16.0,
> >> contributed by 71 people, 20 of which are new faces.
> >
> > Just a heads up. I think this one might have gotten missed at some point a
> few months back. I think it was submitted back in January. Not sure where it
> fell off or whether it was just dropped.
> >
> > diff --git a/transport-helper.c b/transport-helper.c index
> > 3f380d87d..5ee7007f6 100644
> > --- a/transport-helper.c
> > +++ b/transport-helper.c
> > @@ -1212,7 +1212,7 @@ static int udt_do_read(struct
> unidirectional_transfer *t)
> > return 0;   /* No space for more. */
> >
> > transfer_debug("%s is readable", t->src_name);
> > -   bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
> > +   bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE -
> > + t->bufuse);
> > if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
> > errno != EINTR) {
> > error_errno("read(%s) failed", t->src_name);
> 
> Patch at https://public-
> inbox.org/git/010f01d38a9e$a5c4f290$f14ed7b0$@nexbridge.com/

That was it, thanks.

Cheers,
Randall

[PATCH] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote. This is useful e.g. for the Go
repository after the release of version 1.10, where by default v1.10 is
sorted before v1.2. See:

$ git ls-remote -t https://go.googlesource.com/go
...
205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1
d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1
1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2
bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10
ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1
9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1
594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2
5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1
20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2
1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2
46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3
402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2
9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1
...

Signed-off-by: Harald Nordgren 
---
 Documentation/git-ls-remote.txt | 12 +++-
 builtin/ls-remote.c | 27 +--
 t/t5512-ls-remote.sh| 41 -
 3 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..17fae7218 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,16 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..5521c72f4 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -47,6 +48,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +63,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +73,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -108,9 +115,25 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
continue;
if (!tail_match(pattern, ref->name))
continue;
+
+   struct ref_array_item *item;
+   FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
+   item->symref = ref->symref;
+   item->objectname = ref->old_oid;
+
+   REALLOC_ARRAY(array.items, array.nr + 1);
+   array.items[array.nr++] = item;
+   }
+
+   if (sorting) {
+  

Re: [GSoC] [PATCH] travis-ci: added clang static analysis

[Siddhartha Mishra]

On Sun, Apr 1, 2018, 8:10 PM Lars Schneider  wrote:

>
> That's a good summary and I don't see a better solution. While (3)
> sounds nice, I think (2) is the fastest/most pragmatic solution.
>
> We already use the Travis cache [1]. You could use that mechanism to
> store a file with the latest number of bugs in the cache directory
> $HOME/travis-cache
>
> If the "number of bugs" file does not exist, then create it and don't
> complain. If the file exists and the previous number of bugs is higher
> or equal, then don't complain either. If the file exists and the previous
> number of bugs is lower, then let the build fail.
>
> Do you think that could work?

Apologies for having sent this message twice, the last one wasn't in
plain text and were therefore not properly forwarded to the mail
group.

If we only consider the number of bugs in the last pushed commit, a
situation might happen that a commit creates a lot of bugs. If a
subsequent commit then fixes only a few of the previously created bugs
the build would still pass since the number of bugs from the last
commit has decreased.

Should this be the intended behaviour or should adding the number of
allowed bugs be a more deliberate process?

Thanks,
Siddhartha

Re: [PATCH] ls-remote: create option to sort by versions

[Harald Nordgren]

Thanks for all the discussion!

I think I figured out a way to reuse more ref-filter.c machinery. I
will submit another patchset shortly.

On Mon, Apr 2, 2018 at 8:32 PM, Junio C Hamano  wrote:
> Ævar Arnfjörð Bjarmason  writes:
>
>> This is a sensible thing to want, but why not follow the UI we have for
>> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course
>> ls-remote doesn't just show tags, so maybe we'd want --tag-sort=
>> and --ignore-tag-case or something, but the rest should be equivalent,
>> no?
>
> Yeah, and if we can reuse more of ref-filter.c machinery (which was
> factored out of for-each-ref and tag you suggested borrows from),
> that would be even better.  In the context of ls-remote, however, we
> cannot inspect the object (we typically do not have them yet), so it
> may not be practical, but I agree with your suggestion---we should
> match the behaviour at the UI level at least when we can.
>

Re: [ANNOUNCE] Git v2.17.0

[Stefan Beller]

On Mon, Apr 2, 2018 at 12:57 PM, Randall S. Becker
 wrote:
> On April 2, 2018 3:34 PM, Junio C Hamano wrote:
>> The latest feature release Git v2.17.0 is now available at the usual places. 
>>  It is
>> comprised of 516 non-merge commits since v2.16.0, contributed by 71
>> people, 20 of which are new faces.
>
> Just a heads up. I think this one might have gotten missed at some point a 
> few months back. I think it was submitted back in January. Not sure where it 
> fell off or whether it was just dropped.
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 3f380d87d..5ee7007f6 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -1212,7 +1212,7 @@ static int udt_do_read(struct unidirectional_transfer 
> *t)
> return 0;   /* No space for more. */
>
> transfer_debug("%s is readable", t->src_name);
> -   bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
> +   bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
> if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
> errno != EINTR) {
> error_errno("read(%s) failed", t->src_name);

Patch at 
https://public-inbox.org/git/010f01d38a9e$a5c4f290$f14ed7b0$@nexbridge.com/

RE: [ANNOUNCE] Git v2.17.0

[Randall S. Becker]

On April 2, 2018 3:34 PM, Junio C Hamano wrote:
> The latest feature release Git v2.17.0 is now available at the usual places.  
> It is
> comprised of 516 non-merge commits since v2.16.0, contributed by 71
> people, 20 of which are new faces.

Just a heads up. I think this one might have gotten missed at some point a few 
months back. I think it was submitted back in January. Not sure where it fell 
off or whether it was just dropped.

diff --git a/transport-helper.c b/transport-helper.c
index 3f380d87d..5ee7007f6 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1212,7 +1212,7 @@ static int udt_do_read(struct unidirectional_transfer *t)
return 0;   /* No space for more. */

transfer_debug("%s is readable", t->src_name);
-   bytes = read(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
+   bytes = xread(t->src, t->buf + t->bufuse, BUFFERSIZE - t->bufuse);
if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
errno != EINTR) {
error_errno("read(%s) failed", t->src_name);

Cheers,
Randall

Re: [PATCH] Support long format for log-based submodule diff

[Stefan Beller]

On Sun, Apr 1, 2018 at 6:07 PM, Robert Dailey  wrote:
> On Tue, Mar 27, 2018 at 5:17 PM, Stefan Beller  wrote:
>>> >> $ git diff --submodule=log --submodule-log-detail=(long|short)
>>> >>
>>> >> I'm not sure what makes sense here. I welcome thoughts/discussion and
>>> >> will provide follow-up patches.
>>> >
>>> > The case of merges is usually configured with --[no-]merges, or
>>> > --min-parents=.
>>
>>> But that is a knob that controls an irrelevant aspect of the detail
>>> in the context of this discussion, isn't it?  This code is about "to
>>> what degree the things that happened between two submodule commits
>>> in an adjacent pair of commits in the superproject are summarized?"
>>
>> And I took it a step further and wanted to give a general solution, which
>> allows giving any option that the diff machinery accepts to only apply
>> to the submodule diffing part of the current diff.
>>
>>> The hack Robert illustrates below is to change it to stop favouring
>>> such projects with "clean" histories, and show "log --oneline
>>> --no-merges --left-right".  When presented that way, clean histories
>>> of topic-branch based projects will suffer by losing conciseness,
>>> but clean histories of totally linear projects will still be shown
>>> the same way, and messy history that sometimes merges, sometimes
>>> merges mergy histories, and sometimes directly builds on the trunk
>>> will be shown as an enumeration of individual commits in a flat way
>>> by ignoring merges and not restricting the traversal to the first
>>> parent chains, which would appear more uniform than what the current
>>> code shows.
>>
>> Oh, I realize this is in the *summary* code path, I was thinking about the
>> show_submodule_inline_diff, which would benefit from more diff options.
>>
>>> I do not see a point in introducing --min/max-parents as a knob to
>>> control how the history is summarized.
>>
>> For a summary a flat list of commits may be fine, ignoring
>> (ideally non-evil) merges.
>>
>>> This is a strongly related tangent, but I wonder if we can and/or
>>> want to share more code with the codepath that prepares the log
>>> message for a merge.  It summarizes what happened on the side branch
>>> since it forked from the history it is joining back to (I think it
>>> is merge.c::shortlog() that computes this)
>>
>> I do not find code there. To me it looks like builtin/fmt-merge-msg.c
>> is responsible for coming up with a default merge message?
>> In that file there is a shortlog() function, which walks revisions
>> and puts together the subject lines of commits.
>>
>>> and it is quite similar
>>> to what Robert wants to use for submodules here.  On the other hand,
>>> in a project _without_ submodule, if you are pulling history made by
>>> your lieutenant whose history is full of linear merges of topic
>>> branches to the mainline, it may not be a bad idea to allow
>>> fmt-merge-msg to alternatively show something similar to the "diff
>>> --submodule=log" gives us, i.e. summarize the history of the side
>>> branch being merged by just listing the commits on the first-parent
>>> chain.  So I sense some opportunity for cross pollination here.
>>
>> The cross pollination that I sense is the desire in both cases to freely
>> specify the format as it may depend on the workflow.
>
> First I want to apologize for having taken so long to get back with
> each of you about this. I actually have a lot of work started to
> expand the --submodule option to add a "full-log" option in addition
> to the existing "log". This is a pretty big task for me already,
> mostly because I'm unfamiliar with git and have limited personal time
> to do this at home (this is part of what I am apologizing for).

No worries wrt. time.

> I kind
> of get what Stefan and Junio are saying. There's a lot of opportunity
> for cleanup. More specific to my use case, adding some functionality
> to generate a log message (although I've developed a bash script to do
> this since I wrote my original email. I'll attach it to this email for
> those interested).

The functionality looks very similar what Gerrit does in its
"superproject subscription mode", which would update the submodules in
the superproject automatically, when you submit on the submodule.
For example [1] is an update of the Gerrit project itself, that has some
submodules. This commit only updates the replication plugin, but
provides a summary what happened in that plugin.

[1] 
https://gerrit.googlesource.com/gerrit/+/db20af7123221b0b2f01d1f06e4eaac32a04cef6


I wonder if there is need for this in upstream git as well, e.g.
"git submodule update --remote" would also want to have a
switch "--commit-with-proposed-commit-message" or if the
standard commit message template would provide a submodule
summary for you. I realize that there is the config option
status.submoduleSummary already, but it is not as clear as either
your script or the Gerrit example.

> Also 

[ANNOUNCE] Git v2.17.0

[Junio C Hamano]

The latest feature release Git v2.17.0 is now available at the
usual places.  It is comprised of 516 non-merge commits since
v2.16.0, contributed by 71 people, 20 of which are new faces.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.17.0'
tag and the 'master' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.16.0 are as follows.
Welcome to the Git development community!

  Adam Borowski, Alban Gruin, Andreas G. Schacker, Bernhard
  M. Wiedemann, Christian Ludwig, Christopher Diaz Riveros,
  Gargi Sharma, Genki Sky, Gregory Herrero, Jon Simons, Juan
  F. Codagnone, Kim Gybels, Lucas Werkmeister, Mathias Rav,
  Michele Locati, Motoki Seki, Stefan Moch, Stephen R Guglielmo,
  Tatyana Krasnukha, and Thomas Levesque.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Ævar Arnfjörð Bjarmason, Alexander Shopov, Alex Bennée,
  Ben Peart, Brandon Williams, brian m. carlson, Changwoo Ryu,
  Christian Couder, Daniel Knittl-Frank, David Pursehouse,
  Derrick Stolee, Elijah Newren, Eric Sunshine, Eric Wong, Jason
  Merrill, Jean-Noël Avila, Jeff Hostetler, Jeff King, Jiang Xin,
  Johannes Schindelin, Jonathan Nieder, Jonathan Tan, Jordi Mas,
  Junio C Hamano, Kaartic Sivaraam, Mårten Kongstad, Martin
  Ågren, Matthieu Moy, Michael Haggerty, Nathan Payre, Nguyễn
  Thái Ngọc Duy, Nicolas Morey-Chaisemartin, Olga Telezhnaya,
  Patryk Obara, Peter Krefting, Phillip Wood, Prathamesh Chavan,
  Ralf Thielow, Ramsay Jones, Randall S. Becker, Rasmus Villemoes,
  Ray Chen, René Scharfe, Robert P. J. Day, Stefan Beller, SZEDER
  Gábor, Thomas Gummerer, Todd Zullinger, Torsten Bögershausen,
  Trần Ngọc Quân, and Yasushi SHOJI.



Git 2.17 Release Notes
==

Updates since v2.16
---

UI, Workflows & Features

 * "diff" family of commands learned "--find-object=" option
   to limit the findings to changes that involve the named object.

 * "git format-patch" learned to give 72-cols to diffstat, which is
   consistent with other line length limits the subcommand uses for
   its output meant for e-mails.

 * The log from "git daemon" can be redirected with a new option; one
   relevant use case is to send the log to standard error (instead of
   syslog) when running it from inetd.

 * "git rebase" learned to take "--allow-empty-message" option.

 * "git am" has learned the "--quit" option, in addition to the
   existing "--abort" option; having the pair mirrors a few other
   commands like "rebase" and "cherry-pick".

 * "git worktree add" learned to run the post-checkout hook, just like
   "git clone" runs it upon the initial checkout.

 * "git tag" learned an explicit "--edit" option that allows the
   message given via "-m" and "-F" to be further edited.

 * "git fetch --prune-tags" may be used as a handy short-hand for
   getting rid of stale tags that are locally held.

 * The new "--show-current-patch" option gives an end-user facing way
   to get the diff being applied when "git rebase" (and "git am")
   stops with a conflict.

 * "git add -p" used to offer "/" (look for a matching hunk) as a
   choice, even there was only one hunk, which has been corrected.
   Also the single-key help is now given only for keys that are
   enabled (e.g. help for '/' won't be shown when there is only one
   hunk).

 * Since Git 1.7.9, "git merge" defaulted to --no-ff (i.e. even when
   the side branch being merged is a descendant of the current commit,
   create a merge commit instead of fast-forwarding) when merging a
   tag object.  This was appropriate default for integrators who pull
   signed tags from their downstream contributors, but caused an
   unnecessary merges when used by downstream contributors who
   habitually "catch up" their topic branches with tagged releases
   from the upstream.  Update "git merge" to default to --no-ff only
   when merging a tag object that does *not* sit at its usual place in
   refs/tags/ hierarchy, and allow fast-forwarding otherwise, to
   mitigate the problem.

 * "git status" can spend a lot of cycles to compute the relation
   between the current branch and its upstream, which can now be
   disabled with "--no-ahead-behind" option.

 * "git diff" and friends learned funcname patterns for Go language
   source files.

 * "git send-email" learned "--reply-to=" option.

 * Funcname pattern used for C# now recognizes "async" keyword.

 * In a way similar to how "git tag" learned to honor the pager
   setting only in the list mode, "git config" learned to ignore the
   pager setting when it is used for setting values (i.e. when the
   purpose of the operation is not to "show").


Performance, Internal 

Re: [PATCH] ls-remote: create option to sort by versions

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> This is a sensible thing to want, but why not follow the UI we have for
> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course
> ls-remote doesn't just show tags, so maybe we'd want --tag-sort=
> and --ignore-tag-case or something, but the rest should be equivalent,
> no?

Yeah, and if we can reuse more of ref-filter.c machinery (which was
factored out of for-each-ref and tag you suggested borrows from),
that would be even better.  In the context of ls-remote, however, we
cannot inspect the object (we typically do not have them yet), so it
may not be practical, but I agree with your suggestion---we should
match the behaviour at the UI level at least when we can.

Re: [PATCH v12 00/10] convert: add support for different encodings

[Lars Schneider]

> On 29 Mar 2018, at 20:37, Junio C Hamano  wrote:
> 
> lars.schnei...@autodesk.com writes:
> 
>> From: Lars Schneider 
>> 
>> Patches 1-6,9 are preparation and helper functions. Patch 4 is new.
>> Patch 7,8,10 are the actual change.
>> 
>> This series depends on Torsten's 8462ff43e4 (convert_to_git():
>> safe_crlf/checksafe becomes int conv_flags, 2018-01-13) which is
>> already in master.
> 
> I didn't see any further review comments on this round, and as far
> as I can tell from my reading, these patches looked more-or-less
> ready.  
> 
> Except for 04/10 that had a few messages around "who should be
> responsible for handling the 'NULL is for the default UTF-8'?", that
> is.
> 
> So, what's the doneness of this thing?

Almost. I'll send a new round tomorrow. I hope to make this the final
round.

- Lars

Re: [PATCH] graph.c: log.showRootMark to indicate root commits

[Junio C Hamano]

Junio C Hamano  writes:

> Lyubomyr Shaydariv  writes:
>
>> When log.showRootMark is set, root commits are marked with
>> the at sign (@).
>>
>> When log.showRootMark is not set, root commits are marked with
>> the asterisk sign (*). This is the default behavior.
>>
>> Signed-off-by: Lyubomyr Shaydariv 
>> ---
>
> So the idea is when you have a history like this:
> ...
> and that would work well with --left-right automatically (as you
> would just do the same thing as you would to a normal asterisk).

There was some discussion a while back on making root commits more
apparent in the graph view, e.g.

https://public-inbox.org/git/1382717268-21884-1-git-send-email-milton.soares.fi...@gmail.com/

Re: [PATCH] l10n: de.po: translate 132 new messages

[Matthias Rüster]

Hi Ralf,

thanks a lot for your translations!

I've only found a small issue:


  #: git-add--interactive.perl:1405
-#, fuzzy, perl-format
+#, perl-format
  msgid "Discard this hunk from worktree [y,n,q,a,d%s,?]? "
-msgstr "diesen Patch-Block im Arbeitsverzeichnis verwerfen [y,n,q,a,d,/%s,?]? "
+msgstr "diesen Patch-Block im Arbeitsverzeichnis verwerfen [y,n,q,a,d%s,?]? "


"Diesen ..."


Kind regards,
Matthias

Re: [PATCH v4 00/13] Serialized Git Commit Graph

[Stefan Beller]

> Currently, the format includes 8 bytes to share between the generation
> number and commit date. Due to alignment concerns, we will want to keep this
> as 8 bytes or truncate it to 4-bytes. Either we would be wasting at least 3
> bytes or truncating dates too much (presenting the 2038 problem [1] since
> dates are signed).

Good point. I forgot about them while writing the previous email.
That is reason enough to keep the generation numbers, sorry
for the noise.

>
>> I only glanced at the paper, but it looks like a "more advanced 2d
>> generation number" that seems to be able to answer questions
>> that gen numbers can answer, but that paper also refers
>> to SCARAB as well as GRAIL as the state of the art, so maybe
>> there are even more papers to explore?
>
>
> The biggest reason I can say to advance this series (and the small follow-up
> series that computes and consumes generation numbers) is that generation
> numbers are _extremely simple_. You only need to know your parents and their
> generation numbers to compute your own. These other reachability indexes
> require examining the entire graph to create "good" index values.

Yes, that is a good point, too. Generation numbers can be computed
"commit locally" and do not need expensive setups, which the others
presumably need.

> The hard part about using generation numbers (or any other reachability
> index) in Git is refactoring the revision-walk machinery to take advantage
> of them; current code requires O(reachable commits) to topo-order instead of
> O(commits that will be output). I think we should table any discussion of
> these advanced indexes until that work is done and a valuable comparison can
> be done. "Premature optimization is the root of all evil" and all that.

agreed,

Stefan

Re: [PATCH] graph.c: log.showRootMark to indicate root commits

[Junio C Hamano]

Lyubomyr Shaydariv  writes:

> When log.showRootMark is set, root commits are marked with
> the at sign (@).
>
> When log.showRootMark is not set, root commits are marked with
> the asterisk sign (*). This is the default behavior.
>
> Signed-off-by: Lyubomyr Shaydariv 
> ---

So the idea is when you have a history like this:

R1--A---B
  \
R2--C---D---M

to show "git log --oneline --graph D B" like

* B
* A
@ R1
* D
* C
@ R2

because you cannot tell that between R1 and D there is no
parent-child relationship otherwise?

One downside of that approach is that it is not clear how this
feature should interact with --left-right.  I do not think there is
a clean way to do that, unless you come up with a pair of symbols,
not just a single '@'.

Another way to show that there is no parent-child relationship
between R1 and D is to show it like this instead:

* B
* A
 \
  * R1
* D
* C
* R2

and that would work well with --left-right automatically (as you
would just do the same thing as you would to a normal asterisk).

Re: [PATCH v4 00/13] Serialized Git Commit Graph

[Derrick Stolee]

On 4/2/2018 1:35 PM, Stefan Beller wrote:

On Mon, Apr 2, 2018 at 8:02 AM, Derrick Stolee  wrote:

I would be happy to review any effort to extend the commit-graph
format to include such indexes, as long as the performance benefits
outweigh the complexity to create them.

[2]
http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.719.8396=rep1=pdf

The complexity of calculating FELINE index is O(|V| log(|V|) + |E|), the
storage complexity is 2*|V|.


This would be very easy to add as an optional chunk, since it can use one
row per commit.

Given this discussion, I wonder if we want to include generation numbers
as a first class citizen in the current format. They could also go as
an optional
chunk and we may want to discuss further if we want generation numbers or
FELINE numbers or GRAIL or SCARAB, which are all graph related speedup
mechanism AFAICT.
In case we decide against generation numbers in the long run,
the row of mandatory generation numbers would be dead weight
that we'd need to carry.


Currently, the format includes 8 bytes to share between the generation 
number and commit date. Due to alignment concerns, we will want to keep 
this as 8 bytes or truncate it to 4-bytes. Either we would be wasting at 
least 3 bytes or truncating dates too much (presenting the 2038 problem 
[1] since dates are signed).



I only glanced at the paper, but it looks like a "more advanced 2d
generation number" that seems to be able to answer questions
that gen numbers can answer, but that paper also refers
to SCARAB as well as GRAIL as the state of the art, so maybe
there are even more papers to explore?


The biggest reason I can say to advance this series (and the small 
follow-up series that computes and consumes generation numbers) is that 
generation numbers are _extremely simple_. You only need to know your 
parents and their generation numbers to compute your own. These other 
reachability indexes require examining the entire graph to create "good" 
index values.


The hard part about using generation numbers (or any other reachability 
index) in Git is refactoring the revision-walk machinery to take 
advantage of them; current code requires O(reachable commits) to 
topo-order instead of O(commits that will be output). I think we should 
table any discussion of these advanced indexes until that work is done 
and a valuable comparison can be done. "Premature optimization is the 
root of all evil" and all that.


Thanks,
-Stolee

[1] https://en.wikipedia.org/wiki/Year_2038_problem

Re: [PATCH] ls-remote: create option to sort by versions

[Jeff King]

On Mon, Apr 02, 2018 at 06:26:49PM +0200, Harald Nordgren wrote:

> It would be nice to have a uniform option like
> '--sort=version:refname'. But spending a few hours to look over the
> code, it seems that ls-remote.c would require a lot of rewrites if we
> wanted to start using `ref_array` and `ref_array_item` for storing the
> refs.
> 
> Which seems necessary in order to hook in to the sorting flow used in
> other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe
> I'm missing something?

I haven't looked at how painful it might be to use ref-filter.c, but it
would buy us even more if we could. That would open up other options
like --format, I think (OTOH there may be some funny corner cases; that
code assumes we're talking about local refs, so if you were to ask for
"%(committerdate)" or something, we might have to more cleanly handle
the case where we don't actually have the object).

-Peff

Re: [PATCH] ls-remote: create option to sort by versions

[Harald Nordgren]

Both points make sense and it sounds like a very pragmatic approach.
I'll look into it!

On Mon, Apr 2, 2018 at 7:32 PM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Mon, Apr 02 2018, Harald Nordgren wrote:
>
>> In regards the the print statement, it was only moved down according
>> to the diff because I added more logic above. Basically there is 1)
>> the unrolling of the linked list to an array and 2) the printing
>> logic. I could move it and make the diff smaller, but that probably
>> makes the code a tiny bit more complicated.
>
> I was just wondering since it wasn't explained in the commit message,
> makes sense to copy this explanation into v2, or lead with a purely code
> re-arrangement patch.
>
>> It would be nice to have a uniform option like
>> '--sort=version:refname'. But spending a few hours to look over the
>> code, it seems that ls-remote.c would require a lot of rewrites if we
>> wanted to start using `ref_array` and `ref_array_item` for storing the
>> refs.
>>
>> Which seems necessary in order to hook in to the sorting flow used in
>> other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe
>> I'm missing something?
>
> I'm thinking just in terms of UI. If it's the case that porting this to
> whatever guts git-tag uses for sorting would be hard, then we could
> still use the same command-line option convention (and perhaps just die
> if anything except --sort=version:refname is supplied). Changing the
> underlying implementation is easier than cleaning up UI-differences that
> (seemingly) only arose due to underlying implementation details at the
> time.
>
>> On Mon, Apr 2, 2018 at 8:37 AM, Ævar Arnfjörð Bjarmason
>>  wrote:
>>>
>>> On Mon, Apr 02 2018, Harald Nordgren wrote:
>>>
 Create the options '-V ' and '--version-sort' to sort
 'git ls-remote' output by version semantics. This is useful e.g. for
 the Go repository after the release of version 1.10, where otherwise
 v1.10 is sorted before v1.2. See:

   $ git ls-remote -t https://go.googlesource.com/go
   ...
   205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1
   d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1
   1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2
   bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10
   ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1
   9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1
   594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2
   5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1
   20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2
   1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2
   46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3
   402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2
   9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1
   ...
>>>
>>> This is a sensible thing to want, but why not follow the UI we have for
>>> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course
>>> ls-remote doesn't just show tags, so maybe we'd want --tag-sort=
>>> and --ignore-tag-case or something, but the rest should be equivalent,
>>> no?
>>>
 [...]
 @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const 
 char *prefix)
   if (transport_disconnect(transport))
   return 1;

 - if (!dest && !quiet)
 - fprintf(stderr, "From %s\n", *remote->url);
   for ( ; ref; ref = ref->next) {
   if (!check_ref_type(ref, flags))
   continue;
   if (!tail_match(pattern, ref->name))
   continue;
 + REALLOC_ARRAY(refs, nr + 1);
 + refs[nr++] = ref;
 + }
 +
 + if (version_sort)
 + QSORT(refs, nr, cmp_ref_versions);
 +
 + if (!dest && !quiet)
 + fprintf(stderr, "From %s\n", *remote->url);
>>>
>>> Is there some subtlety here I'm missing which means that when sorting
>>> we'd now need to print this "From" line later (i.e. after sorting?

Re: [PATCH v4 00/13] Serialized Git Commit Graph

[Stefan Beller]

On Mon, Apr 2, 2018 at 8:02 AM, Derrick Stolee  wrote:
>>>
>>> I would be happy to review any effort to extend the commit-graph
>>> format to include such indexes, as long as the performance benefits
>>> outweigh the complexity to create them.
>>>
>>> [2]
>>> http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.719.8396=rep1=pdf
>>
>> The complexity of calculating FELINE index is O(|V| log(|V|) + |E|), the
>> storage complexity is 2*|V|.
>>
>
> This would be very easy to add as an optional chunk, since it can use one
> row per commit.

Given this discussion, I wonder if we want to include generation numbers
as a first class citizen in the current format. They could also go as
an optional
chunk and we may want to discuss further if we want generation numbers or
FELINE numbers or GRAIL or SCARAB, which are all graph related speedup
mechanism AFAICT.
In case we decide against generation numbers in the long run,
the row of mandatory generation numbers would be dead weight
that we'd need to carry.

I only glanced at the paper, but it looks like a "more advanced 2d
generation number" that seems to be able to answer questions
that gen numbers can answer, but that paper also refers
to SCARAB as well as GRAIL as the state of the art, so maybe
there are even more papers to explore?

Stefan

Re: [PATCH] ls-remote: create option to sort by versions

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 02 2018, Harald Nordgren wrote:

> In regards the the print statement, it was only moved down according
> to the diff because I added more logic above. Basically there is 1)
> the unrolling of the linked list to an array and 2) the printing
> logic. I could move it and make the diff smaller, but that probably
> makes the code a tiny bit more complicated.

I was just wondering since it wasn't explained in the commit message,
makes sense to copy this explanation into v2, or lead with a purely code
re-arrangement patch.

> It would be nice to have a uniform option like
> '--sort=version:refname'. But spending a few hours to look over the
> code, it seems that ls-remote.c would require a lot of rewrites if we
> wanted to start using `ref_array` and `ref_array_item` for storing the
> refs.
>
> Which seems necessary in order to hook in to the sorting flow used in
> other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe
> I'm missing something?

I'm thinking just in terms of UI. If it's the case that porting this to
whatever guts git-tag uses for sorting would be hard, then we could
still use the same command-line option convention (and perhaps just die
if anything except --sort=version:refname is supplied). Changing the
underlying implementation is easier than cleaning up UI-differences that
(seemingly) only arose due to underlying implementation details at the
time.

> On Mon, Apr 2, 2018 at 8:37 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>>
>> On Mon, Apr 02 2018, Harald Nordgren wrote:
>>
>>> Create the options '-V ' and '--version-sort' to sort
>>> 'git ls-remote' output by version semantics. This is useful e.g. for
>>> the Go repository after the release of version 1.10, where otherwise
>>> v1.10 is sorted before v1.2. See:
>>>
>>>   $ git ls-remote -t https://go.googlesource.com/go
>>>   ...
>>>   205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1
>>>   d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1
>>>   1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2
>>>   bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10
>>>   ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1
>>>   9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1
>>>   594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2
>>>   5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1
>>>   20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2
>>>   1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2
>>>   46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3
>>>   402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2
>>>   9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1
>>>   ...
>>
>> This is a sensible thing to want, but why not follow the UI we have for
>> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course
>> ls-remote doesn't just show tags, so maybe we'd want --tag-sort=
>> and --ignore-tag-case or something, but the rest should be equivalent,
>> no?
>>
>>> [...]
>>> @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const 
>>> char *prefix)
>>>   if (transport_disconnect(transport))
>>>   return 1;
>>>
>>> - if (!dest && !quiet)
>>> - fprintf(stderr, "From %s\n", *remote->url);
>>>   for ( ; ref; ref = ref->next) {
>>>   if (!check_ref_type(ref, flags))
>>>   continue;
>>>   if (!tail_match(pattern, ref->name))
>>>   continue;
>>> + REALLOC_ARRAY(refs, nr + 1);
>>> + refs[nr++] = ref;
>>> + }
>>> +
>>> + if (version_sort)
>>> + QSORT(refs, nr, cmp_ref_versions);
>>> +
>>> + if (!dest && !quiet)
>>> + fprintf(stderr, "From %s\n", *remote->url);
>>
>> Is there some subtlety here I'm missing which means that when sorting
>> we'd now need to print this "From" line later (i.e. after sorting?

Re: [GIT PULL] l10n updates for 2.17.0 round 1

[Junio C Hamano]

Jiang Xin  writes:

> Would you please pull the following git l10n updates.
>
> The following changes since commit 0afbf6caa5b16dcfa3074982e5b48e27d452dbbb:

Thanks, done.

Re: [PATCH] ls-remote: create option to sort by versions

[Harald Nordgren]

Thanks for your comment Ævar!

In regards the the print statement, it was only moved down according
to the diff because I added more logic above. Basically there is 1)
the unrolling of the linked list to an array and 2) the printing
logic. I could move it and make the diff smaller, but that probably
makes the code a tiny bit more complicated.

It would be nice to have a uniform option like
'--sort=version:refname'. But spending a few hours to look over the
code, it seems that ls-remote.c would require a lot of rewrites if we
wanted to start using `ref_array` and `ref_array_item` for storing the
refs.

Which seems necessary in order to hook in to the sorting flow used in
other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe
I'm missing something?

On Mon, Apr 2, 2018 at 8:37 AM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Mon, Apr 02 2018, Harald Nordgren wrote:
>
>> Create the options '-V ' and '--version-sort' to sort
>> 'git ls-remote' output by version semantics. This is useful e.g. for
>> the Go repository after the release of version 1.10, where otherwise
>> v1.10 is sorted before v1.2. See:
>>
>>   $ git ls-remote -t https://go.googlesource.com/go
>>   ...
>>   205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1
>>   d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1
>>   1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2
>>   bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10
>>   ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1
>>   9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1
>>   594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2
>>   5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1
>>   20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2
>>   1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2
>>   46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3
>>   402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2
>>   9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1
>>   ...
>
> This is a sensible thing to want, but why not follow the UI we have for
> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course
> ls-remote doesn't just show tags, so maybe we'd want --tag-sort=
> and --ignore-tag-case or something, but the rest should be equivalent,
> no?
>
>> [...]
>> @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const 
>> char *prefix)
>>   if (transport_disconnect(transport))
>>   return 1;
>>
>> - if (!dest && !quiet)
>> - fprintf(stderr, "From %s\n", *remote->url);
>>   for ( ; ref; ref = ref->next) {
>>   if (!check_ref_type(ref, flags))
>>   continue;
>>   if (!tail_match(pattern, ref->name))
>>   continue;
>> + REALLOC_ARRAY(refs, nr + 1);
>> + refs[nr++] = ref;
>> + }
>> +
>> + if (version_sort)
>> + QSORT(refs, nr, cmp_ref_versions);
>> +
>> + if (!dest && !quiet)
>> + fprintf(stderr, "From %s\n", *remote->url);
>
> Is there some subtlety here I'm missing which means that when sorting
> we'd now need to print this "From" line later (i.e. after sorting?

git stash push -u deletes untracked files

[Hosam Aly Mahmoud]

Hi,

Using Git 2.16.3 on MacOS 10.13.3, running `git stash push
--include-untracked` deletes the untracked files specified in its
arguments and creates an empty stash commit.

In the example below, I create a repository with a single file and a
single commit. Then I create two untracked files and push one of them
to the stash. Although I get an error, an empty stash commit is
generated and the specified file is deleted.

```
$ git init . && touch README && git commit -m "README"
$ touch my-file my-other-file
$ git status
On branch master
Untracked files:
my-file
my-other-file

nothing added to commit but untracked files present
$ git stash push -u my-file
Saved working directory and index state WIP on master: e89afc6 README
fatal: pathspec 'my-file' did not match any files
error: unrecognized input
$ git status
On branch master
Untracked files:
my-other-file

nothing added to commit but untracked files present
$ git stash list
stash@{0}: WIP on master: e89afc6 README
$ git stash show
$
$ ls
READMEmy-other-file
```

I tested this using git built from the latest commit on master at the
time of writing (c2a499e6c31ed613a606ffdeb5bb74ab41e9a586) and got the
same results. Could you please check it out?


Thank you,

Hosam Aly

Re: [PATCH v4 00/13] Serialized Git Commit Graph

[Derrick Stolee]

On 4/2/2018 10:46 AM, Jakub Narebski wrote:

Derrick Stolee  writes:

[...]

At one point, I was investigating these reachability indexes (I read
"SCARAB: Scaling Reachability Computation on Large Graphs" by Jihn,
Ruan, Dey, and Xu [2]) but find the question that these indexes target
to be lacking for most of the Git uses. That is, they ask the boolean
question "Can X reach Y?". More often, Git needs to answer "What is
the set of commits reachable from X but not from Y" or "Topologically
sort commits reachable from X" or "How many commits are in each part
of the symmetric difference between reachable from X or reachable from
Y?"

In the "Reachability Queries in Very Large Graphs..." by Veloso, Cerf,
Meira and Zaki FELINE-index work, authors mention SCARAB as something
that can be used in addition to FELINE-index, as a complementary data
(FELINE-SCARAB in the work, section 4.4).

I see the FELINE-index as a stronger form of generation numbers (called
also level of the vertex / node), in that it allows to negative-cut even
more, pruning paths that are known to be unreachable (or marking nodes
known to be unreachable in the "calculate difference" scenario).

Also, FELINE-index uses two integer numbers (coordinates in 2d space);
one of those indices can be the topological numbering (topological
sorting order) of nodes in the commit graph.  That would help to answer
even more Git questions.


This two-dimensional generation number is helpful for non-reachability 
queries, but is something that needs the "full" commit graph in order to 
define the value for a single commit (hence the O(N lg N) performance 
mentioned below). Generation numbers are effective while being easy to 
compute and immutable.


I wonder if FELINE was compared directly to a one-dimensional index (I 
apologize that I have not read the paper in detail, so I don't 
understand the indexes they compare against). It also appears the graphs 
they use for their tests are not commit graphs, which have a different 
shape than many of the digraphs studies by that work.


This is all to say: I would love to see an interesting study in this 
direction, specifically comparing this series' definition of generation 
numbers to the 2-dimensional system in FELINE, and on a large sample of 
commit graphs available in open-source data sets (Linux kernel, Git, 
etc.) and possibly on interesting closed-source data sets.





The case for "Can X reach Y?" is mostly for commands like 'git branch
--contains', when 'git fetch' checks for forced-updates of branches,
or when the server decides enough negotiation has occurred during a
'git fetch'. While these may be worth investigating, they also benefit
greatly from the accelerated graph walk introduced in the current
format.

I would be happy to review any effort to extend the commit-graph
format to include such indexes, as long as the performance benefits
outweigh the complexity to create them.

[2] 
http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.719.8396=rep1=pdf

The complexity of calculating FELINE index is O(|V| log(|V|) + |E|), the
storage complexity is 2*|V|.



This would be very easy to add as an optional chunk, since it can use 
one row per commit.


Thanks,
-Stolee

Re: [PATCH v4 00/13] Serialized Git Commit Graph

[Jakub Narebski]

Derrick Stolee  writes:

> On 3/30/2018 7:10 AM, Jakub Narebski wrote:
>> I hope that I am addressing the most recent version of this series.
>
> Hi Jakub. Thanks for the interest in this patch series.
>
> The most-recent version is v6 [1], but I will re-roll to v7 soon
> (after v2.17.0 is marked).
>
> [1] 
> https://public-inbox.org/git/20180314192736.70602-1-dsto...@microsoft.com/T/#u

Ooops.  Sorry about that.

>> Derrick Stolee  writes:
[...]

>> What are the assumptions about the serialized commit graph format? Does
>> it need to be:
>>   - extensible without rewriting (e.g. append-only)?
>>   - like the above, but may need rewriting for optimal performance?
>>   - extending it needs to rewrite whole file?
>>
>> Excuse me if it waas already asked and answered.
>
> It is not extensible without rewriting. Reducing write time was not a
> main goal, since the graph will be written only occasionally during
> data management phases (like 'gc' or 'repack'; this integration is not
> implemented yet).

Ah.  I thought that it could be something easily extensible in-place,
and thus easy to keep up to date on each commit.

Recalculating it on 'gc' or 'repack' is still good, especially that it
works even when there are come commits outside commit-graph, without
this information.

>>
>>> The file format has room to store generation numbers, which will be
>>> provided as a patch after this framework is merged. Generation numbers
>>> are referenced by the design document but not implemented in order to
>>> make the current patch focus on the graph construction process. Once
>>> that is stable, it will be easier to add generation numbers and make
>>> graph walks aware of generation numbers one-by-one.
>>>
>> As the serialized commit graph format is versioned, I wonder if it would
>> be possible to speed up graph walks even more by adding to it FELINE
>> index (pair of numbers) from "Reachability Queries in Very Large Graphs:
>> A Fast Refined Olnine Search Approach" (2014) - available at
>> http://openproceedings.org/EDBT/2014/paper_166.pdf
>>
>> The implementation would probably need adjustments to make it
>> unambiguous and unambiguously extensible; unless there is place for
>> indices that are local-only and need to be recalculated from scratch
>> when graph changes (to cover all graph).
>
> The chunk-based format is intended to allow extra indexes like the one
> you recommend, without needing to increase the version number. Using
> an optional chunk allows older versions of Git to read the file
> without error, since the data is "extra", and newer versions can take
> advantage of the acceleration.

That's good.

> At one point, I was investigating these reachability indexes (I read
> "SCARAB: Scaling Reachability Computation on Large Graphs" by Jihn,
> Ruan, Dey, and Xu [2]) but find the question that these indexes target
> to be lacking for most of the Git uses. That is, they ask the boolean
> question "Can X reach Y?". More often, Git needs to answer "What is
> the set of commits reachable from X but not from Y" or "Topologically
> sort commits reachable from X" or "How many commits are in each part
> of the symmetric difference between reachable from X or reachable from
> Y?"

In the "Reachability Queries in Very Large Graphs..." by Veloso, Cerf,
Meira and Zaki FELINE-index work, authors mention SCARAB as something
that can be used in addition to FELINE-index, as a complementary data
(FELINE-SCARAB in the work, section 4.4).

I see the FELINE-index as a stronger form of generation numbers (called
also level of the vertex / node), in that it allows to negative-cut even
more, pruning paths that are known to be unreachable (or marking nodes
known to be unreachable in the "calculate difference" scenario). 

Also, FELINE-index uses two integer numbers (coordinates in 2d space);
one of those indices can be the topological numbering (topological
sorting order) of nodes in the commit graph.  That would help to answer
even more Git questions.

> The case for "Can X reach Y?" is mostly for commands like 'git branch
> --contains', when 'git fetch' checks for forced-updates of branches,
> or when the server decides enough negotiation has occurred during a
> 'git fetch'. While these may be worth investigating, they also benefit
> greatly from the accelerated graph walk introduced in the current
> format.
>
> I would be happy to review any effort to extend the commit-graph
> format to include such indexes, as long as the performance benefits
> outweigh the complexity to create them.
>
> [2] 
> http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.719.8396=rep1=pdf

The complexity of calculating FELINE index is O(|V| log(|V|) + |E|), the
storage complexity is 2*|V|.

>>
>>> Here are some performance results for a copy of the Linux repository
>>> where 'master' has 704,766 reachable commits and is behind 'origin/master'
>>> by 19,610 commits.
>>>
>>> | Command  | 

Re: [PATCH v4 01/13] commit-graph: add format document

[Jakub Narebski]

Derrick Stolee  writes:

> On 3/30/2018 9:25 AM, Jakub Narebski wrote:
>> Derrick Stolee  writes:
>>
>>> +== graph-*.graph files have the following format:
>> What is this '*' here?
>
> No longer necessary. It used to be a placeholder for a hash value, but
> now the graph is stored in objects/info/commit-graph.

All right.

Excuse me replying to v4 instead of v6 of the patch series, where it
would be answered or rather made moot already.

>>
>> [...]
>>> +  The remaining data in the body is described one chunk at a time, and
>>> +  these chunks may be given in any order. Chunks are required unless
>>> +  otherwise specified.
>> Does Git need to understand all chunks, or could there be optional
>> chunks that can be safely ignored (like in PNG format)?  Though this may
>> be overkill, and could be left for later revision of the format if
>> deemed necessary.
>
> In v6, the format and design documents are edited to make clear the
> use of optional chunks, specifically for future extension without
> increasing the version number.

That's good.

>>> +CHUNK DATA:
>>> +
>>> +  OID Fanout (ID: {'O', 'I', 'D', 'F'}) (256 * 4 bytes)
>>> +  The ith entry, F[i], stores the number of OIDs with first
>>> +  byte at most i. Thus F[255] stores the total
>>> +  number of commits (N).
>> All right, it is small enough that can be required even for a very small
>> number of commits.
>>
>>> +
>>> +  OID Lookup (ID: {'O', 'I', 'D', 'L'}) (N * H bytes)
>>> +  The OIDs for all commits in the graph, sorted in ascending order.
>>> +
>>> +  Commit Data (ID: {'C', 'G', 'E', 'T' }) (N * (H + 16) bytes)
>> Do commits need to be put here in the ascending order of OIDs?
>
> Yes.
>
>> If so, this would mean that it is not possible to add information about
>> new commits by only appending data and maybe overwriting some fields, I
>> think.  You would need to do full rewrite to insert new commit in
>> appropriate place.
>
> That is the idea. This file is not updated with every new commit, but
> instead will be updated on some scheduled cleanup events. The
> commit-graph file is designed in a way to be non-critical, and not
> tied to the packfile layout. This allows flexibility for when to do
> the write.
>
> For example, in GVFS, we will write a new commit-graph when there are
> new daily prefetch packs.
>
> This could also integrate with 'gc' and 'repack' so whenever they are
> triggered the commit-graph is written as well.

I wonder if it would be possible to use existing hooks...

> Commits that do not exist in the commit-graph file will load from the
> object database as normal (after a failed lookup in the commit-graph
> file).

Ah. I thought wrongly that it would (or at least could) be something
that can be kept up to date, and extended when adding any new commit.

>>> +* The first H bytes are for the OID of the root tree.
>>> +* The next 8 bytes are for the int-ids of the first two parents
>>> +  of the ith commit. Stores value 0x if no parent in that
>>> +  position. If there are more than two parents, the second value
>>> +  has its most-significant bit on and the other bits store an array
>>> +  position into the Large Edge List chunk.
>>> +* The next 8 bytes store the generation number of the commit and
>>> +  the commit time in seconds since EPOCH. The generation number
>>> +  uses the higher 30 bits of the first 4 bytes, while the commit
>>> +  time uses the 32 bits of the second 4 bytes, along with the lowest
>>> +  2 bits of the lowest byte, storing the 33rd and 34th bit of the
>>> +  commit time.
>>> +
>>> +  Large Edge List (ID: {'E', 'D', 'G', 'E'}) [Optional]
>>> +  This list of 4-byte values store the second through nth parents for
>>> +  all octopus merges. The second parent value in the commit data stores
>>> +  an array position within this list along with the most-significant 
>>> bit
>>> +  on. Starting at that array position, iterate through this list of 
>>> int-ids
>>> +  for the parents until reaching a value with the most-significant bit 
>>> on.
>>> +  The other bits correspond to the int-id of the last parent.
>>
>> All right, that is one chunk that cannot use fixed-length records; this
>> shouldn't matter much, as we iterate only up to the number of parents
>> less two.
>
> Less one: the second "parent" column of the commit data chunk is used
> to point into this list, so (P-1) parents are in this chunk for a
> commit with P parents.

Right.

>> A question: what happens to the last list of parents?  Is there a
>> guardian value of 0x at last place?
>
> The termination condition is in the position of the last parent, since
> the most-significant bit is on. The other 31 bits contain the int-id
> of the parent.

Ah. I have misunderstood the format: I thought that first entry is
marked with most-significant bit set to 1, and all the rest to 0, while
it is last entry (last 

Re: [PATCH v4 01/13] commit-graph: add format document

[Derrick Stolee]

On 3/30/2018 9:25 AM, Jakub Narebski wrote:

Derrick Stolee  writes:


+== graph-*.graph files have the following format:

What is this '*' here?


No longer necessary. It used to be a placeholder for a hash value, but 
now the graph is stored in objects/info/commit-graph.




[...]

+  The remaining data in the body is described one chunk at a time, and
+  these chunks may be given in any order. Chunks are required unless
+  otherwise specified.

Does Git need to understand all chunks, or could there be optional
chunks that can be safely ignored (like in PNG format)?  Though this may
be overkill, and could be left for later revision of the format if
deemed necessary.


In v6, the format and design documents are edited to make clear the use 
of optional chunks, specifically for future extension without increasing 
the version number.





+
+CHUNK DATA:
+
+  OID Fanout (ID: {'O', 'I', 'D', 'F'}) (256 * 4 bytes)
+  The ith entry, F[i], stores the number of OIDs with first
+  byte at most i. Thus F[255] stores the total
+  number of commits (N).

All right, it is small enough that can be required even for a very small
number of commits.


+
+  OID Lookup (ID: {'O', 'I', 'D', 'L'}) (N * H bytes)
+  The OIDs for all commits in the graph, sorted in ascending order.
+
+  Commit Data (ID: {'C', 'G', 'E', 'T' }) (N * (H + 16) bytes)

Do commits need to be put here in the ascending order of OIDs?


Yes.


If so, this would mean that it is not possible to add information about
new commits by only appending data and maybe overwriting some fields, I
think.  You would need to do full rewrite to insert new commit in
appropriate place.


That is the idea. This file is not updated with every new commit, but 
instead will be updated on some scheduled cleanup events. The 
commit-graph file is designed in a way to be non-critical, and not tied 
to the packfile layout. This allows flexibility for when to do the write.


For example, in GVFS, we will write a new commit-graph when there are 
new daily prefetch packs.


This could also integrate with 'gc' and 'repack' so whenever they are 
triggered the commit-graph is written as well.


Commits that do not exist in the commit-graph file will load from the 
object database as normal (after a failed lookup in the commit-graph file).



+* The first H bytes are for the OID of the root tree.
+* The next 8 bytes are for the int-ids of the first two parents
+  of the ith commit. Stores value 0x if no parent in that
+  position. If there are more than two parents, the second value
+  has its most-significant bit on and the other bits store an array
+  position into the Large Edge List chunk.
+* The next 8 bytes store the generation number of the commit and
+  the commit time in seconds since EPOCH. The generation number
+  uses the higher 30 bits of the first 4 bytes, while the commit
+  time uses the 32 bits of the second 4 bytes, along with the lowest
+  2 bits of the lowest byte, storing the 33rd and 34th bit of the
+  commit time.
+
+  Large Edge List (ID: {'E', 'D', 'G', 'E'}) [Optional]
+  This list of 4-byte values store the second through nth parents for
+  all octopus merges. The second parent value in the commit data stores
+  an array position within this list along with the most-significant bit
+  on. Starting at that array position, iterate through this list of int-ids
+  for the parents until reaching a value with the most-significant bit on.
+  The other bits correspond to the int-id of the last parent.

All right, that is one chunk that cannot use fixed-length records; this
shouldn't matter much, as we iterate only up to the number of parents
less two.


Less one: the second "parent" column of the commit data chunk is used to 
point into this list, so (P-1) parents are in this chunk for a commit 
with P parents.



A question: what happens to the last list of parents?  Is there a
guardian value of 0x at last place?


The termination condition is in the position of the last parent, since 
the most-significant bit is on. The other 31 bits contain the int-id of 
the parent.


Thanks,
-Stolee

Re: [PATCH v4 00/13] Serialized Git Commit Graph

[Derrick Stolee]

On 3/30/2018 7:10 AM, Jakub Narebski wrote:

I hope that I am addressing the most recent version of this series.


Hi Jakub. Thanks for the interest in this patch series.

The most-recent version is v6 [1], but I will re-roll to v7 soon (after 
v2.17.0 is marked).


[1] 
https://public-inbox.org/git/20180314192736.70602-1-dsto...@microsoft.com/T/#u



Derrick Stolee  writes:


As promised [1], this patch contains a way to serialize the commit graph.
The current implementation defines a new file format to store the graph
structure (parent relationships) and basic commit metadata (commit date,
root tree OID) in order to prevent parsing raw commits while performing
basic graph walks. For example, we do not need to parse the full commit
when performing these walks:

* 'git log --topo-order -1000' walks all reachable commits to avoid
   incorrect topological orders, but only needs the commit message for
   the top 1000 commits.

* 'git merge-base  ' may walk many commits to find the correct
   boundary between the commits reachable from A and those reachable
   from B. No commit messages are needed.

* 'git branch -vv' checks ahead/behind status for all local branches
   compared to their upstream remote branches. This is essentially as
   hard as computing merge bases for each.

The current patch speeds up these calculations by injecting a check in
parse_commit_gently() to check if there is a graph file and using that
to provide the required metadata to the struct commit.

That's nice.

What are the assumptions about the serialized commit graph format? Does
it need to be:
  - extensible without rewriting (e.g. append-only)?
  - like the above, but may need rewriting for optimal performance?
  - extending it needs to rewrite whole file?

Excuse me if it waas already asked and answered.


It is not extensible without rewriting. Reducing write time was not a 
main goal, since the graph will be written only occasionally during data 
management phases (like 'gc' or 'repack'; this integration is not 
implemented yet).





The file format has room to store generation numbers, which will be
provided as a patch after this framework is merged. Generation numbers
are referenced by the design document but not implemented in order to
make the current patch focus on the graph construction process. Once
that is stable, it will be easier to add generation numbers and make
graph walks aware of generation numbers one-by-one.

As the serialized commit graph format is versioned, I wonder if it would
be possible to speed up graph walks even more by adding to it FELINE
index (pair of numbers) from "Reachability Queries in Very Large Graphs:
A Fast Refined Olnine Search Approach" (2014) - available at
http://openproceedings.org/EDBT/2014/paper_166.pdf

The implementation would probably need adjustments to make it
unambiguous and unambiguously extensible; unless there is place for
indices that are local-only and need to be recalculated from scratch
when graph changes (to cover all graph).


The chunk-based format is intended to allow extra indexes like the one 
you recommend, without needing to increase the version number. Using an 
optional chunk allows older versions of Git to read the file without 
error, since the data is "extra", and newer versions can take advantage 
of the acceleration.


At one point, I was investigating these reachability indexes (I read 
"SCARAB: Scaling Reachability Computation on Large Graphs" by Jihn, 
Ruan, Dey, and Xu [2]) but find the question that these indexes target 
to be lacking for most of the Git uses. That is, they ask the boolean 
question "Can X reach Y?". More often, Git needs to answer "What is the 
set of commits reachable from X but not from Y" or "Topologically sort 
commits reachable from X" or "How many commits are in each part of the 
symmetric difference between reachable from X or reachable from Y?"


The case for "Can X reach Y?" is mostly for commands like 'git branch 
--contains', when 'git fetch' checks for forced-updates of branches, or 
when the server decides enough negotiation has occurred during a 'git 
fetch'. While these may be worth investigating, they also benefit 
greatly from the accelerated graph walk introduced in the current format.


I would be happy to review any effort to extend the commit-graph format 
to include such indexes, as long as the performance benefits outweigh 
the complexity to create them.


[2] 
http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.719.8396=rep1=pdf





Here are some performance results for a copy of the Linux repository
where 'master' has 704,766 reachable commits and is behind 'origin/master'
by 19,610 commits.

| Command  | Before | After  | Rel % |
|--|||---|
| log --oneline --topo-order -1000 |  5.9s  |  0.7s  | -88%  |
| branch -vv   |  0.42s |  0.27s | -35%  |
| rev-list --all   |  6.4s  

Re: git log --oneline cannot display dates

[Jeff King]

On Mon, Apr 02, 2018 at 01:05:01PM +0100, David Hoyle wrote:

> When using the git log command with the --oneline switch, you cannot
> add the --date= switch to see the dates.

Right. --date is just about selecting which date format to show, not
whether to show one. The decision of what to show is up to the format,
and --oneline does not include a date.

> The only workaround is
> to create an alias and use the --pretty-format which doesn't provide
> the ability to colour the branch and tag information (as far as i can
> tell).

You can use "%C(auto)" to turn on auto-coloring for various components:

  git log --format='%C(auto)%h%d %s'

which reproduces the usual coloring and format of --oneline. Then you
should be able to add in your own date, colored as you wish. E.g.:

  git log --format='%C(auto)%h%d %C(blue)%ad%C(reset) %s'

If you want to use this a lot, you may want to look into the "pretty.*"
config so you can trigger it with a short name.

-Peff

Re: From Katie

[katie higgins]

Hello, this is the second times am sending you this mail and you
refused to reply to my email why? write to me on this email

Re: Possible bug in git log with date ranges

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 02 2018, David Hoyle wrote:

> Hi,
>
> Hopefully I've read the readme file correctly for submitting something
> that might be a bug.
>
> I've recently migrated projects from an old version control system
> (JEDI VCS) to Git (which I really like BTW). The way this was done was
> by extracting the files from the original database and saving them to
> a folder layout and then running git add / commit on the files. When
> using the commit command I've used the --date switch to commit the
> files using their original dates. However if I run git log with say
> --since=date it seems as if this command uses the actual date the
> commit was entered not the date given for the commit. The same seems
> to apply to the other date filtering switches.

The --date=* switch to git-commit sets the author date, but the date
narrowing options to git-log use the committer date. See if when you
run:

git log --pretty=format:"%aD - %cD"

Whether what you're getting doesn't make sense in terms of the second
date.

You can use GIT_COMMITTER_DATE to get what you want, see "DATE FORMATS"
in git-commit(1).

git log --oneline cannot display dates

[David Hoyle]

When using the git log command with the --oneline switch, you cannot
add the --date= switch to see the dates. The only workaround is
to create an alias and use the --pretty-format which doesn't provide
the ability to colour the branch and tag information (as far as i can
tell).

regards
David Hoyle.

Possible bug in git log with date ranges

[David Hoyle]

Hi,

Hopefully I've read the readme file correctly for submitting something
that might be a bug.

I've recently migrated projects from an old version control system
(JEDI VCS) to Git (which I really like BTW). The way this was done was
by extracting the files from the original database and saving them to
a folder layout and then running git add / commit on the files. When
using the commit command I've used the --date switch to commit the
files using their original dates. However if I run git log with say
--since=date it seems as if this command uses the actual date the
commit was entered not the date given for the commit. The same seems
to apply to the other date filtering switches.

Below is an example using a log alias switch shows dates in a single
line format.

Date: Mon  2 Apr 2018, Time: 12:39:21, Location: D:\Documents\RAD
Studio\Applications\Eidolon.GIT
>git lg1 --since=01/Jan/2018
* 9ce470f - (Sat Jan 20 11:54:54 2018 +) Prevent an overflow with
an Int64 for integers by not converting. - DGH2112 (HEAD ->
Development, master)
* 863988f - (Sat Jan 20 11:53:44 2018 +) Tested large hard coded
integer conversion integers - stopped conversion and left as string. -
DGH2112
* e14ecc9 - (Thu Jan 4 16:33:49 2018 +) Added new sub-option for
Hard Coded Integers to skip 'DIV 2'. - DGH2112
* 6039285 - (Wed Jan 3 21:51:08 2018 +) Bracketed CodeSiteLogging
with a CODESITE IFDEF. - DGH2112
* 651c682 - (Wed Jan 3 21:50:39 2018 +) Bracketed CodeSiteLogging
with a CODESITE IFDEF. - DGH2112
* c94ba4e - (Wed Jan 3 19:40:42 2018 +) Fixed unit name
correction. - DGH2112
* 368258b - (Wed Jan 3 14:09:48 2018 +) Separated Metric and Check
options from their sub-options (made them a simple enumerte set). -
DGH2112
* d7aa03e - (Tue Jan 2 18:10:20 2018 +) Fixed issues with disabled
metrics and checks showing up in the editor reports. - DGH2112
* f7fbe87 - (Fri Dec 29 20:59:22 2017 +) Fixed cyclometric
complexity test as the method default is 1 not 0. - DGH2112
* ab609f9 - (Thu Dec 28 22:54:45 2017 +) Added two new document
options for auto hiding checks and metrics with no issues. - DGH2112
* 6ed4786 - (Thu Dec 28 22:54:24 2017 +) Added two new document
options for auto hiding checks and metrics with no issues. - DGH2112
* e422751 - (Thu Dec 28 22:42:31 2017 +) Added two new document
options for auto hiding checks and metrics with no issues. - DGH2112
* d8a9b06 - (Thu Dec 28 18:11:11 2017 +) Updated all reference to
AddModuleCheck to AddCheck and update the method with checks. -
DGH2112
* 52bd768 - (Wed Dec 27 23:31:52 2017 +) Fixed depreciated
IsLetter().\nAdd the ability for metrics to be marked as overridden. -
DGH2112
* 0b05b16 - (Wed Dec 27 22:51:53 2017 +) Split metrics and checks. - DGH2112
* e94ab97 - (Wed Dec 27 16:13:00 2017 +) Fixed unit backward
compatibility. - DGH2112
* d6fde37 - (Wed Dec 27 15:12:53 2017 +) Fixed TParallel.For(). - DGH2112
* 9161ded - (Sat Dec 23 16:51:33 2017 +) Updated code for
VirtualTress 5.5.2. - DGH2112
* 2fa264d - (Sun Dec 17 14:02:54 2017 +) Fixed tests. - DGH2112
* 75d438f - (Sun Dec 17 12:09:04 2017 +) Fixed cyclometric
comlpexity sub options for boolean expressions. - DGH2112
* 9075a62 - (Sun Dec 17 11:56:55 2017 +) Broke a part metrics and
checks and their sub options. - DGH2112
* 76b5ec9 - (Sat Dec 16 21:01:18 2017 +) Broke a part metrics and
checks and sub options. - DGH2112
* 9807ad4 - (Tue Dec 12 20:43:04 2017 +) Tested unicode
identifiers. - DGH2112
* d831bc0 - (Sat Dec 9 20:35:05 2017 +) Fixed missing CONST in
parameters. - DGH2112
* fab9981 - (Sun Nov 26 19:22:21 2017 +) Moved some of the metric
checks so that they only work on implemented methods not declarations.
- DGH2112
* c61f460 - (Sun Nov 19 20:22:43 2017 +) Updated the special tags
to have custom fonts styles, fore and background colours. - DGH2112
* 7d1fced - (Sun Nov 12 19:47:00 2017 +) Fixed metrics for non
unit implementations. - DGH2112
* 32fe4de - (Sun Nov 12 19:45:21 2017 +) Added test for checks and
metrics. - DGH2112
* 6827a22 - (Sun Nov 12 13:40:54 2017 +) Fixed
TestGrammarForErrors. - DGH2112
* 8713e52 - (Sun Nov 12 13:07:57 2017 +) Added Doc Conflicts and
Mertrics to the list of checks. - DGH2112
* 0e5c169 - (Sun Nov 12 10:03:10 2017 +) Added an END line to
record, objects ,classes and interfaces. - DGH2112
* ac8091c - (Sun Nov 5 20:22:23 2017 +) Updated the special tags
to have custom fonts styles, fore and background colours. - DGH2112
* 10dacab - (Sun Nov 5 16:19:31 2017 +) Fixed tests for Program,
Library and Packages where Uses does not have Interface and
Implementation sections. - DGH2112
* e7996fa - (Sun Nov 5 16:17:44 2017 +) Ensured metrics are
expanded. - DGH2112
* 3f6d401 - (Sun Nov 5 16:17:25 2017 +) Added checks for Empty
FOR, WHILE, REPEAT, and BEGIN END. - DGH2112
* 579ebc8 - (Fri Nov 3 19:07:44 2017 +) Fixed capitalised USES
clause. - DGH2112
* 

Re: [PATCH v3 0/3] add -p: select individual hunk lines

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 02 2018, Phillip Wood wrote:

> On 31/03/18 20:20, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Fri, Mar 30 2018, Phillip Wood wrote:
>>
>>> On 29/03/18 19:32, Junio C Hamano wrote:
 Phillip Wood  writes:

> From: Phillip Wood 
>
> Since v2 I've updated the patches to use '-' instead of '^' to invert
> the selection to match the rest of add -i and clean -i.
>
> These patches build on top of the recount fixes in [1]. The commit
> message for the first patch describes the motivation:
>
> "When I end up editing hunks it is almost always because I want to
> stage a subset of the lines in the hunk. Doing this by editing the
> hunk is inconvenient and error prone (especially so if the patch is
> going to be reversed before being applied). Instead offer an option
> for add -p to stage individual lines. When the user presses 'l' the
> hunk is redrawn with labels by the insertions and deletions and they
> are prompted to enter a list of the lines they wish to stage. Ranges
> of lines may be specified using 'a-b' where either 'a' or 'b' may be
> omitted to mean all lines from 'a' to the end of the hunk or all lines
> from 1 upto and including 'b'."

 I haven't seen any review comments on this round, and as I am not a
 heavy user of "add -i" interface (even though I admit that I
 originally wrote it), I haven't had a chance to exercise the code
 myself in the two weeks since the patches have been queued in my
 tree.

 I am inclihned to merge them to 'next' soonish, but please stop me
 if anybody (including the original author) has further comments.

 Thanks.

>>> Hi Junio, if no one else has any comments, then I think it's ready for
>>> next. I've not used this latest incarnation much but I've used the
>>> previous versions quite a bit.
>
> Ah it seems I spoke too soon.
>
> Thanks for taking a look at this Ævar
>
>> First of all thinks for working on this. Something like this is a
>> feature I've long wanted to have and have just been manually using edit.
>>
>> As for the code, one comment: For reasons of avoiding something like the
>> 2.17.0-rc* bug I just sent a patch for, I think you should change your
>> use of the implicit $_ to something where you explicitly create lexical
>> variables instead.
>>
>> It's bad style in Perl to use $_ for anything except a one-liner, and
>> similar to the $1 bug with your other patch, you'll get buggy code
>> (regardless of your use of local $_) if one of the functions you're
>> calling in these >10 line for-loops starts doing something to set $_
>> itself, as demonstrated by:
>>
>> $ perl -wE 'sub foo { local $_; for (1..3) { bar(); say } } sub bar { $_ 
>> = $_ ** 2; } foo()'
>> 1
>> 4
>> 9
>>
>> Let's just name these variables, even if it wasn't for that caveat it
>> would still be a good idea, since for any non-trivial use of $_ you've
>> got to mentally keep track of what set $_ where, so it's hard to read.
>
> Right, I'll use lexical variables.
>
>>
>> As for the implementation, I *want* to love this, but it seems the way
>> it works is just fatally flawed, consider. *The* use-case I've had for
>> something like this (maybe yours differs?) is something where I do e.g.:
>
> I've used it for selecting a subset of additions or deletions when my
> work has run ahead of a logical commit boundary. I've also used it in
> cases such as
>
>   -original
>   +modified
>   +new stuff
>
> To separate the modification from the addition of new stuff, but I've
> not used it on a list of modifications as in your example.

Right. I was wrong in saying that it wouldn't work as expected for hunks
with removed/added lines, but only for a subset of those cases.

>> $ perl -pi -e 's/git/Git/g' README.md
>>
>> Which gives me (among other things):
>>
>> -See [Documentation/gittutorial.txt][] to get started, then see
>> -[Documentation/giteveryday.txt][] for a useful minimum set of commands, 
>> and
>> -Documentation/git-.txt for documentation of each command.
>> -If git has been correctly installed, then the tutorial can also be
>> -read with `man gittutorial` or `git help tutorial`, and the
>> -documentation of each command with `man git-` or `git help
>> +See [Documentation/Gittutorial.txt][] to get started, then see
>> +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, 
>> and
>> +Documentation/Git-.txt for documentation of each command.
>> +If Git has been correctly installed, then the tutorial can also be
>> +read with `man Gittutorial` or `Git help tutorial`, and the
>> +documentation of each command with `man Git-` or `Git help
>>
>> Which to me, is a perfect use-case for this feature. Here I
>> hypothetically want to change "git" to "Git" in prose, so I only want to
>> change that "If git has been" 

Re: [PATCH v3 0/3] add -p: select individual hunk lines

[Phillip Wood]

On 31/03/18 20:20, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Mar 30 2018, Phillip Wood wrote:
> 
>> On 29/03/18 19:32, Junio C Hamano wrote:
>>> Phillip Wood  writes:
>>>
 From: Phillip Wood 

 Since v2 I've updated the patches to use '-' instead of '^' to invert
 the selection to match the rest of add -i and clean -i.

 These patches build on top of the recount fixes in [1]. The commit
 message for the first patch describes the motivation:

 "When I end up editing hunks it is almost always because I want to
 stage a subset of the lines in the hunk. Doing this by editing the
 hunk is inconvenient and error prone (especially so if the patch is
 going to be reversed before being applied). Instead offer an option
 for add -p to stage individual lines. When the user presses 'l' the
 hunk is redrawn with labels by the insertions and deletions and they
 are prompted to enter a list of the lines they wish to stage. Ranges
 of lines may be specified using 'a-b' where either 'a' or 'b' may be
 omitted to mean all lines from 'a' to the end of the hunk or all lines
 from 1 upto and including 'b'."
>>>
>>> I haven't seen any review comments on this round, and as I am not a
>>> heavy user of "add -i" interface (even though I admit that I
>>> originally wrote it), I haven't had a chance to exercise the code
>>> myself in the two weeks since the patches have been queued in my
>>> tree.
>>>
>>> I am inclihned to merge them to 'next' soonish, but please stop me
>>> if anybody (including the original author) has further comments.
>>>
>>> Thanks.
>>>
>> Hi Junio, if no one else has any comments, then I think it's ready for
>> next. I've not used this latest incarnation much but I've used the
>> previous versions quite a bit.

Ah it seems I spoke too soon.

Thanks for taking a look at this Ævar

> First of all thinks for working on this. Something like this is a
> feature I've long wanted to have and have just been manually using edit.
> 
> As for the code, one comment: For reasons of avoiding something like the
> 2.17.0-rc* bug I just sent a patch for, I think you should change your
> use of the implicit $_ to something where you explicitly create lexical
> variables instead.
> 
> It's bad style in Perl to use $_ for anything except a one-liner, and
> similar to the $1 bug with your other patch, you'll get buggy code
> (regardless of your use of local $_) if one of the functions you're
> calling in these >10 line for-loops starts doing something to set $_
> itself, as demonstrated by:
> 
> $ perl -wE 'sub foo { local $_; for (1..3) { bar(); say } } sub bar { $_ 
> = $_ ** 2; } foo()'
> 1
> 4
> 9
> 
> Let's just name these variables, even if it wasn't for that caveat it
> would still be a good idea, since for any non-trivial use of $_ you've
> got to mentally keep track of what set $_ where, so it's hard to read.

Right, I'll use lexical variables.

> 
> As for the implementation, I *want* to love this, but it seems the way
> it works is just fatally flawed, consider. *The* use-case I've had for
> something like this (maybe yours differs?) is something where I do e.g.:

I've used it for selecting a subset of additions or deletions when my
work has run ahead of a logical commit boundary. I've also used it in
cases such as

-original
+modified
+new stuff

To separate the modification from the addition of new stuff, but I've
not used it on a list of modifications as in your example.

> $ perl -pi -e 's/git/Git/g' README.md
> 
> Which gives me (among other things):
> 
> -See [Documentation/gittutorial.txt][] to get started, then see
> -[Documentation/giteveryday.txt][] for a useful minimum set of commands, 
> and
> -Documentation/git-.txt for documentation of each command.
> -If git has been correctly installed, then the tutorial can also be
> -read with `man gittutorial` or `git help tutorial`, and the
> -documentation of each command with `man git-` or `git help
> +See [Documentation/Gittutorial.txt][] to get started, then see
> +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, 
> and
> +Documentation/Git-.txt for documentation of each command.
> +If Git has been correctly installed, then the tutorial can also be
> +read with `man Gittutorial` or `Git help tutorial`, and the
> +documentation of each command with `man Git-` or `Git help
> 
> Which to me, is a perfect use-case for this feature. Here I
> hypothetically want to change "git" to "Git" in prose, so I only want to
> change that "If git has been" line, the rest are all references to
> filenames or command names.
> 
> So I would manually edit the hunk via "e" to:
> 
>  See [Documentation/gittutorial.txt][] to get started, then see
>  [Documentation/giteveryday.txt][] for a useful minimum set of commands, 
> and
>  

Re: [RFC] git clone add option to track all remote branches

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 02 2018, Yubin Ruan wrote:

> I am writing to ask that whether or not you think will be appropriate to add
> an option to "git clone" so that whenever a repo is cloned, branches are
> created automatically to track corresponding remote branches. (or is there any
> equivelant option?)
>
> You can obviously do this by a bash command like this
>
> git branch -r | grep -v '\->' | while read remote; do git branch --track 
> "${remote#origin/}" "$remote"; done

Aside from this specific suggestion, we should be careful when adding
more special snowflakes to git-clone that aren't available through
git-fetch or via after the fact config, such as git clone
--single-branch which has no fetch equivalent (i.e. in finding out what
the remote HEAD is).

Actually now that I mention that it occurs to me that what you want
could just be a special case of generalizing our existing
--single-branch feature. I.e. we would clone repos as:

[remote "origin"]
url = g...@github.com:git/git.git
fetch = +refs/heads/*:refs/remotes/origin/*
branch = $symref(HEAD):refs/remotes/origin/HEAD:TRACK

Or some other such syntax to create a refs/heads/master from the remote
HEAD symref if it doesn't exist (note the lack of a +), then for your
feature:

branch = refs/heads/*:refs/remotes/origin/*:TRACK

But you could also do:

branch = +refs/heads/origin/*:refs/remotes/origin/*:TRACK

Or whatever. I don't know what syntax would be sensible, but something
like this is a missing feature of our existing --single-branch feature,
and because of that you can only get its semantics by re-cloning.

Re: Need help debugging issue in git

[Johannes Sixt]

Am 02.04.2018 um 02:36 schrieb Robert Dailey:

I'm struggling with a bug that I found introduced in git v2.13.2. The
bug was not reproducible in v2.13.1. The issue is that using arguments
like "@{-1}" to aliases causes those curly braces to be removed, so
once the command is executed after alias processing the argument looks
like "@-1". This breaks any aliases you have that wrap `git log` and
such. I originally opened the bug on the Git for Windows project
(since I use Git mostly on Windows):

https://github.com/git-for-windows/git/issues/1220

...

Here is the alias being used for a test:

[alias]
 lgtest = !git log --oneline \"$@\"

And here is the command I invoke for the test:

$ git lgtest @{-1}

I should get logs for the previously-checked-out branch.

When `prepare_shell_cmd()` is called in run-command.c, it gets expanded like so:

+ [0] "sh" const char *
+ [1] "-c" const char *
+ [2] "git log --oneline \"$@\" \"$@\"" const char *
+ [3] "git log --oneline \"$@\"" const char *
+ [4] "@{-1}" const char *

With my modifications (again, patch inline below) I get this:

+ [0] "sh" const char *
+ [1] "-c" const char *
+ [2] "git log --oneline \"$@\"" const char *
+ [3] "@{-1}" const char *

The second version looks much better.


But this is wrong. Try this on the command line:

  sh -c 'echo "$@"' a b c

Notice how this prints only 'b c', not 'a b c'. The reason is that the 
argument 'a' is treated like a "script" name, i.e. what you get for 
"$0", and 'b' and 'c' as the actual arguments to the "script".


That is, you must fill in some dummy "script" name at slot [3], and 
run_command chooses to put the alias text there.



I think the constant nesting of
commands inside each other that the first version does is somehow
causing curly braces to be removed. I don't understand enough about
shell processing to know why it would do this.


Some shells expand the curly braces. They must get lost somewhere by one 
of the two shell invocations that happen on the way.


BTW, you don't happen to have a file named '@-1' in your directory, most 
likely by accident?


-- Hannes

Re: [PATCH] ls-remote: create option to sort by versions

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 02 2018, Harald Nordgren wrote:

> Create the options '-V ' and '--version-sort' to sort
> 'git ls-remote' output by version semantics. This is useful e.g. for
> the Go repository after the release of version 1.10, where otherwise
> v1.10 is sorted before v1.2. See:
>
>   $ git ls-remote -t https://go.googlesource.com/go
>   ...
>   205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1
>   d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1
>   1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2
>   bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10
>   ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1
>   9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1
>   594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2
>   5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1
>   20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2
>   1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2
>   46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3
>   402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2
>   9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1
>   ...

This is a sensible thing to want, but why not follow the UI we have for
this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course
ls-remote doesn't just show tags, so maybe we'd want --tag-sort=
and --ignore-tag-case or something, but the rest should be equivalent,
no?

> [...]
> @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const 
> char *prefix)
>   if (transport_disconnect(transport))
>   return 1;
>
> - if (!dest && !quiet)
> - fprintf(stderr, "From %s\n", *remote->url);
>   for ( ; ref; ref = ref->next) {
>   if (!check_ref_type(ref, flags))
>   continue;
>   if (!tail_match(pattern, ref->name))
>   continue;
> + REALLOC_ARRAY(refs, nr + 1);
> + refs[nr++] = ref;
> + }
> +
> + if (version_sort)
> + QSORT(refs, nr, cmp_ref_versions);
> +
> + if (!dest && !quiet)
> + fprintf(stderr, "From %s\n", *remote->url);

Is there some subtlety here I'm missing which means that when sorting
we'd now need to print this "From" line later (i.e. after sorting?

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

[Sergey Organov]

Hi Johannes,

Johannes Schindelin  writes:

> Hi Sergey,
>

[...]

> In the parlance of your RFC v2, where you start with this history (which I
> translated into the left-to-right notation that is used in pretty much all
> of Git's own documentation about interactive rebases, which you apparently
> either did not read, or chose *not* to imitate, creating yet another
> unnecessary diversion):
>
>
> - B1
>  \
> - B2 - M

First, it should rather be:

- B1
\
 M
/ 
- B2

as RFC presents essentially symmetric approach and I'd like it to be
explicit. Representation in RFC simply saves some vertical
space: 

  M
 / \
B1  B2

Another reason to use it is that I liked to somehow indicate that this
is about abstract DAG representation of Git history, not to be confused
with some actual practical Git history. So that, for example, the reader
can't be tempted to even try to assume that M has been necessarily
created by "git merge" operation in the first place.

That said, I'm sorry if it upsets you. I'll stick to your preferred
notation below.

>
> You now insert U1 and U2 with trees identical to M:
>
> - B1 - U1
>   \
> - B2 - U2 - M

 - B1 - U1 
  \
   UM
  / 
 - B2 - U2

_YES_. You've slightly screwed RFC as UM is not M anymore, having
different parents, but otherwise it's still right.

> So U1 is essentially B2 cherry-picked on top of B1, and U2 is essentially
> B1 cherry-picked on top of B2.

_NO_. No any cherry-picking has been involved, and I see absolutely no
reason to pretend there has, except to intentionally make otherwise
simple thing look tricky.

U1 tree is still M tree, and U2 tree is still M tree, and UM tree is
still M tree. That's what actually matters from RFC POV.

> These U1/U2 commits are now to be cherry-picked on top of the rebased B1'
> and B2'. I spare you more diagrams, you get the idea.

_YES_. Exactly 2 cherry-picks.

> Now, the changes in U1/U2 *are* the changes of the merge parents, that's
> how they were constructed.

Either _YES_, or _NO_, depending on the exact meaning of the term "the
changes of the merge parents" you've used, but I suspect it's _NO_,
taking into account your further inferences.

The U1/U2 are constructed by simply duplicating the tree of the original
merge commit M and thus they represent the changes _to_ the merge
parents B1/B2 introduced by M, and not the changes "_of_ the merge
parents" B1/B2, provided the latter meant to have some relation to the
changes introduced by the merge parents B1/B2 themselves.

>
> Since they repeat what B1 and B2 are about,

_NO_, they do not repeat what B1 and B2 are about at all. They rather
represent what M is about. In other words, whatever B1 and B2 are about,
the RFC method doesn't care.

And as this is fundamental misinterpretation of the RFC on your side, it
starts to be big _NO_ from now on...

> and since B1'/B2' means they are rebased, and since U1'/U2' are *also*
> rebased, but independently...
>
>   ...  you essentially have to rebase *every parent's changes
>   twice*.

_NO_. U1' is rebase of U1 (on top of B1'), and U2' is rebase of U2 (on
top of B2'). Each of U1/U2 is rebased only once.

> The answer "No" to this is... astonishing.

It's still _NO_, sorry.

In fact, I could have said _NO_ the first time you started to assign
some arbitrary "meaning" to the commits, as RFC is about somewhat formal
proof of the method, using already well-known operations on the DAG, and
to criticize the RFC, you need to either find and show a _formal_
mistake somewhere in the proof logic, or to show a use-case where it
fails, as you did for RFC v1. Assigning arbitrary "meaning" to the DAG
nodes and operations on them won't do the trick, sorry.

I'd like to reach some agreement on formal correctness of the RFC first,
and then discuss the meanings, the implementations, and other
consequences based on well-established formal base.

-- Sergey