Help

[Senoir Aeron]

Account been flagged 

How to repair 

nLiz

Re: [PATCH 31/41] wt-status: convert two uses of EMPTY_TREE_SHA1_HEX

[brian m. carlson]

On Tue, Apr 24, 2018 at 12:03:35PM +0200, Martin Ågren wrote:
> Just a thought: Maybe it would make sense to have a function
> `oid_hex_empty_tree()` or similar to replace the
> oid_to_hex[_r](the_hash_algo->empty_tree) idiom. It would help avoid the
> buffer here, but also get rid of a few instances of code peeking into
> the_hash_algo. I dunno.

At first I wasn't going to include this change in the series, but then I
thought about what a good idea it was and decided to redo the
corresponding patches.  So I hope to have a v2 out tomorrow with this
change (and the rest of them) in it.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH] format-patch: make cover letters always text/plain

[Eric Sunshine]

On Mon, Apr 30, 2018 at 8:02 PM, brian m. carlson
 wrote:
> When formatting a series of patches using --attach and --cover-letter,
> the cover letter lacks the closing MIME boundary, violating RFC 2046.
> Certain clients, such as Thunderbird, discard the message body in such a
> case.
>
> Since the cover letter is just one part and sending it as
> multipart/mixed is not very useful, always emit it as text/plain,
> avoiding the boundary problem altogether.
>
> Reported-by: Patrick Hemmer 
> Signed-off-by: brian m. carlson 
> ---
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> @@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with 
> --attach' '
> +test_expect_success 'format-patch --attach cover-letter only is 
> non-multipart' '
> +   test_when_finished "rm -r patches" &&
> +   git format-patch -o patches --cover-letter --attach=mimemime 
> --base=HEAD~ -1 &&

Nit: "rm -rf" would be a bit more robust against git-format-patch
somehow crashing before creating the "patches" directory.

> +   ! egrep "^--+mimemime" patches/*.patch &&
> +   egrep "^--+mimemime$" patches/0001*.patch >output &&
> +   test_line_count = 2 output &&
> +   egrep "^--+mimemime--$" patches/0001*.patch >output &&
> +   test_line_count = 1 output
> +'

Re: [PATCH 0/9] get_short_oid UI improvements

[brian m. carlson]

On Mon, Apr 30, 2018 at 10:07:25PM +, Ævar Arnfjörð Bjarmason wrote:
> I started out just wanting to do 04/09 so I'd get prettier output, but
> then noticed that ^{tag}, ^{commit}< ^{blob} and ^{tree} didn't behave
> as expected with the disambiguation output, and that core.disambiguate
> had never been documented.
> 
> Ævar Arnfjörð Bjarmason (9):
>   sha1-name.c: remove stray newline
>   sha1-array.h: align function arguments
>   sha1-name.c: move around the collect_ambiguous() function
>   get_short_oid: sort ambiguous objects by type, then SHA-1
>   get_short_oid: learn to disambiguate by ^{tag}
>   get_short_oid: learn to disambiguate by ^{blob}
>   get_short_oid / peel_onion: ^{tree} should mean tree, not treeish
>   get_short_oid / peel_onion: ^{tree} should mean commit, not commitish
>   config doc: document core.disambiguate

As mentioned, I'm a bit unsure that patches 7 and 8 are entirely
correct.  I've mostly convinced myself that they are after looking at
peel_onion, but I'm still harboring lingering doubts for some reason.

The rest of the series looked fine to me.  Thanks for cleaning up my
stray newline.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 7/9] get_short_oid / peel_onion: ^{tree} should mean tree, not treeish

[brian m. carlson]

On Mon, Apr 30, 2018 at 10:07:32PM +, Ævar Arnfjörð Bjarmason wrote:
> diff --git a/sha1-name.c b/sha1-name.c
> index 023f9471a8..b61c0558d9 100644
> --- a/sha1-name.c
> +++ b/sha1-name.c
> @@ -970,7 +970,7 @@ static int peel_onion(const char *name, int len, struct 
> object_id *oid,
>   else if (expected_type == OBJ_TAG)
>   lookup_flags |= GET_OID_TAG;
>   else if (expected_type == OBJ_TREE)
> - lookup_flags |= GET_OID_TREEISH;
> + lookup_flags |= GET_OID_TREE;
>   else if (expected_type == OBJ_BLOB)
>   lookup_flags |= GET_OID_BLOB;
>  

I was concerned at first that this might lead to some sort of wrong
behavior when we do something like "git rev-parse v2.17.0^{tree}", but
looking at the code I've mostly convinced myself that that should still
work.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

[PATCH] format-patch: make cover letters always text/plain

[brian m. carlson]

When formatting a series of patches using --attach and --cover-letter,
the cover letter lacks the closing MIME boundary, violating RFC 2046.
Certain clients, such as Thunderbird, discard the message body in such a
case.

Since the cover letter is just one part and sending it as
multipart/mixed is not very useful, always emit it as text/plain,
avoiding the boundary problem altogether.

Reported-by: Patrick Hemmer 
Signed-off-by: brian m. carlson 
---
 builtin/log.c   | 2 +-
 log-tree.c  | 7 ---
 log-tree.h  | 3 ++-
 t/t4014-format-patch.sh | 9 +
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 71f68a3e4f..24868ed070 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1019,7 +1019,7 @@ static void make_cover_letter(struct rev_info *rev, int 
use_stdout,
open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", 
rev, quiet))
return;
 
-   log_write_email_headers(rev, head, _subject, _8bit_cte);
+   log_write_email_headers(rev, head, _subject, _8bit_cte, 
0);
 
for (i = 0; !need_8bit_cte && i < nr; i++) {
const char *buf = get_commit_buffer(list[i], NULL);
diff --git a/log-tree.c b/log-tree.c
index d1c0bedf24..9f5eb346a4 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -362,7 +362,8 @@ void fmt_output_email_subject(struct strbuf *sb, struct 
rev_info *opt)
 
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 const char **extra_headers_p,
-int *need_8bit_cte_p)
+int *need_8bit_cte_p,
+int maybe_multipart)
 {
const char *extra_headers = opt->extra_headers;
const char *name = oid_to_hex(opt->zero_commit ?
@@ -385,7 +386,7 @@ void log_write_email_headers(struct rev_info *opt, struct 
commit *commit,
   opt->ref_message_ids->items[i].string);
graph_show_oneline(opt->graph);
}
-   if (opt->mime_boundary) {
+   if (opt->mime_boundary && maybe_multipart) {
static char subject_buffer[1024];
static char buffer[1024];
struct strbuf filename =  STRBUF_INIT;
@@ -610,7 +611,7 @@ void show_log(struct rev_info *opt)
 
if (cmit_fmt_is_mail(opt->commit_format)) {
log_write_email_headers(opt, commit, _headers,
-   _8bit_cte);
+   _8bit_cte, 1);
ctx.rev = opt;
ctx.print_email_subject = 1;
} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
diff --git a/log-tree.h b/log-tree.h
index deba035187..e668628074 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -27,7 +27,8 @@ void format_decorations_extended(struct strbuf *sb, const 
struct commit *commit,
 void show_decorations(struct rev_info *opt, struct commit *commit);
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 const char **extra_headers_p,
-int *need_8bit_cte_p);
+int *need_8bit_cte_p,
+int maybe_multipart);
 void load_ref_decorations(struct decoration_filter *filter, int flags);
 
 #define FORMAT_PATCH_NAME_MAX 64
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 482112ca33..83c596a842 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1661,6 +1661,15 @@ test_expect_success 'format-patch --base with --attach' '
test_write_lines 1 2 >expect &&
test_cmp expect actual
 '
+test_expect_success 'format-patch --attach cover-letter only is non-multipart' 
'
+   test_when_finished "rm -r patches" &&
+   git format-patch -o patches --cover-letter --attach=mimemime 
--base=HEAD~ -1 &&
+   ! egrep "^--+mimemime" patches/*.patch &&
+   egrep "^--+mimemime$" patches/0001*.patch >output &&
+   test_line_count = 2 output &&
+   egrep "^--+mimemime--$" patches/0001*.patch >output &&
+   test_line_count = 1 output
+'
 
 test_expect_success 'format-patch --pretty=mboxrd' '
sp=" " &&

Re: [PATCH 00/41] object_id part 13

[brian m. carlson]

On Mon, Apr 30, 2018 at 08:03:12PM +0200, Duy Nguyen wrote:
> On Tue, Apr 24, 2018 at 1:39 AM, brian m. carlson
>  wrote:
> > [0] I can synthesize blobs, trees, and commits, but things are currently
> > totally broken, which is, I suppose, to be expected.
> 
> Yup. I was tired and bored so I went playing with the new hash.
> Writing and reading blobs (with hash-object/cat-file) were relatively
> easy after fixing up fill_sha1_path and get_oid_basic). Then I worked
> my way up to update-index/ls-files so that I could make trees with
> write-tree. And I hit the first road block: struct ondisk_cache_entry
> hard codes hash size so I would need to re-organize the code for more
> flexibility (or even redesign the file format if I want to keep byte
> alignment). Eck...
> 
> I guess I'll be helping review this series instead :D

Yeah, I have code to fix that, but it's ugly.

You can see the work on part2 and part3 of the test fixes, plus the
fixes for all of that stuff on my object-id-part14 branch.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH v4 10/10] commit-graph.txt: update design document

[Jakub Narebski]

Derrick Stolee  writes:

> We now calculate generation numbers in the commit-graph file and use
> them in paint_down_to_common().
>
> Expand the section on generation numbers to discuss how the three
> special generation numbers GENERATION_NUMBER_INFINITY, _ZERO, and
> _MAX interact with other generation numbers.
>
> Signed-off-by: Derrick Stolee 

Looks good.

> ---
>  Documentation/technical/commit-graph.txt | 30 +++-
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/technical/commit-graph.txt 
> b/Documentation/technical/commit-graph.txt
> index 0550c6d0dc..d9f2713efa 100644
> --- a/Documentation/technical/commit-graph.txt
> +++ b/Documentation/technical/commit-graph.txt
> @@ -77,6 +77,29 @@ in the commit graph. We can treat these commits as having 
> "infinite"
>  generation number and walk until reaching commits with known generation
>  number.
>  
> +We use the macro GENERATION_NUMBER_INFINITY = 0x to mark commits not
> +in the commit-graph file. If a commit-graph file was written by a version
> +of Git that did not compute generation numbers, then those commits will
> +have generation number represented by the macro GENERATION_NUMBER_ZERO = 0.
> +
> +Since the commit-graph file is closed under reachability, we can guarantee
> +the following weaker condition on all commits:
> +
> +If A and B are commits with generation numbers N amd M, respectively,
> +and N < M, then A cannot reach B.
> +
> +Note how the strict inequality differs from the inequality when we have
> +fully-computed generation numbers. Using strict inequality may result in
> +walking a few extra commits,

The linux kernel commit graph has maximum of 513 commits sharing the
same generation number, but is is 5.43 commits sharing the same
generation number on average, with standard deviation 10.70; median is
even lower: it is 2, with 5.35 median absolute deviation (MAD).

So on average it would be a few extra commits.  Right.

>   but the simplicity in dealing with commits
> +with generation number *_INFINITY or *_ZERO is valuable.

As I wrote before, handling those corner cases in more complicated, but
not that complicated.  We could simply use stronger condition if both
generation numbers are ordinary generation numbers, and weaker condition
when at least one generation number has one of those special values.

> +
> +We use the macro GENERATION_NUMBER_MAX = 0x3FFF to for commits whose
> +generation numbers are computed to be at least this value. We limit at
> +this value since it is the largest value that can be stored in the
> +commit-graph file using the 30 bits available to generation numbers. This
> +presents another case where a commit can have generation number equal to
> +that of a parent.

Ordinary generation numbers, where stronger condition holds, are those
between GENERATION_NUMBER_ZERO < gen(C) < GENERATION_NUMBER_MAX.

> +
>  Design Details
>  --
>  
> @@ -98,17 +121,12 @@ Future Work
>  - 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 all generation numbers will be marked as
> -  0 (or "uncomputed"). A later patch will include this calculation.
> -

Good.

>  - After computing and storing generation numbers, we must make graph
>walks aware of generation numbers to gain the performance benefits they
>enable. This will mostly be accomplished by swapping a commit-date-ordered
>priority queue with one ordered by generation number. The following
> -  operations are important candidates:
> +  operation is an important candidate:
>  
> -- paint_down_to_common()
>  - 'log --topo-order'

Another possible candidates:

   - remove_redundant() - see comment in previous patch
   - still_interesting() - where Git uses date slop to stop walking
 too far

>  
>  - Currently, parse_commit_gently() requires filling in the root tree

One important issue left is handling features that change view of
project history, and their interaction with commit-graph feature.

What would happen, if we turn on commit-graph feature, generate commit
graph file, and then:

  * use graft file or remove graft entries to cut history, or remove cut
or join two [independent] histories.
  * use git-replace mechanims to do the same
  * in shallow clone, deepen or shorten the clone

What would happen if without re-generating commit-graph file (assuming
tha Git wouldn't do it for us), we run some feature that makes use of
commit-graph data:

  - git branch --contains
  - git tag --contains
  - git rev-list A..B

Best,
-- 
Jakub Narębski

Re: [PATCH 8/9] get_short_oid / peel_onion: ^{tree} should mean commit, not commitish

[Eric Sunshine]

On Mon, Apr 30, 2018 at 6:07 PM, Ævar Arnfjörð Bjarmason
 wrote:
> get_short_oid / peel_onion: ^{tree} should mean commit, not commitish

s/tree/commit/

> Continue the untangling of peel disambiguation syntax. Before this
> e8f2^{commit} would show the v2.17.0 tag, but now it'll just show
> ambiguous commits:
>
> $ git rev-parse e8f2^{commit}
> error: short SHA1 e8f2 is ambiguous
> hint: The candidates are:
> hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached 
> HEAD abbreviated object name
> hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from 
> jeffhostetler/jeffhostetler/register_rename_src
> hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for 
> multiple remote.url entries
> [...]
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 

Re: [PATCH v4 09/10] merge: check config before loading commits

[Jakub Narebski]

Derrick Stolee  writes:

> Now that we use generation numbers from the commit-graph, we must
> ensure that all commits that exist in the commit-graph are loaded
> from that file instead of from the object database. Since the
> commit-graph file is only checked if core.commitGraph is true, we
> must check the default config before we load any commits.
>
> In the merge builtin, the config was checked after loading the HEAD
> commit. This was due to the use of the global 'branch' when checking
> merge-specific config settings.
>
> Move the config load to be between the initialization of 'branch' and
> the commit lookup.

Sidenote: I wonder why reading config was postponed to later in the
command lifetime... I guess it was to avoid having to read config if
HEAD was invalid.

>
> Without this change, a fast-forward merge would hit a BUG("bad
> generation skip") statement in commit.c during paint_down_to_common().
> This is because the HEAD commit would be loaded with "infinite"
> generation but then reached by commits with "finite" generation
> numbers.

I guess this is because we avoid re-parsing objects at all costs; we
want to avoid re-reading commit graph too.

>
> Add a test to t5318-commit-graph.sh that exercises this code path to
> prevent a regression.

I would prefer if this commit was put earlier in the series, to avoid
having broken Git (and thus a possibility of problems when bisecting) in
between those two commits.

>
> Signed-off-by: Derrick Stolee 
> ---
>  builtin/merge.c | 7 ---
>  t/t5318-commit-graph.sh | 9 +
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 5e5e4497e3..b819756946 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1148,14 +1148,15 @@ int cmd_merge(int argc, const char **argv, const char 
> *prefix)
>   branch = branch_to_free = resolve_refdup("HEAD", 0, _oid, NULL);
>   if (branch)
>   skip_prefix(branch, "refs/heads/", );
> +
> + init_diff_ui_defaults();
> + git_config(git_merge_config, NULL);
> +
>   if (!branch || is_null_oid(_oid))
>   head_commit = NULL;
>   else
>   head_commit = lookup_commit_or_die(_oid, "HEAD");
>  
> - init_diff_ui_defaults();
> - git_config(git_merge_config, NULL);
> -

Good.

>   if (branch_mergeoptions)
>   parse_branch_merge_options(branch_mergeoptions);
>   argc = parse_options(argc, argv, prefix, builtin_merge_options,
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index a380419b65..77d85aefe7 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -221,4 +221,13 @@ test_expect_success 'write graph in bare repo' '
>  graph_git_behavior 'bare repo with graph, commit 8 vs merge 1' bare 
> commits/8 merge/1
>  graph_git_behavior 'bare repo with graph, commit 8 vs merge 2' bare 
> commits/8 merge/2
>  
> +test_expect_success 'perform fast-forward merge in full repo' '
> + cd "$TRASH_DIRECTORY/full" &&
> + git checkout -b merge-5-to-8 commits/5 &&
> + git merge commits/8 &&
> + git show-ref -s merge-5-to-8 >output &&
> + git show-ref -s commits/8 >expect &&
> + test_cmp expect output
> +'

All right.  (though I wonder if this tests catches all problems where
BUG("bad generation skip") could have been encountered.

> +
>  test_done

Best,
-- 
Jakub Narębski

Re: [PATCH 0/9] get_short_oid UI improvements

[Stefan Beller]

On Mon, Apr 30, 2018 at 3:07 PM, Ævar Arnfjörð Bjarmason
 wrote:
> I started out just wanting to do 04/09 so I'd get prettier output, but
> then noticed that ^{tag}, ^{commit}< ^{blob} and ^{tree} didn't behave
> as expected with the disambiguation output, and that core.disambiguate
> had never been documented.
>

This whole series, including the comment in which you wonder
if the code is overly smart, is

Reviewed-by: Stefan Beller 

Re: [PATCH 0/4] subtree: move out of contrib

[Avery Pennarun]

On Mon, Apr 30, 2018 at 6:21 PM, Ævar Arnfjörð Bjarmason
 wrote:
> Pretty clear it's garbage data, unless we're to believe that the
> relative interest of submodules in the US, Germany and Sweden is 51, 64
> & 84, but 75, 100 and 0 for subtree.

Oh yeah, Swedish people hate git-subtree.  Nobody knows why.

Avery

Re: [PATCH 0/4] subtree: move out of contrib

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 30 2018, Avery Pennarun wrote:

> On Mon, Apr 30, 2018 at 5:38 PM, Stefan Beller  wrote:
> There's one exception, which is doing a one-time permanent merge of
> two projects into one.  That's a nice feature, but is probably used
> extremely rarely.

FWIW this is the only thing I've used it for. I do this occasionally and
used to do this manually with format-patch + "perl -pe" before or
similar when I needed to merge some repositories together, and then some
other times I was less stupid and manually started doing something
similar to what subtree is doing with a "move everything" commit just
before the merge of the two histories.

>> https://trends.google.com/trends/explore?date=all=git%20subtree,git%20submodule
>>
>> Not sure what to make of this data.
>
> Clearly people need a lot more help when using submodules than when
> using subtree :)

Pretty clear it's garbage data, unless we're to believe that the
relative interest of submodules in the US, Germany and Sweden is 51, 64
& 84, but 75, 100 and 0 for subtree.

Re: [PATCH v4 08/10] commit: add short-circuit to paint_down_to_common()

[Jakub Narebski]

Derrick Stolee  writes:

> When running 'git branch --contains', the in_merge_bases_many()
> method calls paint_down_to_common() to discover if a specific
> commit is reachable from a set of branches. Commits with lower
> generation number are not needed to correctly answer the
> containment query of in_merge_bases_many().
>
> Add a new parameter, min_generation, to paint_down_to_common() that
> prevents walking commits with generation number strictly less than
> min_generation. If 0 is given, then there is no functional change.

This is thanks to the fact that generation numbers start at zero (as
special case, though, with _ZERO), and we use strict inequality to avoid
handling _ZERO etc. in a special way.

As you wrote in response in previous version of this series, because
paint_down_to_common() is file-local, there is no need to come up with
symbolic name for GENERATION_NO_CUTOFF case.

All right.

>
> For in_merge_bases_many(), we can pass commit->generation as the
> cutoff, and this saves time during 'git branch --contains' queries
> that would otherwise walk "around" the commit we are inspecting.

All right, and when using paint_down_to_common() to actually find merge
bases, and not only check for containment, we cannot use cutoff.
Therefore at least one call site needs to run it without functional
change... which we can do.  Good.

>
> For a copy of the Linux repository, where HEAD is checked out at
> v4.13~100, we get the following performance improvement for
> 'git branch --contains' over the previous commit:
>
> Before: 0.21s
> After:  0.13s
> Rel %: -38%

Nice.

>
> Signed-off-by: Derrick Stolee 
> ---
>  commit.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)

Let me reorder chunks a bit to make it easier to review.

>
> diff --git a/commit.c b/commit.c
> index 7bb007f56a..e2e16ea1a7 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1070,7 +1080,7 @@ int in_merge_bases_many(struct commit *commit, int 
> nr_reference, struct commit *
>   if (commit->generation > min_generation)
>   return ret;
>  
> - bases = paint_down_to_common(commit, nr_reference, reference);
> + bases = paint_down_to_common(commit, nr_reference, reference, 
> commit->generation);
>   if (commit->object.flags & PARENT2)
>   ret = 1;
>   clear_commit_marks(commit, all_flags);
> @@ -808,11 +808,14 @@ static int queue_has_nonstale(struct prio_queue *queue)
>  }
>  
>  /* all input commits in one and twos[] must have been parsed! */
> -static struct commit_list *paint_down_to_common(struct commit *one, int n, 
> struct commit **twos)
> +static struct commit_list *paint_down_to_common(struct commit *one, int n,
> + struct commit **twos,
> + int min_generation)
>  {
>   struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
>   struct commit_list *result = NULL;
>   int i;
> + uint32_t last_gen = GENERATION_NUMBER_INFINITY;
>  
>   one->object.flags |= PARENT1;
>   if (!n) {
> @@ -831,6 +834,13 @@ static struct commit_list *paint_down_to_common(struct 
> commit *one, int n, struc
>   struct commit_list *parents;
>   int flags;
>  
> + if (commit->generation > last_gen)
> + BUG("bad generation skip");
> + last_gen = commit->generation;

Shouldn't we provide more information about where the problem is to the
user, to make it easier to debug the repository / commit-graph data?

Good to have this sanity check here.

> +
> + if (commit->generation < min_generation)
> + break;

So the reasoning for this, as far as I understand, is the following.
Please correct me if I am wrong.

The callsite with non-zero min_generation, in_merge_bases_many(), tries
to find out if "commit" is an ancestor of one of the "references".  At
least one of "references" is above "commit", so in_merge_bases_many()
uses paint_down_to_common() - but is interested only if "commit" was
painted as reachable from one of "references".

Thus we can interrupt the walk if we know that none of [considered]
commits in the queue can reach "commit"/"one" - as if they were all
STALE.

The search is done using priority queue (a bit like in Dijkstra
algorithm), with newer commits - with larger generation numbers -
considered first.  Thus if current commit has generation number less
than min_generation cutoff, i.e. if it is below "commit", then all
remaining commits in the queue are below cutoff.

Good.

> +
>   flags = commit->object.flags & (PARENT1 | PARENT2 | STALE);
>   if (flags == (PARENT1 | PARENT2)) {
>   if (!(commit->object.flags & RESULT)) {
> @@ -879,7 +889,7 @@ static struct commit_list *merge_bases_many(struct commit 
> *one, int n, struct co
>   return NULL;
> 

Re: [PATCH 0/4] subtree: move out of contrib

[Stefan Beller]

On Mon, Apr 30, 2018 at 2:53 PM, Avery Pennarun  wrote:

> For the best of both worlds, I've often thought that a good balance
> would be to use the same data structure that submodule uses, but to
> store all the code in a single git repo under different refs, which we
> might or might not download (or might or might not have different
> ACLs) under different circumstances.

There has been some experimentation with having a simpler ref
surface on the submodule side,
https://public-inbox.org/git/cover.1512168087.git.jonathanta...@google.com/

The way you describe the future of submodules, all we'd have to do
is to teach git-clone how to select the the "interesting" refs for your use
case. Any other command would assume all submodule data to be
in the main repository.

The difference to Jonathans proposal linked above, would be the
object store to be in the main repo and the refs to be prefixed
per submodule instead of "shadowed".

>  However, when some projects get
> really huge (lots of very big submodule dependencies), then repacking
> one-big-repo starts becoming unwieldy; in that situation git-subtree
> also fails completely.

Yes, but that is a general scaling problem of Git that could be tackled,
e.g. repack into multiple packs serially instead of putting everything
into one pack.

>> Submodules do not need to produce a synthetic project history
>> when splitting off again, as the history is genuine. This allows
>> for easier work with upstream.
>
> Splitting for easier work upstream is great, and there really ought to
> be an official version of 'git subtree split', which is good for all
> sorts of purposes.
>
> However, I suspect almost all uses of the split feature are a)
> splitting a subtree that you previously merged in, or b) splitting a
> subtree into a separate project that you want to maintain separately
> from now on.  Repeated splits in case (a) are only necessary because
> you're not using submodules, or in case (b) are only necessary because
> you didn't *switch* to submodules when it finally came time to split
> the projects.  (In both cases you probably didn't switch to submodules
> because you didn't like one of its tradeoffs, especially the need to
> track multiple repos when you fork.)

That makes sense.

>
> There's one exception, which is doing a one-time permanent merge of
> two projects into one.  That's a nice feature, but is probably used
> extremely rarely.  More often people get into a
> merge-split-merge-split cycle that would be better served by a
> slightly improved git-submodule.

This rare use case is how git-subtree came into existence in gits
contrib directory AFAICT,
https://kernel.googlesource.com/pub/scm/git/git/+/634392b26275fe5436c0ea131bc89b46476aa4ae
which is interesting to view in git-show, but I think defaults could
be tweaked there, as it currently shows me mostly a license file.

>> Conceptually Gerrit is doing
>>
>>   while true:
>> git submodule update --remote
>> if worktree is dirty:
>> git commit "update the submodules"
>>
>> just that Gerrit doesn't poll but does it event based.
>
> ...and it's super handy :)  The problem is it's fundamentally
> centralized: because gerrit can serialize merges into the submodule,
> it also knows exactly how to update the link in the supermodule.  If
> there was wild branching and merging (as there often is in git) and
> you had to resolve conflicts between two submodules, I don't think it
> would be obvious at all how to do it automatically when pushing a
> submodule.  (This also works quite badly with git subtree --squash.)

With the poll based solution I don't think you'd run into many more
problems than you would with Gerrits solution.

In a nearby thread, we were just discussing the submodule merging
strategies,
https://public-inbox.org/git/1524739599.20251.17.ca...@klsmartin.com/
which might seem confusing, but the implementation is actually easy
as we just fastforward-only in submodules.

>>
>> https://trends.google.com/trends/explore?date=all=git%20subtree,git%20submodule
>>
>> Not sure what to make of this data.
>
> Clearly people need a lot more help when using submodules than when
> using subtree :)

That could be true. :)

Thanks,
Stefan

[PATCH 7/9] get_short_oid / peel_onion: ^{tree} should mean tree, not treeish

[Ævar Arnfjörð Bjarmason]

After the recent series of patches ^{tag} and ^{blob} now work to get
just the tags and blobs, but ^{tree} will still list any
tree-ish (commits, tags and trees).

The previous behavior was added in ed1ca6025f ("peel_onion:
disambiguate to favor tree-ish when we know we want a tree-ish",
2013-03-31). I may have missed some special-case but this makes more
sense to me.

Now "$sha1:" can be used as before to mean treeish

$ git rev-parse e8f2:
error: short SHA1 e8f2 is ambiguous
hint: The candidates are:
hint:   e8f2650052 tag v2.17.0
hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached 
HEAD abbreviated object name
hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from 
jeffhostetler/jeffhostetler/register_rename_src
hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for 
multiple remote.url entries
hint:   e8f2093055 tree
hint:   e8f25a3a50 tree
hint:   e8f28d537c tree
hint:   e8f2cf6ec0 tree
[...]

But ^{tree} shows just the trees, but would previously be equivalent
to the above:

$ git rev-parse e8f2^{tree}
error: short SHA1 e8f2 is ambiguous
hint: The candidates are:
hint:   e8f2093055 tree
hint:   e8f25a3a50 tree
hint:   e8f28d537c tree
hint:   e8f2cf6ec0 tree
[...]

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 sha1-name.c |  2 +-
 t/t1512-rev-parse-disambiguation.sh | 18 ++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/sha1-name.c b/sha1-name.c
index 023f9471a8..b61c0558d9 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -970,7 +970,7 @@ static int peel_onion(const char *name, int len, struct 
object_id *oid,
else if (expected_type == OBJ_TAG)
lookup_flags |= GET_OID_TAG;
else if (expected_type == OBJ_TREE)
-   lookup_flags |= GET_OID_TREEISH;
+   lookup_flags |= GET_OID_TREE;
else if (expected_type == OBJ_BLOB)
lookup_flags |= GET_OID_BLOB;
 
diff --git a/t/t1512-rev-parse-disambiguation.sh 
b/t/t1512-rev-parse-disambiguation.sh
index 08ae73e2a5..2acf564714 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -159,9 +159,13 @@ test_expect_failure 'two semi-ambiguous commit-ish' '
git log 00...
 '
 
-test_expect_failure 'three semi-ambiguous tree-ish' '
+test_expect_success 'three semi-ambiguous tree-ish' '
# Likewise for tree-ish.  HEAD, v1.0.0 and HEAD^{tree} share
# the prefix but peeling them to tree yields the same thing
+   test_must_fail git rev-parse --verify 00: &&
+
+   # For ^{tree} we can disambiguate because HEAD and v1.0.0 will
+   # be excluded.
git rev-parse --verify 00^{tree}
 '
 
@@ -267,8 +271,12 @@ test_expect_success 'ambiguous commit-ish' '
 # There are three objects with this prefix: a blob, a tree, and a tag. We know
 # the blob will not pass as a treeish, but the tree and tag should (and thus
 # cause an error).
-test_expect_success 'ambiguous tags peel to treeish' '
-   test_must_fail git rev-parse 00f^{tree}
+test_expect_success 'ambiguous tags peel to treeish or tree' '
+   test_must_fail git rev-parse 00f: &&
+   git rev-parse 00f^{tree} >stdout &&
+   test_line_count = 1 stdout &&
+   grep -q ^00fd8bcc56 stdout
+
 '
 
 test_expect_success 'rev-parse --disambiguate' '
@@ -365,7 +373,9 @@ test_expect_success 'core.disambiguate config can prefer 
types' '
 test_expect_success 'core.disambiguate does not override context' '
# treeish ambiguous between tag and tree
test_must_fail \
-   git -c core.disambiguate=committish rev-parse $sha1^{tree}
+   git -c core.disambiguate=committish rev-parse $sha1: &&
+   # tree not ambiguous between tag and tree
+   git -c core.disambiguate=committish rev-parse $sha1^{tree}
 '
 
 test_done
-- 
2.17.0.290.gded63e768a

Good News

[Mrs Julie Leach]

You are a recipient to Mrs Julie Leach Donation of $2 million USD. Contact 
(julieleach...@gmail.com) for claims.

[PATCH 2/9] sha1-array.h: align function arguments

[Ævar Arnfjörð Bjarmason]

The arguments weren't lined up with the opening parenthesis. Fixes up
code added in cff38a5e11 ("receive-pack: eliminate duplicate .have
refs", 2011-05-19).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 sha1-array.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sha1-array.h b/sha1-array.h
index 04b0756334..1e1d24b009 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -17,7 +17,7 @@ void oid_array_clear(struct oid_array *array);
 typedef int (*for_each_oid_fn)(const struct object_id *oid,
   void *data);
 int oid_array_for_each_unique(struct oid_array *array,
-  for_each_oid_fn fn,
-  void *data);
+ for_each_oid_fn fn,
+ void *data);
 
 #endif /* SHA1_ARRAY_H */
-- 
2.17.0.290.gded63e768a

[PATCH 9/9] config doc: document core.disambiguate

[Ævar Arnfjörð Bjarmason]

The core.disambiguate variable was added in
5b33cb1fd7 ("get_short_sha1: make default disambiguation
configurable", 2016-09-27) but never documented.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 Documentation/config.txt | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2659153cb3..6fee67c12d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -910,6 +910,20 @@ core.abbrev::
abbreviated object names to stay unique for some time.
The minimum length is 4.
 
+core.disambiguate::
+   If Git is given a SHA-1 that's ambigous it'll suggest what
+   objects you might mean. By default it'll print out all
+   potential objects with that prefix regardless of their
+   type. This setting, along with the `^{}` peel syntax
+   (see linkgit:gitrevisions[7]), allows for narrowing that down.
+
++
+Is set to `none` by default, can also be `commit` (peel syntax:
+`$sha1^{commit}`), `commitish` (commits and tags), `tree` (peel:
+`$sha1^{tree}`), `treeish` (everything except blobs), `blob` (peel:
+`$sha1^{blob}`) or `tag` (peel: `$sha1^{tag}`). The peel syntax will
+override any config value.
+
 add.ignoreErrors::
 add.ignore-errors (deprecated)::
Tells 'git add' to continue adding files when some files cannot be
-- 
2.17.0.290.gded63e768a

[PATCH 8/9] get_short_oid / peel_onion: ^{tree} should mean commit, not commitish

[Ævar Arnfjörð Bjarmason]

Continue the untangling of peel disambiguation syntax. Before this
e8f2^{commit} would show the v2.17.0 tag, but now it'll just show
ambiguous commits:

$ git rev-parse e8f2^{commit}
error: short SHA1 e8f2 is ambiguous
hint: The candidates are:
hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached 
HEAD abbreviated object name
hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from 
jeffhostetler/jeffhostetler/register_rename_src
hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for 
multiple remote.url entries
[...]

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 sha1-name.c | 2 +-
 t/t1512-rev-parse-disambiguation.sh | 7 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/sha1-name.c b/sha1-name.c
index b61c0558d9..1d2a74a29c 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -966,7 +966,7 @@ static int peel_onion(const char *name, int len, struct 
object_id *oid,
 
lookup_flags &= ~GET_OID_DISAMBIGUATORS;
if (expected_type == OBJ_COMMIT)
-   lookup_flags |= GET_OID_COMMITTISH;
+   lookup_flags |= GET_OID_COMMIT;
else if (expected_type == OBJ_TAG)
lookup_flags |= GET_OID_TAG;
else if (expected_type == OBJ_TREE)
diff --git a/t/t1512-rev-parse-disambiguation.sh 
b/t/t1512-rev-parse-disambiguation.sh
index 2acf564714..b3ef236db8 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -341,8 +341,8 @@ test_expect_success C_LOCALE_OUTPUT 'ambiguity hints' '
 test_expect_success C_LOCALE_OUTPUT 'ambiguity hints respect type' '
test_must_fail git rev-parse 0^{commit} 2>stderr &&
grep ^hint: stderr >hints &&
-   # 5 commits, 1 tag (which is a commitish), plus intro line
-   test_line_count = 7 hints &&
+   # 5 commits plus intro line
+   test_line_count = 6 hints &&
git rev-parse 0^{tag} >stdout &&
test_line_count = 1 stdout &&
grep -q ^00f8f stdout &&
@@ -366,7 +366,8 @@ test_expect_success 'core.disambiguate config can prefer 
types' '
# ambiguous between tree and tag
sha1=00f &&
test_must_fail git rev-parse $sha1 &&
-   git rev-parse $sha1^{commit} &&
+   # there is no commit so ^{commit} comes up empty
+   test_must_fail git rev-parse $sha1^{commit} &&
git -c core.disambiguate=committish rev-parse $sha1
 '
 
-- 
2.17.0.290.gded63e768a

[PATCH 5/9] get_short_oid: learn to disambiguate by ^{tag}

[Ævar Arnfjörð Bjarmason]

Add support for ^{tag} to the disambiguation logic. Before this ^{tag}
would simply be ignored:

$ git rev-parse e8f2^{tag}
error: short SHA1 e8f2 is ambiguous
hint: The candidates are:
hint:   e8f2650052 tag v2.17.0
hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached 
HEAD abbreviated object name
hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from 
jeffhostetler/jeffhostetler/register_rename_src
hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for 
multiple remote.url entries
hint:   e8f2093055 tree
hint:   e8f25a3a50 tree
hint:   e8f28d537c tree
hint:   e8f2cf6ec0 tree
hint:   e8f21d02f7 blob
hint:   e8f21d577c blob
hint:   e8f2867228 blob
hint:   e8f2a35526 blob
e8f2^{tag}

Now the logic added in ed1ca6025f ("peel_onion: disambiguate to favor
tree-ish when we know we want a tree-ish", 2013-03-31) has been
extended to support it.

$ git rev-parse e8f2^{tag}
e8f2650052f3ff646023725e388ea1112b020e79

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 cache.h |  5 +++--
 sha1-name.c | 13 -
 t/t1512-rev-parse-disambiguation.sh |  5 -
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index 77b7acebb6..3f6a292ba6 100644
--- a/cache.h
+++ b/cache.h
@@ -1322,8 +1322,9 @@ struct object_context {
 #define GET_OID_TREE 010
 #define GET_OID_TREEISH  020
 #define GET_OID_BLOB 040
-#define GET_OID_FOLLOW_SYMLINKS 0100
-#define GET_OID_RECORD_PATH 0200
+#define GET_OID_TAG 0100
+#define GET_OID_FOLLOW_SYMLINKS 0200
+#define GET_OID_RECORD_PATH 0400
 #define GET_OID_ONLY_TO_DIE04000
 
 #define GET_OID_DISAMBIGUATORS \
diff --git a/sha1-name.c b/sha1-name.c
index 46d8b1afa6..68d5f65362 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -221,6 +221,12 @@ static int finish_object_disambiguation(struct 
disambiguate_state *ds,
return 0;
 }
 
+static int disambiguate_tag_only(const struct object_id *oid, void 
*cb_data_unused)
+{
+   int kind = oid_object_info(oid, NULL);
+   return kind == OBJ_TAG;
+}
+
 static int disambiguate_commit_only(const struct object_id *oid, void 
*cb_data_unused)
 {
int kind = oid_object_info(oid, NULL);
@@ -288,7 +294,8 @@ int set_disambiguate_hint_config(const char *var, const 
char *value)
{ "committish", disambiguate_committish_only },
{ "tree", disambiguate_tree_only },
{ "treeish", disambiguate_treeish_only },
-   { "blob", disambiguate_blob_only }
+   { "blob", disambiguate_blob_only },
+   { "tag", disambiguate_tag_only }
};
int i;
 
@@ -429,6 +436,8 @@ static int get_short_oid(const char *name, int len, struct 
object_id *oid,
ds.fn = disambiguate_treeish_only;
else if (flags & GET_OID_BLOB)
ds.fn = disambiguate_blob_only;
+   else if (flags & GET_OID_TAG)
+   ds.fn = disambiguate_tag_only;
else
ds.fn = default_disambiguate_hint;
 
@@ -958,6 +967,8 @@ static int peel_onion(const char *name, int len, struct 
object_id *oid,
lookup_flags &= ~GET_OID_DISAMBIGUATORS;
if (expected_type == OBJ_COMMIT)
lookup_flags |= GET_OID_COMMITTISH;
+   else if (expected_type == OBJ_TAG)
+   lookup_flags |= GET_OID_TAG;
else if (expected_type == OBJ_TREE)
lookup_flags |= GET_OID_TREEISH;
 
diff --git a/t/t1512-rev-parse-disambiguation.sh 
b/t/t1512-rev-parse-disambiguation.sh
index 711704ba5a..c7ceda2f21 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -334,7 +334,10 @@ test_expect_success C_LOCALE_OUTPUT 'ambiguity hints 
respect type' '
test_must_fail git rev-parse 0^{commit} 2>stderr &&
grep ^hint: stderr >hints &&
# 5 commits, 1 tag (which is a commitish), plus intro line
-   test_line_count = 7 hints
+   test_line_count = 7 hints &&
+   git rev-parse 0^{tag} >stdout &&
+   test_line_count = 1 stdout &&
+   grep -q ^00f8f stdout
 '
 
 test_expect_success C_LOCALE_OUTPUT 'failed type-selector still shows hint' '
-- 
2.17.0.290.gded63e768a

[PATCH 6/9] get_short_oid: learn to disambiguate by ^{blob}

[Ævar Arnfjörð Bjarmason]

The disambiguation logic had all the pieces necessary to only print
out those blobs that were ambiguous, but they hadn't been connected.

The initial logic was added in daba53aeaf ("sha1_name.c: add support
for disambiguating other types", 2012-07-02), and when the flags were
propagated in 8a10fea49b ("get_sha1: propagate flags to child
functions", 2016-09-26) GET_OID_BLOB wasn't added to lookup_flags.

Before this change requests for blobs were simply ignored:

$ git rev-parse e8f2^{blob}
error: short SHA1 e8f2 is ambiguous
hint: The candidates are:
hint:   e8f2650052 tag v2.17.0
hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached 
HEAD abbreviated object name
hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from 
jeffhostetler/jeffhostetler/register_rename_src
hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for 
multiple remote.url entries
hint:   e8f2093055 tree
hint:   e8f25a3a50 tree
hint:   e8f28d537c tree
hint:   e8f2cf6ec0 tree
hint:   e8f21d02f7 blob
hint:   e8f21d577c blob
hint:   e8f2867228 blob
hint:   e8f2a35526 blob
e8f2^{blob}
[...]

But now we'll do the right thing and only print the blobs:

$ git rev-parse e8f2^{blob}
error: short SHA1 e8f2 is ambiguous
hint: The candidates are:
hint:   e8f21d02f7 blob
hint:   e8f21d577c blob
hint:   e8f2867228 blob
hint:   e8f2a35526 blob
e8f2^{blob}
[...]

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 sha1-name.c | 2 ++
 t/t1512-rev-parse-disambiguation.sh | 6 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/sha1-name.c b/sha1-name.c
index 68d5f65362..023f9471a8 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -971,6 +971,8 @@ static int peel_onion(const char *name, int len, struct 
object_id *oid,
lookup_flags |= GET_OID_TAG;
else if (expected_type == OBJ_TREE)
lookup_flags |= GET_OID_TREEISH;
+   else if (expected_type == OBJ_BLOB)
+   lookup_flags |= GET_OID_BLOB;
 
if (get_oid_1(name, sp - name - 2, , lookup_flags))
return -1;
diff --git a/t/t1512-rev-parse-disambiguation.sh 
b/t/t1512-rev-parse-disambiguation.sh
index c7ceda2f21..08ae73e2a5 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -337,7 +337,11 @@ test_expect_success C_LOCALE_OUTPUT 'ambiguity hints 
respect type' '
test_line_count = 7 hints &&
git rev-parse 0^{tag} >stdout &&
test_line_count = 1 stdout &&
-   grep -q ^00f8f stdout
+   grep -q ^00f8f stdout &&
+   test_must_fail git rev-parse 0^{blob} 2>stderr &&
+   grep ^hint: stderr >hints &&
+   # 5 blobs plus intro line &&
+   test_line_count = 6 hints
 '
 
 test_expect_success C_LOCALE_OUTPUT 'failed type-selector still shows hint' '
-- 
2.17.0.290.gded63e768a

[PATCH 1/9] sha1-name.c: remove stray newline

[Ævar Arnfjörð Bjarmason]

This stray newline was accidentally introduced in
d2b7d9c7ed ("sha1_name: convert disambiguate_hint_fn to take
object_id", 2017-03-26).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 sha1-name.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sha1-name.c b/sha1-name.c
index 5b93bf8da3..cd3b133aae 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -346,7 +346,6 @@ static int show_ambiguous_object(const struct object_id 
*oid, void *data)
struct strbuf desc = STRBUF_INIT;
int type;
 
-
if (ds->fn && !ds->fn(oid, ds->cb_data))
return 0;
 
-- 
2.17.0.290.gded63e768a

[PATCH 0/9] get_short_oid UI improvements

[Ævar Arnfjörð Bjarmason]

I started out just wanting to do 04/09 so I'd get prettier output, but
then noticed that ^{tag}, ^{commit}< ^{blob} and ^{tree} didn't behave
as expected with the disambiguation output, and that core.disambiguate
had never been documented.

Ævar Arnfjörð Bjarmason (9):
  sha1-name.c: remove stray newline
  sha1-array.h: align function arguments
  sha1-name.c: move around the collect_ambiguous() function
  get_short_oid: sort ambiguous objects by type, then SHA-1
  get_short_oid: learn to disambiguate by ^{tag}
  get_short_oid: learn to disambiguate by ^{blob}
  get_short_oid / peel_onion: ^{tree} should mean tree, not treeish
  get_short_oid / peel_onion: ^{tree} should mean commit, not commitish
  config doc: document core.disambiguate

 Documentation/config.txt| 14 ++
 cache.h |  5 ++-
 sha1-array.c| 15 +++
 sha1-array.h|  7 ++-
 sha1-name.c | 69 -
 t/t1512-rev-parse-disambiguation.sh | 32 ++---
 6 files changed, 120 insertions(+), 22 deletions(-)

-- 
2.17.0.290.gded63e768a

[PATCH 4/9] get_short_oid: sort ambiguous objects by type, then SHA-1

[Ævar Arnfjörð Bjarmason]

Change the output emitted when an ambiguous object is encountered so
that we show tags first, then commits, followed by trees, and finally
blobs. Within each type we show objects in hashcmp(). Before this
change the objects were only ordered by hashcmp().

The reason for doing this is that the output looks better as a result,
e.g. the v2.17.0 tag before this change on "git show e8f2" would
display:

hint: The candidates are:
hint:   e8f2093055 tree
hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached 
HEAD abbreviated object name
hint:   e8f21d02f7 blob
hint:   e8f21d577c blob
hint:   e8f25a3a50 tree
hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from 
jeffhostetler/jeffhostetler/register_rename_src
hint:   e8f2650052 tag v2.17.0
hint:   e8f2867228 blob
hint:   e8f28d537c tree
hint:   e8f2a35526 blob
hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for 
multiple remote.url entries
hint:   e8f2cf6ec0 tree

Now we'll instead show:

hint:   e8f2650052 tag v2.17.0
hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached 
HEAD abbreviated object name
hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from 
jeffhostetler/jeffhostetler/register_rename_src
hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for 
multiple remote.url entries
hint:   e8f2093055 tree
hint:   e8f25a3a50 tree
hint:   e8f28d537c tree
hint:   e8f2cf6ec0 tree
hint:   e8f21d02f7 blob
hint:   e8f21d577c blob
hint:   e8f2867228 blob
hint:   e8f2a35526 blob

Since we show the commit data in the output that's nicely aligned once
we sort by object type. The decision to show tags before commits is
pretty arbitrary, but it's much less likely that we'll display a tag,
so if there is one it makes sense to show it first.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 sha1-array.c | 15 +++
 sha1-array.h |  3 +++
 sha1-name.c  | 37 -
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/sha1-array.c b/sha1-array.c
index 838b3bf847..48bd9e9230 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -41,6 +41,21 @@ void oid_array_clear(struct oid_array *array)
array->sorted = 0;
 }
 
+
+int oid_array_for_each(struct oid_array *array,
+  for_each_oid_fn fn,
+  void *data)
+{
+   int i;
+
+   for (i = 0; i < array->nr; i++) {
+   int ret = fn(array->oid + i, data);
+   if (ret)
+   return ret;
+   }
+   return 0;
+}
+
 int oid_array_for_each_unique(struct oid_array *array,
for_each_oid_fn fn,
void *data)
diff --git a/sha1-array.h b/sha1-array.h
index 1e1d24b009..232bf95017 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -16,6 +16,9 @@ void oid_array_clear(struct oid_array *array);
 
 typedef int (*for_each_oid_fn)(const struct object_id *oid,
   void *data);
+int oid_array_for_each(struct oid_array *array,
+  for_each_oid_fn fn,
+  void *data);
 int oid_array_for_each_unique(struct oid_array *array,
  for_each_oid_fn fn,
  void *data);
diff --git a/sha1-name.c b/sha1-name.c
index 9d7bbd3e96..46d8b1afa6 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -378,6 +378,34 @@ static int collect_ambiguous(const struct object_id *oid, 
void *data)
return 0;
 }
 
+static int sort_ambiguous(const void *a, const void *b)
+{
+   int a_type = oid_object_info(a, NULL);
+   int b_type = oid_object_info(b, NULL);
+   int a_type_sort;
+   int b_type_sort;
+
+   /*
+* Sorts by hash within the same object type, just as
+* oid_array_for_each_unique() would do.
+*/
+   if (a_type == b_type)
+   return oidcmp(a, b);
+
+   /*
+* Between object types show tags, then commits, and finally
+* trees and blobs.
+*
+* The object_type enum is commit, tree, blob, tag, but we
+* want tag, commit, tree blob. Cleverly (perhaps too
+* cleverly) do that with modulus, since the enum assigns 1 to
+* commit, so tag becomes 0.
+*/
+   a_type_sort = a_type % 4;
+   b_type_sort = b_type % 4;
+   return a_type_sort > b_type_sort ? 1 : -1;
+}
+
 static int get_short_oid(const char *name, int len, struct object_id *oid,
  unsigned flags)
 {
@@ -409,6 +437,8 @@ static int get_short_oid(const char *name, int len, struct 
object_id *oid,
status = finish_object_disambiguation(, oid);
 
if (!quietly && (status == SHORT_NAME_AMBIGUOUS)) {
+   struct oid_array collect = OID_ARRAY_INIT;
+
error(_("short SHA1 %s is ambiguous"), ds.hex_pfx);
 
   

[PATCH 3/9] sha1-name.c: move around the collect_ambiguous() function

[Ævar Arnfjörð Bjarmason]

A subsequent change will make use of this static function in the
get_short_oid() function, which is defined above where the
collect_ambiguous() function is now, which would result in a
compilation error due to a forward declaration.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 sha1-name.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sha1-name.c b/sha1-name.c
index cd3b133aae..9d7bbd3e96 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -372,6 +372,12 @@ static int show_ambiguous_object(const struct object_id 
*oid, void *data)
return 0;
 }
 
+static int collect_ambiguous(const struct object_id *oid, void *data)
+{
+   oid_array_append(data, oid);
+   return 0;
+}
+
 static int get_short_oid(const char *name, int len, struct object_id *oid,
  unsigned flags)
 {
@@ -421,12 +427,6 @@ static int get_short_oid(const char *name, int len, struct 
object_id *oid,
return status;
 }
 
-static int collect_ambiguous(const struct object_id *oid, void *data)
-{
-   oid_array_append(data, oid);
-   return 0;
-}
-
 int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 {
struct oid_array collect = OID_ARRAY_INIT;
-- 
2.17.0.290.gded63e768a

Re: [PATCH 0/4] subtree: move out of contrib

[Avery Pennarun]

On Mon, Apr 30, 2018 at 5:38 PM, Stefan Beller  wrote:
> On Mon, Apr 30, 2018 at 1:45 PM, Avery Pennarun  wrote:
> No objections from me either.
>
> Submodules seem to serve a slightly different purpose, though?

I think the purpose is actually the same - it's just the tradeoffs
that are difference.  Both sets of tradeoffs kind of suck.

> With Subtrees the superproject always contains all the code,
> even when you squash the subtree histroy when merging it in.
> In the submodule world, you may not have access to one of the
> submodules.

Right.  Personally I think it's a disadvantage of subtree that it
always contains all the code (what if some people don't want the code
for a particular build variant?).  However, it's a huge pain that
submodules *don't* contain all the code (what if I'm not online right
now, or the site supposedly containing the code goes offline, or I
want to make my own fork?).

For the best of both worlds, I've often thought that a good balance
would be to use the same data structure that submodule uses, but to
store all the code in a single git repo under different refs, which we
might or might not download (or might or might not have different
ACLs) under different circumstances.  However, when some projects get
really huge (lots of very big submodule dependencies), then repacking
one-big-repo starts becoming unwieldy; in that situation git-subtree
also fails completely.

> Submodules do not need to produce a synthetic project history
> when splitting off again, as the history is genuine. This allows
> for easier work with upstream.

Splitting for easier work upstream is great, and there really ought to
be an official version of 'git subtree split', which is good for all
sorts of purposes.

However, I suspect almost all uses of the split feature are a)
splitting a subtree that you previously merged in, or b) splitting a
subtree into a separate project that you want to maintain separately
from now on.  Repeated splits in case (a) are only necessary because
you're not using submodules, or in case (b) are only necessary because
you didn't *switch* to submodules when it finally came time to split
the projects.  (In both cases you probably didn't switch to submodules
because you didn't like one of its tradeoffs, especially the need to
track multiple repos when you fork.)

> Subtrees present you the whole history by default and the user
> needs to be explicit about not wanting to see history from the
> subtree, which is the opposite of submodules (though this
> may be planned in the future to switch).

It turns out that AFAIK, almost everyone prefers 'git subtree
--squash', which squashes into a single commit each time you merge,
much like git submodule does.  I doubt people would cry too much if
the full-history feature went away.

There's one exception, which is doing a one-time permanent merge of
two projects into one.  That's a nice feature, but is probably used
extremely rarely.  More often people get into a
merge-split-merge-split cycle that would be better served by a
slightly improved git-submodule.

>> The gerrit team (eg. Stefan Beller) has been doing some really great
>> stuff to make submodules more usable by helping with relative
>> submodule links and by auto-updating links in supermodules at the
>> right times.  Unfortunately doing that requires help from the server
>> side, which kind of messes up decentralization and so doesn't solve
>> the problem in the general case.
>
> Conceptually Gerrit is doing
>
>   while true:
> git submodule update --remote
> if worktree is dirty:
> git commit "update the submodules"
>
> just that Gerrit doesn't poll but does it event based.

...and it's super handy :)  The problem is it's fundamentally
centralized: because gerrit can serialize merges into the submodule,
it also knows exactly how to update the link in the supermodule.  If
there was wild branching and merging (as there often is in git) and
you had to resolve conflicts between two submodules, I don't think it
would be obvious at all how to do it automatically when pushing a
submodule.  (This also works quite badly with git subtree --squash.)

>> I really wish there were a good answer, but I don't know what it is.
>> I do know that lots of people seem to at least be happy using
>> git-subtree, and would be even happier if it were installed
>> automatically with git.
>
> https://trends.google.com/trends/explore?date=all=git%20subtree,git%20submodule
>
> Not sure what to make of this data.

Clearly people need a lot more help when using submodules than when
using subtree :)

Have fun,

Avery

Re: [PATCH v2 3/5] mem-pool: fill out functionality

[Stefan Beller]

On Mon, Apr 30, 2018 at 8:31 AM, Jameson Miller  wrote:
> Adds the following functionality to memory pools:
>
>  - Lifecycle management functions (init, discard)
>
>  - Test whether a memory location is part of the managed pool
>
>  - Function to combine 2 pools
>
> This also adds logic to track all memory allocations made by a memory
> pool.
>
> These functions will be used in a future commit in this commit series.
>
> Signed-off-by: Jameson Miller 
> ---
>  mem-pool.c | 114 
> ++---
>  mem-pool.h |  32 +

> diff --git a/mem-pool.h b/mem-pool.h
> index 829ad58ecf..34df4fa709 100644
> --- a/mem-pool.h
> +++ b/mem-pool.h
> @@ -19,8 +19,27 @@ struct mem_pool {
>
> /* The total amount of memory allocated by the pool. */
> size_t pool_alloc;
> +
> +   /*
> +* Array of pointers to "custom size" memory allocations.
> +* This is used for "large" memory allocations.
> +* The *_end variables are used to track the range of memory
> +* allocated.
> +*/
> +   void **custom, **custom_end;
> +   int nr, nr_end, alloc, alloc_end;
>  };

What is the design goal of this mem pool?
What is it really good at, which patterns of use should we avoid?

It looks like internally the mem-pool can either use mp_blocks
that are stored as a linked list, or it can have custom allocations
stored in an array.

Is the linked list or the custom part sorted by some key?
Does it need to be sorted?

I am currently looking at alloc.c, which is really good for
allocating memory for equally sized parts, i.e. it is very efficient
at providing memory for fixed sized structs. And on top of that
it is not tracking any memory as it relies on program termination
for cleanup.

This memory pool seems to be optimized for allocations of
varying sizes, some of them huge (to be stored in the custom
part) and most of them rather small as they go into the mp_blocks?

Thanks,
Stefan

Proposal

[Miss Zeliha Omer Faruk]

Hello

   Greetings to you today i asked before but i did't get a response please
i know this might come to you as a surprise because you do not know me
personally i have a business proposal for our mutual benefit please let
me know if you are interested.



Best Regards,

Esentepe Mahallesi Büyükdere
Caddesi Kristal Kule Binasi
No:215
 Sisli - Istanbul, Turkey

Re: git-submodule is missing --dissociate option

[Casey Fitzpatrick]

Sure, I'll take a crack at it and submit a patch.

On Mon, Apr 30, 2018 at 2:19 PM, Stefan Beller  wrote:
> On Mon, Apr 30, 2018 at 1:29 AM, Casey Fitzpatrick  wrote:
>> This seems to be a hole in the git feature set. I believe it is fairly
>> easily worked around, but it would be best to provide the option for
>> ease of use (and maybe performance?).
>>
>> git clone has both a --reference feature and a --dissociate option,
>> with dissociate allowing for a reference to *only* speed up network
>> transfers rather than have the resulting clone rely upon the reference
>> always being there (creates an independent repo).
>
> With the advent of git-worktree, I claim that --reference without further
> --dissociate is dangerous, such that the combination of these two are
> not the best UX, you could wish for.
>
>> But git submodule only allows for --reference, so there isn't a an
>> option to make a speedy independent submodule clone in one shot:
>> https://git-scm.com/docs/git-submodule
>> I checked the latest online documentation (currently at 2.16.3) and
>> the documentation in the latest sources (almost 2.18):
>> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>>
>> As far as I am aware this can be worked around with 'git repack -a'
>> and manual removal of the objects/info/alternates file afterward.
>> Though I don't know if this results in a less speedy clone than
>> dissociate would.
>
> That is an interesting workaround!
> I agree we should have the --dissociate option available for submodules.
> Care to implement it?
>
> Thanks,
> Stefan

Re: [PATCH 0/4] subtree: move out of contrib

[Stefan Beller]

On Mon, Apr 30, 2018 at 1:45 PM, Avery Pennarun  wrote:
> On Mon, Apr 30, 2018 at 5:50 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>> I think at this point git-subtree is widely used enough to move out of
>> contrib/, maybe others disagree, but patches are always better for
>> discussion that patch-less ML posts.
>
> I really was hoping git-subtree would be completely obsoleted by
> git-submodule by now, but... it hasn't been, so... no objections on
> this end.

No objections from me either.

Submodules seem to serve a slightly different purpose, though?

With Subtrees the superproject always contains all the code,
even when you squash the subtree histroy when merging it in.
In the submodule world, you may not have access to one of the
submodules.

Submodules do not need to produce a synthetic project history
when splitting off again, as the history is genuine. This allows
for easier work with upstream.

Subtrees present you the whole history by default and the user
needs to be explicit about not wanting to see history from the
subtree, which is the opposite of submodules (though this
may be planned in the future to switch).

> The gerrit team (eg. Stefan Beller) has been doing some really great
> stuff to make submodules more usable by helping with relative
> submodule links and by auto-updating links in supermodules at the
> right times.  Unfortunately doing that requires help from the server
> side, which kind of messes up decentralization and so doesn't solve
> the problem in the general case.

Conceptually Gerrit is doing

  while true:
git submodule update --remote
if worktree is dirty:
git commit "update the submodules"

just that Gerrit doesn't poll but does it event based.

> I really wish there were a good answer, but I don't know what it is.
> I do know that lots of people seem to at least be happy using
> git-subtree, and would be even happier if it were installed
> automatically with git.

https://trends.google.com/trends/explore?date=all=git%20subtree,git%20submodule

Not sure what to make of this data.

Proposal

[Miss Zeliha Omer Faruk]

Hello

   Greetings to you today i asked before but i did't get a response please
i know this might come to you as a surprise because you do not know me
personally i have a business proposal for our mutual benefit please let
me know if you are interested.



Best Regards,

Esentepe Mahallesi Büyükdere
Caddesi Kristal Kule Binasi
No:215
 Sisli - Istanbul, Turkey

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

[Martin Ågren]

The diagram renders fine in AsciiDoc before and after this patch.
Asciidoctor, on the other hand, ignores the tabs entirely, which results
in different indentation for different lines. The graph illustration
earlier in the document already uses spaces instead of a tab.

Signed-off-by: Martin Ågren 
---
Can be seen at the end of https://git-scm.com/docs/gitrevisions

 Documentation/revisions.txt | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index dfcc49c72c..011746b74f 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -355,15 +355,15 @@ spelt out:
B..C   = ^B CC
B...C  = B ^F C  G H D E B C
B^-= B^..B
- = ^B^1 B  E I J F B
+  = ^B^1 B  E I J F B
C^@= C^1
- = F   I J F
+  = F   I J F
B^@= B^1 B^2 B^3
- = D E F   D G H E F I J
+  = D E F   D G H E F I J
C^!= C ^C^@
- = C ^C^1
- = C ^FC
+  = C ^C^1
+  = C ^FC
B^!= B ^B^@
- = B ^B^1 ^B^2 ^B^3
- = B ^D ^E ^F  B
+  = B ^B^1 ^B^2 ^B^3
+  = B ^D ^E ^F  B
F^! D  = F ^I ^J D   G H D F
-- 
2.17.0

Re: [PATCH 0/4] subtree: move out of contrib

[Avery Pennarun]

On Mon, Apr 30, 2018 at 5:50 AM, Ævar Arnfjörð Bjarmason
 wrote:
> I think at this point git-subtree is widely used enough to move out of
> contrib/, maybe others disagree, but patches are always better for
> discussion that patch-less ML posts.

I really was hoping git-subtree would be completely obsoleted by
git-submodule by now, but... it hasn't been, so... no objections on
this end.

The gerrit team (eg. Stefan Beller) has been doing some really great
stuff to make submodules more usable by helping with relative
submodule links and by auto-updating links in supermodules at the
right times.  Unfortunately doing that requires help from the server
side, which kind of messes up decentralization and so doesn't solve
the problem in the general case.

I really wish there were a good answer, but I don't know what it is.
I do know that lots of people seem to at least be happy using
git-subtree, and would be even happier if it were installed
automatically with git.

Have fun,

Avery

Re: [PATCH 1/1] wt-status: use rename settings from init_diff_ui_defaults

[Elijah Newren]

Hi Eckhard,

On Mon, Apr 30, 2018 at 2:34 AM, Eckhard S. Maaß
 wrote:
> Since the very beginning, git status behaved differently for rename
> detection than other rename aware commands like git log or git show as
> it has the use of rename hard coded into it.  After 5404c116aa ("diff:
> activate diff.renames by default", 2016-02-25) the default behaves the
> same by coincidence, but a work flow like
>
> - git add .
> - git status
> - git commit
> - git show
>
> should give you the same information on renames (and/or copies if
> activated) accordingly to the diff.renames setting.

Thanks for sending this change in.  I agree with the logic.  I think
the last sentence needs a s/diff.renames setting/diff.renames and
diff.renameLimit settings/, though, because...

> With this commit the hard coded rename settings are dropped from the
> status command.
>
> Signed-off-by: Eckhard S. Maaß 
> ---
>  builtin/commit.c   |  2 +-
>  t/t4001-diff-rename.sh | 12 
>  wt-status.c|  4 
>  3 files changed, 13 insertions(+), 5 deletions(-)
>

> diff --git a/wt-status.c b/wt-status.c
> index 50815e5faf..32f3bcaebd 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -625,9 +625,6 @@ static void wt_status_collect_changes_index(struct 
> wt_status *s)
> rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
> rev.diffopt.format_callback = wt_status_collect_updated_cb;
> rev.diffopt.format_callback_data = s;
> -   rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
> -   rev.diffopt.rename_limit = 200;

By removing the hard-coded value of 200, the rename limit will instead
match whatever the user specified (or, if they didn't specify
anything, then the value of 400 from diff_rename_limit_default).  I
think that's a good change, for the exact same reasons as you argue
for making diff.renames be used everywhere in the commit message, it
just needs to be mentioned in the commit message.

Everything else in the patch looks good to me, so with that change
feel free to add:

Reviewed-by: Elijah Newren 

RE: Branch deletion question / possible bug?

[Tang (US), Pik S]

Hello,

Thank you for all your replies.  I am on a case insensitive system (Windows 10) 
running git version 2.14.1.windows.1.  

While I can't comment on what the fix would be, it has been enlightening to 
learn a bit more about what's under the cover of git.  

TIL :)
Pik

-Original Message-
From: Johannes Schindelin [mailto:johannes.schinde...@gmx.de] 
Sent: Saturday, April 28, 2018 5:44 AM
To: Philip Oakley 
Cc: Jacob Keller ; Tang (US), Pik S 
; Git List 
Subject: Re: Branch deletion question / possible bug?

Hi,

On Sat, 28 Apr 2018, Philip Oakley wrote:

> From: "Jacob Keller" 
> > On Fri, Apr 27, 2018 at 5:29 PM, Tang (US), Pik S 
> > 
> > wrote:
> > > Hi,
> > >
> > > I discovered that I was able to delete the feature branch I was 
> > > in, due to some fat fingering on my part and case insensitivity.  
> > > I never realized this could be done before.  A quick google search 
> > > did not give me a whole lot to work with...
> > >
> > > Steps to reproduce:
> > > 1. Create a feature branch, "editCss"
> > > 2. git checkout master
> > > 3. git checkout editCSS
> > > 4. git checkout editCss
> > > 5. git branch -d editCSS
> > >
> >
> > Are you running on a case-insensitive file system? What version of 
> > git? I thought I recalled seeing commits to help avoid creating 
> > branches of the same name with separate case when we know we're on a 
> > file system which is case-insensitive..
> >
> > > Normally, it should have been impossible for a user to delete the 
> > > branch they're on.  And the deletion left me in a weird state that 
> > > took a while to dig out of.
> > >
> > > I know this was a user error, but I was also wondering if this was a bug.
> >
> > If we have not yet done this, I think we should. Long term this 
> > would be fixed by using a separate format to store refs than the 
> > filesystem, which has a few projects being worked on but none have 
> > been put into a release.
> 
> Yes, this is an on-going problem on Windows and other case insentive 
> systems. At the moment the branch name becomes embedded as a file 
> name, so when Git requests details of a branch from the filesystem, it 
> can get a case insensitive equivalent. Meanwhile, internally Git is 
> checking for equality in a case sensitive [Linux] way with obvious 
> consequences such as this - The most obvious being when there is no 
> "*" current branch marker in the branch status list.
> 
> It's a bit tricky to fix (internally the name and the path are passed 
> down different call chains), and depends on how one expects the case 
> insensitivity to work - the kicker is when someone does an edit of the 
> name via the file system and expects Git to cope (i.e. devs knowing, 
> or think they know, too much detail ;-).
> 
> The refs can also get packed, so the "bad spelling" gets baked in.
> Ultimately it probably means that GfW and other systems will need  a 
> case sensitivity check when opening paths...

FWIW I outlined what I think is the best route to fix this for good:

https://github.com/git-for-windows/git/issues/1623#issuecomment-380085257

Essentially, I think we should teach Git the trick to check the spelling before 
calling lstat() in refs/files-backend.c.

To check the spelling, we would need an API to get the on-disk representation 
of a given path. On Windows, I know this call. On Linux, apparently 
canonicalize_file_name() might do the job, but that is a GNU libc extension, 
and won't help us on macOS.

Any ideas?

Ciao,
Dscho

Account Receivables (Job Offer)

[Yokohama Rubber Company]

--
We are interested in employing you to represent our company in the  
United States and Canada for the position of a Collection Agent to our  
Company Yokohama Rubber Company Limited Japan, you will be on a 20%  
commission for the amount owed and collected on our behalf from our  
customers, if interested get back to us for more details  
(yokohamarubbergroup...@gmail.com)

Re: [PATCH 2/6] t1406: prepare for the refs code to fail with BUG()

[Johannes Sixt]

Am 30.04.2018 um 00:17 schrieb Johannes Schindelin:

t1406 specifically verifies that certain code paths fail with a BUG: ...
message.

In the upcoming commit, we will convert that message to be generated via
BUG() instead of die("BUG: ..."), which implies SIGABRT instead of a
regular exit code.

Signed-off-by: Johannes Schindelin 
---
  t/t1406-submodule-ref-store.sh | 15 +--
  1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index e093782cc37..0ea3457cae3 100755
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -16,7 +16,7 @@ test_expect_success 'setup' '
  '
  
  test_expect_success 'pack_refs() not allowed' '

-   test_must_fail $RUN pack-refs 3
+   test_must_fail ok=sigabrt $RUN pack-refs 3
  '
  
  test_expect_success 'peel_ref(new-tag)' '

@@ -27,15 +27,18 @@ test_expect_success 'peel_ref(new-tag)' '
  '
  
  test_expect_success 'create_symref() not allowed' '

-   test_must_fail $RUN create-symref FOO refs/heads/master nothing
+   test_must_fail ok=sigabrt \
+   $RUN create-symref FOO refs/heads/master nothing
  '
  
  test_expect_success 'delete_refs() not allowed' '

-   test_must_fail $RUN delete-refs 0 nothing FOO refs/tags/new-tag
+   test_must_fail ok=sigabrt \
+   $RUN delete-refs 0 nothing FOO refs/tags/new-tag
  '
  
  test_expect_success 'rename_refs() not allowed' '

-   test_must_fail $RUN rename-ref refs/heads/master refs/heads/new-master
+   test_must_fail ok=sigabrt \
+   $RUN rename-ref refs/heads/master refs/heads/new-master
  '
  
  test_expect_success 'for_each_ref(refs/heads/)' '

@@ -91,11 +94,11 @@ test_expect_success 'reflog_exists(HEAD)' '
  '
  
  test_expect_success 'delete_reflog() not allowed' '

-   test_must_fail $RUN delete-reflog HEAD
+   test_must_fail ok=sigabrt $RUN delete-reflog HEAD
  '
  
  test_expect_success 'create-reflog() not allowed' '

-   test_must_fail $RUN create-reflog HEAD 1
+   test_must_fail ok=sigabrt $RUN create-reflog HEAD 1
  '


I can't quite follow the rationale for this change. A 'BUG' error exit 
must never be reached, otherwise it is a bug in the program by 
definition. It cannot be OK that SIGABRT is a valid result from Git.


If SIGABRT occurs as a result of BUG(), and we know that this happens 
for certain cases, it means we have an unfixed bug. Should then not run 
these cases under test_expect_failure instead of test_expect_success to 
identify them as known bugs?


Confused.

-- Hannes

Re: [PATCH v1] test-drop-caches: simplify delay loading of NtSetSystemInformation

[Johannes Sixt]

Am 30.04.2018 um 16:26 schrieb Ben Peart:

@@ -82,8 +83,6 @@ static int cmd_dropcaches(void)
  {
HANDLE hProcess = GetCurrentProcess();
HANDLE hToken;
-   HMODULE ntdll;
-   DWORD(WINAPI *NtSetSystemInformation)(INT, PVOID, ULONG);
SYSTEM_MEMORY_LIST_COMMAND command;
int status;
  
@@ -95,14 +94,9 @@ static int cmd_dropcaches(void)
  
  	CloseHandle(hToken);
  
-	ntdll = LoadLibrary("ntdll.dll");

-   if (!ntdll)
-   return error("Can't load ntdll.dll, wrong Windows version?");
-
-   NtSetSystemInformation =
-   (DWORD(WINAPI *)(INT, PVOID, ULONG))GetProcAddress(ntdll, 
"NtSetSystemInformation");
-   if (!NtSetSystemInformation)
-   return error("Can't get function addresses, wrong Windows 
version?");
+   DECLARE_PROC_ADDR(ntdll.dll, DWORD, NtSetSystemInformation, INT, PVOID, 
ULONG);


This is a declaration-after-statement. Even though this is not in 
generic code, it is all too easy to trigger a warning from GCC if a 
corresponding warning is turned on.



+   if (!INIT_PROC_ADDR(NtSetSystemInformation))
+   return error("Could not find NtSetSystemInformation() 
function");
  
  	command = MemoryPurgeStandbyList;

status = NtSetSystemInformation(
@@ -115,8 +109,6 @@ static int cmd_dropcaches(void)
else if (status != STATUS_SUCCESS)
error("Unable to execute the memory list command %d", status);
  
-	FreeLibrary(ntdll);

-
return status;
  }


-- Hannes

js/no-pager-shorthand [was: What's cooking in git.git (Apr 2018, #04; Mon, 30)]

[Johannes Sixt]

Am 30.04.2018 um 05:25 schrieb Junio C Hamano:

* js/no-pager-shorthand (2018-04-25) 1 commit
  - git: add -N as a short option for --no-pager

  "git --no-pager cmd" did not have short-and-sweet single letter
  option. Now it does.

  Will merge to 'next'.


I consider your argument that -N is only an abbreviation for an 
unspecific "no" a valid one. So, I would like to be sure that we are not 
painting us into the wrong corner by squatting -N for --no-pager.


I find -P is not that bad after all.

-- Hannes

Re: [PATCH v1] test-drop-caches: simplify delay loading of NtSetSystemInformation

[Ben Peart]

On 4/30/2018 2:12 PM, Stefan Beller wrote:

On Mon, Apr 30, 2018 at 7:26 AM, Ben Peart  wrote:

Take advantage of the recent addition of support for lazy loading functions


Care to specify "recent additions"? Are these in Git code or somewhere else?

I find this alias handy, as then I can describe commits
in commit messages via "git gcs 

   alias.gcs=show --date=short -s --pretty='format:%h (%s, %ad)'

Thanks,
Stefan



I guess "recent" is relative. :)

db2f7c48cb (Win32: simplify loading of DLL functions, 2017-09-25)

Re: git-submodule is missing --dissociate option

[Stefan Beller]

On Mon, Apr 30, 2018 at 1:29 AM, Casey Fitzpatrick  wrote:
> This seems to be a hole in the git feature set. I believe it is fairly
> easily worked around, but it would be best to provide the option for
> ease of use (and maybe performance?).
>
> git clone has both a --reference feature and a --dissociate option,
> with dissociate allowing for a reference to *only* speed up network
> transfers rather than have the resulting clone rely upon the reference
> always being there (creates an independent repo).

With the advent of git-worktree, I claim that --reference without further
--dissociate is dangerous, such that the combination of these two are
not the best UX, you could wish for.

> But git submodule only allows for --reference, so there isn't a an
> option to make a speedy independent submodule clone in one shot:
> https://git-scm.com/docs/git-submodule
> I checked the latest online documentation (currently at 2.16.3) and
> the documentation in the latest sources (almost 2.18):
> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>
> As far as I am aware this can be worked around with 'git repack -a'
> and manual removal of the objects/info/alternates file afterward.
> Though I don't know if this results in a less speedy clone than
> dissociate would.

That is an interesting workaround!
I agree we should have the --dissociate option available for submodules.
Care to implement it?

Thanks,
Stefan

Re: [PATCH v1] test-drop-caches: simplify delay loading of NtSetSystemInformation

[Stefan Beller]

On Mon, Apr 30, 2018 at 7:26 AM, Ben Peart  wrote:
> Take advantage of the recent addition of support for lazy loading functions

Care to specify "recent additions"? Are these in Git code or somewhere else?

I find this alias handy, as then I can describe commits
in commit messages via "git gcs 

  alias.gcs=show --date=short -s --pretty='format:%h (%s, %ad)'

Thanks,
Stefan

Re: [PATCH 00/41] object_id part 13

[Duy Nguyen]

On Tue, Apr 24, 2018 at 1:39 AM, brian m. carlson
 wrote:
> [0] I can synthesize blobs, trees, and commits, but things are currently
> totally broken, which is, I suppose, to be expected.

Yup. I was tired and bored so I went playing with the new hash.
Writing and reading blobs (with hash-object/cat-file) were relatively
easy after fixing up fill_sha1_path and get_oid_basic). Then I worked
my way up to update-index/ls-files so that I could make trees with
write-tree. And I hit the first road block: struct ondisk_cache_entry
hard codes hash size so I would need to re-organize the code for more
flexibility (or even redesign the file format if I want to keep byte
alignment). Eck...

I guess I'll be helping review this series instead :D
-- 
Duy

Re: git merge banch w/ different submodule revision

[Heiko Voigt]

On Thu, Apr 26, 2018 at 03:19:36PM -0700, Stefan Beller wrote:
> Stefan wrote:
> > See 
> > https://github.com/git/git/commit/68d03e4a6e448aa557f52adef92595ac4d6cd4bd
> > (68d03e4a6e (Implement automatic fast-forward merge for submodules, 
> > 2010-07-07)
> > to explain the situation you encounter. (specifically merge_submodule
> > at the end of the diff)
> 
> +cc Heiko, author of that commit.

In that commit we tried to be very careful about. I do not understand
the situation in which the current strategy would be wrong by default.

We only merge if the following applies:

 * The changes in the superproject on both sides point forward in the
   submodule.

 * One side is contained in the other. Contained from the submodule
   perspective. Sides from the superproject merge perspective.

So in case of the mentioned rewind of a submodule: Only one side of the
3-way merge would point forward and the merge would fail.

I can imagine, that in case of a temporary revert of a commit in the
submodule that you would not want that merged into some other branch.
But that would be the same without submodules. If you merge a temporary
revert from another branch you will not get any conflict.

So maybe someone can explain the use case in which one would get the
results that seem wrong?

Cheers Heiko

Re: [PATCH v4 07/10] commit: use generation numbers for in_merge_bases()

[Jakub Narebski]

Derrick Stolee  writes:

> The containment algorithm for 'git branch --contains' is different
> from that for 'git tag --contains' in that it uses is_descendant_of()
> instead of contains_tag_algo(). The expensive portion of the branch
> algorithm is computing merge bases.
>
> When a commit-graph file exists with generation numbers computed,
> we can avoid this merge-base calculation when the target commit has
> a larger generation number than the initial commits.

Right.

>
> Performance tests were run on a copy of the Linux repository where
> HEAD is contained in v4.13 but no earlier tag. Also, all tags were
> copied to branches and 'git branch --contains' was tested:

I guess that it is equivalent of 'git tag --contains' setup from
previous commit, just for 'git branch --contains', isn't it?

>
> Before: 60.0s
> After:   0.4s
> Rel %: -99.3%

Very nice.

Sidenote: an alternative to using "Rel %" of -99.3% (which is calculated
as (before-after)/before) would be to use "Speedup" of 150 x (calculated
as before/after).  One one hand it might be more readable, on the other
hand it might be a bit misleading.

Yet another alternative would be to use a chart like the following:

   time  Before   After
  Before  60.0s  --  -99.3%
  After0.4s   +149%  --

Anyway, consistency in presentation in patch series is good.  So I am
for keeping your notation thorough the series.

>
> Reported-by: Jeff King 
> Signed-off-by: Derrick Stolee 
> ---
>  commit.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/commit.c b/commit.c
> index 39a3749abd..7bb007f56a 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1056,12 +1056,19 @@ int in_merge_bases_many(struct commit *commit, int 
> nr_reference, struct commit *

Let's give it a bit more context:

   /*
* Is "commit" an ancestor of one of the "references"?
*/
   int in_merge_bases_many(struct commit *commit, int nr_reference, struct 
commit **reference)
>  {
>   struct commit_list *bases;
>   int ret = 0, i;
> + uint32_t min_generation = GENERATION_NUMBER_INFINITY;
>  
>   if (parse_commit(commit))
>   return ret;
> - for (i = 0; i < nr_reference; i++)
> + for (i = 0; i < nr_reference; i++) {
>   if (parse_commit(reference[i]))
>   return ret;

We use parse_commit(), so there is no need for calling
load_commit_graph_info(), like in previous patch.

All right.

> + if (min_generation > reference[i]->generation)

At first glance, I thought it was wrong; but it is the same as the
following, it is just a matter of taste (which feels more natural):

  + if (reference[i]->generation < min_generation)

> + min_generation = reference[i]->generation;
> + }
> +
> + if (commit->generation > min_generation)
> + return ret;

All right, using weak version of generation numbers based negative-cut
nicely handles automatically all corner-cases, including the case where
commit-graaph feature is turned off.

If commit-graph feature is not available, it costs only few memory
access and few comparisons than before, and performance is dominated by
something else anyway.  Negligible and possibly unnoticeable change, I
guess.  Good.

>  
>   bases = paint_down_to_common(commit, nr_reference, reference);
>   if (commit->object.flags & PARENT2)

Re: [PATCH v1] test-drop-caches: simplify delay loading of NtSetSystemInformation

[Eric Sunshine]

On Mon, Apr 30, 2018 at 10:26 AM, Ben Peart  wrote:
> Take advantage of the recent addition of support for lazy loading functions
> on Windows to simplfy the loading of NtSetSystemInformation.

s/simplfy/simplify/

> Signed-off-by: Ben Peart 

Re: [PATCH v3 2/3] merge: Add merge.renames config setting

[Elijah Newren]

Hi Eckhard,

On Mon, Apr 30, 2018 at 1:03 AM, Eckhard Maaß
 wrote:
> On Fri, Apr 27, 2018 at 01:23:20PM -0700, Elijah Newren wrote:
>> I doubt it has ever been discussed before this thread.  But, if you're
>> curious, I'll try to dump a few thoughts.
>
> Thank you, I try to dump some of mine, too. Maybe let me first stress
> that for me copy detection without --find-copies-harder is much more a
> "find content extracted" (like methods being factored out). In a way
> this is nearer to a rename than to a real copy.

Ooh, if you wanted to detect movement of code between files (as blame
does, I think searches for PICKAXE_BLAME_MOVE would point you in the
right direction) and then try to use that during merge to allow
applied changes to move with the code, that would be awesome.
Expensive, and might be a lot of work to wire it all up, but it'd be
very interesting.  I was only discussing DIFF_DETECT_COPY in my
previous email, which was all about duplicating entire files; that's
something I don't see utility in right now for resolving merges.


> I admit that a "real" copy would get unnoticed that way. But the
> semantics of such a copy isn't too clear for me either - did I copy the
> other part to make it independent of the other or did I just employ a
> copy and paste tactic? The former does not want the changes, the later
> does. But I am happy catering to the former here.

Right, if you have to assume that the copy was made to make the code
independent, then there's no value for merge resolution to having
detected the copy in the first place.  That has the advantage of
side-stepping the possible new edge and corner cases I mentioned in
the rest of my email, but it means we shouldn't even spend time
detecting copies -- whether whole file (via DIFF_DETECT_COPY) or
individual lines (via PICKAXE_BLAME_COPY and variants).


Elijah

Re: public-inbox.org down?

[Eric Wong]

Oops, some upgrades went awry for the non-Tor HTTPS endpoint termination.

The Tor .onions should remain available if that happens
http://ou63pmih66umazou.onion/git
http://czquwvybam4bgbro.onion/git
http://hjrcffqmbrq6wope.onion/git

nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git


And my monitoring scripts didn't test HTTPS :x  Just fixed that, at least.

Re: public-inbox.org down?

[Stefan Beller]

Hi Samuel,

public-inbox.org is reachable from here, which may not add a lot of
information to your debugging.

I cc'd Eric Wong, who runs the infrastructure of public-inbox.org, so
he knows of your problem.

Stefan

On Mon, Apr 30, 2018 at 8:59 AM, Samuel Lijin  wrote:
> DNS lookup is working but the website (possibly the server) seems to be
> down. The TLS handshake is never acked:
>
> $ curl -v https://public-inbox.org
> * Rebuilt URL to: https://public-inbox.org/
> *   Trying 64.71.152.64...
> * TCP_NODELAY set
> * Connected to public-inbox.org (64.71.152.64) port 443 (#0)
> * ALPN, offering h2
> * ALPN, offering http/1.1
> * successfully set certificate verify locations:
>CAfile: /etc/ssl/certs/ca-certificates.crt
>CApath: none
> * TLSv1.2 (OUT), TLS handshake, Client hello (1):

Re: [PATCH v4 06/10] ref-filter: use generation number for --contains

[Jakub Narebski]

Derrick Stolee  writes:

> A commit A can reach a commit B only if the generation number of A
> is strictly larger than the generation number of B. This condition
> allows significantly short-circuiting commit-graph walks.
>
> Use generation number for '--contains' type queries.
>
> On a copy of the Linux repository where HEAD is containd in v4.13

Minor typo: containd -> contained.

> but no earlier tag, the command 'git tag --contains HEAD' had the
> following peformance improvement:
>
> Before: 0.81s
> After:  0.04s
> Rel %:  -95%

Very nice.  I guess that any performance changes for when commit-graph
feature is not available are negligible / not measurable.

Rel % = (before - after)/before * 100%, isn't it?.

Good.

>
> Helped-by: Jeff King 
> Signed-off-by: Derrick Stolee 
> ---
>  ref-filter.c | 24 
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index aff24d93be..fb35067fc9 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -16,6 +16,7 @@
>  #include "trailer.h"
>  #include "wt-status.h"
>  #include "commit-slab.h"
> +#include "commit-graph.h"
>  
>  static struct ref_msg {
>   const char *gone;
> @@ -1587,7 +1588,8 @@ static int in_commit_list(const struct commit_list 
> *want, struct commit *c)
>   */
>  static enum contains_result contains_test(struct commit *candidate,
> const struct commit_list *want,
> -   struct contains_cache *cache)
> +   struct contains_cache *cache,
> +   uint32_t cutoff)
>  {
>   enum contains_result *cached = contains_cache_at(cache, candidate);
>  
> @@ -1603,6 +1605,10 @@ static enum contains_result contains_test(struct 
> commit *candidate,
>  
>   /* Otherwise, we don't know; prepare to recurse */
>   parse_commit_or_die(candidate);
> +
> + if (candidate->generation < cutoff)
> + return CONTAINS_NO;
> +

We use here weaker negative-cut criteria, which has the advantage of
simply automatic handling of special values: _INFINITY, _MAX, _ZERO.

Stronger version:

  if A != B and A ---> B, then gen(A) > gen(B)

  if gen(A) <= gen(B) and A != B, then A -/-> B

Weaker version:

  if gen(A) < gen(B), then A -/-> B

If commit-graph feature is not available, then all generation numbers
would be _INFINITY, and cutoff would also be _INFINITY - which means
this operation is practically no-op.  One memory access (probably from
cache) and one comparison is very cheap.

All right.

>   return CONTAINS_UNKNOWN;
>  }
>  
> @@ -1618,8 +1624,18 @@ static enum contains_result contains_tag_algo(struct 
> commit *candidate,
> struct contains_cache *cache)
>  {
>   struct contains_stack contains_stack = { 0, 0, NULL };
> - enum contains_result result = contains_test(candidate, want, cache);
> + enum contains_result result;
> + uint32_t cutoff = GENERATION_NUMBER_INFINITY;
> + const struct commit_list *p;
> +
> + for (p = want; p; p = p->next) {
> + struct commit *c = p->item;
> + load_commit_graph_info(c);
> + if (c->generation < cutoff)
> + cutoff = c->generation;
> + }

For each in wants, load generation numbers if needed and find lowest
one.  Anything lower cannot reach any of wants.  All right.

If commit-graph feature is not available, this is practically no-op.  It
is fast, as it only accesses memory - it does not access disk, nor do it
needs to do any decompression, un-deltafication or parsing.

All right.

>  
> + result = contains_test(candidate, want, cache, cutoff);
>   if (result != CONTAINS_UNKNOWN)
>   return result;
>  
> @@ -1637,7 +1653,7 @@ static enum contains_result contains_tag_algo(struct 
> commit *candidate,
>* If we just popped the stack, parents->item has been marked,
>* therefore contains_test will return a meaningful yes/no.
>*/
> - else switch (contains_test(parents->item, want, cache)) {
> + else switch (contains_test(parents->item, want, cache, cutoff)) 
> {
>   case CONTAINS_YES:
>   *contains_cache_at(cache, commit) = CONTAINS_YES;
>   contains_stack.nr--;
> @@ -1651,7 +1667,7 @@ static enum contains_result contains_tag_algo(struct 
> commit *candidate,
>   }
>   }
>   free(contains_stack.contains_stack);
> - return contains_test(candidate, want, cache);
> + return contains_test(candidate, want, cache, cutoff);

Those two just update callsite to new signatore.  All right.

>  }
>  
>  static int commit_contains(struct ref_filter *filter, struct commit *commit,

Re: [PATCH v3] unpack_trees: fix breakage when o->src_index != o->dst_index

[Duy Nguyen]

On Mon, Apr 30, 2018 at 6:19 PM, Elijah Newren  wrote:
> Hi Duy,
>
> On Mon, Apr 30, 2018 at 7:45 AM, Duy Nguyen  wrote:
>> On Mon, Apr 30, 2018 at 4:42 PM, Duy Nguyen  wrote:
>>> On Sun, Apr 29, 2018 at 10:53 PM, Johannes Schindelin
>>>  wrote:
> > @@ -1412,12 +1422,13 @@ int unpack_trees(unsigned len, struct tree_desc 
> > *t, struct unpack_trees_options
> >   WRITE_TREE_SILENT |
> >   WRITE_TREE_REPAIR);
> > }
> > -   move_index_extensions(>result, o->dst_index);
> > +   move_index_extensions(>result, o->src_index);
>
> While this looks like the right thing to do on paper, I believe it's
> actually broken for a specific case of untracked cache. In short,
> please do not touch this line. I will send a patch to revert
> edf3b90553 (unpack-trees: preserve index extensions - 2017-05-08),
> which essentially deletes this line, with proper explanation and
> perhaps a test if I could come up with one.
>
> When we update the index, we depend on the fact that all updates must
> invalidate the right untracked cache correctly. In this unpack
> operations, we start copying entries over from src to result. Since
> 'result' (at least from the beginning) does not have an untracked
> cache, it has nothing to invalidate when we copy entries over. By the
> time we have done preparing 'result', what's recorded in src's (or
> dst's for that matter) untracked cache may or may not apply to
> 'result'  index anymore. This copying only leads to more problems when
> untracked cache is used.

 Is there really no way to invalidate just individual entries?
>>>
>>> Grr the short answer is the current code (i.e. without Elijah's
>>> changes) works but in a twisted way. So you get to keep untracked
>>> cache in the end.
>>
>> GAAAHH.. it works _with_ Elijah's changes (since he made the change
>> from dst to src) not without (and no performance regression).
>
> So...is that an Acked-by for the patch

Yes, Acked-by: me.

> or does the "two wrong make a
> right, I guess" comment suggest that we should still drop the
> move_index_extensions change (essentially reverting to v1 of the PATCH
> as found at 20180421193736.12722-1-new...@gmail.com), and you'll fix
> things up further in a separate series?

I think I'll stay away from this file for a while. When I gather
enough courage, I'll need to read it through since it sounds like a
mine field.
-- 
Duy

Re: [PATCH v3] unpack_trees: fix breakage when o->src_index != o->dst_index

[Elijah Newren]

Hi Duy,

On Mon, Apr 30, 2018 at 7:45 AM, Duy Nguyen  wrote:
> On Mon, Apr 30, 2018 at 4:42 PM, Duy Nguyen  wrote:
>> On Sun, Apr 29, 2018 at 10:53 PM, Johannes Schindelin
>>  wrote:
 > @@ -1412,12 +1422,13 @@ int unpack_trees(unsigned len, struct tree_desc 
 > *t, struct unpack_trees_options
 >   WRITE_TREE_SILENT |
 >   WRITE_TREE_REPAIR);
 > }
 > -   move_index_extensions(>result, o->dst_index);
 > +   move_index_extensions(>result, o->src_index);

 While this looks like the right thing to do on paper, I believe it's
 actually broken for a specific case of untracked cache. In short,
 please do not touch this line. I will send a patch to revert
 edf3b90553 (unpack-trees: preserve index extensions - 2017-05-08),
 which essentially deletes this line, with proper explanation and
 perhaps a test if I could come up with one.

 When we update the index, we depend on the fact that all updates must
 invalidate the right untracked cache correctly. In this unpack
 operations, we start copying entries over from src to result. Since
 'result' (at least from the beginning) does not have an untracked
 cache, it has nothing to invalidate when we copy entries over. By the
 time we have done preparing 'result', what's recorded in src's (or
 dst's for that matter) untracked cache may or may not apply to
 'result'  index anymore. This copying only leads to more problems when
 untracked cache is used.
>>>
>>> Is there really no way to invalidate just individual entries?
>>
>> Grr the short answer is the current code (i.e. without Elijah's
>> changes) works but in a twisted way. So you get to keep untracked
>> cache in the end.
>
> GAAAHH.. it works _with_ Elijah's changes (since he made the change
> from dst to src) not without (and no performance regression).

So...is that an Acked-by for the patch, or does the "two wrong make a
right, I guess" comment suggest that we should still drop the
move_index_extensions change (essentially reverting to v1 of the PATCH
as found at 20180421193736.12722-1-new...@gmail.com), and you'll fix
things up further in a separate series?

> This file really messes my brain up.

I'm glad I'm not the only one.  :-)


Elijah

Re:

[Elijah Newren]

On Mon, Apr 30, 2018 at 6:11 AM, Ben Peart  wrote:
> On 4/27/2018 2:19 PM, Elijah Newren wrote:
>>
>> From: Elijah Newren 
>>
>> On Thu, Apr 26, 2018 at 5:54 PM, Ben Peart  wrote:
>>
>>> Can you write the documentation that clearly explains the exact behavior
>>> you
>>> want?  That would kill two birds with one stone... :)
>>
>>
>> Sure, something like the following is what I envision, and I've tried to
>> include the suggestion from Junio to document the copy behavior in the
>> merge-recursive documentation.
>>

>
> Thanks Elijah. I've applied this patch and reviewed and tested it.  It works
> and addresses the concerns around the settings inheritance from
> diff.renames.  I still _prefer_ the simpler model that doesn't do the
> partial inheritance but I can use this model as well.
>
> I'm unsure on the protocol here.  Should I incorporate this patch and submit
> a reroll or can it just be applied as is?

I suspect you'll want to re-roll anyway, to base your series on
en/rename-directory-detection-reboot instead of on master.  (Junio
plans to merge it down to next, and your series has four different
merge conflicts with it.)

There are two other loose ends with this series that Junio will need
to weigh in on:

- I'm obviously a strong proponent of the inherited setting, but Junio
may change his mind after reading Dscho's arguments against it (or
after reading my arguments for it).

- I like the setting as-is, and think we could allow a "copy" setting
for merge.renames to specify that the post-merge diffstat should
detect copies (not part of your series, but a useful addition I'd like
to tackle afterwards).  However, Junio had comments in
xmqqwox19ohw@gitster-ct.c.googlers.com about merge.renames
handling the scoring as well, like -Xfind-renames.  Those sound
incompatible to me for a single setting, and I'm unsure if Junio would
resolve them the way I do or still feels strongly about the scoring.

public-inbox.org down?

[Samuel Lijin]

DNS lookup is working but the website (possibly the server) seems to be
down. The TLS handshake is never acked:

$ curl -v https://public-inbox.org
* Rebuilt URL to: https://public-inbox.org/
*   Trying 64.71.152.64...
* TCP_NODELAY set
* Connected to public-inbox.org (64.71.152.64) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
   CAfile: /etc/ssl/certs/ca-certificates.crt
   CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):

[PATCH v2 2/2] wt-status: const-ify all printf helper methods

[Samuel Lijin]

Change the method signatures of all printf helper methods to take a
`const struct wt_status *` rather than a `struct wt_status *`.

Signed-off-by: Samuel Lijin 
---
 wt-status.c | 18 +-
 wt-status.h |  4 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 2e5452731..4360bbfd7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -33,7 +33,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_NIL,/* WT_STATUS_ONBRANCH */
 };
 
-static const char *color(int slot, struct wt_status *s)
+static const char *color(int slot, const struct wt_status *s)
 {
const char *c = "";
if (want_color(s->use_color))
@@ -43,7 +43,7 @@ static const char *color(int slot, struct wt_status *s)
return c;
 }
 
-static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
+static void status_vprintf(const struct wt_status *s, int at_bol, const char 
*color,
const char *fmt, va_list ap, const char *trail)
 {
struct strbuf sb = STRBUF_INIT;
@@ -89,7 +89,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, 
const char *color,
strbuf_release();
 }
 
-void status_printf_ln(struct wt_status *s, const char *color,
+void status_printf_ln(const struct wt_status *s, const char *color,
const char *fmt, ...)
 {
va_list ap;
@@ -99,7 +99,7 @@ void status_printf_ln(struct wt_status *s, const char *color,
va_end(ap);
 }
 
-void status_printf(struct wt_status *s, const char *color,
+void status_printf(const struct wt_status *s, const char *color,
const char *fmt, ...)
 {
va_list ap;
@@ -109,7 +109,7 @@ void status_printf(struct wt_status *s, const char *color,
va_end(ap);
 }
 
-static void status_printf_more(struct wt_status *s, const char *color,
+static void status_printf_more(const struct wt_status *s, const char *color,
   const char *fmt, ...)
 {
va_list ap;
@@ -192,7 +192,7 @@ static void wt_longstatus_print_unmerged_header(struct 
wt_status *s)
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_longstatus_print_cached_header(struct wt_status *s)
+static void wt_longstatus_print_cached_header(const struct wt_status *s)
 {
const char *c = color(WT_STATUS_HEADER, s);
 
@@ -239,7 +239,7 @@ static void wt_longstatus_print_other_header(struct 
wt_status *s,
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_longstatus_print_trailer(struct wt_status *s)
+static void wt_longstatus_print_trailer(const struct wt_status *s)
 {
status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
 }
@@ -332,7 +332,7 @@ static void wt_longstatus_print_unmerged_data(struct 
wt_status *s,
strbuf_release();
 }
 
-static void wt_longstatus_print_change_data(struct wt_status *s,
+static void wt_longstatus_print_change_data(const struct wt_status *s,
int change_type,
struct string_list_item *it)
 {
@@ -768,7 +768,7 @@ static void wt_longstatus_print_unmerged(struct wt_status 
*s)
 
 }
 
-static void wt_longstatus_print_updated(struct wt_status *s)
+static void wt_longstatus_print_updated(const struct wt_status *s)
 {
int i;
 
diff --git a/wt-status.h b/wt-status.h
index 430770b85..83a1f7c00 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -135,9 +135,9 @@ int wt_status_check_bisect(const struct worktree *wt,
   struct wt_status_state *state);
 
 __attribute__((format (printf, 3, 4)))
-void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, 
...);
+void status_printf_ln(const struct wt_status *s, const char *color, const char 
*fmt, ...);
 __attribute__((format (printf, 3, 4)))
-void status_printf(struct wt_status *s, const char *color, const char *fmt, 
...);
+void status_printf(const struct wt_status *s, const char *color, const char 
*fmt, ...);
 
 /* The following functions expect that the caller took care of reading the 
index. */
 int has_unstaged_changes(int ignore_submodules);
-- 
2.17.0

[PATCH v2 1/2] commit: fix --short and --porcelain options

[Samuel Lijin]

Mark the commitable flag in the wt_status object in the call to
`wt_status_collect()`, instead of in `wt_longstatus_print_updated()`,
and simplify the logic in the latter function to take advantage of the
logic shifted to the former. This means that callers do not need to use
`wt_longstatus_print_updated()` to collect the `commitable` flag;
calling `wt_status_collect()` is sufficient.

As a result, invoking `git commit` with `--short` or `--porcelain`
(which imply `--dry-run`, but previously returned an inconsistent error
code inconsistent with dry run behavior) correctly returns status code
zero when there is something to commit. This fixes two bugs documented
in the test suite.

Signed-off-by: Samuel Lijin 
---
 t/t7501-commit.sh |  4 ++--
 wt-status.c   | 38 +++---
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index fa61b1a4e..85a8217fd 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -87,12 +87,12 @@ test_expect_success '--dry-run with stuff to commit returns 
ok' '
git commit -m next -a --dry-run
 '
 
-test_expect_failure '--short with stuff to commit returns ok' '
+test_expect_success '--short with stuff to commit returns ok' '
echo bongo bongo bongo >>file &&
git commit -m next -a --short
 '
 
-test_expect_failure '--porcelain with stuff to commit returns ok' '
+test_expect_success '--porcelain with stuff to commit returns ok' '
echo bongo bongo bongo >>file &&
git commit -m next -a --porcelain
 '
diff --git a/wt-status.c b/wt-status.c
index 50815e5fa..2e5452731 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -718,6 +718,19 @@ static void wt_status_collect_untracked(struct wt_status 
*s)
s->untracked_in_ms = (getnanotime() - t_begin) / 100;
 }
 
+static void wt_status_mark_commitable(struct wt_status *s) {
+   int i;
+
+   for (i = 0; i < s->change.nr; i++) {
+   struct wt_status_change_data *d = (s->change.items[i]).util;
+
+   if (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) 
{
+   s->commitable = 1;
+   return;
+   }
+   }
+}
+
 void wt_status_collect(struct wt_status *s)
 {
wt_status_collect_changes_worktree(s);
@@ -726,7 +739,10 @@ void wt_status_collect(struct wt_status *s)
wt_status_collect_changes_initial(s);
else
wt_status_collect_changes_index(s);
+
wt_status_collect_untracked(s);
+
+   wt_status_mark_commitable(s);
 }
 
 static void wt_longstatus_print_unmerged(struct wt_status *s)
@@ -754,26 +770,26 @@ static void wt_longstatus_print_unmerged(struct wt_status 
*s)
 
 static void wt_longstatus_print_updated(struct wt_status *s)
 {
-   int shown_header = 0;
int i;
 
+   if (!s->commitable) {
+   return;
+   }
+
+   wt_longstatus_print_cached_header(s);
+
for (i = 0; i < s->change.nr; i++) {
struct wt_status_change_data *d;
struct string_list_item *it;
it = &(s->change.items[i]);
d = it->util;
-   if (!d->index_status ||
-   d->index_status == DIFF_STATUS_UNMERGED)
-   continue;
-   if (!shown_header) {
-   wt_longstatus_print_cached_header(s);
-   s->commitable = 1;
-   shown_header = 1;
+   if (d->index_status &&
+   d->index_status != DIFF_STATUS_UNMERGED) {
+   wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, 
it);
}
-   wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);
}
-   if (shown_header)
-   wt_longstatus_print_trailer(s);
+
+   wt_longstatus_print_trailer(s);
 }
 
 /*
-- 
2.17.0

[PATCH v2 0/2] Fix --short and --porcelain options for commit

[Samuel Lijin]

Rerolling patch series to fix t7501.

Samuel Lijin (2):
  commit: fix --short and --porcelain options
  wt-status: const-ify all printf helper methods

 t/t7501-commit.sh |  4 ++--
 wt-status.c   | 56 ++-
 wt-status.h   |  4 ++--
 3 files changed, 40 insertions(+), 24 deletions(-)

-- 
2.17.0

Re: [PATCH v5 00/10] Keep all info in command-list.txt in git binary

[Duy Nguyen]

This is probably scope creep for this series, but do you guys think we
should do the same for config variables completion? We currently
maintain a giant list at the end of _git_config(). Extracting the list
from Documentation/config.txt to keep it in a C array does not look
super hard. There will be some special handling for advice.* or
color but overall I still think it's a net gain.

Listing all recognizable config variables from "git config" (or "git
help") would be lovely, but I don't think it helps unless we could
print a short summary line each variable, but this info is not
available anywhere.
--
Duy

Re: [PATCH v1 1/1] test: Correct detection of UTF8_NFD_TO_NFC for APFS

[Elijah Newren]

On Mon, Apr 30, 2018 at 8:41 AM, Torsten Bögershausen  wrote:
> On 30.04.18 17:33, Elijah Newren wrote:
>
>> On Sun, Apr 29, 2018 at 11:35 PM,   wrote:
>>> From: Torsten Bögershausen 
>>>

>>> @@ -1106,12 +1106,7 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
>>
>> I'm not sure what "NFD" and "NFC" stand for, but I suspect the test
>> prerequisite name may be specific to how HFS handled things.  If so,
>> should it be renamed from UTF8_NFD_TO_NFC to something else, such as
>> UTF8_NORMALIZATION?
>
> NFD and NFC both come from the unicode standard, and are just taken
> "as is" into the Git world:
> https://en.wikipedia.org/wiki/Unicode_equivalence
>
> If you are otherwise happy with the patch, would it be possible
> to run it on your system ?
> (I don't have a High Sierra box, but I am confident that the test work
>  for you).

Sure, feel free to add a

Tested-by: Elijah Newren 

if you like.  (Works with both 'test -r', as in your patch, and 'test
-f', as suggested by Junio.)

Re: [PATCH v5 0/5] Convert some stash functionality to a builtin

[Joel Teichroeb]

Re-sending, but this time in plain text (why doesn't gmail for android
support that...)

On Mon, Apr 30, 2018 at 7:35 AM, Joel Teichroeb  wrote:
> Hi Paul,
>
> That sounds great to me!
>
> Thanks,
> Joel
>
>
> On Sat, Apr 28, 2018, 3:06 PM Paul-Sebastian Ungureanu
>  wrote:
>>
>> Hello Joel,
>>
>> >> Since there seems to be interest from GSOC students who want to
>> >> work on converting builtins, I figured I should finish what I
>> >> have that works now so they could build on top of it.
>>
>> First of all, I must thank you for submitting this series of patches. It
>> is a great starting point to convert 'git stash'.
>>
>> I would like to continue your work on "git stash" if that is fine with
>> you. I will continue to build on top of it, starting with applying some
>> patches in order to implement what was already suggested in this thread.
>>   During the summer, I am planning to finish this process and,
>> hopefully, have a 100% C, built-in 'git stash'.
>>
>> Best regards,
>> Paul

Re: [PATCH v1 1/1] test: Correct detection of UTF8_NFD_TO_NFC for APFS

[Torsten Bögershausen]

On 30.04.18 17:33, Elijah Newren wrote:
> Hi,
> 
> On Sun, Apr 29, 2018 at 11:35 PM,   wrote:
>> From: Torsten Bögershausen 
>>
>> On HFS (which is the default Mac filesystem prior to High Sierra),
>> unicode names are "decomposed" before recording.
>> On APFS, which appears to be the new default filesystem in Mac OS High
>> Sierra, filenames are recorded as specified by the user.
>>
>> APFS continues to allow the user to access it via any name
>> that normalizes to the same thing.
>>
>> This difference causes t0050-filesystem.sh to fail two tests.
>>
>> Improve the test for a NFD/NFC in test-lib.sh:
>> Test if the same file can be reached in pre- and decomposed unicode.
>>
>> Reported-By: Elijah Newren 
>> Signed-off-by: Torsten Bögershausen 
>> ---
>>  t/test-lib.sh | 7 +--
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index ea2bbaaa7a..e206250d1b 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -1106,12 +1106,7 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
> 
> I'm not sure what "NFD" and "NFC" stand for, but I suspect the test
> prerequisite name may be specific to how HFS handled things.  If so,
> should it be renamed from UTF8_NFD_TO_NFC to something else, such as
> UTF8_NORMALIZATION?

NFD and NFC both come from the unicode standard, and are just taken
"as is" into the Git world:
https://en.wikipedia.org/wiki/Unicode_equivalence

If you are otherwise happy with the patch, would it be possible
to run it on your system ?
(I don't have a High Sierra box, but I am confident that the test work
 for you).

The other comments may be addressed later, may be.
In any case, they should go into a different commit.
 

[PATCH] doc: keep first level section header in upper case

[Nguyễn Thái Ngọc Duy]

When formatted as a man page, 1st section header is always in upper
case even if we write it otherwise. Make all 1st section headers
uppercase to keep it close to the final output.

This does affect html since case is kept there, but I still think it's
a good idea to maintain a consistent style for 1st section headers.

Some sections perhaps should become second sections instead, where
case is kept, and for better organization. I will update if anyone has
suggestions about this.

While at there I also make some header more consistent (e.g. examples
vs example) and fix a couple minor things here and there.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-add.txt   |  4 ++--
 Documentation/git-apply.txt |  4 ++--
 Documentation/git-branch.txt|  4 ++--
 Documentation/git-bundle.txt|  4 ++--
 Documentation/git-clone.txt |  2 +-
 Documentation/git-cvsserver.txt | 10 +-
 Documentation/git-diff-index.txt|  6 +++---
 Documentation/git-diff-tree.txt |  2 +-
 Documentation/git-fast-export.txt   |  2 +-
 Documentation/git-fast-import.txt   | 22 +++---
 Documentation/git-filter-branch.txt |  6 +++---
 Documentation/git-fmt-merge-msg.txt |  4 ++--
 Documentation/git-gc.txt|  4 ++--
 Documentation/git-grep.txt  |  2 +-
 Documentation/git-http-push.txt |  2 +-
 Documentation/git-imap-send.txt |  4 ++--
 Documentation/git-index-pack.txt|  4 ++--
 Documentation/git-ls-files.txt  |  4 ++--
 Documentation/git-name-rev.txt  |  4 ++--
 Documentation/git-p4.txt|  4 ++--
 Documentation/git-prune.txt |  6 +++---
 Documentation/git-push.txt  |  4 ++--
 Documentation/git-read-tree.txt |  4 ++--
 Documentation/git-receive-pack.txt  | 10 +-
 Documentation/git-remote-ext.txt| 12 ++--
 Documentation/git-remote.txt|  2 +-
 Documentation/git-request-pull.txt  |  4 ++--
 Documentation/git-send-email.txt|  4 ++--
 Documentation/git-send-pack.txt |  2 +-
 Documentation/git-shell.txt |  4 ++--
 Documentation/git-show-branch.txt   |  4 ++--
 Documentation/git-show-ref.txt  |  4 ++--
 Documentation/git-show.txt  |  2 +-
 Documentation/git-update-index.txt  | 20 ++--
 Documentation/git-update-ref.txt|  2 +-
 Documentation/git-var.txt   |  4 ++--
 Documentation/git-web--browse.txt   |  2 +-
 Documentation/gitattributes.txt |  4 ++--
 38 files changed, 96 insertions(+), 96 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index d50fa339dc..45652fe4a6 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -193,7 +193,7 @@ for "git add --no-all ...", i.e. ignored removed 
files.
for command-line options).
 
 
-Configuration
+CONFIGURATION
 -
 
 The optional configuration variable `core.excludesFile` indicates a path to a
@@ -226,7 +226,7 @@ Because this example lets the shell expand the asterisk 
(i.e. you are
 listing the files explicitly), it does not consider
 `subdir/git-foo.sh`.
 
-Interactive mode
+INTERACTIVE MODE
 
 When the command enters the interactive mode, it shows the
 output of the 'status' subcommand, and then goes into its
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 4ebc3d3271..ad0888bfcb 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -240,7 +240,7 @@ When `git apply` is used as a "better GNU patch", the user 
can pass
 the `--unsafe-paths` option to override this safety check.  This option
 has no effect when `--index` or `--cached` is in use.
 
-Configuration
+CONFIGURATION
 -
 
 apply.ignoreWhitespace::
@@ -251,7 +251,7 @@ apply.whitespace::
When no `--whitespace` flag is given from the command
line, this configuration item is used as the default.
 
-Submodules
+SUBMODULES
 --
 If the patch contains any changes to submodules then 'git apply'
 treats these changes as follows.
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index b3084c99c1..02eccbb931 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -287,7 +287,7 @@ CONFIGURATION
 `--list` is used or implied. The default is to use a pager.
 See linkgit:git-config[1].
 
-Examples
+EXAMPLES
 
 
 Start development from a known tag::
@@ -318,7 +318,7 @@ See linkgit:git-fetch[1].
 is currently checked out) does not have all commits from the test branch.
 
 
-Notes
+NOTES
 -
 
 If you are creating a branch that you want to checkout immediately, it is
diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt
index 3a8120c3b3..7d6c9dcd17 100644
--- a/Documentation/git-bundle.txt
+++ b/Documentation/git-bundle.txt
@@ -92,8 +92,8 @@ It is okay to err on the side of caution, causing the bundle 
file
 to contain objects already in the destination, as these are 

Re: What's cooking in git.git (Apr 2018, #04; Mon, 30)

[SZEDER Gábor]

On Mon, Apr 30, 2018 at 5:25 AM, Junio C Hamano  wrote:
> * js/rebase-i-clean-msg-after-fixup-continue (2018-04-30) 4 commits
>   - rebase --skip: clean up commit message after a failed fixup/squash
>   - sequencer: always commit without editing when asked for
>   - rebase -i: Handle "combination of  commits" with GETTEXT_POISON
>   - rebase -i: demonstrate bugs with fixup!/squash! commit messages

>   "git rebase -i" sometimes left intermediate "# This is a
>   combination of N commits" message meant for the human consumption
>   inside an editor in the final result in certain corner cases, which
>   has been fixed.

>   Will merge to 'next'.

This topic branches off from v2.16.3.  However, its last patch uses
the sequencer's parse_head() function, which was only added in
v2.17.0-rc0~110^2~6 (sequencer: try to commit without forking 'git
commit', 2017-11-24), in topic 'pw/sequencer-in-process-commit',
leading to compilation errors.

Re: [PATCH v1 1/1] test: Correct detection of UTF8_NFD_TO_NFC for APFS

[Elijah Newren]

Hi,

On Sun, Apr 29, 2018 at 11:35 PM,   wrote:
> From: Torsten Bögershausen 
>
> On HFS (which is the default Mac filesystem prior to High Sierra),
> unicode names are "decomposed" before recording.
> On APFS, which appears to be the new default filesystem in Mac OS High
> Sierra, filenames are recorded as specified by the user.
>
> APFS continues to allow the user to access it via any name
> that normalizes to the same thing.
>
> This difference causes t0050-filesystem.sh to fail two tests.
>
> Improve the test for a NFD/NFC in test-lib.sh:
> Test if the same file can be reached in pre- and decomposed unicode.
>
> Reported-By: Elijah Newren 
> Signed-off-by: Torsten Bögershausen 
> ---
>  t/test-lib.sh | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index ea2bbaaa7a..e206250d1b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1106,12 +1106,7 @@ test_lazy_prereq UTF8_NFD_TO_NFC '

I'm not sure what "NFD" and "NFC" stand for, but I suspect the test
prerequisite name may be specific to how HFS handled things.  If so,
should it be renamed from UTF8_NFD_TO_NFC to something else, such as
UTF8_NORMALIZATION?

> auml=$(printf "\303\244")
> aumlcdiar=$(printf "\141\314\210")
> >"$auml" &&
> -   case "$(echo *)" in
> -   "$aumlcdiar")
> -   true ;;
> -   *)
> -   false ;;
> -   esac
> +   test -r "$aumlcdiar"
>  '
>
>  test_lazy_prereq AUTOIDENT '
> --
> 2.16.0.rc0.8.g5497051b43

[PATCH v2 1/5] read-cache: teach refresh_cache_entry() to take istate

[Jameson Miller]

Refactor refresh_cache_entry() to work on a specific index, instead of
implicitly using the_index. This is in preparation for making the
make_cache_entry function work on a specific index.

Signed-off-by: Jameson Miller 
---
 cache.h   | 2 +-
 merge-recursive.c | 2 +-
 read-cache.c  | 7 ---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 77b7acebb6..31f8f0420a 100644
--- a/cache.h
+++ b/cache.h
@@ -743,7 +743,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, 
struct stat *st);
 #define REFRESH_IGNORE_SUBMODULES  0x0010  /* ignore submodules */
 #define REFRESH_IN_PORCELAIN   0x0020  /* user friendly output, not "needs 
update" */
 extern int refresh_index(struct index_state *, unsigned int flags, const 
struct pathspec *pathspec, char *seen, const char *header_msg);
-extern struct cache_entry *refresh_cache_entry(struct cache_entry *, unsigned 
int);
+extern struct cache_entry *refresh_cache_entry(struct index_state *, struct 
cache_entry *, unsigned int);
 
 /*
  * Opportunistically update the index but do not complain if we can't.
diff --git a/merge-recursive.c b/merge-recursive.c
index 0c0d48624d..693f60e0a3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -260,7 +260,7 @@ static int add_cacheinfo(struct merge_options *o,
if (refresh) {
struct cache_entry *nce;
 
-   nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | 
CE_MATCH_IGNORE_MISSING);
+   nce = refresh_cache_entry(_index, ce, CE_MATCH_REFRESH | 
CE_MATCH_IGNORE_MISSING);
if (!nce)
return err(o, _("addinfo_cache failed for path '%s'"), 
path);
if (nce != ce)
diff --git a/read-cache.c b/read-cache.c
index 10f1c6bb8a..2cb4f53b57 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -767,7 +767,7 @@ struct cache_entry *make_cache_entry(unsigned int mode,
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);
 
-   ret = refresh_cache_entry(ce, refresh_options);
+   ret = refresh_cache_entry(_index, ce, refresh_options);
if (ret != ce)
free(ce);
return ret;
@@ -1448,10 +1448,11 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
return has_errors;
 }
 
-struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
+struct cache_entry *refresh_cache_entry(struct index_state *istate,
+  struct cache_entry *ce,
   unsigned int options)
 {
-   return refresh_cache_ent(_index, ce, options, NULL, NULL);
+   return refresh_cache_ent(istate, ce, options, NULL, NULL);
 }
 
 
-- 
2.14.3

[PATCH v2 3/5] mem-pool: fill out functionality

[Jameson Miller]

Adds the following functionality to memory pools:

 - Lifecycle management functions (init, discard)

 - Test whether a memory location is part of the managed pool

 - Function to combine 2 pools

This also adds logic to track all memory allocations made by a memory
pool.

These functions will be used in a future commit in this commit series.

Signed-off-by: Jameson Miller 
---
 mem-pool.c | 114 ++---
 mem-pool.h |  32 +
 2 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/mem-pool.c b/mem-pool.c
index 389d7af447..a495885c4b 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -5,6 +5,8 @@
 #include "cache.h"
 #include "mem-pool.h"
 
+#define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
+
 static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t 
block_alloc)
 {
struct mp_block *p;
@@ -19,6 +21,60 @@ static struct mp_block *mem_pool_alloc_block(struct mem_pool 
*mem_pool, size_t b
return p;
 }
 
+static void *mem_pool_alloc_custom(struct mem_pool *mem_pool, size_t 
block_alloc)
+{
+   char *p;
+   ALLOC_GROW(mem_pool->custom, mem_pool->nr + 1, mem_pool->alloc);
+   ALLOC_GROW(mem_pool->custom_end, mem_pool->nr_end + 1, 
mem_pool->alloc_end);
+
+   p = xmalloc(block_alloc);
+   mem_pool->custom[mem_pool->nr++] = p;
+   mem_pool->custom_end[mem_pool->nr_end++] = p + block_alloc;
+
+   mem_pool->pool_alloc += block_alloc;
+
+   return mem_pool->custom[mem_pool->nr];
+}
+
+void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size)
+{
+   if (!(*mem_pool))
+   {
+   *mem_pool = xmalloc(sizeof(struct mem_pool));
+   (*mem_pool)->pool_alloc = 0;
+   (*mem_pool)->mp_block = NULL;
+   (*mem_pool)->block_alloc = BLOCK_GROWTH_SIZE;
+   (*mem_pool)->custom = NULL;
+   (*mem_pool)->nr = 0;
+   (*mem_pool)->alloc = 0;
+   (*mem_pool)->custom_end = NULL;
+   (*mem_pool)->nr_end = 0;
+   (*mem_pool)->alloc_end = 0;
+
+   if (initial_size > 0)
+   mem_pool_alloc_block(*mem_pool, initial_size);
+   }
+}
+
+void mem_pool_discard(struct mem_pool *mem_pool)
+{
+   int i;
+   struct mp_block *block, *block_to_free;
+   for (block = mem_pool->mp_block; block;)
+   {
+   block_to_free = block;
+   block = block->next_block;
+   free(block_to_free);
+   }
+
+   for (i = 0; i < mem_pool->nr; i++)
+   free(mem_pool->custom[i]);
+
+   free(mem_pool->custom);
+   free(mem_pool->custom_end);
+   free(mem_pool);
+}
+
 void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)
 {
struct mp_block *p;
@@ -33,10 +89,8 @@ void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)
break;
 
if (!p) {
-   if (len >= (mem_pool->block_alloc / 2)) {
-   mem_pool->pool_alloc += len;
-   return xmalloc(len);
-   }
+   if (len >= (mem_pool->block_alloc / 2))
+   return mem_pool_alloc_custom(mem_pool, len);
 
p = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);
}
@@ -53,3 +107,55 @@ void *mem_pool_calloc(struct mem_pool *mem_pool, size_t 
count, size_t size)
memset(r, 0, len);
return r;
 }
+
+int mem_pool_contains(struct mem_pool *mem_pool, void *mem)
+{
+   int i;
+   struct mp_block *p;
+
+   /* Check if memory is allocated in a block */
+   for (p = mem_pool->mp_block; p; p = p->next_block)
+   if ((mem >= ((void *)p->space)) &&
+   (mem < ((void *)p->end)))
+   return 1;
+
+   /* Check if memory is allocated in custom block */
+   for (i = 0; i < mem_pool->nr; i++)
+   if ((mem >= mem_pool->custom[i]) &&
+   (mem < mem_pool->custom_end[i]))
+   return 1;
+
+   return 0;
+}
+
+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src)
+{
+   int i;
+   struct mp_block **tail = >mp_block;
+
+   /* Find pointer of dst's last block (if any) */
+   while (*tail)
+   tail = &(*tail)->next_block;
+
+   /* Append the blocks from src to dst */
+   *tail = src->mp_block;
+
+   /* Combine custom allocations */
+   ALLOC_GROW(dst->custom, dst->nr + src->nr, dst->alloc);
+   ALLOC_GROW(dst->custom_end, dst->nr_end + src->nr_end, dst->alloc_end);
+
+   for (i = 0; i < src->nr; i++) {
+   dst->custom[dst->nr++] = src->custom[i];
+   dst->custom_end[dst->nr_end++] = src->custom_end[i];
+   }
+
+   dst->pool_alloc += src->pool_alloc;
+   src->pool_alloc = 0;
+   src->mp_block = NULL;
+   src->custom = NULL;
+   src->nr = 0;
+ 

[PATCH v2 5/5] block alloc: add validations around cache_entry lifecyle

[Jameson Miller]

Add an option (controlled by an environment variable) perform extra
validations on mem_pool allocated cache entries. When set:

  1) Invalidate cache_entry memory when discarding cache_entry.

  2) When discarding index_state struct, verify that all cache_entries
 were allocated from expected mem_pool.

  3) When discarding mem_pools, invalidate mem_pool memory.

This should provide extra checks that mem_pools and their allocated
cache_entries are being used as expected.

Signed-off-by: Jameson Miller 
---
 cache.h  |  7 +++
 git.c|  3 +++
 mem-pool.c   | 24 +++-
 mem-pool.h   |  2 ++
 read-cache.c | 47 +++
 5 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 7ed68f28e0..8f10f0649b 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,13 @@ void discard_index_cache_entry(struct cache_entry *ce);
  */
 void discard_transient_cache_entry(struct cache_entry *ce);
 
+/*
+ * Validate the cache entries in the index.  This is an internal
+ * consistency check that the cache_entry structs are allocated from
+ * the expected memory pool.
+ */
+void validate_cache_entries(const struct index_state *istate);
+
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
 #define active_cache (the_index.cache)
 #define active_nr (the_index.cache_nr)
diff --git a/git.c b/git.c
index f598fae7b7..28ec7a6c4f 100644
--- a/git.c
+++ b/git.c
@@ -347,7 +347,10 @@ static int run_builtin(struct cmd_struct *p, int argc, 
const char **argv)
 
trace_argv_printf(argv, "trace: built-in: git");
 
+   validate_cache_entries(_index);
status = p->fn(argc, argv, prefix);
+   validate_cache_entries(_index);
+
if (status)
return status;
 
diff --git a/mem-pool.c b/mem-pool.c
index a495885c4b..77adb5d5b9 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -60,21 +60,43 @@ void mem_pool_discard(struct mem_pool *mem_pool)
 {
int i;
struct mp_block *block, *block_to_free;
+   int invalidate_memory = should_validate_cache_entries();
+
for (block = mem_pool->mp_block; block;)
{
block_to_free = block;
block = block->next_block;
+
+   if (invalidate_memory)
+   memset(block_to_free->space, 0xDD, ((char 
*)block_to_free->end) - ((char *)block_to_free->space));
+
free(block_to_free);
}
 
-   for (i = 0; i < mem_pool->nr; i++)
+   for (i = 0; i < mem_pool->nr; i++) {
+   memset(mem_pool->custom[i], 0xDD, ((char 
*)mem_pool->custom_end[i]) - ((char *)mem_pool->custom[i]));
free(mem_pool->custom[i]);
+   }
 
free(mem_pool->custom);
free(mem_pool->custom_end);
free(mem_pool);
 }
 
+int should_validate_cache_entries(void)
+{
+   static int validate_index_cache_entries = -1;
+
+   if (validate_index_cache_entries < 0) {
+   if (getenv("GIT_TEST_VALIDATE_INDEX_CACHE_ENTRIES"))
+   validate_index_cache_entries = 1;
+   else
+   validate_index_cache_entries = 0;
+   }
+
+   return validate_index_cache_entries;
+}
+
 void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)
 {
struct mp_block *p;
diff --git a/mem-pool.h b/mem-pool.h
index 34df4fa709..b1f9a920ba 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -63,4 +63,6 @@ void mem_pool_combine(struct mem_pool *dst, struct mem_pool 
*src);
  */
 int mem_pool_contains(struct mem_pool *mem_pool, void *mem);
 
+int should_validate_cache_entries(void);
+
 #endif
diff --git a/read-cache.c b/read-cache.c
index 01cd7fea41..e1dc9f7f33 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1270,6 +1270,8 @@ int add_index_entry(struct index_state *istate, struct 
cache_entry *ce, int opti
 {
int pos;
 
+   validate_cache_entries(istate);
+
if (option & ADD_CACHE_JUST_APPEND)
pos = istate->cache_nr;
else {
@@ -1290,6 +1292,8 @@ int add_index_entry(struct index_state *istate, struct 
cache_entry *ce, int opti
   istate->cache_nr - pos - 1);
set_index_entry(istate, pos, ce);
istate->cache_changed |= CE_ENTRY_ADDED;
+
+   validate_cache_entries(istate);
return 0;
 }
 
@@ -2013,6 +2017,8 @@ int is_index_unborn(struct index_state *istate)
 
 int discard_index(struct index_state *istate)
 {
+   validate_cache_entries(istate);
+
resolve_undo_clear_index(istate);
istate->cache_nr = 0;
istate->cache_changed = 0;
@@ -2035,6 +2041,43 @@ int discard_index(struct index_state *istate)
return 0;
 }
 
+
+/*
+ * Validate the cache entries of this index.
+ * All cache entries associated with this index
+ * should have been allocated by the memory pool
+ * associated with this index, or by a referenced
+ * split index.
+ */
+void validate_cache_entries(const struct index_state *istate)
+{
+ 

[PATCH v2 4/5] block alloc: allocate cache entries from mem_pool

[Jameson Miller]

When reading large indexes from disk, a portion of the time is
dominated in malloc() calls. This can be mitigated by allocating a
large block of memory and manage it ourselves via memory pools.

This change moves the cache entry allocation to be on top of memory
pools.

Design:
The index_state struct will gain a notion of an associated memory_pool
from which cache_entries will be allocated from. When reading in the
index from disk, we have information on the number of entries and
their size, which can guide us in deciding how large our initial
memory allocation should be. When an index is discarded, the
associated memory_pool will be discarded as well - so the lifetime of
a cache_entry is tied to the lifetime of the index_state that it was
allocated for.

In the case of a Split Index, the following rules are followed. 1st,
some terminology is defined:

Terminology:
  - 'the_index': represents the logical view of the index

  - 'split_index': represents the "base" cache entries. Read from the
split index file.

'the_index' can reference a single split_index, as well as
cache_entries from the split_index. `the_index` will be discarded
before the `split_index` is.  This means that when we are allocating
cache_entries in the presence of a split index, we need to allocate
the entries from the `split_index`'s memory pool.  This allows us to
follow the pattern that `the_index` can reference cache_entries from
the `split_index`, and that the cache_entries will not be freed while
they are still being referenced.

Alternatives:
The current design does not track whether a cache_entry is allocated
from a pool or not. Instead, it relies on the caller to know how the
cache_entry will be used and to handle its lifecyle appropriately,
including calling the correct free() method. Instead, we could include
a bit of information in the cache_entry (either a bit indicating
whether cache_entry was allocated from a pool or not, or possibly even
a pointer back to the allocating pool), which can then be used to make
informed decisions about these objects. The downside of this approach
is that the cache_entry type would need to grow to incorporate this
information.

Signed-off-by: Jameson Miller 
---
 cache.h   |  2 ++
 read-cache.c  | 95 +--
 split-index.c | 23 +--
 3 files changed, 95 insertions(+), 25 deletions(-)

diff --git a/cache.h b/cache.h
index 3760adbe25..7ed68f28e0 100644
--- a/cache.h
+++ b/cache.h
@@ -15,6 +15,7 @@
 #include "path.h"
 #include "sha1-array.h"
 #include "repository.h"
+#include "mem-pool.h"
 
 #include 
 typedef struct git_zstream {
@@ -328,6 +329,7 @@ struct index_state {
struct untracked_cache *untracked;
uint64_t fsmonitor_last_update;
struct ewah_bitmap *fsmonitor_dirty;
+   struct mem_pool *ce_mem_pool;
 };
 
 extern struct index_state the_index;
diff --git a/read-cache.c b/read-cache.c
index 2a61cee130..01cd7fea41 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -46,6 +46,42 @@
 CE_ENTRY_ADDED | CE_ENTRY_REMOVED | CE_ENTRY_CHANGED | \
 SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED | FSMONITOR_CHANGED)
 
+
+/*
+ * This is a guess of an pathname in the index.  We use this for V4
+ * index files to guess the un-deltafied size of the index in memory
+ * because of pathname deltafication.  This is not required for V2/V3
+ * index formats because their pathnames are not compressed.  If the
+ * initial amount of memory set aside is not sufficient, the mem pool
+ * will allocate extra memory.
+ */
+#define CACHE_ENTRY_PATH_LENGTH 80
+
+static inline struct cache_entry *mem_pool__ce_alloc(struct mem_pool 
*mem_pool, size_t len)
+{
+   return mem_pool_alloc(mem_pool, cache_entry_size(len));
+}
+
+static inline struct cache_entry *mem_pool__ce_calloc(struct mem_pool 
*mem_pool, size_t len)
+{
+   return mem_pool_calloc(mem_pool, 1, cache_entry_size(len));
+}
+
+static struct mem_pool *find_mem_pool(struct index_state *istate)
+{
+   struct mem_pool **pool_ptr;
+
+   if (istate->split_index && istate->split_index->base)
+   pool_ptr = >split_index->base->ce_mem_pool;
+   else
+   pool_ptr = >ce_mem_pool;
+
+   if (!*pool_ptr)
+   mem_pool_init(pool_ptr, 0);
+
+   return *pool_ptr;
+}
+
 struct index_state the_index;
 static const char *alternate_index_output;
 
@@ -746,7 +782,7 @@ int add_file_to_index(struct index_state *istate, const 
char *path, int flags)
 
 struct cache_entry *make_empty_index_cache_entry(struct index_state *istate, 
size_t len)
 {
-   return xcalloc(1, cache_entry_size(len));
+   return mem_pool__ce_calloc(find_mem_pool(istate), len);
 }
 
 struct cache_entry *make_empty_transient_cache_entry(size_t len)
@@ -1641,13 +1677,13 @@ int read_index(struct index_state *istate)
return read_index_from(istate, get_index_file(), get_git_dir());
 }
 
-static struct 

[PATCH v2 2/5] block alloc: add lifecycle APIs for cache_entry structs

[Jameson Miller]

Add an API around managing the lifetime of cache_entry
structs. Abstracting memory management details behind an API will
allow for alternative memory management strategies without affecting
all the call sites.  This commit does not change how memory is
allocated / freed. A later commit in this series will allocate cache
entries from memory pools as appropriate.

Motivation:
It has been observed that the time spent loading an index with a large
number of entries is partly dominated by malloc() calls. This change
is in preparation for using memory pools to reduce the number of
malloc() calls made when loading an index.

This API makes a distinction between cache entries that are intended
for use with a particular index and cache entries that are not. This
enables us to use the knowledge about how a cache entry will be used
to make informed decisions about how to handle the corresponding
memory.

Signed-off-by: Jameson Miller 
---
 apply.c|  26 ++--
 blame.c|   5 +--
 builtin/checkout.c |   8 ++--
 builtin/difftool.c |   8 ++--
 builtin/reset.c|   6 +--
 builtin/update-index.c |  26 ++--
 cache.h|  29 +-
 merge-recursive.c  |   2 +-
 read-cache.c   | 105 +++--
 resolve-undo.c |   6 ++-
 split-index.c  |   8 ++--
 tree.c |   4 +-
 unpack-trees.c |  27 -
 13 files changed, 166 insertions(+), 94 deletions(-)

diff --git a/apply.c b/apply.c
index 7e5792c996..123646e1aa 100644
--- a/apply.c
+++ b/apply.c
@@ -4090,12 +4090,12 @@ static int build_fake_ancestor(struct apply_state 
*state, struct patch *list)
return error(_("sha1 information is lacking or useless "
   "(%s)."), name);
 
-   ce = make_cache_entry(patch->old_mode, oid.hash, name, 0, 0);
+   ce = make_index_cache_entry(, patch->old_mode, oid.hash, 
name, 0, 0);
if (!ce)
-   return error(_("make_cache_entry failed for path '%s'"),
+   return error(_("make_index_cache_entry failed for path 
'%s'"),
 name);
if (add_index_entry(, ce, ADD_CACHE_OK_TO_ADD)) {
-   free(ce);
+   discard_index_cache_entry(ce);
return error(_("could not add %s to temporary index"),
 name);
}
@@ -4263,12 +4263,11 @@ static int add_index_file(struct apply_state *state,
struct stat st;
struct cache_entry *ce;
int namelen = strlen(path);
-   unsigned ce_size = cache_entry_size(namelen);
 
if (!state->update_index)
return 0;
 
-   ce = xcalloc(1, ce_size);
+   ce = make_empty_index_cache_entry(_index, namelen);
memcpy(ce->name, path, namelen);
ce->ce_mode = create_ce_mode(mode);
ce->ce_flags = create_ce_flags(0);
@@ -4278,13 +4277,13 @@ static int add_index_file(struct apply_state *state,
 
if (!skip_prefix(buf, "Subproject commit ", ) ||
get_oid_hex(s, >oid)) {
-   free(ce);
-  return error(_("corrupt patch for submodule %s"), path);
+   discard_index_cache_entry(ce);
+   return error(_("corrupt patch for submodule %s"), path);
}
} else {
if (!state->cached) {
if (lstat(path, ) < 0) {
-   free(ce);
+   discard_index_cache_entry(ce);
return error_errno(_("unable to stat newly "
 "created file '%s'"),
   path);
@@ -4292,13 +4291,13 @@ static int add_index_file(struct apply_state *state,
fill_stat_cache_info(ce, );
}
if (write_object_file(buf, size, blob_type, >oid) < 0) {
-   free(ce);
+   discard_index_cache_entry(ce);
return error(_("unable to create backing store "
   "for newly created file %s"), path);
}
}
if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
-   free(ce);
+   discard_index_cache_entry(ce);
return error(_("unable to add cache entry for %s"), path);
}
 
@@ -4422,27 +4421,26 @@ static int add_conflicted_stages_file(struct 
apply_state *state,
   struct patch *patch)
 {
int stage, namelen;
-   unsigned ce_size, mode;
+   unsigned mode;
struct cache_entry *ce;
 
if (!state->update_index)

[PATCH v2 0/5] Allocate cache entries from memory pool

[Jameson Miller]

Changes from V1:

 - Based patch series off of commit in master

 - Minor updates based on initial code review feedback

Summary:

This patch series improves the performance of loading indexes by
reducing the number of malloc() calls. Loading the index from disk is
partly dominated by the time in malloc(), which is called for each
index entry. This patch series reduces the number of times malloc() is
called as part of loading the index, and instead allocates a block of
memory upfront that is large enough to hold all of the cache entries,
and chunks this memory itself. This change builds on [1].


Git previously allocated block of memory for the index cache entries
until [2].

This 5 part patch series is broken up as follows:

  1/5, 2/5 - Move cache entry lifecycle methods behind an API

  3/5 - Fill out memory pool API to include lifecycle and other
methods used in later patches

  4/5 - Allocate cache entry structs from memory pool

  5/5 - Add extra optional validation

Performance Benchmarks:

To evaluate the performance of this approach, the p0002-read-cache.sh
test was run with several combinations of allocators (glibc default,
tcmalloc, jemalloc), with and without block allocation, and across
several different index sized (100K, 1M, 2M entries). The details on
how these repositories were constructed can be found in [3].The
p0002-read-cache.sh was run with the iteration count set to 1 and
$GIT_PERF_REPEAT_COUNT=10.

The tests were run with iteration count set to 1 because this best
approximates the real behavior. The read_cache/discard_cache test will
load / free the index N times, and the performance of this logic is
different between N = 1 and N > 1. As the production code does not
read / discard the index in a loop, a better approximation is when N =
1.

100K

Test   baseline   block_allocation
 

0002.1: read_cache/discard_cache 1 times   0.03(0.01+0.01)0.02(0.01+0.01) 
-33.3%

1M:

Test   baseline   block_allocation
 

0002.1: read_cache/discard_cache 1 times   0.23(0.12+0.11)0.17(0.07+0.09) 
-26.1%

2M:

Test   baseline   block_allocation
 

0002.1: read_cache/discard_cache 1 times   0.45(0.26+0.19)0.39(0.17+0.20) 
-13.3%

With 100K entries, it only takes 0.3 seconds to read the entries even
without using block allocation, so there is only a small change in the
wall clock time. We can see a larger wall clack improvement with 1M
and 2M entries.

For completeness, here is the p0002-read-cache tests for git.git and
linux.git:

git.git:

Test  baseline  block_allocation
 
-
0002.1: read_cache/discard_cache 1000 times   0.30(0.26+0.03)   0.17(0.13+0.03) 
-43.3%

linux.git:

Test  baseline  block_allocation
 
-
0002.1: read_cache/discard_cache 1000 times   7.05(6.01+0.84)   4.61(3.74+0.66) 
-34.6% 


We also investigated the performance of just using different
allocators. We can see that there is not a consistent performance
gain.

100K

Test   baseline  tcmalloc   
   jemalloc
 
--
0002.1: read_cache/discard_cache 1 times   0.03(0.01+0.01)   0.03(0.01+0.01) 
+0.0% 0.03(0.02+0.01) +0.0% 

1M:

Test   baseline  tcmalloc   
   jemalloc
 
--
0002.1: read_cache/discard_cache 1 times   0.23(0.12+0.11)   0.21(0.10+0.10) 
-8.7% 0.27(0.16+0.10) +17.4%

2M:

Test   baseline  tcmalloc   
   jemalloc
 
--
0002.1: read_cache/discard_cache 1 times   0.45(0.26+0.19)   0.46(0.25+0.21) 
+2.2% 0.57(0.36+0.21) +26.7%



[1] https://public-inbox.org/git/20180321164152.204869-1-jam...@microsoft.com/

[2] debed2a629 (read-cache.c: allocate index entries individually - 2011-10-24)

[3] Constructing test repositories:

The test repositories were constructed with t/perf/repos/many_files.sh with the 
following parameters:

100K:many-files.sh 4 10 9
1M:  many-files.sh 5 10 9
2M:  many-files.sh 6 8 7

Base Ref: master
Web-Diff: 

Re: BUG report: unicode normalization on APFS (Mac OS High Sierra)

[Elijah Newren]

Hi,

On Fri, Apr 27, 2018 at 2:45 PM, Totsten Bögershausen  wrote:
> On 2018-04-26 19:23, Elijah Newren wrote:

>> Sure.  First, though, note that I can make it pass (or at least "not
>> ok...TODO known breakage") with the following patch (may be
>> whitespace-damaged by gmail):
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 483c8d6d7..770b91f8c 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -1106,12 +1106,7 @@ test_lazy_prereq UTF8_NFD_TO_NFC '
>>  auml=$(printf "\303\244")
>>  aumlcdiar=$(printf "\141\314\210")
>>  >"$auml" &&
>> -   case "$(echo *)" in
>> -   "$aumlcdiar")
>> -   true ;;
>> -   *)
>> -   false ;;
>> -   esac
>> +   stat "$aumlcdiar" >/dev/null 2>/dev/null
>
>
> Nicely analyzed and improved.
>
> The "stat" statement is technically correct.
> I think that a more git-style fix would be
> [] ---
> +   test -r "$aumlcdiar"
>
> instead of the stat.
>
> I looked into the 2 known breakages.
> In short: they test use cases which are not sooo important for a user in
> practice, but do a good test if the code is broken.
> IOW: I can't see a need for immediate action.
>
> As you already did all the analyzes:
> Do you want to send a patch ?

You know, despite seeing the "test_expect_failure" and "TODO...known
breakage" with these tests and even mentioning them, it somehow didn't
sink in and I was still thinking that there might be some kind of
unicode normalization handling in the codebase somewhere (similar to
the case insensitivy handling that I've seen in a place or two) that
now needed to be extended.  I should have realized that
test_expect_failure meant there wasn't, and thus all we needed to do
was to mark it as continuing to fail with the new filesystem,  Should
have realized, but didn't.  Oops.

Anyway, it looks like you've already submitted a patch and marked it
as having been reported by me, which is just fine.  Thanks!

Elijah

Re: [PATCH v1] test-drop-caches: simplify delay loading of NtSetSystemInformation

[Ben Peart]

On 4/30/2018 10:57 AM, Duy Nguyen wrote:

On Mon, Apr 30, 2018 at 4:26 PM, Ben Peart  wrote:

Take advantage of the recent addition of support for lazy loading functions
on Windows to simplfy the loading of NtSetSystemInformation.

Signed-off-by: Ben Peart 
---

Notes:
 Base Ref: master
 Web-Diff: https://github.com/benpeart/git/commit/6e6ce4a788
 Checkout: git fetch https://github.com/benpeart/git test-drop-caches-v1 && 
git checkout 6e6ce4a788

  t/helper/test-drop-caches.c | 16 
  1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/t/helper/test-drop-caches.c b/t/helper/test-drop-caches.c
index 838760898b..dd41da1a2c 100644
--- a/t/helper/test-drop-caches.c
+++ b/t/helper/test-drop-caches.c
@@ -1,5 +1,6 @@
  #include "test-tool.h"
  #include "git-compat-util.h"
+#include "lazyload.h"


This is in compat/win32, should it be inside the "if defined
(GIT_WINDOWS_NATIVE)" block instead of here?



Yes, that does make sense.  No other platform will need/want it.  I'll 
wait to see if there is any other feedback and will submit an updated 
patch.  Thanks for the catch.

Re: [RFC PATCH] checkout: Force matching mtime between files

[Duy Nguyen]

On Mon, Apr 30, 2018 at 1:56 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> Target revision should be available in the index. But this gives me an
>> idea to another thing that bugs me: sending the list to the hook means
>> I have to deal with separator (\n or NUL?) or escaping. This mentions
>> of index makes me take a different direction. I could produce a small
>> index that contains just what is modified, then you can retrieve
>> whatever info you want with `git ls-files` or even `git show` after
>> pointing $GIT_INDEX_FILE to it.
>
> That's somewhere in between a tail wagging the dog and a hammer
> looking like a solution even when you have a screw.  By passing a
> temporary index, you may be able to claim that you are feeding the
> necessary information without corruption and in a readable and
> native format of Git, and make it up to the reader to grab the paths
> out of it, but
>
>  (1) the contents, and probably the cached stat information, in that
>  temporary index is duplicated and wasted; you know from the
>  time you design this thing that all you want to convey is a
>  list of paths.

Yep, I was not happy about this. Which I why I moved to update the
hook calling convention to pass pathspec to the hook instead.

>  (2) it is totally unclear who is responsible for cleaning the
>  temporary file up.

The one that creates it deletes it, which is git.

>  (3) the recipient must walk and carefully grab the path, certainly
>  has to "deal with separator (\n or NUL?) or escaping" anyway,
>  especially if the reason you use a temporary index is because
>  "they can use ls-files on it".  They need to read from ls-files
>  anyway, so that is no better than feeding ls-files compatible
>  input from the standard input of the hook script.

Err "ls-files compatible input" or "ls-files compatible _output_"? If
by input you mean the pathspec we give to ls-files, I agree. If it's
standard ls-files --stage output , letting the hook call ls-files lets
it select the output format it wants (including the potential json
output in the future), which is more flexible.

>
> no?



-- 
Duy

Re: [PATCH v1] test-drop-caches: simplify delay loading of NtSetSystemInformation

[Duy Nguyen]

On Mon, Apr 30, 2018 at 4:26 PM, Ben Peart  wrote:
> Take advantage of the recent addition of support for lazy loading functions
> on Windows to simplfy the loading of NtSetSystemInformation.
>
> Signed-off-by: Ben Peart 
> ---
>
> Notes:
> Base Ref: master
> Web-Diff: https://github.com/benpeart/git/commit/6e6ce4a788
> Checkout: git fetch https://github.com/benpeart/git test-drop-caches-v1 
> && git checkout 6e6ce4a788
>
>  t/helper/test-drop-caches.c | 16 
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/t/helper/test-drop-caches.c b/t/helper/test-drop-caches.c
> index 838760898b..dd41da1a2c 100644
> --- a/t/helper/test-drop-caches.c
> +++ b/t/helper/test-drop-caches.c
> @@ -1,5 +1,6 @@
>  #include "test-tool.h"
>  #include "git-compat-util.h"
> +#include "lazyload.h"

This is in compat/win32, should it be inside the "if defined
(GIT_WINDOWS_NATIVE)" block instead of here?

>
>  #if defined(GIT_WINDOWS_NATIVE)
>
> @@ -82,8 +83,6 @@ static int cmd_dropcaches(void)
>  {
> HANDLE hProcess = GetCurrentProcess();
> HANDLE hToken;
> -   HMODULE ntdll;
> -   DWORD(WINAPI *NtSetSystemInformation)(INT, PVOID, ULONG);
> SYSTEM_MEMORY_LIST_COMMAND command;
> int status;
>
> @@ -95,14 +94,9 @@ static int cmd_dropcaches(void)
>
> CloseHandle(hToken);
>
> -   ntdll = LoadLibrary("ntdll.dll");
> -   if (!ntdll)
> -   return error("Can't load ntdll.dll, wrong Windows version?");
> -
> -   NtSetSystemInformation =
> -   (DWORD(WINAPI *)(INT, PVOID, ULONG))GetProcAddress(ntdll, 
> "NtSetSystemInformation");
> -   if (!NtSetSystemInformation)
> -   return error("Can't get function addresses, wrong Windows 
> version?");
> +   DECLARE_PROC_ADDR(ntdll.dll, DWORD, NtSetSystemInformation, INT, 
> PVOID, ULONG);
> +   if (!INIT_PROC_ADDR(NtSetSystemInformation))
> +   return error("Could not find NtSetSystemInformation() 
> function");
>
> command = MemoryPurgeStandbyList;
> status = NtSetSystemInformation(
> @@ -115,8 +109,6 @@ static int cmd_dropcaches(void)
> else if (status != STATUS_SUCCESS)
> error("Unable to execute the memory list command %d", status);
>
> -   FreeLibrary(ntdll);
> -
> return status;
>  }
>
>
> base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
> --
> 2.17.0.windows.1
>



-- 
Duy

Re: [PATCH v3] unpack_trees: fix breakage when o->src_index != o->dst_index

[Duy Nguyen]

On Mon, Apr 30, 2018 at 4:42 PM, Duy Nguyen  wrote:
> On Sun, Apr 29, 2018 at 10:53 PM, Johannes Schindelin
>  wrote:
>>> > @@ -1412,12 +1422,13 @@ int unpack_trees(unsigned len, struct tree_desc 
>>> > *t, struct unpack_trees_options
>>> >   WRITE_TREE_SILENT |
>>> >   WRITE_TREE_REPAIR);
>>> > }
>>> > -   move_index_extensions(>result, o->dst_index);
>>> > +   move_index_extensions(>result, o->src_index);
>>>
>>> While this looks like the right thing to do on paper, I believe it's
>>> actually broken for a specific case of untracked cache. In short,
>>> please do not touch this line. I will send a patch to revert
>>> edf3b90553 (unpack-trees: preserve index extensions - 2017-05-08),
>>> which essentially deletes this line, with proper explanation and
>>> perhaps a test if I could come up with one.
>>>
>>> When we update the index, we depend on the fact that all updates must
>>> invalidate the right untracked cache correctly. In this unpack
>>> operations, we start copying entries over from src to result. Since
>>> 'result' (at least from the beginning) does not have an untracked
>>> cache, it has nothing to invalidate when we copy entries over. By the
>>> time we have done preparing 'result', what's recorded in src's (or
>>> dst's for that matter) untracked cache may or may not apply to
>>> 'result'  index anymore. This copying only leads to more problems when
>>> untracked cache is used.
>>
>> Is there really no way to invalidate just individual entries?
>
> Grr the short answer is the current code (i.e. without Elijah's
> changes) works but in a twisted way. So you get to keep untracked
> cache in the end.

GAAAHH.. it works _with_ Elijah's changes (since he made the change
from dst to src) not without (and no performance regression). This
file really messes my brain up.
-- 
Duy

Re: [PATCH v3] unpack_trees: fix breakage when o->src_index != o->dst_index

[Duy Nguyen]

On Sun, Apr 29, 2018 at 10:53 PM, Johannes Schindelin
 wrote:
>> > @@ -1412,12 +1422,13 @@ int unpack_trees(unsigned len, struct tree_desc 
>> > *t, struct unpack_trees_options
>> >   WRITE_TREE_SILENT |
>> >   WRITE_TREE_REPAIR);
>> > }
>> > -   move_index_extensions(>result, o->dst_index);
>> > +   move_index_extensions(>result, o->src_index);
>>
>> While this looks like the right thing to do on paper, I believe it's
>> actually broken for a specific case of untracked cache. In short,
>> please do not touch this line. I will send a patch to revert
>> edf3b90553 (unpack-trees: preserve index extensions - 2017-05-08),
>> which essentially deletes this line, with proper explanation and
>> perhaps a test if I could come up with one.
>>
>> When we update the index, we depend on the fact that all updates must
>> invalidate the right untracked cache correctly. In this unpack
>> operations, we start copying entries over from src to result. Since
>> 'result' (at least from the beginning) does not have an untracked
>> cache, it has nothing to invalidate when we copy entries over. By the
>> time we have done preparing 'result', what's recorded in src's (or
>> dst's for that matter) untracked cache may or may not apply to
>> 'result'  index anymore. This copying only leads to more problems when
>> untracked cache is used.
>
> Is there really no way to invalidate just individual entries?

Grr the short answer is the current code (i.e. without Elijah's
changes) works but in a twisted way. So you get to keep untracked
cache in the end.

I was right about the invalidation stuff. I knew about
invalidate_ce_path() in this file. What I didn't remember was this
function actually invalidates entries from the _source_ index, not the
result one. What kind of logic is that? You copy/move entries from
source to result than you go invalidate the source. Since the original
move_index_extensions() call moves extensions from the source, these
are already properly invalidated (both untracked cache and cache
tree), it it looks like it does the right thing. Two wrongs make a
right, I guess.

Sorry for venting. I was not happy with what I found. And sorry for
wasting your time making this move_index.. change then remove it.

> I have a couple of worktrees which are *huge*. And edf3b90553 really
> helped relieve the pain a bit when running `git status`. Now you say that
> even a `git checkout -b new-branch` would blow the untracked cache away
> again?
>
> It would be *really* nice if we could prevent that performance regression
> somehow.
>
> Ciao,
> Dscho



-- 
Duy

[PATCH v1] test-drop-caches: simplify delay loading of NtSetSystemInformation

[Ben Peart]

Take advantage of the recent addition of support for lazy loading functions
on Windows to simplfy the loading of NtSetSystemInformation.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: master
Web-Diff: https://github.com/benpeart/git/commit/6e6ce4a788
Checkout: git fetch https://github.com/benpeart/git test-drop-caches-v1 && 
git checkout 6e6ce4a788

 t/helper/test-drop-caches.c | 16 
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/t/helper/test-drop-caches.c b/t/helper/test-drop-caches.c
index 838760898b..dd41da1a2c 100644
--- a/t/helper/test-drop-caches.c
+++ b/t/helper/test-drop-caches.c
@@ -1,5 +1,6 @@
 #include "test-tool.h"
 #include "git-compat-util.h"
+#include "lazyload.h"
 
 #if defined(GIT_WINDOWS_NATIVE)
 
@@ -82,8 +83,6 @@ static int cmd_dropcaches(void)
 {
HANDLE hProcess = GetCurrentProcess();
HANDLE hToken;
-   HMODULE ntdll;
-   DWORD(WINAPI *NtSetSystemInformation)(INT, PVOID, ULONG);
SYSTEM_MEMORY_LIST_COMMAND command;
int status;
 
@@ -95,14 +94,9 @@ static int cmd_dropcaches(void)
 
CloseHandle(hToken);
 
-   ntdll = LoadLibrary("ntdll.dll");
-   if (!ntdll)
-   return error("Can't load ntdll.dll, wrong Windows version?");
-
-   NtSetSystemInformation =
-   (DWORD(WINAPI *)(INT, PVOID, ULONG))GetProcAddress(ntdll, 
"NtSetSystemInformation");
-   if (!NtSetSystemInformation)
-   return error("Can't get function addresses, wrong Windows 
version?");
+   DECLARE_PROC_ADDR(ntdll.dll, DWORD, NtSetSystemInformation, INT, PVOID, 
ULONG);
+   if (!INIT_PROC_ADDR(NtSetSystemInformation))
+   return error("Could not find NtSetSystemInformation() 
function");
 
command = MemoryPurgeStandbyList;
status = NtSetSystemInformation(
@@ -115,8 +109,6 @@ static int cmd_dropcaches(void)
else if (status != STATUS_SUCCESS)
error("Unable to execute the memory list command %d", status);
 
-   FreeLibrary(ntdll);
-
return status;
 }
 

base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
-- 
2.17.0.windows.1

Re: branch --contains / tag --merged inconsistency

[Derrick Stolee]

On 4/27/2018 12:03 PM, SZEDER Gábor wrote:

Szia Feri,


I'm moving the IRC discussion here, because this might be a bug report
in the end.  So, kindly try these steps (103 MB free space required):

$ git clone https://github.com/ClusterLabs/pacemaker.git && cd pacemaker
[...]
$ git branch --contains Pacemaker-0.6.1
* master
$ git tag --merged master | fgrep Pacemaker-0.6
Pacemaker-0.6.0
Pacemaker-0.6.2
Pacemaker-0.6.3
Pacemaker-0.6.4
Pacemaker-0.6.5
Pacemaker-0.6.6

Notice that Pacemaker-0.6.1 is missing from the output.  Kind people on
IRC didn't find a quick explanation, and we all had to go eventually.
Is this expected behavior?  Reproduced with git 2.11.0 and 2.17.0.

The commit pointed to by the tag Pacemaker-0.6.1 and its parent have a
serious clock skew, i.e. they are a few months older than their parents:

$ git log --format='%h %ad %cd%d%n%s' --date=short 
Pacemaker-0.6.1^..47a8ef4c
47a8ef4ce 2008-02-15 2008-02-15
 Low: TE: Logging - display the op's magic field for unexpected and foreign 
events
b9cfcd6b4 2007-12-10 2007-12-10 (tag: Pacemaker-0.6.2)
 haresources2cib.py: set default-action-timeout to the default (20s)
52e7793e0 2007-12-10 2007-12-10
 haresources2cib.py: update ra parameters lists
dea277271 2008-02-14 2008-02-14
 Medium: Build: Turn on snmp support in rpm packages (patch from MATSUDA, 
Daiki)
f418742fe 2008-02-14 2008-02-14
 Low: Build: Update the .spec file with the one used by build service
ccfa716a5 2008-02-14 2008-02-14
 Medium: SNMP: Allow the snmp subagent to be built (patch from MATSUDA, 
Daiki)
50f0ade2d 2008-02-14 2008-02-14
 Low: Build: Update last release number
90f11667f 2008-02-14 2008-02-14
 Medium: Tools: Make sure the autoconf variables in haresources2cib are 
expanded
9d2383c46 2008-02-11 2008-02-11 (tag: Pacemaker-0.6.1)
 High: cib: Ensure the archived file hits the disk before returning

(branch|tag|describe|...) (--contains|--merged) use the commit timestamp
information as a heuristic to avoid traversing parts of the history,
which makes these operations, especially on big histories, an order of
magnitude or two faster.  Yeah, commit timestamps can't always be
trusted, but skewed commits are rare, and skewed commits with this much
skew are even rarer.

I'm not sure how (or if it's at all possible) to turn off this
timestamp-based optimisation.


This is actually a bit more complicated. The "--merged" check in 'git 
tag' uses a different mechanism to detect which tags are reachable. It 
uses a revision walk starting at the "merge commit" (master in your 
case) and all tags with the "limited" option (to ensure the walk happens 
during prepare_revision_walk()) but marks the merge commit as 
UNINTERESTING. The limit_list() method stops when all commits are marked 
UNINTERESTING - minus some "slop" related to the commits that start the 
walk.


One important note: the set of tags is important here. If you add a new 
tag to the root commit (git tag MyTag a2d71961f) then the walk succeeds 
by ensuring it walks until MyTag. This gets around the clock skew issue. 
There may be other more-recent tags with a clock-skew issue, but since 
Pacemaker-0.6.0 is the oldest tag, that requires the walk to continue 
until at least that date.


The commit-walk machinery in revision.c is rather complicated, and is 
used for a lot of different reasons, such as "git log" and this 
application in "git tag". It is on my list to refactor this code to use 
the commit-graph and generation numbers, but as we can see by this 
example, it is not easy to tease out what is happening in the code.


In a world where generation numbers are expected to be available, we 
could rewrite do_merge_filter() in ref-filter.c to call into 
paint_down_to_common() in commit.c using the new "min_generation" 
marker. By assigning the tags to be in the "twos" list and the merge 
commit in the "one" commit, we can check if the tags have the PARENT1 
flag after the walk in paint_down_to_common(). Due to the static nature 
of paint_down_to_common(), we will likely want to abstract this into an 
external method in commit.c, say can_reach_many(struct commit *from, 
struct commit_list *to).



FWIW, much work is being done on a cached commit graph including commit
generation numbers, which will solve this issue both correctly and more
efficiently.  Perhaps it will already be included in the next release.


The work in ds/generation-numbers is focused on the "git tag --contains" 
method, which does return correctly here (it is the reverse of the 
--merged condition):


Which tags can reach Pacemaker-0.6.1?

$ git tag --contains Pacemaker-0.6.1
(returns a big list)

This is the actual reverse lookup (which branches contain this tag?)

$ git branch --contains Pacemaker-0.6.1 | grep master
* master

These commands work despite clock skew. The commit-graph feature makes 
them faster.


Thanks,
-Stolee

Hello compliment of the season

[Mrs. Sufia SALWA]

Hello dear

Mrs. Sufia SALWA is my name a childless widow suffering from a long
time cancer decease and from all indication my condition is really
deteriorating and as a result of my present situation I have nothing
in mind other than to help the poor .

Recently my doctor have confirmed and courageously advised me that I
may not live beyond two weeks that I have shorter days to live.
Likewise from my daily health report it is also clear to me that I
have but little time to pass on,therefore I wish to offer my fortune
US$4,500,000.00 to charity organization or anybody ready to handle
charity work in favour of less privileges in your country, 60% will go
for charity work while 40% will be for you and family.

If you are interested with God fearing heart contact me so i can give
you the contact details of my bank with a letter of authorization to
claim the fund.

Yours Dying Sister,
Mrs. Sufia SALWA
>From France (Paris) living in Burkina Faso till date.

Re: git-submodule is missing --dissociate option

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 30, 2018 at 1:30 PM, Casey Fitzpatrick  wrote:
> It also seems to be missing "--progress", and I imagine others.
> Perhaps submodule add/update should be reworked to automatically
> accept all the options that clone would?

--progress is not missing, but I see that it isn't documented. It was
added in 72c5f88311 ("clone: pass --progress decision to recursive
submodules", 2016-09-22). What you're suggesting makes sense, but as
shown in that commit it's not easy for it to happen automatically,
there's a lot of boilerplate involved.

But since you're interested you can see how to add new options with
that patch, it should be easy for anyone not experienced with the
codebase, it's all just boilerplate + adding a test.

> On Mon, Apr 30, 2018 at 4:29 AM, Casey Fitzpatrick  wrote:
>> This seems to be a hole in the git feature set. I believe it is fairly
>> easily worked around, but it would be best to provide the option for
>> ease of use (and maybe performance?).
>>
>> git clone has both a --reference feature and a --dissociate option,
>> with dissociate allowing for a reference to *only* speed up network
>> transfers rather than have the resulting clone rely upon the reference
>> always being there (creates an independent repo).
>> But git submodule only allows for --reference, so there isn't a an
>> option to make a speedy independent submodule clone in one shot:
>> https://git-scm.com/docs/git-submodule
>> I checked the latest online documentation (currently at 2.16.3) and
>> the documentation in the latest sources (almost 2.18):
>> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>>
>> As far as I am aware this can be worked around with 'git repack -a'
>> and manual removal of the objects/info/alternates file afterward.
>> Though I don't know if this results in a less speedy clone than
>> dissociate would.

Re:

[Ben Peart]

On 4/27/2018 2:19 PM, Elijah Newren wrote:

From: Elijah Newren 

On Thu, Apr 26, 2018 at 5:54 PM, Ben Peart  wrote:


Can you write the documentation that clearly explains the exact behavior you
want?  That would kill two birds with one stone... :)


Sure, something like the following is what I envision, and I've tried to
include the suggestion from Junio to document the copy behavior in the
merge-recursive documentation.

-- 8< --
Subject: [PATCH] fixup! merge: Add merge.renames config setting

---
  Documentation/merge-config.txt | 3 +--
  Documentation/merge-strategies.txt | 5 +++--
  merge-recursive.c  | 8 
  3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 59848e5634..662c2713ca 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -41,8 +41,7 @@ merge.renameLimit::
  merge.renames::
Whether and how Git detects renames.  If set to "false",
rename detection is disabled. If set to "true", basic rename
-   detection is enabled.  If set to "copies" or "copy", Git will
-   detect copies, as well.  Defaults to the value of diff.renames.
+   detection is enabled.  Defaults to the value of diff.renames.
  
  merge.renormalize::

Tell Git that canonical representation of files in the
diff --git a/Documentation/merge-strategies.txt 
b/Documentation/merge-strategies.txt
index 1e0728aa12..aa66cbe41e 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -23,8 +23,9 @@ recursive::
causing mismerges by tests done on actual merge commits
taken from Linux 2.6 kernel development history.
Additionally this can detect and handle merges involving
-   renames.  This is the default merge strategy when
-   pulling or merging one branch.
+   renames, but currently cannot make use of detected
+   copies.  This is the default merge strategy when pulling
+   or merging one branch.
  +
  The 'recursive' strategy can take the following options:
  
diff --git a/merge-recursive.c b/merge-recursive.c

index 6cc4404144..b618f134d2 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -564,6 +564,14 @@ static struct string_list *get_renames(struct 
merge_options *o,
opts.flags.recursive = 1;
opts.flags.rename_empty = 0;
opts.detect_rename = merge_detect_rename(o);
+   /*
+* We do not have logic to handle the detection of copies.  In
+* fact, it may not even make sense to add such logic: would we
+* really want a change to a base file to be propagated through
+* multiple other files by a merge?
+*/
+   if (opts.detect_rename > DIFF_DETECT_RENAME)
+   opts.detect_rename = DIFF_DETECT_RENAME;
opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :
o->diff_rename_limit >= 0 ? o->diff_rename_limit :
1000;



Thanks Elijah. I've applied this patch and reviewed and tested it.  It 
works and addresses the concerns around the settings inheritance from 
diff.renames.  I still _prefer_ the simpler model that doesn't do the 
partial inheritance but I can use this model as well.


I'm unsure on the protocol here.  Should I incorporate this patch and 
submit a reroll or can it just be applied as is?

Re: [RFC 00/10] Make .the gitmodules file path configurable

[Antonio Ospite]

On Mon, 23 Apr 2018 10:47:09 -0700
Jonathan Nieder  wrote:

> Hi,
>

Hi Jonathan, thank you for your comment.

> Antonio Ospite wrote:
> 
> > vcsh[1] uses bare git repositories and detached work-trees to manage
> > *distinct* sets of configuration files directly into $HOME.
> 
> Cool!  I like the tooling you're creating for this, though keep in mind
> that Git has some weaknesses as a tool for deployment.
>

I am not the author BTW, just a user trying to address the remaining
corner cases.

> In particular, keep in mind that when git updates a file, there is a
> period of time while it is missing from the filesystem, which can be
> problematic for dotfiles.
>

Thanks for the reminder, it may be worth mentioning this in vcsh
documentation, however I don't have knowledge of users experiencing
problems related to that.

> [...]
> > However when multiple repositories take turns using the same directory
> > as their work-tree, and more than one of them want to use submodules,
> > there could still be conflicts about the '.gitmodules' file because git
> > hardcodes this path.
> >
> > For comparison, in case of '.gitignore' a similar conflict might arise,
> > but git has alternative ways to specify exclude files, so vcsh solves
> > this by setting core.excludesFile for each repository and track ignored
> > files somewhere else (in ~/.gitignore.d/$VCSH_REPO_NAME).
> 
> For reference:
> 
>   core.excludesFile
>   Specifies the pathname to the file that contains
>   patterns to describe paths that are not meant to be
>   tracked, in addition to .gitignore (per-directory) and
>   .git/info/exclude. Defaults to
>   $XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is
>   either not set or empty, $HOME/.config/git/ignore is
>   used instead. See gitignore(5).
> 
> Using this as a substitute for /.gitignore is a bit of a
> hack.  It happens to work, though, so reading on. :)
> 
> [...]
> > So this series proposes a mechanism to set an alternative path for the
> > submodules configuration file (from now on "gitmodules file").
> 
> I am nervous about this.  I wonder if there is another way to
> accomplish the goal.
>
> One possibility would be to handle the case where .gitmodules is
> excluded by a sparse checkout specification and use .gitmodules from
> the index in that case.  Would that work for you?
> 

Since part of the problem is that .gitmodules *collide* between
repositories, a sparse-checkout approach make sense indeed.

As discussed[1] with Stefan Beller having git use .gitmodules from the
index without the need to have it checked out should work for us.

[1] https://www.spinics.net/lists/git/msg329153.html

Ideally git should also be able to write to that file when it's not
checked out (e.g. when running "git submodule add"), to save the
user from tedious sparse/unsparse rounds when operating with submodules.

As suggested by Stefan I'll first try to remove the hardcoded references
to .gitmodules in git-submodule.sh adding a helper sub-command to
access .gitmodules in a more robust way, and after that git could
be taught to use the file from the index, but this second part
is currently beyond my current git knowledge.

If this mechanism of using unchecked-out files from the index could be
extended to .gitignore (and .gitattributes), then vcsh might even stop
abusing core.excludesFile; sparse checkouts seem the more natural git
way to deal with colliding files in a shared-workdir scenario.

However, having users *write* to .gitignore and .gitattributes while
they are not checked out still sounds quite problematic to me, but maybe
this could be handled by vcsh itself, similarly to what is done for the
file pointed by core.excludesFile.

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Re: [PATCH 0/4] subtree: move out of contrib

[Philip Oakley]

From: "Ævar Arnfjörð Bjarmason" 

I think at this point git-subtree is widely used enough to move out of
contrib/, maybe others disagree, but patches are always better for
discussion that patch-less ML posts.



Assuming this lands in Git, then there will also need to be a simple follow 
on into Duy's series that is updating the command-list.txt (Message-Id: 
<20180429181844.21325-10-pclo...@gmail.com>). Duy's series also does the 
completions thing IIUC;-).

--
Philip


Ævar Arnfjörð Bjarmason (4):
 git-subtree: move from contrib/subtree/
 subtree: remove support for git version <1.7
 subtree: fix a test failure under GETTEXT_POISON
 i18n: translate the git-subtree command

.gitignore|   1 +
Documentation/git-submodule.txt   |   2 +-
.../subtree => Documentation}/git-subtree.txt |   3 +
Makefile  |   1 +
contrib/subtree/.gitignore|   7 -
contrib/subtree/COPYING   | 339 --
contrib/subtree/INSTALL   |  28 --
contrib/subtree/Makefile  |  97 -
contrib/subtree/README|   8 -
contrib/subtree/t/Makefile|  86 -
contrib/subtree/todo  |  48 ---
.../subtree/git-subtree.sh => git-subtree.sh  | 109 +++---
{contrib/subtree/t => t}/t7900-subtree.sh |  21 +-
13 files changed, 78 insertions(+), 672 deletions(-)
rename {contrib/subtree => Documentation}/git-subtree.txt (99%)
delete mode 100644 contrib/subtree/.gitignore
delete mode 100644 contrib/subtree/COPYING
delete mode 100644 contrib/subtree/INSTALL
delete mode 100644 contrib/subtree/Makefile
delete mode 100644 contrib/subtree/README
delete mode 100644 contrib/subtree/t/Makefile
delete mode 100644 contrib/subtree/todo
rename contrib/subtree/git-subtree.sh => git-subtree.sh (84%)
rename {contrib/subtree/t => t}/t7900-subtree.sh (99%)

--
2.17.0.290.gded63e768a

Re: [PATCH v4 02/10] commit: add generation number to struct commmit

[Derrick Stolee]

On 4/28/2018 6:35 PM, Jakub Narebski wrote:

Derrick Stolee  writes:


The generation number of a commit is defined recursively as follows:

* If a commit A has no parents, then the generation number of A is one.
* If a commit A has parents, then the generation number of A is one
   more than the maximum generation number among the parents of A.

Very minor nitpick: it would be more readable wrapped differently:

   * If a commit A has parents, then the generation number of A is
 one more than the maximum generation number among parents of A.

Very minor nitpick: possibly "parents", not "the parents", but I am
not native English speaker.


Add a uint32_t generation field to struct commit so we can pass this
information to revision walks. We use three special values to signal
the generation number is invalid:

GENERATION_NUMBER_INFINITY 0x
GENERATION_NUMBER_MAX 0x3FFF
GENERATION_NUMBER_ZERO 0

The first (_INFINITY) means the generation number has not been loaded or
computed. The second (_MAX) means the generation number is too large to
store in the commit-graph file. The third (_ZERO) means the generation
number was loaded from a commit graph file that was written by a version
of git that did not support generation numbers.

Good explanation; I wonder if we want to have it in some shortened form
also in comments, and not only in the commit message.


Signed-off-by: Derrick Stolee 
---
  alloc.c| 1 +
  commit-graph.c | 2 ++
  commit.h   | 4 
  3 files changed, 7 insertions(+)

I have reordered patches to make it easier to review.


diff --git a/commit.h b/commit.h
index 23a3f364ed..aac3b8c56f 100644
--- a/commit.h
+++ b/commit.h
@@ -10,6 +10,9 @@
  #include "pretty.h"
  
  #define COMMIT_NOT_FROM_GRAPH 0x

+#define GENERATION_NUMBER_INFINITY 0x
+#define GENERATION_NUMBER_MAX 0x3FFF
+#define GENERATION_NUMBER_ZERO 0

I wonder if it wouldn't be good to have some short in-line comments
explaining those constants, or a block comment above them.

  
  struct commit_list {

struct commit *item;
@@ -30,6 +33,7 @@ struct commit {
 */
struct tree *maybe_tree;
uint32_t graph_pos;
+   uint32_t generation;
  };
  
  extern int save_commit_buffer;

All right, simple addition of the new field.  Nothing to go wrong here.

Sidenote: With 0x7FFF being (if I am not wrong) maximum graph_pos
and maximum number of nodes in commit graph, we won't hit 0x3FFF
generation number limit for all except very, very linear histories.


Both of these limits are far away from being realistic. But we could 
extend the maximum graph_pos independently from the maximum generation 
number now that we have the "capped" logic.





diff --git a/alloc.c b/alloc.c
index cf4f8b61e1..e8ab14f4a1 100644
--- a/alloc.c
+++ b/alloc.c
@@ -94,6 +94,7 @@ void *alloc_commit_node(void)
c->object.type = OBJ_COMMIT;
c->index = alloc_commit_index();
c->graph_pos = COMMIT_NOT_FROM_GRAPH;
+   c->generation = GENERATION_NUMBER_INFINITY;
return c;
  }

All right, start with initializing it with "not from commit-graph" value
after allocation.

  
diff --git a/commit-graph.c b/commit-graph.c

index 70fa1b25fd..9ad21c3ffb 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -262,6 +262,8 @@ static int fill_commit_in_graph(struct commit *item, struct 
commit_graph *g, uin
date_low = get_be32(commit_data + g->hash_len + 12);
item->date = (timestamp_t)((date_high << 32) | date_low);
  
+	item->generation = get_be32(commit_data + g->hash_len + 8) >> 2;

+

I guess we should not worry about these "magical constants" sprinkled
here, like "+ 8" above.

Let's examine how it goes, taking a look at commit-graph-format.txt
in Documentation/technical/commit-graph-format.txt

  * The first H (g->hash_len) bytes are for the OID of the root tree.
  * The next 8 bytes are for the positions of the first two parents [...]

So 'commit_data + g->hash_len + 8' is our offset from the start of
commit data.  All right.

   * 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. [...]

The higher 30 bits of the 4 bytes, which is 32 bits, means that we need
to shift 32-bit value 2 bits right, so that we get lower 30 bits of
32-bit value.  All right.

   All 4-byte numbers are in network order.

Shouldn't it be ntohl() to convert from network order to host order, and
not get_be32()?  I guess they are the same (network order is big-endian
order), and get_be32() is what rest of git uses...


ntohl() takes a 32-bit value, while get_be32() takes a pointer. This 
makes pulling network-bytes out of streams much cleaner with get_be32(), 
so I try to use that whenever possible.

Re: Bug: format-patch MIME boundary not added to cover letter when attach enabled

[brian m. carlson]

On Mon, Apr 30, 2018 at 12:30:57PM +0900, Junio C Hamano wrote:
> Thanks.  It is true that the current output from the tool is corrupt
> mime multi-part, and we need to do something about it.
> 
> I however have to wonder if it even makes sense for --cover to pay
> attention to --attach and produce the cover template that has "BLURB
> HERE" etc.  in a multi-part format.  Shouldn't we be making a simple
> plain text file instead?

I agree that multipart/mixed is not a useful content-type for only one
plain text part.  I have a patch to add the trailing boundary, but I
think you make a good argument that perhaps omitting the entire
multipart portion would be better.

I'll have to work on this after work, so expect a patch later today.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH] coccinelle: avoid wrong transformation suggestions from commit.cocci

[Derrick Stolee]

On 4/30/2018 5:31 AM, SZEDER Gábor wrote:

The semantic patch 'contrib/coccinelle/commit.cocci' added in
2e27bd7731 (treewide: replace maybe_tree with accessor methods,
2018-04-06) is supposed to "ensure that all references to the
'maybe_tree' member of struct commit are either mutations or accesses
through get_commit_tree()".  So get_commit_tree() clearly must be able
to directly access the 'maybe_tree' member, and 'commit.cocci' has a
bit of a roundabout workaround to ensure that get_commit_tree()'s
direct access in its return statement is not transformed: after all
references to 'maybe_tree' have been transformed to a call to
get_commit_tree(), including the reference in get_commit_tree()
itself, the last rule transforms back a 'return get_commit_tree()'
statement, back then found only in get_commit_tree() itself, to a
direct access.

Unfortunately, already the very next commit shows that this workaround
is insufficient: 7b8a21dba1 (commit-graph: lazy-load trees for
commits, 2018-04-06) extends get_commit_tree() with a condition
directly accessing the 'maybe_tree' member, and Coccinelle with
'commit.cocci' promptly detects it and suggests a transformation to
avoid it.  This transformation is clearly wrong, because calling
get_commit_tree() to access 'maybe_tree' _in_ get_commit_tree() would
obviously lead to recursion.  Furthermore, the same commit added
another, more specialized getter function get_commit_tree_in_graph(),
whose legitimate direct access to 'maybe_tree' triggers a similar
wrong transformation suggestion.


Thanks for catching this, Szeder. Sorry for the noise.


Exclude both of these getter functions from the general rule in
'commit.cocci' that matches their direct accesses to 'maybe_tree'.
Also exclude load_tree_for_commit(), which, as static helper funcion
of get_commit_tree_in_graph(), has legitimate direct access to
'maybe_tree' as well.


This is an interesting feature of Coccinelle. Happy to learn it.


The last rule transforming back 'return get_commit_tree()' statements
to direct accesses thus became unnecessary, remove it.

Signed-off-by: SZEDER Gábor 


I applied this locally on 'next' and ran the check. I succeeded with no 
changes.


Thanks!

Reviewed-by: Derrick Stolee 



---
  contrib/coccinelle/commit.cocci | 10 --
  1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/contrib/coccinelle/commit.cocci b/contrib/coccinelle/commit.cocci
index ac38525941..a7e9215ffc 100644
--- a/contrib/coccinelle/commit.cocci
+++ b/contrib/coccinelle/commit.cocci
@@ -10,11 +10,15 @@ expression c;
  - c->maybe_tree->object.oid.hash
  + get_commit_tree_oid(c)->hash
  
+// These excluded functions must access c->maybe_tree direcly.

  @@
+identifier f !~ 
"^(get_commit_tree|get_commit_tree_in_graph|load_tree_for_commit)$";
  expression c;
  @@
+  f(...) {...
  - c->maybe_tree
  + get_commit_tree(c)
+  ...}
  
  @@

  expression c;
@@ -22,9 +26,3 @@ expression s;
  @@
  - get_commit_tree(c) = s
  + c->maybe_tree = s
-
-@@
-expression c;
-@@
-- return get_commit_tree(c);
-+ return c->maybe_tree;

Re: [PATCH] Create '--merges-only' option for 'git bisect'

[Tiago Botelho]

On Mon, Apr 30, 2018 at 12:31 PM, Johannes Schindelin
 wrote:
> Hi Tiago,
>
>> > On Thu, 12 Apr 2018, Tiago Botelho wrote:
>> >
>> >> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder
>> >>  wrote:
>> >> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren
>> >> >  wrote:
>> >> >> I think it looks similar. But if I'm reading that thread correctly
>> >> >> then there was never a patch created, right?
>> >> >
>> >> > (It is customary on this mailing list to reply after the sentences we
>> >> > reply to. We don't "top post".)
>> >> >
>> >> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we
>> >> > have been suggesting "Implement git bisect --first-parent" and there
>> >> > are the following related links:
>> >> >
>> >> > https://public-inbox.org/git/2015030405.ga9...@peff.net/
>> >> > https://public-inbox.org/git/4d3cddf9.6080...@intel.com/
>> >> >
>> >> > Tiago in Cc also tried at a recent London hackathon to implement it
>> >> > and came up with the following:
>> >> >
>> >> > https://github.com/tiagonbotelho/git/pull/1/files
>> >> >
>> >> > I tried to help him by reworking his commit in the following branch:
>> >> >
>> >> > https://github.com/chriscool/git/commits/myfirstparent
>> >>
>> >> Thank you for the cc Christian, I’ve been quite busy and was not able
>> >> to work on the PR for quite some time.
>> >>
>> >> I intended to pick it back up again next week. If it is ok with
>> >> Harald I would love to finish the PR that I started, since it is
>> >> quite close to being finished (I think it was just specs missing if I
>> >> am not mistaken).
>
> It is now well after "next week". Are there any news? Or could you unblock
> Harald by stating that you won't come back to it any time soon (in
> particular since the PR is not quite as finished from my reading as you
> made it sound...)?
>
> Ciao,
> Johannes

I've been working on the feature for the past week
https://github.com/tiagonbotelho/git/commit/709e2e248ebfb1deab12fd7d3da4611002dfaf86#diff-118df990fd68a0929bca5441fea06fc7

I have some comments sent by Christian I plan on fixing this week

Thank you,

-- 
Tiago Botelho

Re: [PATCH] Create '--merges-only' option for 'git bisect'

[Johannes Schindelin]

Hi Tiago,

> > On Thu, 12 Apr 2018, Tiago Botelho wrote:
> >
> >> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder
> >>  wrote:
> >> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren
> >> >  wrote:
> >> >> I think it looks similar. But if I'm reading that thread correctly
> >> >> then there was never a patch created, right?
> >> >
> >> > (It is customary on this mailing list to reply after the sentences we
> >> > reply to. We don't "top post".)
> >> >
> >> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we
> >> > have been suggesting "Implement git bisect --first-parent" and there
> >> > are the following related links:
> >> >
> >> > https://public-inbox.org/git/2015030405.ga9...@peff.net/
> >> > https://public-inbox.org/git/4d3cddf9.6080...@intel.com/
> >> >
> >> > Tiago in Cc also tried at a recent London hackathon to implement it
> >> > and came up with the following:
> >> >
> >> > https://github.com/tiagonbotelho/git/pull/1/files
> >> >
> >> > I tried to help him by reworking his commit in the following branch:
> >> >
> >> > https://github.com/chriscool/git/commits/myfirstparent
> >>
> >> Thank you for the cc Christian, I’ve been quite busy and was not able
> >> to work on the PR for quite some time.
> >>
> >> I intended to pick it back up again next week. If it is ok with
> >> Harald I would love to finish the PR that I started, since it is
> >> quite close to being finished (I think it was just specs missing if I
> >> am not mistaken).

It is now well after "next week". Are there any news? Or could you unblock
Harald by stating that you won't come back to it any time soon (in
particular since the PR is not quite as finished from my reading as you
made it sound...)?

Ciao,
Johannes

Re: git-submodule is missing --dissociate option

[Casey Fitzpatrick]

It also seems to be missing "--progress", and I imagine others.
Perhaps submodule add/update should be reworked to automatically
accept all the options that clone would?

On Mon, Apr 30, 2018 at 4:29 AM, Casey Fitzpatrick  wrote:
> This seems to be a hole in the git feature set. I believe it is fairly
> easily worked around, but it would be best to provide the option for
> ease of use (and maybe performance?).
>
> git clone has both a --reference feature and a --dissociate option,
> with dissociate allowing for a reference to *only* speed up network
> transfers rather than have the resulting clone rely upon the reference
> always being there (creates an independent repo).
> But git submodule only allows for --reference, so there isn't a an
> option to make a speedy independent submodule clone in one shot:
> https://git-scm.com/docs/git-submodule
> I checked the latest online documentation (currently at 2.16.3) and
> the documentation in the latest sources (almost 2.18):
> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>
> As far as I am aware this can be worked around with 'git repack -a'
> and manual removal of the objects/info/alternates file afterward.
> Though I don't know if this results in a less speedy clone than
> dissociate would.

[PATCH 3/4] subtree: fix a test failure under GETTEXT_POISON

[Ævar Arnfjörð Bjarmason]

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 t/t7900-subtree.sh | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/t/t7900-subtree.sh b/t/t7900-subtree.sh
index eb223ff049..a6e7103f92 100755
--- a/t/t7900-subtree.sh
+++ b/t/t7900-subtree.sh
@@ -38,6 +38,16 @@ check_equal()
fi
 }
 
+check_equal_i18n()
+{
+   if test_have_prereq C_LOCALE_OUTPUT
+   then
+   check_equal "$@"
+   else
+   return 0
+   fi
+}
+
 undo()
 {
git reset --hard HEAD~
@@ -250,7 +260,7 @@ test_expect_success 'merge the added subproj again, should 
do nothing' '
# this shouldn not actually do anything, since FETCH_HEAD
# is already a parent
result=$(git merge -s ours -m "merge -s -ours" FETCH_HEAD) &&
-   check_equal "${result}" "Already up to date."
+   check_equal_i18n "${result}" "Already up to date."
)
 '
 
-- 
2.17.0.290.gded63e768a

[PATCH 4/4] i18n: translate the git-subtree command

[Ævar Arnfjörð Bjarmason]

One of the advantages of having git-subtree out of contrib/ is being
able to treat it as a first-class citizen when it comes to
translations.

Mark those messages that make sense to translate for translation, and
add a comment to the ones that shouldn't be translated. This has been
tested under GIT_GETTEXT_POISON=1.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 git-subtree.sh | 90 +++---
 t/t7900-subtree.sh |  4 +--
 2 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/git-subtree.sh b/git-subtree.sh
index 1b8cd53c7f..03ef175428 100755
--- a/git-subtree.sh
+++ b/git-subtree.sh
@@ -141,7 +141,7 @@ do
break
;;
*)
-   die "Unexpected option: $opt"
+   die "$(eval_gettext "Unexpected option: \$opt")"
;;
esac
 done
@@ -157,23 +157,23 @@ split|push)
default="--default HEAD"
;;
 *)
-   die "Unknown command '$command'"
+   die "$(eval_gettext "Unknown command '\$command'")"
;;
 esac
 
 if test -z "$prefix"
 then
-   die "You must provide the --prefix option."
+   die "$(gettext "You must provide the --prefix option.")"
 fi
 
 case "$command" in
 add)
test -e "$prefix" &&
-   die "prefix '$prefix' already exists."
+   die "$(eval_gettext "prefix '\$prefix' already exists.")"
;;
 *)
test -e "$prefix" ||
-   die "'$prefix' does not exist; use 'git subtree add'"
+   die "$(eval_gettext "'\$prefix' does not exist; use 'git 
subtree add'")"
;;
 esac
 
@@ -187,7 +187,7 @@ then
dirs=$(git rev-parse --no-revs --no-flags "$@") || exit $?
if test -n "$dirs"
then
-   die "Error: Use --prefix instead of bare filenames."
+   die "$(gettext "Error: Use --prefix instead of bare 
filenames.")"
fi
 fi
 
@@ -201,12 +201,12 @@ debug
 cache_setup () {
cachedir="$GIT_DIR/subtree-cache/$$"
rm -rf "$cachedir" ||
-   die "Can't delete old cachedir: $cachedir"
+   die "$(eval_gettext "Can't delete old cachedir: \$cachedir")"
mkdir -p "$cachedir" ||
-   die "Can't create new cachedir: $cachedir"
+   die "$(eval_gettext "Can't create new cachedir: \$cachedir")"
mkdir -p "$cachedir/notree" ||
-   die "Can't create new cachedir: $cachedir/notree"
-   debug "Using cachedir: $cachedir" >&2
+   die "$(eval_gettext "Can't create new cachedir: 
\$cachedir/notree")"
+   debug "$(eval_gettext "Using cachedir: \$cachedir")" >&2
 }
 
 cache_get () {
@@ -252,7 +252,7 @@ cache_set () {
test "$oldrev" != "latest_new" &&
test -e "$cachedir/$oldrev"
then
-   die "cache for $oldrev already exists!"
+   die "$(eval_gettext "cache for \$oldrev already exists!")"
fi
echo "$newrev" >"$cachedir/$oldrev"
 }
@@ -311,7 +311,7 @@ find_latest_squash () {
;;
git-subtree-split:)
sub="$(git rev-parse "$b^0")" ||
-   die "could not rev-parse split hash $b from commit $sq"
+   die "$(eval_gettext "could not rev-parse split hash \$b 
from commit \$sq")"
;;
END)
if test -n "$sub"
@@ -353,7 +353,7 @@ find_existing_splits () {
;;
git-subtree-split:)
sub="$(git rev-parse "$b^0")" ||
-   die "could not rev-parse split hash $b from commit $sq"
+   die "$(eval_gettext "could not rev-parse split hash \$b 
from commit \$sq")"
;;
END)
debug "  Main is: '$main'"
@@ -401,7 +401,10 @@ copy_commit () {
cat
) |
git commit-tree "$2" $3  # reads the rest of stdin
-   ) || die "Can't copy commit $1"
+   ) || {
+   commit="$1"
+   die "$(eval_gettext "Can't copy commit \$commit")"
+   }
 }
 
 add_msg () {
@@ -412,6 +415,7 @@ add_msg () {
then
commit_message="$message"
else
+   # i18n: This message must not be translated!
commit_message="Add '$dir/' from commit '$latest_new'"
fi
cat <<-EOF
@@ -428,6 +432,7 @@ add_squashed_msg () {
then
echo "$message"
else
+   # i18n: This message must not be translated!
echo "Merge commit '$1' as '$2'"
fi
 }
@@ -440,6 +445,7 @@ rejoin_msg () {
then
commit_message="$message"
else
+   # i18n: This message must not be translated!
commit_message="Split '$dir/' into commit '$latest_new'"
fi
cat <<-EOF
@@ 

[PATCH 2/4] subtree: remove support for git version <1.7

[Ævar Arnfjörð Bjarmason]

Now that git-subtree is in-tree there's no reason to be supporting
older git Versions. This reverts & amends 448e71e263 ("Use 'git merge
-Xsubtree' when git version >= 1.7.0.", 2010-05-07).

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 git-subtree.sh | 19 ---
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/git-subtree.sh b/git-subtree.sh
index d3f39a862a..1b8cd53c7f 100755
--- a/git-subtree.sh
+++ b/git-subtree.sh
@@ -810,23 +810,12 @@ cmd_merge () {
rev="$new"
fi
 
-   version=$(git version)
-   if test "$version" \< "git version 1.7"
+   if test -n "$message"
then
-   if test -n "$message"
-   then
-   git merge -s subtree --message="$message" "$rev"
-   else
-   git merge -s subtree "$rev"
-   fi
+   git merge -Xsubtree="$prefix" \
+   --message="$message" "$rev"
else
-   if test -n "$message"
-   then
-   git merge -Xsubtree="$prefix" \
-   --message="$message" "$rev"
-   else
-   git merge -Xsubtree="$prefix" $rev
-   fi
+   git merge -Xsubtree="$prefix" $rev
fi
 }
 
-- 
2.17.0.290.gded63e768a

[PATCH 0/4] subtree: move out of contrib

[Ævar Arnfjörð Bjarmason]

I think at this point git-subtree is widely used enough to move out of
contrib/, maybe others disagree, but patches are always better for
discussion that patch-less ML posts.

Ævar Arnfjörð Bjarmason (4):
  git-subtree: move from contrib/subtree/
  subtree: remove support for git version <1.7
  subtree: fix a test failure under GETTEXT_POISON
  i18n: translate the git-subtree command

 .gitignore|   1 +
 Documentation/git-submodule.txt   |   2 +-
 .../subtree => Documentation}/git-subtree.txt |   3 +
 Makefile  |   1 +
 contrib/subtree/.gitignore|   7 -
 contrib/subtree/COPYING   | 339 --
 contrib/subtree/INSTALL   |  28 --
 contrib/subtree/Makefile  |  97 -
 contrib/subtree/README|   8 -
 contrib/subtree/t/Makefile|  86 -
 contrib/subtree/todo  |  48 ---
 .../subtree/git-subtree.sh => git-subtree.sh  | 109 +++---
 {contrib/subtree/t => t}/t7900-subtree.sh |  21 +-
 13 files changed, 78 insertions(+), 672 deletions(-)
 rename {contrib/subtree => Documentation}/git-subtree.txt (99%)
 delete mode 100644 contrib/subtree/.gitignore
 delete mode 100644 contrib/subtree/COPYING
 delete mode 100644 contrib/subtree/INSTALL
 delete mode 100644 contrib/subtree/Makefile
 delete mode 100644 contrib/subtree/README
 delete mode 100644 contrib/subtree/t/Makefile
 delete mode 100644 contrib/subtree/todo
 rename contrib/subtree/git-subtree.sh => git-subtree.sh (84%)
 rename {contrib/subtree/t => t}/t7900-subtree.sh (99%)

-- 
2.17.0.290.gded63e768a

[PATCH 1/4] git-subtree: move from contrib/subtree/

[Ævar Arnfjörð Bjarmason]

Change the git-subtree command from living in contrib to being a
"real" git command.

I think it's useful enough that it should be shipped by default, and
unlike git-submodule doesn't need any changes to git's object
format. It's just a helper to do things you can do manually already
with plumbing tools (but that would be much more tedious).

It'll also help to avoid the confusion noted in
https://public-inbox.org/git/alpine.LFD.2.21.1802070547040.5530@android-a172fe96dd584b41/
where users know git-subtree exists, but can't find its
documentation (or it isn't installed).

This change removes config/subtree's copy of COPYING because it's
redundant to the one in the root of the tree. The contrib/subtree/todo
file is being removed because we don't usually store TODOs
in-tree (and this'll be available in the history), the rest like the
.gitignore, Makefile etc. are all redundant to things we have in-tree.

Signed-off-by: Ævar Arnfjörð Bjarmason 
---
 .gitignore|   1 +
 Documentation/git-submodule.txt   |   2 +-
 .../subtree => Documentation}/git-subtree.txt |   3 +
 Makefile  |   1 +
 contrib/subtree/.gitignore|   7 -
 contrib/subtree/COPYING   | 339 --
 contrib/subtree/INSTALL   |  28 --
 contrib/subtree/Makefile  |  97 -
 contrib/subtree/README|   8 -
 contrib/subtree/t/Makefile|  86 -
 contrib/subtree/todo  |  48 ---
 .../subtree/git-subtree.sh => git-subtree.sh  |   0
 {contrib/subtree/t => t}/t7900-subtree.sh |   5 +-
 13 files changed, 7 insertions(+), 618 deletions(-)
 rename {contrib/subtree => Documentation}/git-subtree.txt (99%)
 delete mode 100644 contrib/subtree/.gitignore
 delete mode 100644 contrib/subtree/COPYING
 delete mode 100644 contrib/subtree/INSTALL
 delete mode 100644 contrib/subtree/Makefile
 delete mode 100644 contrib/subtree/README
 delete mode 100644 contrib/subtree/t/Makefile
 delete mode 100644 contrib/subtree/todo
 rename contrib/subtree/git-subtree.sh => git-subtree.sh (100%)
 rename {contrib/subtree/t => t}/t7900-subtree.sh (99%)

diff --git a/.gitignore b/.gitignore
index 833ef3b0b7..b78a5b6e0a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -155,6 +155,7 @@
 /git-status
 /git-stripspace
 /git-submodule
+/git-subtree
 /git-submodule--helper
 /git-svn
 /git-symbolic-ref
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 71c5618e82..ba718d2c9b 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -402,7 +402,7 @@ for details.
 
 SEE ALSO
 
-linkgit:gitsubmodules[7], linkgit:gitmodules[5].
+linkgit:gitsubmodules[7], linkgit:gitmodules[5], linkgit:git-subtree[1].
 
 GIT
 ---
diff --git a/contrib/subtree/git-subtree.txt b/Documentation/git-subtree.txt
similarity index 99%
rename from contrib/subtree/git-subtree.txt
rename to Documentation/git-subtree.txt
index 352deda69d..f206a32c8f 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/Documentation/git-subtree.txt
@@ -345,6 +345,9 @@ AUTHOR
 --
 Written by Avery Pennarun 
 
+SEE ALSO
+
+linkgit:git-submodule[1].
 
 GIT
 ---
diff --git a/Makefile b/Makefile
index 50da82b016..82cdac3aa2 100644
--- a/Makefile
+++ b/Makefile
@@ -576,6 +576,7 @@ SCRIPT_SH += git-remote-testgit.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
+SCRIPT_SH += git-subtree.sh
 SCRIPT_SH += git-web--browse.sh
 
 SCRIPT_LIB += git-mergetool--lib
diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
deleted file mode 100644
index 0b9381abca..00
--- a/contrib/subtree/.gitignore
+++ /dev/null
@@ -1,7 +0,0 @@
-*~
-git-subtree
-git-subtree.1
-git-subtree.html
-git-subtree.xml
-mainline
-subproj
diff --git a/contrib/subtree/COPYING b/contrib/subtree/COPYING
deleted file mode 100644
index d511905c16..00
--- a/contrib/subtree/COPYING
+++ /dev/null
@@ -1,339 +0,0 @@
-   GNU GENERAL PUBLIC LICENSE
-  Version 2, June 1991
-
- Copyright (C) 1989, 1991 Free Software Foundation, Inc.,
- 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- Everyone is permitted to copy and distribute verbatim copies
- of this license document, but changing it is not allowed.
-
-   Preamble
-
-  The licenses for most software are designed to take away your
-freedom to share and change it.  By contrast, the GNU General Public
-License is intended to guarantee your freedom to share and change free
-software--to make sure the software is free for all its users.  This
-General Public License applies to most of the Free Software
-Foundation's software and to any other program whose authors commit to
-using it.  (Some other Free Software Foundation software is covered by
-the GNU Lesser 

[PATCH 1/1] wt-status: use rename settings from init_diff_ui_defaults

[Eckhard S. Maaß]

Since the very beginning, git status behaved differently for rename
detection than other rename aware commands like git log or git show as
it has the use of rename hard coded into it.  After 5404c116aa ("diff:
activate diff.renames by default", 2016-02-25) the default behaves the
same by coincidence, but a work flow like

- git add .
- git status
- git commit
- git show

should give you the same information on renames (and/or copies if
activated) accordingly to the diff.renames setting.

With this commit the hard coded rename settings are dropped from the
status command.

Signed-off-by: Eckhard S. Maaß 
---
 builtin/commit.c   |  2 +-
 t/t4001-diff-rename.sh | 12 
 wt-status.c|  4 
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 5571d4a3e2..5240f11225 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -161,9 +161,9 @@ static void determine_whence(struct wt_status *s)
 static void status_init_config(struct wt_status *s, config_fn_t fn)
 {
wt_status_prepare(s);
+   init_diff_ui_defaults();
git_config(fn, s);
determine_whence(s);
-   init_diff_ui_defaults();
s->hints = advice_status_hints; /* must come after git_config() */
 }
 
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index a07816d560..bf4030371a 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -138,6 +138,18 @@ test_expect_success 'favour same basenames over different 
ones' '
test_i18ngrep "renamed: .*path1 -> subdir/path1" out
 '
 
+test_expect_success 'test diff.renames=true for git status' '
+   git -c diff.renames=true status >out &&
+   test_i18ngrep "renamed: .*path1 -> subdir/path1" out
+'
+
+test_expect_success 'test diff.renames=false for git status' '
+   git -c diff.renames=false status >out &&
+   test_i18ngrep ! "renamed: .*path1 -> subdir/path1" out &&
+   test_i18ngrep "new file: .*subdir/path1" out &&
+   test_i18ngrep "deleted: .*[^/]path1" out
+'
+
 test_expect_success 'favour same basenames even with minor differences' '
git show HEAD:path1 | sed "s/15/16/" > subdir/path1 &&
git status >out &&
diff --git a/wt-status.c b/wt-status.c
index 50815e5faf..32f3bcaebd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -625,9 +625,6 @@ static void wt_status_collect_changes_index(struct 
wt_status *s)
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = wt_status_collect_updated_cb;
rev.diffopt.format_callback_data = s;
-   rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
-   rev.diffopt.rename_limit = 200;
-   rev.diffopt.break_opt = 0;
copy_pathspec(_data, >pathspec);
run_diff_index(, 1);
 }
@@ -985,7 +982,6 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
setup_revisions(0, NULL, , );
 
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
-   rev.diffopt.detect_rename = DIFF_DETECT_RENAME;
rev.diffopt.file = s->fp;
rev.diffopt.close_file = 0;
/*
-- 
2.17.0.252.gfe0a9eaf31

[PATCH 0/1] Use the diff.rename configuration for git status

[Eckhard S. Maaß]

Hello,

I have been irritated that the output of git status does not adhere to
the same diff settings than the other commands like git show. I think it
is more of an oversight and git status is also intended to show the same
kind of rename detection (without any more options passed a long the
command line) than other similar commands do. This patch should fix that
issue.

I am aware that this change would also change how the porcelain
variations of git status behave for rename detection. However, I could
not find any hints that the rename detection should be fixed. As the
default would not change (50% rename threshold), this still seems
reasonable to me.

As for documentation, I am unsure what and where to add: for me it seems
like git status should already behave that way, but it is a change and
so far none else seemed to have brought it up.

Greetings,
Eckhard

Eckhard S. Maaß (1):
  wt-status: use rename settings from init_diff_ui_defaults

 builtin/commit.c   |  2 +-
 t/t4001-diff-rename.sh | 12 
 wt-status.c|  4 
 3 files changed, 13 insertions(+), 5 deletions(-)

-- 
2.17.0.252.gfe0a9eaf31

[PATCH] coccinelle: avoid wrong transformation suggestions from commit.cocci

[SZEDER Gábor]

The semantic patch 'contrib/coccinelle/commit.cocci' added in
2e27bd7731 (treewide: replace maybe_tree with accessor methods,
2018-04-06) is supposed to "ensure that all references to the
'maybe_tree' member of struct commit are either mutations or accesses
through get_commit_tree()".  So get_commit_tree() clearly must be able
to directly access the 'maybe_tree' member, and 'commit.cocci' has a
bit of a roundabout workaround to ensure that get_commit_tree()'s
direct access in its return statement is not transformed: after all
references to 'maybe_tree' have been transformed to a call to
get_commit_tree(), including the reference in get_commit_tree()
itself, the last rule transforms back a 'return get_commit_tree()'
statement, back then found only in get_commit_tree() itself, to a
direct access.

Unfortunately, already the very next commit shows that this workaround
is insufficient: 7b8a21dba1 (commit-graph: lazy-load trees for
commits, 2018-04-06) extends get_commit_tree() with a condition
directly accessing the 'maybe_tree' member, and Coccinelle with
'commit.cocci' promptly detects it and suggests a transformation to
avoid it.  This transformation is clearly wrong, because calling
get_commit_tree() to access 'maybe_tree' _in_ get_commit_tree() would
obviously lead to recursion.  Furthermore, the same commit added
another, more specialized getter function get_commit_tree_in_graph(),
whose legitimate direct access to 'maybe_tree' triggers a similar
wrong transformation suggestion.

Exclude both of these getter functions from the general rule in
'commit.cocci' that matches their direct accesses to 'maybe_tree'.
Also exclude load_tree_for_commit(), which, as static helper funcion
of get_commit_tree_in_graph(), has legitimate direct access to
'maybe_tree' as well.

The last rule transforming back 'return get_commit_tree()' statements
to direct accesses thus became unnecessary, remove it.

Signed-off-by: SZEDER Gábor 
---
 contrib/coccinelle/commit.cocci | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/contrib/coccinelle/commit.cocci b/contrib/coccinelle/commit.cocci
index ac38525941..a7e9215ffc 100644
--- a/contrib/coccinelle/commit.cocci
+++ b/contrib/coccinelle/commit.cocci
@@ -10,11 +10,15 @@ expression c;
 - c->maybe_tree->object.oid.hash
 + get_commit_tree_oid(c)->hash
 
+// These excluded functions must access c->maybe_tree direcly.
 @@
+identifier f !~ 
"^(get_commit_tree|get_commit_tree_in_graph|load_tree_for_commit)$";
 expression c;
 @@
+  f(...) {...
 - c->maybe_tree
 + get_commit_tree(c)
+  ...}
 
 @@
 expression c;
@@ -22,9 +26,3 @@ expression s;
 @@
 - get_commit_tree(c) = s
 + c->maybe_tree = s
-
-@@
-expression c;
-@@
-- return get_commit_tree(c);
-+ return c->maybe_tree;
-- 
2.17.0.551.g86756ed296

Re: [PATCH v1 1/1] test: Correct detection of UTF8_NFD_TO_NFC for APFS

[Torsten Bögershausen]

On 30.04.18 09:56, Junio C Hamano wrote:
> tbo...@web.de writes:
> 
>> From: Torsten Bögershausen 
>>
>> On HFS (which is the default Mac filesystem prior to High Sierra),
>> unicode names are "decomposed" before recording.
>> On APFS, which appears to be the new default filesystem in Mac OS High
>> Sierra, filenames are recorded as specified by the user.
>>
>> APFS continues to allow the user to access it via any name
>> that normalizes to the same thing.
>>
>> This difference causes t0050-filesystem.sh to fail two tests.
>>
>> Improve the test for a NFD/NFC in test-lib.sh:
>> Test if the same file can be reached in pre- and decomposed unicode.
>>
>> Reported-By: Elijah Newren 
>> Signed-off-by: Torsten Bögershausen 
>> ---
>>  t/test-lib.sh | 7 +--
>>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> Thanks.  
> 
> Wouldn't it logically make more sense to check for the target being
> an existing file with "-f"?  It is not an essential part of the test
> for the target to be "readable", but "can be stat(2)ed with the
> other UTF-8 representation" is.

That make sense.
Would you like to amend the patch ?

git-submodule is missing --dissociate option

[Casey Fitzpatrick]

This seems to be a hole in the git feature set. I believe it is fairly
easily worked around, but it would be best to provide the option for
ease of use (and maybe performance?).

git clone has both a --reference feature and a --dissociate option,
with dissociate allowing for a reference to *only* speed up network
transfers rather than have the resulting clone rely upon the reference
always being there (creates an independent repo).
But git submodule only allows for --reference, so there isn't a an
option to make a speedy independent submodule clone in one shot:
https://git-scm.com/docs/git-submodule
I checked the latest online documentation (currently at 2.16.3) and
the documentation in the latest sources (almost 2.18):
https://github.com/git/git/blob/next/Documentation/git-submodule.txt

As far as I am aware this can be worked around with 'git repack -a'
and manual removal of the objects/info/alternates file afterward.
Though I don't know if this results in a less speedy clone than
dissociate would.