Re: [PATCH 8/8] t0012: test "-h" with builtins

[Junio C Hamano]

Junio C Hamano  writes:

> For now, I will mix this in when queuing the whole thing in 'pu', as
> I hate to push out something that does not even work for me to the
> general public.
> 
> -- >8 --
> Subject: [PATCH] diff- and log- family: handle "git cmd -h" early
> ...

And then the check_help_option() thing may look like this.  

I am not proud of the way it "unifies" the two styles of usage
strings, obviously.

One benefit this patch has is that it makes it easier to highlight
what it does *not* touch.

$ git grep -A2 -E -e 'a(rg)?c [!=]= 2 .*strcmp.*-h'

shows there are somewhat curious construct

if (argc != 2 || !strcmp(argv[1], "-h"))
usage(...);

left in the code.  Upon closer inspection, they all happen to be
doing the right thing for their current set of options and
arguments, but they are somewhat ugly.


 builtin/am.c   |  3 +--
 builtin/branch.c   |  3 +--
 builtin/check-ref-format.c |  3 +--
 builtin/checkout-index.c   |  7 ---
 builtin/commit.c   |  6 ++
 builtin/diff-files.c   |  3 +--
 builtin/diff-index.c   |  3 +--
 builtin/diff-tree.c|  3 +--
 builtin/gc.c   |  3 +--
 builtin/index-pack.c   |  3 +--
 builtin/ls-files.c |  3 +--
 builtin/merge-ours.c   |  3 +--
 builtin/merge.c|  3 +--
 builtin/pack-redundant.c   |  3 +--
 builtin/rev-list.c |  3 +--
 builtin/update-index.c |  3 +--
 builtin/upload-archive.c   |  3 +--
 fast-import.c  |  3 +--
 git-compat-util.h  |  3 +++
 usage.c| 11 +++
 20 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 8881d73615..12b7298907 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2307,8 +2307,7 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
-   if (argc == 2 && !strcmp(argv[1], "-h"))
-   usage_with_options(usage, options);
+   check_help_option(argc, argv, usage, options);
 
git_config(git_am_config, NULL);
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda43dc..8c4465f0e4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -599,8 +599,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
filter.kind = FILTER_REFS_BRANCHES;
filter.abbrev = -1;
 
-   if (argc == 2 && !strcmp(argv[1], "-h"))
-   usage_with_options(builtin_branch_usage, options);
+   check_help_option(argc, argv, builtin_branch_usage, options);
 
git_config(git_branch_config, NULL);
 
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index eac499450f..aab5776dd5 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -55,8 +55,7 @@ int cmd_check_ref_format(int argc, const char **argv, const 
char *prefix)
int flags = 0;
const char *refname;
 
-   if (argc == 2 && !strcmp(argv[1], "-h"))
-   usage(builtin_check_ref_format_usage);
+   check_help_option(argc, argv, builtin_check_ref_format_usage, NULL);
 
if (argc == 3 && !strcmp(argv[1], "--branch"))
return check_ref_format_branch(argv[2]);
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 07631d0c9c..8dd28ae8ba 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -179,9 +179,10 @@ int cmd_checkout_index(int argc, const char **argv, const 
char *prefix)
OPT_END()
};
 
-   if (argc == 2 && !strcmp(argv[1], "-h"))
-   usage_with_options(builtin_checkout_index_usage,
-  builtin_checkout_index_options);
+   check_help_option(argc, argv,
+ builtin_checkout_index_usage,
+ builtin_checkout_index_options);
+
git_config(git_default_config, NULL);
prefix_length = prefix ? strlen(prefix) : 0;
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 66c9ac587b..05c2f61e33 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1376,8 +1376,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
OPT_END(),
};
 
-   if (argc == 2 && !strcmp(argv[1], "-h"))
-   usage_with_options(builtin_status_usage, 
builtin_status_options);
+   check_help_option(argc, argv, builtin_status_usage, 
builtin_status_options);
 
status_init_config(, git_status_config);
argc = parse_options(argc, argv, prefix,
@@ -1661,8 +1660,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
 
-   if (argc == 2 && !strcmp(argv[1], "-h"))
-   usage_with_options(builtin_commit_usage, 
builtin_commit_options);
+   check_help_option(argc, argv, builtin_commit_usage, 
builtin_commit_options);
 
status_init_config(, 

Re: [PATCH 8/8] t0012: test "-h" with builtins

[Junio C Hamano]

Junio C Hamano  writes:

> Heh, I found another ;-)  
>
> 95e98cd9 ("revision.c: use refs_for_each*() instead of
> for_each_*_submodule()", 2017-04-19), which is in the middle of
> Duy's nd/prune-in-worktree series, does this:
> ...
> when jk/consistent-h is merged into it and then "git diff-files -h"
> is run.
>
> I guess anything that calls setup_revisions() from the "git cmd -h"
> bypass need to be prepared with that
>
>   check_help_option(argc, argv, usage, options);
>
> thing.  Which is a bit sad, but I tend to agree with you that
> restructuring to make usage[] of everybody available to git.c
> is probably too noisy for the benefit it would give us.

For now, I will mix this in when queuing the whole thing in 'pu', as
I hate to push out something that does not even work for me to the
general public.

-- >8 --
Subject: [PATCH] diff- and log- family: handle "git cmd -h" early

"git $builtin -h" bypasses the usual repository setup and calls the
cmd_$builtin() function, expecting it to show the help text.

Unfortunately the commands in the log- and the diff- family want to
call into the revisions machinery, which by definition needs to have
a repository already discovered, before they can parse the options.

Handle the "git $builtin -h" special case very early in these
commands to work around potential issues.

Signed-off-by: Junio C Hamano 
---
 builtin/diff-files.c | 3 +++
 builtin/diff-index.c | 3 +++
 builtin/diff-tree.c  | 3 +++
 builtin/rev-list.c   | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 15c61fd8d1..6be1df684a 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -20,6 +20,9 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
int result;
unsigned options = 0;
 
+   if (argc == 2 && !strcmp(argv[1], "-h"))
+   usage(diff_files_usage);
+
init_revisions(, prefix);
gitmodules_config();
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 1af373d002..02dd35ba45 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -17,6 +17,9 @@ int cmd_diff_index(int argc, const char **argv, const char 
*prefix)
int i;
int result;
 
+   if (argc == 2 && !strcmp(argv[1], "-h"))
+   usage(diff_cache_usage);
+
init_revisions(, prefix);
gitmodules_config();
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 5ea1c14317..f633b10b08 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -105,6 +105,9 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
struct setup_revision_opt s_r_opt;
int read_stdin = 0;
 
+   if (argc == 2 && !strcmp(argv[1], "-h"))
+   usage(diff_tree_usage);
+
init_revisions(opt, prefix);
gitmodules_config();
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 718c6059c9..b250c515b1 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -277,6 +277,9 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
int use_bitmap_index = 0;
const char *show_progress = NULL;
 
+   if (argc == 2 && !strcmp(argv[1], "-h"))
+   usage(rev_list_usage);
+
git_config(git_default_config, NULL);
init_revisions(, prefix);
revs.abbrev = DEFAULT_ABBREV;
-- 
2.13.0-513-g1c0955652f

Re: [PATCH 8/8] t0012: test "-h" with builtins

[Jeff King]

On Thu, Jun 01, 2017 at 01:17:55PM +0900, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Anyway, the problem is sk/dash-is-previous, specifically fc5684b47
> > (revision.c: args starting with "-" might be a revision, 2017-02-25). It
> > looks like the revision parser used to just bail on "-h", because
> > revision.c would say "I don't recognize this" and then cmd_rev_list()
> > would similarly say "I don't recognize this" and call usage(). But now
> > we actually try to read it as a ref, which obviously requires being
> > inside a repository.
> 
> Heh, I found another ;-)  
> 
> 95e98cd9 ("revision.c: use refs_for_each*() instead of
> for_each_*_submodule()", 2017-04-19), which is in the middle of
> Duy's nd/prune-in-worktree series, does this:

Hrm, yeah. The problem is that handle_revision_pseudo_opt() initializes
the ref store at the top of the function, even if we don't see any
arguments that require us to use it (and obviously in the "-h" case, we
don't).

That's an implementation detail that we could fix, but I do think in
general that we should probably just declare it forbidden to call
setup_revisions() when the repo hasn't been discovered.

> I guess anything that calls setup_revisions() from the "git cmd -h"
> bypass need to be prepared with that
> 
>   check_help_option(argc, argv, usage, options);
> 
> thing.  Which is a bit sad, but I tend to agree with you that
> restructuring to make usage[] of everybody available to git.c
> is probably too noisy for the benefit it would give us.

The other options are:

  - reverting the "-h" magic in git.c. It really is the source of most
of this confusion, I think, because functions which assume RUN_SETUP
are having that assumption broken. But at the same time I do think
it makes "-h" a lot friendlier, and I'd prefer to keep it.

  - reverting the BUG() in setup_git_env(); this has been flushing out a
lot of bugs, and I think is worth keeping

I did look at writing something like check_help_option(). One of the
annoyances is that we have two different usage formats: one that's a
straight string for usage(), and one that's an array-of-strings for
parse_options(). We could probably unify those.

It doesn't actually save that much code, though. The real value is that
it abstracts the "did git.c decide to skip RUN_SETUP?" logic.

-Peff

[PATCH v3] pull: ff --rebase --autostash works in dirty repo

[Tyler Brazier]

When `git pull --rebase --autostash` in a dirty repository resulted in a
fast-forward, nothing was being autostashed and the pull failed. This
was due to a shortcut to avoid running rebase when we can fast-forward,
but autostash is ignored on that codepath.

Now we will only take the shortcut if autostash is not in effect.
Based on a few tests against the git.git repo, the shortcut does not
seem to give us significant performance benefits, on Linux at least.
Regardless, it is more important to be correct than to be fast.

Signed-off-by: Tyler Brazier 
---
 builtin/pull.c  | 25 ++---
 t/t5520-pull.sh | 18 ++
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index dd1a4a94e41ed..42f0560252e00 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -772,6 +772,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
struct oid_array merge_heads = OID_ARRAY_INIT;
struct object_id orig_head, curr_head;
struct object_id rebase_fork_point;
+   int autostash;
 
if (!getenv("GIT_REFLOG_ACTION"))
set_reflog_message(argc, argv);
@@ -800,8 +801,8 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
if (!opt_rebase && opt_autostash != -1)
die(_("--[no-]autostash option is only valid with --rebase."));
 
+   autostash = config_autostash;
if (opt_rebase) {
-   int autostash = config_autostash;
if (opt_autostash != -1)
autostash = opt_autostash;
 
@@ -862,16 +863,18 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
die(_("Cannot rebase onto multiple branches."));
 
if (opt_rebase) {
-   struct commit_list *list = NULL;
-   struct commit *merge_head, *head;
-
-   head = lookup_commit_reference(orig_head.hash);
-   commit_list_insert(head, );
-   merge_head = lookup_commit_reference(merge_heads.oid[0].hash);
-   if (is_descendant_of(merge_head, list)) {
-   /* we can fast-forward this without invoking rebase */
-   opt_ff = "--ff-only";
-   return run_merge();
+   if (!autostash) {
+   struct commit_list *list = NULL;
+   struct commit *merge_head, *head;
+
+   head = lookup_commit_reference(orig_head.hash);
+   commit_list_insert(head, );
+   merge_head = 
lookup_commit_reference(merge_heads.oid[0].hash);
+   if (is_descendant_of(merge_head, list)) {
+   /* we can fast-forward this without invoking 
rebase */
+   opt_ff = "--ff-only";
+   return run_merge();
+   }
}
return run_rebase(_head, merge_heads.oid, 
_fork_point);
} else {
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 17f4d0fe4e724..f15f7a332960f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -272,6 +272,24 @@ test_expect_success '--rebase fast forward' '
test_cmp reflog.expected reflog.fuzzy
 '
 
+test_expect_success '--rebase --autostash fast forward' '
+   test_when_finished "
+   git reset --hard
+   git checkout to-rebase
+   git branch -D to-rebase-ff
+   git branch -D behind" &&
+   git branch behind &&
+   git checkout -b to-rebase-ff &&
+   echo another modification >>file &&
+   git add file &&
+   git commit -m mod &&
+
+   git checkout behind &&
+   echo dirty >file &&
+   git pull --rebase --autostash . to-rebase-ff &&
+   test "$(git rev-parse HEAD)" = "$(git rev-parse to-rebase-ff)"
+'
+
 test_expect_success '--rebase with conflicts shows advice' '
test_when_finished "git rebase --abort; git checkout -f to-rebase" &&
git checkout -b seq &&

--
https://github.com/git/git/pull/365

Re: [PATCH 8/8] t0012: test "-h" with builtins

[Junio C Hamano]

Jeff King  writes:

> Anyway, the problem is sk/dash-is-previous, specifically fc5684b47
> (revision.c: args starting with "-" might be a revision, 2017-02-25). It
> looks like the revision parser used to just bail on "-h", because
> revision.c would say "I don't recognize this" and then cmd_rev_list()
> would similarly say "I don't recognize this" and call usage(). But now
> we actually try to read it as a ref, which obviously requires being
> inside a repository.

Heh, I found another ;-)  

95e98cd9 ("revision.c: use refs_for_each*() instead of
for_each_*_submodule()", 2017-04-19), which is in the middle of
Duy's nd/prune-in-worktree series, does this:

#0  die (err=0x6128f8 "BUG: setup_git_env called without repository") at 
usage.c:114
#1  0x004f9467 in setup_git_env () at environment.c:172
#2  0x004f966c in get_git_dir () at environment.c:214
#3  0x0055113b in get_main_ref_store () at refs.c:1544
#4  0x00570ee0 in handle_revision_pseudo_opt (submodule=0x0, 
revs=0x7fffd6a0, argc=1, argv=0x7fffe180, flags=0x7fffc59c)
at revision.c:2110
#5  0x005716f5 in setup_revisions (argc=2, argv=0x7fffe178, 
revs=0x7fffd6a0, opt=0x0) at revision.c:2254
#6  0x0043074a in cmd_diff_files (argc=2, argv=0x7fffe178, 
prefix=0x0)
at builtin/diff-files.c:29
#7  0x00405907 in run_builtin (p=0x87ba00 , argc=2, 
argv=0x7fffe178) at git.c:376
#8  0x00405bb5 in handle_builtin (argc=2, argv=0x7fffe178) at 
git.c:584
#9  0x00405e04 in cmd_main (argc=2, argv=0x7fffe178) at git.c:683
#10 0x004a3364 in main (argc=2, argv=0x7fffe178) at common-main.c:43

when jk/consistent-h is merged into it and then "git diff-files -h"
is run.

I guess anything that calls setup_revisions() from the "git cmd -h"
bypass need to be prepared with that

  check_help_option(argc, argv, usage, options);

thing.  Which is a bit sad, but I tend to agree with you that
restructuring to make usage[] of everybody available to git.c
is probably too noisy for the benefit it would give us.

Re: [PATCH 1/2] format-patch: have progress option while generating patches

[Junio C Hamano]

Jeff King  writes:

> As I said above, I think I'd prefer it to require "--progress", as
> format-patch is quite often used as plumbing.

Yes, that sounds sensible.

Initially, my reaction was "Why do we even need --progress for
format-patch, when it gives one-line per patch output to show the
progress anyway?", but if that output is redirected to a file, of
course you'd need --progress independently.


> Should this use start_progress_delay()? In most cases the command will
> complete very quickly, and the progress report is just noise. For many
> commands (e.g., checkout) we wait 1-2 seconds before bothering to show
> progress output.

It is better to use the "delay" version for progress meters for
commands that may or may not last very long, and this may be a good
candidate to heed that principle.

The subcommands that use start_progress() tend to be older and more
batch oriented operations, e.g. fsck, pack-objects, etc., that are
expected to last longer.  It may be a good idea to convert them to
the "delay" variant, but obviously that is outside the scope of this
patch.

> I would have expected this to say "Generating patches"; most of our
> other progress messages are pluralized. You could use Q_() to handle the
> mono/plural case, but I think it's fine to just always say "patches"
> (that's what other messages do).
>
> One final thought is whether callers would want to customize this
> message, since it will often be used as plumbing. E.g., would rebase
> want to say something besides "Generating patches". I'm not sure.
> Anyway, if you're interested in that direction, there's prior art in
> the way rev-list handles "--progress" (and its sole caller,
> check_connected()).

These are all good suggestions.

Re: [PATCH v3 4/4] stash: implement builtin stash

[Jeff King]

On Wed, May 31, 2017 at 08:29:43PM -0700, Joel Teichroeb wrote:

> I'm running into a lot of trouble using argv_array_clear. It seems
> that some of the builtin git cmd functions move the parameters around,
> and write new pointers to argv. There's three options I have now, and
> I'm not sure which is the best one.

Hrm. It's normal for parsing to reorder the parameters (e.g., shifting
non-options to the front), but that should still allow a clear at the
end. New pointers would definitely cause a problem, though. I don't know
of any cases where we do that, but on the other hand I wouldn't be too
surprised to find that the revision.c options parser does some nasty
tricks.

Do you have a specific example? I'd be curious to see if we can just fix
the parser to be less surprising (i.e., your (1) below).

> 1. Fix all the builtin cmd functions that I use to not mess around with argv

If it's just one or two spots, this might be viable.

> 2. Stop using the builtin cmd functions, and use child processes exclusively

That might not be the worst thing in the world for a first cut at a
shell to C transition, because it eliminates a whole class of possible
problems. But it really just side-steps the problem, as we'd want to
eventually deal with it and reduce the process count.

> 3. Don't worry about clearing the memory used for these function calls.

That might be do-able, as long as the leaks are O(1) for a program run
(and not say, a leak per commit). At the very least we should mark
those spots with a "NEEDSWORK" comment and an explanation of the issue
so that your work in finding them isn't wasted.

> It looks like the rest of the code generally does #3.

It looks like we don't actually pass argv arrays to setup_revisions()
all that often. The three I see are:

  - bisect_rev_setup(), which is a known leak. This is trickier, though,
because we actually pass the initialized rev_info out of the
function, and the memory needs to last until we're done with the
traversal

  - http-push, which does seem to free the memory

  - stat_tracking_info(), which does seem to free

I could well believe there are places where we leak, though, especially
for top-level functions that exit the program when they're done.

A fourth option is to massage the argv array into something that can be
massaged by the callee, and retain the original array for freeing. I.e.,
something like:

  struct argv_array argv = ARGV_ARRAY_INIT;
  const char **massaged;

  argv_array_pushl(, ...whatever...);

  ALLOC_ARRAY(massaged, argc);
  COPY_ARRAY(massaged, argv, argc);

  setup_revisions(argv.argc, massaged, , NULL);

  /*
   * No clue what's in "massaged" now, as setup_revisions() may have
   * reordered things, added new elements, deleted some, etc. But we
   * don't have to care because any pointers we need to free are still
   * in the original argv struct, and we should be safe to free the
   * massaged array itself.
   */
  free(massaged);
  argv_array_clear();

That's pretty horrible, though. If setup_revisions() is requiring us to
do that, I'd really prefer to look into fixing it.

-Peff

Re: [PATCH v3 4/4] stash: implement builtin stash

[Joel Teichroeb]

I'm running into a lot of trouble using argv_array_clear. It seems
that some of the builtin git cmd functions move the parameters around,
and write new pointers to argv. There's three options I have now, and
I'm not sure which is the best one.

1. Fix all the builtin cmd functions that I use to not mess around with argv
2. Stop using the builtin cmd functions, and use child processes exclusively
3. Don't worry about clearing the memory used for these function calls.

It looks like the rest of the code generally does #3.

Any advice here would be appreciated.

Re: bug: `git log --grep ... --invert-grep --author=...` negates / ignores --author

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> $ git log --grep=bar --invert-grep --author=Ævar --pretty=format:%an
> -100 origin/pu |sort|uniq -c|sort -nr
>  78 Junio C Hamano
>  14 Jeff King
>   2 Andreas Heiduk
>   1 Sahil Dua
>   1 Rikard Falkeborn
>   1 Johannes Sixt
>   1 Johannes Schindelin
>   1 Ben Peart
>   1 Ævar Arnfjörð Bjarmason
>
> That last command should only find my commits, but instead --author is
> discarded.

Hmph, the way I read revision.c::commit_match() is that we check
with "--grep=bar --author=Ævar" and then emit the commit if that
check fails when --invert-grep is in effect (instead of emitting the
commit when the check succeeds, which is the normal case).

So there is one commit that didn't pass "has 'bar' and written by
Ævar" check that was written by you in the recent past (iow,
recently you were writing bar all over the place).  Changes by other
people by definition does not pass "has 'bar' and written by Ævar"
check, on the other hand.

Re: [PATCH] docs: fix literal quoted spaces

[Junio C Hamano]

Jeff King  writes:

> On Wed, May 31, 2017 at 04:06:24PM +0100, Adam Dinwoodie wrote:
>
>> When compiling the documentation, asciidoc thinks a backtick surrounded
>> by whitespace shouldn't be interpreted as marking the start or end of a
>> literal.  In most cases, that's useful behaviour, but in the git-pull
>> documentation the space is clearly intended to be part of the monospace
>> formatted text.
>
> Good catch.
>
>> Instead, use + to avoid asciidoc's literal passthrough, and encode the
>> space as {sp}.  In particular, this means asciidoc will correctly detect
>> the end of the monospace formatting, rather than having it continue past
>> the backtick.
>
> In these particular cases, is the space adding anything? Would a simpler
> fix be to just use:
>
>   ...the value on `URL:` line
>
> We've had such headaches with other entities like {sp} between different
> asciidoc versions (not to mention asciidoctor) that I tend to reach for
> the simplest solution.

Me, too (and no, I am not from AOL).  If `URL:` is typeset correctly
the approach to drop the space is much more preferred.

> (I'd also suggest the minor English correct of saying "_the_ URL line";
> that's orthogonal to what you're trying to fix, but may make sense on
> top while we're here).
>
> -Peff

Re: [PATCH 1/3] rebase -i: Add test for reflog message

[Junio C Hamano]

Phillip Wood  writes:

> From: Phillip Wood 
>
> Check that the reflog message written to the branch reflog when the
> rebase is completed is correct. This checks for regressions for the
> fix in commit
> 4ab867b8fc rebase -i: fix reflog message
>
> Signed-off-by: Phillip Wood 
> ---

Good idea.  Thanks.

>  t/t3404-rebase-interactive.sh | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 
> 5bd0275930b715c25507fc9744c8946e7f73900b..37821d245433f757fa13f0a3e27da0312bebb7db
>  100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -169,6 +169,13 @@ test_expect_success 'reflog for the branch shows state 
> before rebase' '
>   test $(git rev-parse branch1@{1}) = $(git rev-parse original-branch1)
>  '
>  
> +test_expect_success 'reflog for the branch shows correct finish message' '
> + printf "rebase -i (finish): refs/heads/branch1 onto %s\n" \
> + "$(git rev-parse branch2)" >expected &&
> + git log -g --pretty=%gs -1 refs/heads/branch1 >actual &&
> + test_cmp expected actual
> +'
> +
>  test_expect_success 'exchange two commits' '
>   set_fake_editor &&
>   FAKE_LINES="2 1" git rebase -i HEAD~2 &&

Re: [PATCH 2/3] rebase: Add tests for console output

[Junio C Hamano]

Phillip Wood  writes:

> On 31/05/17 11:42, Phillip Wood wrote:
>> From: Phillip Wood 
>>
>> Check the console output when using --autostash and the stash applies
>> cleanly is what we expect. To avoid this test depending on commit and
>> stash hashes it uses sed to replace them with XXX. The sed script also
>> replaces carriage returns in the output with '\r' to avoid embedded
>> '^M's in the expected output files. Unfortunately this means we still
>> end up with an embedded '^M' in the sed script which may not be
>> preserved when sending this.

Or such a sed script may simply be not portable.

Re: [PATCH v2] pull: ff --rebase --autostash works in dirty repo

[Junio C Hamano]

Tyler Brazier  writes:

> When `git pull --rebase --autostash` in a dirty repository resulted in a
> fast-forward, nothing was being autostashed and the pull failed. This
> was due to a shortcut to avoid running rebase when we can fast-forward,
> but autostash is ignored on that codepath.
>
> Now we will only take the shortcut if autostash is not in effect.
> Based on a few tests against the git.git repo, the shortcut does not
> seem to give us significant performance benefits, on Linux at least.
> Regardless, it is more important to be correct than to be fast.
> ---

Missing sign-off.

Otherwise, nicely done.  Thanks.

Re: [PATCH/RFC v2 2/6] branch: add copy branch option

[Junio C Hamano]

Sahil Dua  writes:

> Adds copy branch option available using -c or -C (forcefully).
>
> Includes a lot of function renames and their signature changes in order
> to introduce a new function parameter - flag 'copy' which determines
> whether those functions should do operation copy or move.
>
> Additionally, this changes a lot of other files wherever the renamed
> functions were used. By default copy=0 is passed at all those places so
> that they keep behaving the way they were, before these changes.

Things like rename_branch() that is narrowly confined inside a
single program (i.e. builtin/branch.c), if renaming and copying
shares a lot of logic and there is only a single caller to rename,
it may be OK to rename the function to rename_or_copy_branch() and
pass a new "are we doing copy or move?" parameter, but for lower
level infrastructure like config_rename_section(), I am afraid to
say that such a change is totally unacceptable.  When the current
callers are content with rename_section(), and have no need to ever
copy, why should they be forced tocall copy-or-rename with copy set
to 0?

When the original code looks like:


== caller (there are many) ==

rename_it(a, b);

== implementation (only one) ==

int rename_it(src, dst) {
... logic to create dst by copying src ...
... logic to remove src ...
}

You could introduce a common helper

== implementation ==

int rename_or_copy_it(src, dst, copy?) {
... logic to create dst by copying src ...
if (!copy?) {
... logic to remove src ...
}
}

but to help the current code (and possibly code somebody _else_ is
developing elsewhere), you can also do it in a much less disruptive
way.

== implementation ==

static int rename_or_copy_it(src, dst, copy?) {
... logic to create dst by copying src ...
if (!copy?) {
... logic to remove src ...
}
}

int rename_it(src, dst) {
return rename_or_copy_it(src, dst, 0);
}

int copy_it(src, dst) {
return rename_or_copy_it(src, dst, 1);
}

Existing callers of "rename" that are not interested in your new
"copy" thing can be left oblivious to it if you did it that way.

Re: [Bug] git branch -v has problems with carriage returns

[Junio C Hamano]

Atousa Duprat  writes:

> Here is my first attempt at fixing the issue.
>
> There are two problems in ref-filter.c:
>
> First, copy_subject() has been modified to turn '\n' into a space and
> every other ascii control character to be ignored.
>
> Second, find_subpos() doesn't realize that a line that only contains a
> '\r\n' is a blank line – at least when using crlf convention.
> I have changed things so that a sequence of either '\n' or "\r\n"
> separate the subject from the body of the commit message.
> I am not looking at the crlf setting because it doesn't seem like a
> useful distinction – when one would we ever care for \r\n not to be a
> blank line?  But it could be done...
>
> Both fixes are minimal, but it feels like they are a issues with the
> specific encoding.  Does git mandate ascii or utf-8 commit messages?
> If not, there may be a larger issue here with encodings and line-end
> conventions at the very least in ref-filter.c
> Guidance would be appreciated for how to deal with this issue...
>
> Patch attached.

Doesn't this share the problem I pointed out in the other attempt in

  https://public-inbox.org/git/xmqqy3tppu13@gitster.mtv.corp.google.com/

In short, any patch that special cases CR will not get the original
behaviour back correctly.  The original never special cased CR; it
stripped isspace() at the end of lines, and turning of CRLF into LF
is merely just one effect of it.

You can apply the attached patch and see what "git branch -v"
produces.  We should only see "one", not "one line3_long line4", for
branch-two in the output.  Then replace the SP between %s and \n in
the printf thing with \r and repeat the experiment.

Thanks.

 t/t3203-branch-output.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index a428ae6703..79b80a2d3f 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -13,7 +13,8 @@ test_expect_success 'make commits' '
 
 test_expect_success 'make branches' '
git branch branch-one &&
-   git branch branch-two HEAD^
+   git branch branch-two $(printf "%s \n" one "" line3_long line4 |
+git commit-tree HEAD:)
 '
 
 test_expect_success 'make remote branches' '

Re: What's cooking in git.git (May 2017, #08; Mon, 29)

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Mon, May 29, 2017 at 8:23 AM, Junio C Hamano  wrote:
>> * ab/pcre-v2 (2017-05-26) 7 commits
>>  - grep: add support for PCRE v2
>>  - grep: un-break building with PCRE < 8.20
>>  - grep: un-break building with PCRE < 8.32
>>  - grep: add support for the PCRE v1 JIT API
>>  - log: add -P as a synonym for --perl-regexp
>>  - grep: skip pthreads overhead when using one thread
>>  - grep: don't redundantly compile throwaway patterns under threading
>>  (this branch uses ab/grep-preparatory-cleanup.)
>>
>>  Update "perl-compatible regular expression" support to enable JIT
>>  and also allow linking with the newer PCRE v2 library.
>>
>>  Will merge to 'next'.
>
> First a general question: When you say "will merge" in these E-Mails,
> that means "before the next what's cooking in ~7 days, right? I.e. if
> there's no issues in a given series does the pu->next->master cycle
> take 3wks?

No.  "What's cooking" is not even once-per-week to begin with ;-)

These merely mean "the current plan is to merge this, not in too far
distant future, and not until the message hits the mailing list, but
the plan can change based on bug reports and people are encouraged
to find and report blocking issues before such a merge happens,
thanks".

Depending on many factors (the impact of the topic, the complexity
of the changes, etc.), the time a series spends in each stage
differs.  A truly trivially correct change may already have been in
'next' for a few days when the first issue of "What's cooking"
report that mentions it is sent out and the topic may graduate to
'master' two days later.  A more complex series may have to spend
more than a few weeks in 'next' before getting merged to 'master'.

> Anyway, the PCRE v1 set of JIT patches break builds on PCRE compiled
> with --disable-jit, which is apparently Johannes's Windows
> configuration.

OK.  Thanks for reporting and stopping me.  I'll mark it as such...

> And a perf test in a commit message that makes no sense since I split
> up the PCRE v1 patch.

... and wait for an update.

[PATCHv3 4/4] builtin/fetch.c: respect 'submodule.recurse' option

[Stefan Beller]

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/fetch.c |  7 +++
 t/t5526-fetch-submodules.sh | 10 ++
 2 files changed, 17 insertions(+)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5f2c2ab23e..c1ec3b03c3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -73,6 +73,13 @@ static int git_fetch_config(const char *k, const char *v, 
void *cb)
fetch_prune_config = git_config_bool(k, v);
return 0;
}
+
+   if (!strcmp(k, "submodule.recurse")) {
+   int r = git_config_bool(k, v) ?
+   RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+   recurse_submodules = r;
+   }
+
return git_default_config(k, v, cb);
 }
 
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index f3b0a8d30a..162baf101f 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -71,6 +71,16 @@ test_expect_success "fetch --recurse-submodules recurses 
into submodules" '
test_i18ncmp expect.err actual.err
 '
 
+test_expect_success "submodule.recurse option triggers recursive fetch" '
+   add_upstream_commit &&
+   (
+   cd downstream &&
+   git -c submodule.recurse fetch >../actual.out 2>../actual.err
+   ) &&
+   test_must_be_empty actual.out &&
+   test_i18ncmp expect.err actual.err
+'
+
 test_expect_success "fetch --recurse-submodules -j2 has the same output 
behaviour" '
add_upstream_commit &&
(
-- 
2.13.0.17.gab62347cd9

[PATCHv3 2/4] builtin/grep.c: respect 'submodule.recurse' option

[Stefan Beller]

In builtin/grep.c we parse the config before evaluating the command line
options. This makes the task of teaching grep to respect the new config
option 'submodule.recurse' very easy by just parsing that option.

As an alternative I had implemented a similar structure to treat
submodules as the fetch/push command have, including
* aligning the meaning of the 'recurse_submodules' to possible submodule
  values RECURSE_SUBMODULES_* as defined in submodule.h.
* having a callback to parse the value and
* reacting to the RECURSE_SUBMODULES_DEFAULT state that was the initial
  state.

However all this is not needed for a true boolean value, so let's keep
it simple. However this adds another place where "submodule.recurse" is
parsed.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/grep.c |  3 +++
 t/t7814-grep-recurse-submodules.sh | 18 ++
 2 files changed, 21 insertions(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index b1095362fb..454e263820 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -302,6 +302,9 @@ static int grep_cmd_config(const char *var, const char 
*value, void *cb)
 #endif
}
 
+   if (!strcmp(var, "submodule.recurse"))
+   recurse_submodules = git_config_bool(var, value);
+
return st;
 }
 
diff --git a/t/t7814-grep-recurse-submodules.sh 
b/t/t7814-grep-recurse-submodules.sh
index 3a58197f47..7184113b9b 100755
--- a/t/t7814-grep-recurse-submodules.sh
+++ b/t/t7814-grep-recurse-submodules.sh
@@ -33,6 +33,24 @@ test_expect_success 'grep correctly finds patterns in a 
submodule' '
test_cmp expect actual
 '
 
+test_expect_success 'grep finds patterns in a submodule via config' '
+   test_config submodule.recurse true &&
+   # expect from previous test
+   git grep -e "(3|4)" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'grep --no-recurse-submodules overrides config' '
+   test_config submodule.recurse true &&
+   cat >expect <<-\EOF &&
+   a:(1|2)d(3|4)
+   b/b:(3|4)
+   EOF
+
+   git grep -e "(3|4)" --no-recurse-submodules >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'grep and basic pathspecs' '
cat >expect <<-\EOF &&
submodule/a:(1|2)d(3|4)
-- 
2.13.0.17.gab62347cd9

[PATCHv3 3/4] builtin/push.c: respect 'submodule.recurse' option

[Stefan Beller]

The closest mapping from the boolean 'submodule.recurse' set to "yes"
to the variety of submodule push modes is "on-demand", so implement that.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/push.c |  4 
 t/t5531-deep-submodule-push.sh | 21 +
 2 files changed, 25 insertions(+)

diff --git a/builtin/push.c b/builtin/push.c
index a597759d8f..258648d5fd 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -498,6 +498,10 @@ static int git_push_config(const char *k, const char *v, 
void *cb)
const char *value;
if (!git_config_get_value("push.recursesubmodules", ))
recurse_submodules = 
parse_push_recurse_submodules_arg(k, value);
+   } else if (!strcmp(k, "submodule.recurse")) {
+   int val = git_config_bool(k, v) ?
+   RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
+   recurse_submodules = val;
}
 
return git_default_config(k, v, NULL);
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 57ba322628..712c595fd8 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -126,6 +126,27 @@ test_expect_success 'push succeeds if submodule commit not 
on remote but using o
)
 '
 
+test_expect_success 'push succeeds if submodule commit not on remote but using 
auto-on-demand via submodule.recurse config' '
+   (
+   cd work/gar/bage &&
+   >recurse-on-demand-from-submodule-recurse-config &&
+   git add recurse-on-demand-from-submodule-recurse-config &&
+   git commit -m "Recurse submodule.recurse from config junk"
+   ) &&
+   (
+   cd work &&
+   git add gar/bage &&
+   git commit -m "Recurse submodule.recurse from config for 
gar/bage" &&
+   git -c submodule.recurse push ../pub.git master &&
+   # Check that the supermodule commit got there
+   git fetch ../pub.git &&
+   git diff --quiet FETCH_HEAD master &&
+   # Check that the submodule commit got there too
+   cd gar/bage &&
+   git diff --quiet origin/master master
+   )
+'
+
 test_expect_success 'push recurse-submodules on command line overrides config' 
'
(
cd work/gar/bage &&
-- 
2.13.0.17.gab62347cd9

[PATCHv3 1/4] Introduce 'submodule.recurse' option for worktree manipulators

[Stefan Beller]

Any command that understands '--recurse-submodules' can have its
default changed to true, by setting the new 'submodule.recurse'
option.

This patch includes read-tree/checkout/reset for working tree
manipulating commands. Later patches will cover other commands.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 Documentation/config.txt  |  5 +
 builtin/checkout.c|  2 +-
 builtin/read-tree.c   | 10 +-
 builtin/reset.c   | 10 +-
 submodule.c   | 23 +--
 submodule.h   |  1 +
 t/lib-submodule-update.sh | 12 
 7 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e0b9fd0bc3..f60c504e86 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3065,6 +3065,11 @@ submodule.active::
submodule's path to determine if the submodule is of interest to git
commands.
 
+submodule.recurse::
+   Specifies if commands recurse into submodules by default. This
+   applies to all commands that have a `--recurse-submodules` option.
+   Defaults to false.
+
 submodule.fetchJobs::
Specifies how many submodules are fetched/cloned at the same time.
A positive integer allows up to that number of submodules fetched
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 56ea723b75..e289b7d477 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -855,7 +855,7 @@ static int git_checkout_config(const char *var, const char 
*value, void *cb)
}
 
if (starts_with(var, "submodule."))
-   return parse_submodule_config_option(var, value);
+   return submodule_config(var, value, NULL);
 
return git_xmerge_config(var, value, NULL);
 }
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 8a889ef4c3..7fd55140db 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -98,6 +98,14 @@ static int debug_merge(const struct cache_entry * const 
*stages,
return 0;
 }
 
+static int git_read_tree_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "submodule.recurse"))
+   return git_default_submodule_config(var, value, cb);
+
+   return git_default_config(var, value, cb);
+}
+
 static struct lock_file lock_file;
 
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
@@ -150,7 +158,7 @@ int cmd_read_tree(int argc, const char **argv, const char 
*unused_prefix)
opts.src_index = _index;
opts.dst_index = _index;
 
-   git_config(git_default_config, NULL);
+   git_config(git_read_tree_config, NULL);
 
argc = parse_options(argc, argv, unused_prefix, read_tree_options,
 read_tree_usage, 0);
diff --git a/builtin/reset.c b/builtin/reset.c
index 6f89dc5494..585cfe0745 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -266,6 +266,14 @@ static int reset_refs(const char *rev, const struct 
object_id *oid)
return update_ref_status;
 }
 
+static int git_reset_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, "submodule.recurse"))
+   return git_default_submodule_config(var, value, cb);
+
+   return git_default_config(var, value, cb);
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
int reset_type = NONE, update_ref_status = 0, quiet = 0;
@@ -294,7 +302,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
-   git_config(git_default_config, NULL);
+   git_config(git_reset_config, NULL);
 
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
diff --git a/submodule.c b/submodule.c
index 78cccb7563..2b157dc995 100644
--- a/submodule.c
+++ b/submodule.c
@@ -16,6 +16,7 @@
 #include "quote.h"
 #include "remote.h"
 #include "worktree.h"
+#include "parse-options.h"
 
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
@@ -170,10 +171,28 @@ static int git_modules_config(const char *var, const char 
*value, void *cb)
return 0;
 }
 
-/* Loads all submodule settings from the config */
+/* Loads all submodule settings from the config. */
 int submodule_config(const char *var, const char *value, void *cb)
 {
-   return git_modules_config(var, value, cb);
+   if (!strcmp(var, "submodule.recurse")) {
+   int v = git_config_bool(var, value) ?
+   RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
+   config_update_recurse_submodules = v;
+   return 0;
+   } else {
+   return git_modules_config(var, value, cb);
+   }
+}
+
+/* Cheap function that only determines if we're interested in 

[PATCHv3 0/4] A reroll of sb/submodule-blanket-recursive

[Stefan Beller]

v3:
* rerolling only the top-4 patches of sb/submodule-blanket-recursive.
  (base: 1d789d089280539ca39b83aabb67860929d39b75)
* fixes function declarations that should be static, thanks Ramsay!

v2:
* A reroll of sb/submodule-blanket-recursive.
* This requires ab/grep-preparatory-cleanup
* It changed a lot from v1, as in v1 the tests did not work,
  hence the code was broken. Now it actually works.
* it also includes grep, fetch, push in addition to plain working tree
  manipulators.

Thanks,
Stefan

Stefan Beller (4):
  Introduce 'submodule.recurse' option for worktree manipulators
  builtin/grep.c: respect 'submodule.recurse' option
  builtin/push.c: respect 'submodule.recurse' option
  builtin/fetch.c: respect 'submodule.recurse' option

 Documentation/config.txt   |  5 +
 builtin/checkout.c |  2 +-
 builtin/fetch.c|  7 +++
 builtin/grep.c |  3 +++
 builtin/push.c |  4 
 builtin/read-tree.c| 10 +-
 builtin/reset.c| 10 +-
 submodule.c| 23 +--
 submodule.h|  1 +
 t/lib-submodule-update.sh  | 12 
 t/t5526-fetch-submodules.sh| 10 ++
 t/t5531-deep-submodule-push.sh | 21 +
 t/t7814-grep-recurse-submodules.sh | 18 ++
 13 files changed, 121 insertions(+), 5 deletions(-)

-- 
2.13.0.17.gab62347cd9

[PATCH] diff.c: color moved lines differently

[Stefan Beller]

When a patch consists mostly of moving blocks of code around, it can
be quite tedious to ensure that the blocks are moved verbatim, and not
undesirably modified in the move. To that end, color blocks that are
moved within the same patch differently. For example (OM, del, add,
and NM are different colors):

[OM]  -void sensitive_stuff(void)
[OM]  -{
[OM]  -if (!is_authorized_user())
[OM]  -die("unauthorized");
[OM]  -sensitive_stuff(spanning,
[OM]  -multiple,
[OM]  -lines);
[OM]  -}

   void another_function()
   {
[del] -printf("foo");
[add] +printf("bar");
   }

[NM]  +void sensitive_stuff(void)
[NM]  +{
[NM]  +if (!is_authorized_user())
[NM]  +die("unauthorized");
[NM]  +sensitive_stuff(spanning,
[NM]  +multiple,
[NM]  +lines);
[NM]  +}

However adjacent blocks may be problematic. For example, in this
potentially malicious patch, the swapping of blocks can be spotted:

[OM]  -void sensitive_stuff(void)
[OM]  -{
[OMA] -if (!is_authorized_user())
[OMA] -die("unauthorized");
[OM]  -sensitive_stuff(spanning,
[OM]  -multiple,
[OM]  -lines);
[OMA] -}

   void another_function()
   {
[del] -printf("foo");
[add] +printf("bar");
   }

[NM]  +void sensitive_stuff(void)
[NM]  +{
[NMA] +sensitive_stuff(spanning,
[NMA] +multiple,
[NMA] +lines);
[NM]  +if (!is_authorized_user())
[NM]  +die("unauthorized");
[NMA] +}

If the moved code is larger, it is easier to hide some permutation in the
code, which is why some alternative coloring is needed.

As the reviewers attention should be brought to the places, where the
difference is introduced to the moved code, we cannot just have one new
color for all of moved code.

First I implemented an alternative design, which would try to fingerprint
a line by its neighbors to detect if we are in a block or at the boundary.
This idea iss error prone as it inspected each line and its neighboring
lines to determine if the line was (a) moved and (b) if was deep inside
a hunk by having matching neighboring lines. This is unreliable as the
we can construct hunks which have equal neighbors that just exceed the
number of lines inspected. (Think of 'AXYZBXYZCXYZD..' with each letter
as a line, that is permutated to AXYZCXYZBXYZD..').

Instead this provides a dynamic programming greedy algorithm that finds
the largest moved hunk and then has several modes on highlighting bounds.

A note on the options '--submodule=diff' and '--color-words/--word-diff':
In the conversion to use emit_line in the prior patches both submodules
as well as word diff output carefully chose to call emit_line with sign=0.
All output with sign=0 is ignored for move detection purposes in this
patch, such that no weird looking output will be generated for these
cases. This leads to another thought: We could pass on '--color-moved' to
submodules such that they color up moved lines for themselves. If we'd do
so only line moves within a repository boundary are marked up.

Helped-by: Jonathan Tan 
Signed-off-by: Stefan Beller 
---

 Replacing the top commit in origin/sb/diff-color-move, 
 this has the spelling fixes by Philip.
 
 Also a minor fix for the 'alternate' mode, to go back to the default
 after empty lines. Thanks to Jacob.
 
 Thanks,
 Stefan

 Documentation/config.txt   |  10 +-
 Documentation/diff-options.txt |  32 
 color.h|   2 +
 diff.c | 343 +++--
 diff.h |  15 +-
 t/t4015-diff-whitespace.sh | 373 +
 6 files changed, 761 insertions(+), 14 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d51..73511a4603 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1051,14 +1051,20 @@ This does not affect linkgit:git-format-patch[1] or the
 'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
 command line with the `--color[=]` option.
 
+diff.colorMoved::
+   If set moved lines in a diff are colored differently,
+   for details see '--color-moved' in linkgit:git-diff[1].
+
 color.diff.::
Use customized color for diff colorization.  `` specifies
which part of the patch to use the specified color, and is one
of `context` (context text - `plain` is a historical synonym),
`meta` (metainformation), `frag`
(hunk header), 'func' (function in hunk header), `old` (removed 

[PATCH v2] send-email: Net::SMTP::starttls was introduced in v2.34 (Re: [BUG] git-send-email broken: Can't locate object method "starttls")

[Jonathan Nieder]

Subject: send-email: Net::SMTP::starttls was introduced in v2.34

We cannot rely on the starttls method being present in Net::SMTP until
c274b798e6881a941d941808c6d89966975cb8c8 (Merge branch 'ipv6_ssl' of
https://github.com/noxxi/perl-libnet into noxxi-ipv6_ssl, 2014-06-02),
which set the module version to 2.34.

This version was first shipped as part of perl in v5.21.5~169 (Update
libnet to CPAN version 3.01, 2014-10-10).

Noticed on an Ubuntu system with perl 5.18.2-2ubuntu1.1, which
provides Net::SMTP version 2.31. The error message is

  Can't locate object method "starttls" via package "Net::SMTP" at 
/usr/lib/git-core/git-send-email line 1410.

Reported-by: Brandon Williams 
Reported-and-tested-by: Eric Biggers 
Signed-off-by: Jonathan Nieder 
---
Jonathan Nieder wrote:

> Thanks.  Mining through https://github.com/gbarr/perl-libnet, I find
[...]
> I think 2.35 is the correct minimum version.

Nah, I'm reading wrong.  The relevant commit is c274b798 (Merge branch
'ipv6_ssl' of https://github.com/noxxi/perl-libnet into
noxxi-ipv6_ssl, 2014-06-02), which bumped VERSION to 2.34.

Here's an updated patch.

Thanks,
Jonathan

 git-send-email.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 0d90439d9..d326238c0 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1354,7 +1354,7 @@ EOF
}
 
require Net::SMTP;
-   my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
version->parse("1.28");
+   my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
version->parse("2.34");
$smtp_domain ||= maildomain();
 
if ($smtp_encryption eq 'ssl') {
-- 
2.13.0.506.g27d5fe0cd

Re: [PATCH] docs/config: mention protocol implications of url.insteadOf

[Brandon Williams]

On 05/31, Jeff King wrote:
> On Fri, May 26, 2017 at 11:22:37AM -0500, Elliott Cable wrote:
> 
> > 1. Most simply, better documentation: mention `GIT_PROTOCOL_FROM_USER`
> >explicitly in the documentation of/near `insteadOf`, most
> >particularly in the README for `contrib/persistent-https`.
> 
> I agree that a hint in both places would be helpful.  The patch for that
> is below.
> 
> > 2. Possibly, special-case “higher-security” porcelain (like
> >`git-submodule`, as described in 33cfccbbf3) to ignore `insteadOf`
> >rewrite-rules without additional, special configuration. This way,
> >`git-submodule` works for ignorant users (like me) out of the box,
> >just as it previously did, and there's no possible security
> >compramise.
> 
> I don't think we can do that. Rewrites of "git://" to "ssh://" are
> pretty common (and completely harmless). Besides, I think submodules are
> a case where you really would want persistent-https to kick in. IIRC,
> the original use case for that helper is Android development, where a
> user is likely to update a ton of repositories from the same server all
> at once. Right now the fetches are all done individually with the "repo"
> tool, but in theory the whole thing could be set up as submodules.

This right here is why Stefan and I have been working on submodules.

> 
> -- >8 --
> Subject: [PATCH] docs/config: mention protocol implications of url.insteadOf
> 
> If a URL rewrite switches the protocol to something
> nonstandard (like "persistent-https" for "https"), the user
> may be bitten by the fact that the default protocol
> restrictions are different between the two. Let's drop a
> note in insteadOf that points the user in the right
> direction.
> 
> It would be nice if we could make this work out of the box,
> but we can't without knowing the security implications of
> the user's rewrite. Only the documentation for a particular
> remote helper can advise one way or the other. Since we do
> include the persistent-https helper in contrib/ (and since
> it was the helper in the real-world case that inspired that
> patch), let's also drop a note there.

Documentation changes look sane to me.  Thanks for whipping this up!

> 
> Suggested-by: Elliott Cable 
> Signed-off-by: Jeff King 
> ---
>  Documentation/config.txt|  7 +++
>  contrib/persistent-https/README | 10 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 43d830ee3..5218ecd37 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -3235,6 +3235,13 @@ url..insteadOf::
>   the best alternative for the particular user, even for a
>   never-before-seen repository on the site.  When more than one
>   insteadOf strings match a given URL, the longest match is used.
> ++
> +Note that any protocol restrictions will be applied to the rewritten
> +URL. If the rewrite changes the URL to use a custom protocol or remote
> +helper, you may need to adjust the `protocol.*.allow` config to permit
> +the request.  In particular, protocols you expect to use for submodules
> +must be set to `always` rather than the default of `user`. See the
> +description of `protocol.allow` above.
>  
>  url..pushInsteadOf::
>   Any URL that starts with this value will not be pushed to;
> diff --git a/contrib/persistent-https/README b/contrib/persistent-https/README
> index f784dd2e6..7c4cd8d25 100644
> --- a/contrib/persistent-https/README
> +++ b/contrib/persistent-https/README
> @@ -35,6 +35,16 @@ to use persistent-https:
>  [url "persistent-http"]
>   insteadof = http
>  
> +You may also want to allow the use of the persistent-https helper for
> +submodule URLs (since any https URLs pointing to submodules will be
> +rewritten, and Git's out-of-the-box defaults forbid submodules from
> +using unknown remote helpers):
> +
> +[protocol "persistent-https"]
> + allow = always
> +[protocol "persistent-http"]
> + allow = always
> +
>  
>  #
>  # BUILDING FROM SOURCE
> -- 
> 2.13.0.678.ga17378094
> 

-- 
Brandon Williams

[PATCH/RFC v2 4/6] config: modify function signature to include copy argument

[Sahil Dua]

Changed git_config_rename_section to git_config_copy_or_rename_section
which will now accept another argument flag "copy" which will determine
if the function will copy the config section or just rename it.

Again, this includes changes at a lot of unrelated other places wherever
the renamed and updated functions were being used. Default value of
copy=0 is passed at all those places in order to make sure the behavior
of the functions doesn't change for those cases.

Signed-off-by: Sahil Dua 
---
 builtin/branch.c | 4 ++--
 builtin/config.c | 4 ++--
 builtin/remote.c | 4 ++--
 cache.h  | 4 ++--
 config.c | 6 +++---
 submodule.c  | 2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 16d01a100cbb9..f3cd180e8d4cb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -176,7 +176,7 @@ static void delete_branch_config(const char *branchname)
 {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(, "branch.%s", branchname);
-   if (git_config_copy_or_rename_section(buf.buf, NULL) < 0)
+   if (git_config_copy_or_rename_section(buf.buf, NULL, 0) < 0)
warning(_("Update of config-file failed"));
strbuf_release();
 }
@@ -502,7 +502,7 @@ static void copy_or_rename_branch(const char *oldname, 
const char *newname, int
strbuf_release();
strbuf_addf(, "branch.%s", newref.buf + 11);
strbuf_release();
-   if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf) < 
0)
+   if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf, 
copy) < 0)
die(_("Branch is %s, but update of config-file failed"),
 (copy ? "copied" : "renamed"));
strbuf_release();
diff --git a/builtin/config.c b/builtin/config.c
index c72972d731bd1..4f0b3d1595709 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -694,7 +694,7 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
check_write();
check_argc(argc, 2, 2);
ret = 
git_config_copy_or_rename_section_in_file(given_config_source.file,
-   argv[0], argv[1]);
+   argv[0], argv[1], 0);
if (ret < 0)
return ret;
if (ret == 0)
@@ -705,7 +705,7 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
check_write();
check_argc(argc, 1, 1);
ret = 
git_config_copy_or_rename_section_in_file(given_config_source.file,
-   argv[0], NULL);
+   argv[0], NULL, 0);
if (ret < 0)
return ret;
if (ret == 0)
diff --git a/builtin/remote.c b/builtin/remote.c
index ade748044b5ab..2abcdfa441599 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -635,7 +635,7 @@ static int mv(int argc, const char **argv)
strbuf_reset();
strbuf_addf(, "remote.%s", rename.old);
strbuf_addf(, "remote.%s", rename.new);
-   if (git_config_copy_or_rename_section(buf.buf, buf2.buf) < 1)
+   if (git_config_copy_or_rename_section(buf.buf, buf2.buf, 0) < 1)
return error(_("Could not rename config section '%s' to '%s'"),
buf.buf, buf2.buf);
 
@@ -804,7 +804,7 @@ static int rm(int argc, const char **argv)
 
if (!result) {
strbuf_addf(, "remote.%s", remote->name);
-   if (git_config_copy_or_rename_section(buf.buf, NULL) < 1)
+   if (git_config_copy_or_rename_section(buf.buf, NULL, 0) < 1)
return error(_("Could not remove config section '%s'"), 
buf.buf);
}
 
diff --git a/cache.h b/cache.h
index b2b043d3505ba..54a7f272bac87 100644
--- a/cache.h
+++ b/cache.h
@@ -1933,8 +1933,8 @@ extern int git_config_set_multivar_gently(const char *, 
const char *, const char
 extern void git_config_set_multivar(const char *, const char *, const char *, 
int);
 extern int git_config_set_multivar_in_file_gently(const char *, const char *, 
const char *, const char *, int);
 extern void git_config_set_multivar_in_file(const char *, const char *, const 
char *, const char *, int);
-extern int git_config_copy_or_rename_section(const char *, const char *);
-extern int git_config_copy_or_rename_section_in_file(const char *, const char 
*, const char *);
+extern int git_config_copy_or_rename_section(const char *, const char *, int);
+extern int git_config_copy_or_rename_section_in_file(const char *, const char 
*, const char *, int);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/config.c b/config.c
index 

[PATCH/RFC v2 1/6] branch: add tests for new copy branch feature

[Sahil Dua]

Adds a few basic tests for getting any suggestions/feedback
about expected behavior for this new feature. Aim is to have an option -c
for copying a branch just like -m option for renaming a branch.

Signed-off-by: Sahil Dua 
---
 t/t3200-branch.sh | 53 +
 1 file changed, 53 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da6..2c95ed6ebf3c5 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
test_must_fail git config branch.s/s/dummy
 '
 
+test_expect_success 'git branch -c dumps usage' '
+   test_expect_code 128 git branch -c 2>err &&
+   test_i18ngrep "branch name required" err
+'
+
+git config branch.d.dummy Hello
+
+test_expect_success 'git branch -c d e should work' '
+   git branch -l d &&
+   git reflog exists refs/heads/d &&
+   git branch -c d e &&
+   git reflog exists refs/heads/d &&
+   git reflog exists refs/heads/e
+'
+
+test_expect_success 'config information was copied, too' '
+   test $(git config branch.e.dummy) = Hello &&
+   test $(git config branch.d.dummy) = Hello
+'
+
+git config branch.f/f.dummy Hello
+
+test_expect_success 'git branch -c f/f g/g should work' '
+   git branch -l f/f &&
+   git reflog exists refs/heads/f/f &&
+   git branch -c f/f g/g &&
+   git reflog exists refs/heads/f/f &&
+   git reflog exists refs/heads/g/g
+'
+
+test_expect_success 'config information was copied, too' '
+   test $(git config branch.f/f.dummy) = Hello &&
+   test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+   git branch -l m2 &&
+   git reflog exists refs/heads/m2 &&
+   git branch -c m2 m2 &&
+   git reflog exists refs/heads/m2
+'
+
+test_expect_success 'git branch -c a a/a should fail' '
+   git branch -l a &&
+   git reflog exists refs/heads/a &&
+   test_must_fail git branch -c a a/a
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+   git branch -l b/b &&
+   test_must_fail git branch -c b/b b
+'
+
 test_expect_success 'deleting a symref' '
git branch target &&
git symbolic-ref refs/heads/symref refs/heads/target &&

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 6/6] branch: don't copy or rename config when same branch name

[Sahil Dua]

It doesn't make sense to trigger config section copy or rename method if
both the branch names are same.

For example - git branch -C a a
In such a case, it shouldn't try to copy or rename the git config
section.

Signed-off-by: Sahil Dua 
---
 builtin/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index f3cd180e8d4cb..df82f196a4bba 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -502,7 +502,7 @@ static void copy_or_rename_branch(const char *oldname, 
const char *newname, int
strbuf_release();
strbuf_addf(, "branch.%s", newref.buf + 11);
strbuf_release();
-   if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf, 
copy) < 0)
+   if (strcmp(oldname, newname) && 
git_config_copy_or_rename_section(oldsection.buf, newsection.buf, copy) < 0)
die(_("Branch is %s, but update of config-file failed"),
 (copy ? "copied" : "renamed"));
strbuf_release();

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 2/6] branch: add copy branch option

[Sahil Dua]

Adds copy branch option available using -c or -C (forcefully).

Includes a lot of function renames and their signature changes in order
to introduce a new function parameter - flag 'copy' which determines
whether those functions should do operation copy or move.

Additionally, this changes a lot of other files wherever the renamed
functions were used. By default copy=0 is passed at all those places so
that they keep behaving the way they were, before these changes.

Signed-off-by: Sahil Dua 
---
 builtin/branch.c  | 48 +++
 builtin/config.c  |  4 ++--
 builtin/remote.c  |  6 +++---
 cache.h   |  4 ++--
 config.c  |  6 +++---
 refs.c| 10 +-
 refs.h|  7 ---
 refs/files-backend.c  | 21 ++---
 refs/refs-internal.h  |  6 +++---
 submodule.c   |  2 +-
 t/helper/test-ref-store.c |  2 +-
 11 files changed, 70 insertions(+), 46 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda43dceec..16d01a100cbb9 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [] [-l] [-f]  []"),
N_("git branch [] [-r] (-d | -D) ..."),
N_("git branch [] (-m | -M) [] "),
+   N_("git branch [] (-c | -C) [] "),
N_("git branch [] [-r | -a] [--points-at]"),
N_("git branch [] [-r | -a] [--format]"),
NULL
@@ -175,7 +176,7 @@ static void delete_branch_config(const char *branchname)
 {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(, "branch.%s", branchname);
-   if (git_config_rename_section(buf.buf, NULL) < 0)
+   if (git_config_copy_or_rename_section(buf.buf, NULL) < 0)
warning(_("Update of config-file failed"));
strbuf_release();
 }
@@ -449,7 +450,7 @@ static void reject_rebase_or_bisect_branch(const char 
*target)
free_worktrees(worktrees);
 }
 
-static void rename_branch(const char *oldname, const char *newname, int force)
+static void copy_or_rename_branch(const char *oldname, const char *newname, 
int copy, int force)
 {
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = 
STRBUF_INIT;
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
@@ -457,7 +458,8 @@ static void rename_branch(const char *oldname, const char 
*newname, int force)
int clobber_head_ok;
 
if (!oldname)
-   die(_("cannot rename the current branch while not on any."));
+   die(_("cannot %s the current branch while not on any."),
+(copy ? "copy" : "rename"));
 
if (strbuf_check_branch_ref(, oldname)) {
/*
@@ -480,17 +482,19 @@ static void rename_branch(const char *oldname, const char 
*newname, int force)
 
reject_rebase_or_bisect_branch(oldref.buf);
 
-   strbuf_addf(, "Branch: renamed %s to %s",
-oldref.buf, newref.buf);
+   strbuf_addf(, "Branch: %s %s to %s",
+(copy ? "copied" : "renamed"), oldref.buf, newref.buf);
 
-   if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
-   die(_("Branch rename failed"));
+   if (copy_or_rename_ref(oldref.buf, newref.buf, logmsg.buf, copy))
+   die(_("Branch %s failed"), (copy ? "copy" : "rename"));
 
if (recovery)
-   warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 
11);
+   warning(_("%s a misnamed branch '%s' away"),
+(copy ? "copied" : "renamed"), oldref.buf + 11);
 
if (replace_each_worktree_head_symref(oldref.buf, newref.buf, 
logmsg.buf))
-   die(_("Branch renamed to %s, but HEAD is not updated!"), 
newname);
+   die(_("Branch %s to %s, but HEAD is not updated!"),
+(copy ? "copied" : "renamed"), newname);
 
strbuf_release();
 
@@ -498,8 +502,9 @@ static void rename_branch(const char *oldname, const char 
*newname, int force)
strbuf_release();
strbuf_addf(, "branch.%s", newref.buf + 11);
strbuf_release();
-   if (git_config_rename_section(oldsection.buf, newsection.buf) < 0)
-   die(_("Branch is renamed, but update of config-file failed"));
+   if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf) < 
0)
+   die(_("Branch is %s, but update of config-file failed"),
+(copy ? "copied" : "renamed"));
strbuf_release();
strbuf_release();
 }
@@ -537,7 +542,7 @@ static int edit_branch_description(const char *branch_name)
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-   int delete = 0, rename = 0, force = 0, list = 0;
+   int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
int reflog = 0, edit_description = 0;

[PATCH/RFC v2 3/6] config: abstract out create section from key logic

[Sahil Dua]

Abstracts out the logic for creating string buffer from given key for
example - 'branch.b' and returns '[branch "b"]'.

We want to keep the original config section intact in case of copy
operation. For this we need to fetch the section with updated new branch
name so that we can write that to the config file.

For example - git branch -c foo bar
The mentioned/edited function renames and overwrites this part in the
config - [branch "foo"] to [branch "bar"]. However, in case of copy, we
want to keep the original [branch "foo"] intact and get [branch "bar"]
from "branch.bar" key. 'store_create_section' function will return
[branch "bar"] when "branch.bar" is passed.

Signed-off-by: Sahil Dua 
---
 config.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 78cf1ffac043e..d3d48bfae3b96 100644
--- a/config.c
+++ b/config.c
@@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
return 4;
 }
 
-static int store_write_section(int fd, const char *key)
+struct strbuf store_create_section(const char *key)
 {
const char *dot;
-   int i, success;
+   int i;
struct strbuf sb = STRBUF_INIT;
 
dot = memchr(key, '.', store.baselen);
@@ -2188,6 +2188,16 @@ static int store_write_section(int fd, const char *key)
strbuf_addf(, "[%.*s]\n", store.baselen, key);
}
 
+   return sb;
+}
+
+static int store_write_section(int fd, const char *key)
+{
+   int success;
+
+   /* Create a section with the given key */
+   struct strbuf sb = store_create_section(key);
+
success = write_in_full(fd, sb.buf, sb.len) == sb.len;
strbuf_release();
 

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 5/6] config: add copy config section logic

[Sahil Dua]

Adds implementation for copying the config section while copying a
branch.

While we're parsing the config file, we need to make sure we start
copying the config section once we find the matching block for our
branch1 (for example when running 'git branch -c branch1 branch2').

There is one flag used - 'copying_section' which can take 0/1/2 values.
0 - not copying currently
1 - just started copying section
2 - currently copying
I thought of making this flag binary to keep things easier. However,
since there was distinction in behavior(adding to currently copied
section) depending upon whether it's the first line of config section or
not.

The copied section has first line which contains the new branch name
(branch2 in our example). This is achieved using store_create_section
method.

Once we're done with reading the entire config file, we write our copied
section. Hence, literally copying the config section from branch1 to
branch2.

However, there's one case which is not handled by this yet - when
branch2 already has some configuration and -C command is used, operation
should delete the present configuration for branch2.

Signed-off-by: Sahil Dua 
---
 config.c | 67 +---
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/config.c b/config.c
index 155274f03b2b6..2bf711ca3e0da 100644
--- a/config.c
+++ b/config.c
@@ -2642,13 +2642,14 @@ static int section_name_is_ok(const char *name)
 int git_config_copy_or_rename_section_in_file(const char *config_filename,
  const char *old_name, const char 
*new_name, int copy)
 {
-   int ret = 0, remove = 0;
+   int ret = 0, remove = 0, copying_section = 0, copied_section_length;
char *filename_buf = NULL;
struct lock_file *lock;
int out_fd;
char buf[1024];
FILE *config_file = NULL;
struct stat st;
+   struct strbuf copied_section;
 
if (new_name && !section_name_is_ok(new_name)) {
ret = error("invalid section name: %s", new_name);
@@ -2689,6 +2690,13 @@ int git_config_copy_or_rename_section_in_file(const char 
*config_filename,
; /* do nothing */
if (buf[i] == '[') {
/* it's a section */
+   if (copying_section) {
+   /* Mark the end of copying the matching
+* section, as this is the beginning
+* of the new section
+*/
+   copying_section = 0;
+   }
int offset = section_name_match([i], old_name);
if (offset > 0) {
ret++;
@@ -2696,26 +2704,41 @@ int git_config_copy_or_rename_section_in_file(const 
char *config_filename,
remove = 1;
continue;
}
-   store.baselen = strlen(new_name);
-   if (!store_write_section(out_fd, new_name)) {
-   ret = 
write_error(get_lock_file_path(lock));
-   goto out;
+   if (!copy) {
+   store.baselen = strlen(new_name);
+   if (!store_write_section(out_fd, 
new_name)) {
+   ret = 
write_error(get_lock_file_path(lock));
+   goto out;
+   }
+   } else {
+   /* Mark the beginning of copying the 
matching section */
+   copying_section = 1;
+
+   /* TODO: Make this work for the
+* case when there are multiple
+* matching sections
+*/
+   /* Create a section with new branch 
name */
+   store.baselen = strlen(new_name);
+   copied_section = 
store_create_section(new_name);
}
/*
 * We wrote out the new section, with
 * a newline, now skip the old
 * section's length
 */
-   output += offset + i;
-   if (strlen(output) > 0) {
-   /*
-* More content means there's
-   

Re: [PATCH] send-email: Net::SMTP::starttls was introduced in v3.01 (Re: [BUG] git-send-email broken: Can't locate object method "starttls")

[Jonathan Nieder]

Eric Biggers wrote:
> On Wed, May 31, 2017 at 03:44:15PM -0700, Jonathan Nieder wrote:

>> Subject: send-email: Net::SMTP::starttls was introduced in v3.01
>>
>> We cannot rely on the starttls method being present in the copy
>> of Net::SMTP shipped with perl until v5.21.5~169 (Update libnet to
>> CPAN version 3.01, 2014-10-10).
>>
>> Reported-by: Brandon Williams 
>> Reported-by: Eric Biggers 
>> Signed-off-by: Jonathan Nieder 
>> ---
[...]
>> +++ b/git-send-email.perl
>> @@ -1354,7 +1354,7 @@ EOF
>>  }
>>  
>>  require Net::SMTP;
>> -my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
>> version->parse("1.28");
>> +my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
>> version->parse("3.01");
>>  $smtp_domain ||= maildomain();
>>  
>>  if ($smtp_encryption eq 'ssl') {
>> -- 
>
> Yes, that solves the problem for me.

Thanks.  Mining through https://github.com/gbarr/perl-libnet, I find

  $ git log -Sstarttls
[...]
  commit b4a7a274a7fe5344c154abc4b3fdd7c446d36370
  Author: Steffen Ullrich 
  Date:   Fri May 9 23:15:48 2014 +0200

  SSL and IPv6 support for Net::SMTP
  $ git show b4a7a274a7fe5344c154abc4b3fdd7c446d36370
[...]
  diff --git a/Net/SMTP.pm b/Net/SMTP.pm
  index 705b5c5..fcc124f 100644
  --- a/Net/SMTP.pm
  +++ b/Net/SMTP.pm
  @@ -18,7 +18,31 @@ use Net::Config;
 
   $VERSION = "2.33";
[...]
  $ git log -p --ancestry-path b4a7a274a7fe5344c154abc4b3fdd7c446d36370..HEAD  
-- Net/SMTP.pm
[...]
  commit 67b37d5c7118f9af50e5a5a00c242992caba3b8d
  Author: Steve Hay 
  Date:   Mon Jun 2 14:13:55 2014 +0100

  Bump $VERSION in changed modules

  diff --git a/Net/SMTP.pm b/Net/SMTP.pm
  index 4496f6f..9dfaadf 100644
  --- a/Net/SMTP.pm
  +++ b/Net/SMTP.pm
  @@ -16,7 +16,7 @@ use IO::Socket;
   use Net::Cmd;
   use Net::Config;
   
  -$VERSION = "2.34";
  +$VERSION = "2.35";
   
   # Code for detecting if we can use SSL
   my $ssl_class = eval {

I think 2.35 is the correct minimum version.

Jonathan

Re: [PATCH 00/31] repository object

[Brandon Williams]

On 05/31, Stefan Beller wrote:
> On Wed, May 31, 2017 at 2:43 PM, Brandon Williams  wrote:
> > Given the vast interest expressed when I sent out my RFC series I decided it
> > would be worth it to invest more time to making a repository object a 
> > reality.
> >
> > This series is an extension of the last series I sent out (in that ls-files 
> > is
> > converted to working on submodules in-process using repository objects 
> > instead
> > of spawning a child process to do the work).  The big difference from the 
> > RFC
> > series is that I went through and did the work to migrate key repository 
> > state
> > from global variables in 'environment.c' to being stored in a repository 
> > object
> > itself.  I migrated the bits of state that seemed reasonable for this 
> > series,
> > there is still a lot of global state which could be migrated in the future.
> >
> > I do think that we need to be slightly cautious about moving global state 
> > into
> > the repository object though, I don't want 'struct repo' to simply become a
> > kitchen sink where everything gets dumped.  But this is just a warning for 
> > the
> > future.
> 
> Or in other words:
> You want to have another struct e.g. 'the_command_line_arguments',
> which would carry the verbosity/color options for example as they are
> not related to a repo object, but to the current command being run?

Yes exactly.  Library code that needs to operate on a repository would
then be able to take arguments like:

  some_library_function(struct repo *repo, struct lib_opts *ops)

Much like how the grep machinery takes a grep_opts struct.

> 
> > Since this is a v1 I'm fairly certain that it still has a lot of rough edges
> > (like I think I need to write better commit messages, and we should probably
> > have more comments documenting object fields/contract) but I want to get the
> > review process started sooner rather than later since I'm sure people will 
> > have
> > opinions (e.g. should it be called 'struct repo' or 'struct repository'?!).
> 
> IMHO this is the most obvious, but bikesheddable part of the series. ;)

I know, that's why I mentioned it ;)

> Keep it short as everyone knows what a 'repo' is.

-- 
Brandon Williams

Re: [PATCH] send-email: Net::SMTP::starttls was introduced in v3.01 (Re: [BUG] git-send-email broken: Can't locate object method "starttls")

[Eric Biggers]

On Wed, May 31, 2017 at 03:44:15PM -0700, Jonathan Nieder wrote:
> Subject: send-email: Net::SMTP::starttls was introduced in v3.01
> 
> We cannot rely on the starttls method being present in the copy
> of Net::SMTP shipped with perl until v5.21.5~169 (Update libnet to
> CPAN version 3.01, 2014-10-10).
> 
> Reported-by: Brandon Williams 
> Reported-by: Eric Biggers 
> Signed-off-by: Jonathan Nieder 
> ---
> Hi Eric,
> 
> Eric Biggers wrote:
> 
> > There seems to be a bug in 'git send-email' caused by this commit:
> >
> > commit 0ead000c3aca13a10ae51a3c74c866981e0d33b8
> > Author: Dennis Kaarsemaker 
> > Date:   Fri Mar 24 22:37:32 2017 +0100
> >
> > send-email: Net::SMTP::SSL is obsolete, use only when necessary
> >
> > When running git send-email I get the following error:
> >
> > Can't locate object method "starttls" via package "Net::SMTP" at 
> > /usr/lib/git-core/git-send-email line 1410.
> >
> > The perl version is 5.18.2, and the Net::SMTP version is 2.31.
> 
> Thanks for reporting.  Does this patch help?
> 
> Regards,
> Jonathan
> 
>  git-send-email.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 0d90439d9..3441e3cf5 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1354,7 +1354,7 @@ EOF
>   }
>  
>   require Net::SMTP;
> - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
> version->parse("1.28");
> + my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
> version->parse("3.01");
>   $smtp_domain ||= maildomain();
>  
>   if ($smtp_encryption eq 'ssl') {
> -- 

Yes, that solves the problem for me.

Eric

Re: [PATCH 00/31] repository object

[Stefan Beller]

On Wed, May 31, 2017 at 2:43 PM, Brandon Williams  wrote:
> Given the vast interest expressed when I sent out my RFC series I decided it
> would be worth it to invest more time to making a repository object a reality.
>
> This series is an extension of the last series I sent out (in that ls-files is
> converted to working on submodules in-process using repository objects instead
> of spawning a child process to do the work).  The big difference from the RFC
> series is that I went through and did the work to migrate key repository state
> from global variables in 'environment.c' to being stored in a repository 
> object
> itself.  I migrated the bits of state that seemed reasonable for this series,
> there is still a lot of global state which could be migrated in the future.
>
> I do think that we need to be slightly cautious about moving global state into
> the repository object though, I don't want 'struct repo' to simply become a
> kitchen sink where everything gets dumped.  But this is just a warning for the
> future.

Or in other words:
You want to have another struct e.g. 'the_command_line_arguments',
which would carry the verbosity/color options for example as they are
not related to a repo object, but to the current command being run?

> Since this is a v1 I'm fairly certain that it still has a lot of rough edges
> (like I think I need to write better commit messages, and we should probably
> have more comments documenting object fields/contract) but I want to get the
> review process started sooner rather than later since I'm sure people will 
> have
> opinions (e.g. should it be called 'struct repo' or 'struct repository'?!).

IMHO this is the most obvious, but bikesheddable part of the series. ;)
Keep it short as everyone knows what a 'repo' is.

Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary

[Jonathan Nieder]

Hi,

Jun 01, 2017 at 07:39:16AM +0900, Junio C Hamano wrote:
> Jonathan Nieder  writes:

>> This broke git send-email for me.  The error message is
>>
>>   Can't locate object method "starttls" via package "Net::SMTP" at 
>> /usr/lib/git-core/git-send-email line 1410.
>>
>> Is 1.28 the right minimum version?
>>
>>   $ perl -e 'require Net::SMTP; print version->parse($Net::SMTP::VERSION); 
>> print "\n"'
[...]
>>   $ grep starttls /usr/share/perl/5.18.2/Net/SMTP.pm
>>   $ dpkg-query -W perl
>>   perl5.18.2-2ubuntu1.1
>
> Thanks.
>
> Let's revert the merge for now until we know (this time for certain)
> what the minimum version is.

Thanks.  I just sent
http://public-inbox.org/git/20170531224415.gc81...@aiede.mtv.corp.google.com
in response to another thread.  That uses 3.01 as minimum version,
since it is the minimum version imported to core perl with starttls
support.

I haven't tried testing with historical Net::SMTP versions, though.
Is there a git repository available with all Net::SMTP versions from
CPAN, or is https://perl5.git.perl.org/perl.git as good as it gets?

Regards,
Jonathan

Re: [PATCH] send-email: Net::SMTP::starttls was introduced in v3.01 (Re: [BUG] git-send-email broken: Can't locate object method "starttls")

[Junio C Hamano]

Jonathan Nieder  writes:

> Subject: send-email: Net::SMTP::starttls was introduced in v3.01
>
> We cannot rely on the starttls method being present in the copy
> of Net::SMTP shipped with perl until v5.21.5~169 (Update libnet to
> CPAN version 3.01, 2014-10-10).
>
> Reported-by: Brandon Williams 
> Reported-by: Eric Biggers 
> Signed-off-by: Jonathan Nieder 
> ---
> Hi Eric,
>
> Eric Biggers wrote:
>
>> There seems to be a bug in 'git send-email' caused by this commit:
>>
>> commit 0ead000c3aca13a10ae51a3c74c866981e0d33b8
>> Author: Dennis Kaarsemaker 
>> Date:   Fri Mar 24 22:37:32 2017 +0100
>>
>> send-email: Net::SMTP::SSL is obsolete, use only when necessary
>>
>> When running git send-email I get the following error:
>>
>>  Can't locate object method "starttls" via package "Net::SMTP" at 
>> /usr/lib/git-core/git-send-email line 1410.
>>
>> The perl version is 5.18.2, and the Net::SMTP version is 2.31.
>
> Thanks for reporting.  Does this patch help?

Good, you beat me to it ;-)  Once we get this confirmed, let's queue
an emergency fix.

Thanks.

>
> Regards,
> Jonathan
>
>  git-send-email.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 0d90439d9..3441e3cf5 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1354,7 +1354,7 @@ EOF
>   }
>  
>   require Net::SMTP;
> - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
> version->parse("1.28");
> + my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
> version->parse("3.01");
>   $smtp_domain ||= maildomain();
>  
>   if ($smtp_encryption eq 'ssl') {

Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary

[Junio C Hamano]

Dennis Kaarsemaker  writes:

> Second ping. This problem is not going away, so if this solution is not
> acceptable, I'd like to know what needs to be improved.

Perhaps you needed to actually test with older installation that
people have, it seems, between pings.  Immediately after this was
merged to 'master', we start getting bug reports X-<.

Eric Biggers' message 

https://public-inbox.org/git/<20170531222455.gd72...@gmail.com>

seems to indicate that we should cut off at 3.01 not 1.28?

Thanks.

> On Thu, 2017-05-04 at 09:01 +0200, Dennis Kaarsemaker wrote:
>> Ping. It's a little over a month since I sent this, but I haven't seen
>> any comments. Is this commit good to go?
>> 
>> On Fri, 2017-03-24 at 22:37 +0100, Dennis Kaarsemaker wrote:
>> > Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine
>> > since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28
>> > isn't that old yet, keep the old code in place and use it when
>> > necessary.
>> > ...
>> > diff --git a/git-send-email.perl b/git-send-email.perl
>> > index eea0a517f7..0d90439d9a 100755
>> > --- a/git-send-email.perl
>> > +++ b/git-send-email.perl
>> > @@ -1353,10 +1353,12 @@ EOF
>> >die __("The required SMTP server is not properly 
>> > defined.")
>> >}
>> >  
>> > +  require Net::SMTP;
>> > +  my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
>> > version->parse("1.28");

[PATCH] send-email: Net::SMTP::starttls was introduced in v3.01 (Re: [BUG] git-send-email broken: Can't locate object method "starttls")

[Jonathan Nieder]

Subject: send-email: Net::SMTP::starttls was introduced in v3.01

We cannot rely on the starttls method being present in the copy
of Net::SMTP shipped with perl until v5.21.5~169 (Update libnet to
CPAN version 3.01, 2014-10-10).

Reported-by: Brandon Williams 
Reported-by: Eric Biggers 
Signed-off-by: Jonathan Nieder 
---
Hi Eric,

Eric Biggers wrote:

> There seems to be a bug in 'git send-email' caused by this commit:
>
> commit 0ead000c3aca13a10ae51a3c74c866981e0d33b8
> Author: Dennis Kaarsemaker 
> Date:   Fri Mar 24 22:37:32 2017 +0100
>
> send-email: Net::SMTP::SSL is obsolete, use only when necessary
>
> When running git send-email I get the following error:
>
>   Can't locate object method "starttls" via package "Net::SMTP" at 
> /usr/lib/git-core/git-send-email line 1410.
>
> The perl version is 5.18.2, and the Net::SMTP version is 2.31.

Thanks for reporting.  Does this patch help?

Regards,
Jonathan

 git-send-email.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 0d90439d9..3441e3cf5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1354,7 +1354,7 @@ EOF
}
 
require Net::SMTP;
-   my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
version->parse("1.28");
+   my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
version->parse("3.01");
$smtp_domain ||= maildomain();
 
if ($smtp_encryption eq 'ssl') {
-- 
2.13.0.506.g27d5fe0cd

Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary

[Junio C Hamano]

Jonathan Nieder  writes:

> Dennis Kaarsemaker wrote:
>
>> Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine
>> since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28
>> isn't that old yet, keep the old code in place and use it when
>> necessary.
>
> This broke git send-email for me.  The error message is
>
>   Can't locate object method "starttls" via package "Net::SMTP" at 
> /usr/lib/git-core/git-send-email line 1410.
>
> Is 1.28 the right minimum version?
>
>   $ perl -e 'require Net::SMTP; print version->parse($Net::SMTP::VERSION); 
> print "\n"'
>   2.31
>   $ grep VERSION /usr/share/perl/5.18.2/Net/SMTP.pm
>   use vars qw($VERSION @ISA);
>   $VERSION = "2.31";
>   $ grep starttls /usr/share/perl/5.18.2/Net/SMTP.pm
>   $ dpkg-query -W perl
>   perl5.18.2-2ubuntu1.1
>
Thanks.  

Let's revert the merge for now until we know (this time for certain)
what the minimum version is.

[BUG] git-send-email broken: Can't locate object method "starttls"

[Eric Biggers]

Hi,

There seems to be a bug in 'git send-email' caused by this commit:

commit 0ead000c3aca13a10ae51a3c74c866981e0d33b8
Author: Dennis Kaarsemaker 
Date:   Fri Mar 24 22:37:32 2017 +0100

send-email: Net::SMTP::SSL is obsolete, use only when necessary

When running git send-email I get the following error:

Can't locate object method "starttls" via package "Net::SMTP" at 
/usr/lib/git-core/git-send-email line 1410.

The perl version is 5.18.2, and the Net::SMTP version is 2.31.

I suspect the following line of code which appears to assume that starttls() is
present in Net::SMTP >= 1.28:

my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
version->parse("1.28");

... but in fact it appears to have been added by the following commit to perl,
which bumped the Net::SMTP version from 2.34 up to 3.01:


https://perl5.git.perl.org/perl.git/commit/2e1731446cd265cddae2ea6c43a375168fdb6f56

Eric

Re: [PATCH 2/2] rebase: turn on progress option by default for format-patch

[Jeff King]

On Wed, May 31, 2017 at 08:04:27AM -0700, Kevin Willford wrote:

> This change passes the progress option of format-patch by
> default and passes the -q --quiet option through to the
> format-patch call so that it is respected as well.

That makes sense. Is it a bug that we aren't propagating "-q" already?

I think the answer is "no", because that option only claims to silence
the printing of the filenames that format-patch writes. But since we're
using --stdout, it wouldn't write those anyway.

Come to think of it, I'm not sure that "-q" silencing "--progress" in
your patch 1 actually makes sense. If you do:

  git format-patch -o out/

you don't need progress, because we're already writing the filenames to
stdout. So it's only if you did:

  git format-patch -o out/ -q

that you'd actually want progress. But "-q" would override any
--progress option!  So I actually think we may be better off keeping the
two as distinct features (especially if we follow the "no progress
unless --progress is given" rule I mentioned in the earlier message).

> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 375239341f..ab2be30abf 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -51,8 +51,9 @@ then
>  else
>   rm -f "$GIT_DIR/rebased-patches"
>  
> - git format-patch -k --stdout --full-index --cherry-pick --right-only \
> - --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
> + git format-patch $git_format_patch_opt -k --stdout --full-index \
> + --cherry-pick --right-only --src-prefix=a/ --dst-prefix=b/ \
> + --no-renames --no-cover-letter --progress \
>   "$revisions" ${restrict_revision+^$restrict_revision} \
>   >"$GIT_DIR/rebased-patches"

Here we pass --progress unconditionally, which tells format-patch to
output progress information. Shouldn't we be checking whether stderr is
a tty before making that decision? And that we're not in --quiet mode?

That explains why you want to pass "-q" in the other hunk; to
countermand the explicit --progress here. But if we separate the two as
I mentioned above, you'd want logic more like:

  if test -t 2 && test "$GIT_QUIET" != "t"
git_format_patch_opt="$git_format_patch_opt --progress"
  fi

-Peff

Re: [PATCH 0/5] Some submodule bugfixes and "reattaching detached HEADs"

[Stefan Beller]

On Mon, May 1, 2017 at 9:04 PM, Stefan Beller  wrote:
>>
>>> I don't know why submodules were originally designed to be in a
>>> detached HEAD state but I much prefer working on branches (as I'm sure
>>> many other developers do) so the prospect of this becoming the norm is
>>> exciting! :D
>>
>
> I'll think about this more.

What the current model is missing is the possibility to have
a symbolic link not just to a ref within a repository, but to the outside
of a repository (such as the superproject in this case).

So we could have a HEAD with a content like:

"super:  [LF ]"

Then we would use the HEAD to determine if the superproject
would touch a submodule at all. Example workflow:

git -C  checkout --reattach-to-superproject

# hack away in the submodule

# This will make a commit in  and add the
# resulting object to the index of the superproject
# because HEAD is tracking the superproject.
# so in order to have HEAD containing the new
# commit we have to change the superproject:
git -C  commit -a -m "message"

# This has also interesting consequences for
# submodule related commands:
git checkout --recurse-submodules 
# Any submodule whose HEAD is attached to the
# superproject would be touched, the others would
# not.

By being directly attached to the superproject, it would be
easy to find all submodules that are changed, via a

git -C  status # no need to recurse, even!




















The whole "checkout --recurse-submodules" series is based on
assumptions of the current mental model of how branches and
detached HEADs work.


A submodule would have a symref

Re: [PATCH 00/33] object id conversion (grep and diff)

[brian m. carlson]

On Tue, May 30, 2017 at 10:30:36AM -0700, Brandon Williams wrote:
> A month or so ago I thought I would lend a hand to Brian and do a round of
> conversions from sha1 -> struct object_id.  Now that Brian's latest series has
> hit master I can finally send these patches out.
> 
> The first couple patches are from Brian which convert some of the notes logic
> to using 'struct object_id'.  The remaining patches are to convert the grep 
> and
> diff machinery to using 'struct object_id'.
> 
> Brandon Williams (26):
>   grep: convert to struct object_id
>   diff: convert get_stat_data to struct object_id
>   diff: convert diff_index_show_file to struct object_id
>   diff: convert diff_addremove to struct object_id
>   diff: convert run_diff_files to struct object_id
>   diff: convert diff_change to struct object_id
>   diff: convert fill_filespec to struct object_id
>   diff: convert reuse_worktree_file to struct object_id
>   diff: finish conversion for prepare_temp_file to struct object_id
>   patch-ids: convert to struct object_id
>   diff: convert diff_flush_patch_id to struct object_id
>   combine-diff: convert diff_tree_combined to struct object_id
>   combine-diff: convert find_paths_* to struct object_id
>   tree-diff: convert diff_root_tree_sha1 to struct object_id
>   notes-merge: convert notes_merge* to struct object_id
>   notes-merge: convert merge_from_diffs to struct object_id
>   notes-merge: convert find_notes_merge_pair_ps to struct object_id
>   notes-merge: convert verify_notes_filepair to struct object_id
>   notes-merge: convert write_note_to_worktree to struct object_id
>   diff-tree: convert diff_tree_sha1 to struct object_id

I've wanted to convert this function for some time.  I'm glad you got
around to it.

Other than the issue I pointed out, the fact that I'm obviously not
qualified to review my own patches, and the issue that Stefan pointed
out with GIT_MAX_HEXSZ, this looks good to me.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 22/33] notes-merge: convert notes_merge* to struct object_id

[brian m. carlson]

On Tue, May 30, 2017 at 10:30:58AM -0700, Brandon Williams wrote:
> @@ -596,47 +596,47 @@ int notes_merge(struct notes_merge_options *o,
>   /* Find merge bases */
>   bases = get_merge_bases(local, remote);
>   if (!bases) {
> - base_sha1 = null_sha1;
> - base_tree_sha1 = EMPTY_TREE_SHA1_BIN;
> + base_oid = _oid;
> + base_tree_oid = _tree_oid;
>   if (o->verbosity >= 4)
>   printf("No merge base found; doing history-less 
> merge\n");
>   } else if (!bases->next) {
> - base_sha1 = bases->item->object.oid.hash;
> - base_tree_sha1 = bases->item->tree->object.oid.hash;
> + base_oid = >item->object.oid;
> + base_tree_oid = >item->tree->object.oid;
>   if (o->verbosity >= 4)
>   printf("One merge base found (%.7s)\n",
> - sha1_to_hex(base_sha1));
> +oid_to_hex(base_oid));

I noticed you fixed up the indentation.  Thanks.

> diff --git a/notes-merge.h b/notes-merge.h
> index 0d890563b..eaa8e3b86 100644
> --- a/notes-merge.h
> +++ b/notes-merge.h
> @@ -33,15 +33,15 @@ void init_notes_merge_options(struct notes_merge_options 
> *o);
>   *
>   * 1. The merge trivially results in an existing commit (e.g. fast-forward or
>   *already-up-to-date). 'local_tree' is untouched, the SHA1 of the result
> - *is written into 'result_sha1' and 0 is returned.
> + *is written into 'result_oid' and 0 is returned.
>   * 2. The merge successfully completes, producing a merge commit. local_tree
>   *contains the updated notes tree, the SHA1 of the resulting commit is
> - *written into 'result_sha1', and 1 is returned.
> + *written into 'result_oid', and 1 is returned.
>   * 3. The merge results in conflicts. This is similar to #2 in that the
>   *partial merge result (i.e. merge result minus the unmerged entries)
>   *are stored in 'local_tree', and the SHA1 or the resulting commit
>   *(to be amended when the conflicts have been resolved) is written into
> - *'result_sha1'. The unmerged entries are written into the
> + *'result_oid'. The unmerged entries are written into the
>   *.git/NOTES_MERGE_WORKTREE directory with conflict markers.
>   *-1 is returned.
>   *

Did you want to change the comment to say "object ID" or "OID" instead
of "SHA1" like you did in an earlier patch?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH 1/2] format-patch: have progress option while generating patches

[Jeff King]

On Wed, May 31, 2017 at 08:04:26AM -0700, Kevin Willford wrote:

> When generating patches for the rebase command if the user does
> not realize the branch they are rebasing onto is thousands of
> commits different there is no progress indication after initial
> rewinding message.
> 
> This patch allows a progress option to be passed to format-patch
> so that the user can be informed the progress of generating the
> patch.  This option will then be used by the rebase command when
> calling format-patch.

I'm generally in favor of progress meters, though it does seem a little
funny to me that we'd need one on format-patch. It's basically the same
operation as "git log -p", after all. I guess one difference is that
usually the output of "log" would stream to the pager, so the user would
see immediate output. That's true of format-patch, too, but in the
instance you care about we're probably doing something more like:

  git format-patch "$@" >patches

and the user sees nothing.

That makes me wonder two things:

  1. Should this really be tied to isatty(2), as the documentation
 claims?  It seems like you'd really only want it to kick in for
 certain cases. Arguably whenever stdout is _not_ going to a tty
 you'd want progress, but I think probably the caller should
 probably just decide whether to ask for it or not.

  2. Should this apply to other commands in the log family besides
 format-patch? E.g., should "git log --progress -p >commits" work?

 I added a progress meter to rev-list a while ago (for connectivity
 checks). I don't think we could push this down as far as the
 revision traversal code, because only its callers really understand
 what's the right unit to be counting.

 It may not be worth the trouble, though. Only "format-patch" does a
 pre-pass to find the total number of commits. So "git log
 --progress" would inherently just be counting up, with no clue what
 the final number is.

> @@ -283,6 +285,12 @@ you can use `--suffix=-patch` to get 
> `0001-description-of-my-change-patch`.
>   range are always formatted as creation patches, independently
>   of this flag.
>  
> +--progress::
> + Progress status is reported on the standard error stream
> + by default when it is attached to a terminal, unless -q
> + is specified. This flag forces progress status even if the
> + standard error stream is not directed to a terminal.

Checking whether stderr is a tty would match our usual progress meters.
But I don't actually see any code in the patch implementing this.

As I said above, I think I'd prefer it to require "--progress", as
format-patch is quite often used as plumbing. But we'd need to fix the
documentation here to match.

> @@ -1739,8 +1744,12 @@ int cmd_format_patch(int argc, const char **argv, 
> const char *prefix)
>   start_number--;
>   }
>   rev.add_signoff = do_signoff;
> +
> + if (show_progress && !quiet)
> + progress = start_progress(_("Generating patch"), total);

The meter code here all looks correct, but let me bikeshed for a moment. :)

Should this use start_progress_delay()? In most cases the command will
complete very quickly, and the progress report is just noise. For many
commands (e.g., checkout) we wait 1-2 seconds before bothering to show
progress output.

I would have expected this to say "Generating patches"; most of our
other progress messages are pluralized. You could use Q_() to handle the
mono/plural case, but I think it's fine to just always say "patches"
(that's what other messages do).

One final thought is whether callers would want to customize this
message, since it will often be used as plumbing. E.g., would rebase
want to say something besides "Generating patches". I'm not sure.
Anyway, if you're interested in that direction, there's prior art in
the way rev-list handles "--progress" (and its sole caller,
check_connected()).

-Peff

Re: [PATCH v2] send-email: Net::SMTP::SSL is obsolete, use only when necessary

[Jonathan Nieder]

Hi,

Dennis Kaarsemaker wrote:

> Net::SMTP itself can do the necessary SSL and STARTTLS bits just fine
> since version 1.28, and Net::SMTP::SSL is now deprecated. Since 1.28
> isn't that old yet, keep the old code in place and use it when
> necessary.

This broke git send-email for me.  The error message is

  Can't locate object method "starttls" via package "Net::SMTP" at 
/usr/lib/git-core/git-send-email line 1410.

Is 1.28 the right minimum version?

  $ perl -e 'require Net::SMTP; print version->parse($Net::SMTP::VERSION); 
print "\n"'
  2.31
  $ grep VERSION /usr/share/perl/5.18.2/Net/SMTP.pm
  use vars qw($VERSION @ISA);
  $VERSION = "2.31";
  $ grep starttls /usr/share/perl/5.18.2/Net/SMTP.pm
  $ dpkg-query -W perl
  perl5.18.2-2ubuntu1.1

Patch left unsnipped for reference.

Thanks,
Jonathan

> While we're in the area, mark some messages for translation that were
> not yet marked as such.
>
> Signed-off-by: Dennis Kaarsemaker 
> ---
>  git-send-email.perl | 54 
> ++---
>  1 file changed, 35 insertions(+), 19 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index eea0a517f7..0d90439d9a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1353,10 +1353,12 @@ EOF
>   die __("The required SMTP server is not properly 
> defined.")
>   }
>  
> + require Net::SMTP;
> + my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < 
> version->parse("1.28");
> + $smtp_domain ||= maildomain();
> +
>   if ($smtp_encryption eq 'ssl') {
>   $smtp_server_port ||= 465; # ssmtp
> - require Net::SMTP::SSL;
> - $smtp_domain ||= maildomain();
>   require IO::Socket::SSL;
>  
>   # Suppress "variable accessed once" warning.
> @@ -1368,34 +1370,48 @@ EOF
>   # Net::SMTP::SSL->new() does not forward any SSL options
>   IO::Socket::SSL::set_client_defaults(
>   ssl_verify_params());
> - $smtp ||= Net::SMTP::SSL->new($smtp_server,
> -   Hello => $smtp_domain,
> -   Port => $smtp_server_port,
> -   Debug => $debug_net_smtp);
> +
> + if ($use_net_smtp_ssl) {
> + require Net::SMTP::SSL;
> + $smtp ||= Net::SMTP::SSL->new($smtp_server,
> +   Hello => 
> $smtp_domain,
> +   Port => 
> $smtp_server_port,
> +   Debug => 
> $debug_net_smtp);
> + }
> + else {
> + $smtp ||= Net::SMTP->new($smtp_server,
> +  Hello => $smtp_domain,
> +  Port => 
> $smtp_server_port,
> +  Debug => 
> $debug_net_smtp,
> +  SSL => 1);
> + }
>   }
>   else {
> - require Net::SMTP;
> - $smtp_domain ||= maildomain();
>   $smtp_server_port ||= 25;
>   $smtp ||= Net::SMTP->new($smtp_server,
>Hello => $smtp_domain,
>Debug => $debug_net_smtp,
>Port => $smtp_server_port);
>   if ($smtp_encryption eq 'tls' && $smtp) {
> - require Net::SMTP::SSL;
> - $smtp->command('STARTTLS');
> - $smtp->response();
> - if ($smtp->code == 220) {
> + if ($use_net_smtp_ssl) {
> + $smtp->command('STARTTLS');
> + $smtp->response();
> + if ($smtp->code != 220) {
> + die sprintf(__("Server does not 
> support STARTTLS! %s"), $smtp->message);
> + }
> + require Net::SMTP::SSL;
>   $smtp = Net::SMTP::SSL->start_SSL($smtp,
> 
> ssl_verify_params())
> - or die "STARTTLS failed! 
> ".IO::Socket::SSL::errstr();
> - $smtp_encryption = '';

[PATCH 09/31] setup: add comment indicating a hack

[Brandon Williams]

'GIT_TOPLEVEL_PREFIX_ENVIRONMENT' was added in (b58a68c1c setup: allow
for prefix to be passed to git commands) to aid in fixing a bug where
'ls-files' and 'grep' were not able to properly recurse when called from
within a subdirectory.  Add a 'NEEDSWORK' comment indicating that this
envvar should be removed once 'ls-files' and 'grep' can recurse
in-process.

Signed-off-by: Brandon Williams 
---
 setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/setup.c b/setup.c
index dba3abd00..c3d21345d 100644
--- a/setup.c
+++ b/setup.c
@@ -1057,6 +1057,12 @@ const char *setup_git_directory_gently(int *nongit_ok)
die("BUG: unhandled setup_git_directory_1() result");
}
 
+   /*
+* NEEDSWORK: This was a hack in order to get ls-files and grep to have
+* properly formated output when recursing submodules.  Once ls-files
+* and grep have been changed to perform this recursing in-process this
+* needs to be removed.
+*/
env_prefix = getenv(GIT_TOPLEVEL_PREFIX_ENVIRONMENT);
if (env_prefix)
prefix = env_prefix;
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 10/31] config: migrate the_configset to the_repository

[Brandon Williams]

Migrate the default config to be stored within 'the_repository'.

Signed-off-by: Brandon Williams 
---
 config.c | 185 ++-
 config.h |   1 +
 repo.c   |  21 
 repo.h   |  11 
 4 files changed, 146 insertions(+), 72 deletions(-)

diff --git a/config.c b/config.c
index ff09b27b8..930333e89 100644
--- a/config.c
+++ b/config.c
@@ -7,6 +7,7 @@
  */
 #include "cache.h"
 #include "config.h"
+#include "repo.h"
 #include "lockfile.h"
 #include "exec_cmd.h"
 #include "strbuf.h"
@@ -72,13 +73,6 @@ static int core_compression_seen;
 static int pack_compression_seen;
 static int zlib_compression_seen;
 
-/*
- * Default config_set that contains key-value pairs from the usual set of 
config
- * config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG
- * config file and the global /etc/gitconfig)
- */
-static struct config_set the_config_set;
-
 static int config_file_fgetc(struct config_source *conf)
 {
return getc_unlocked(conf->u.file);
@@ -1606,28 +1600,6 @@ int git_config_with_options(config_fn_t fn, void *data,
return do_git_config_sequence(opts, fn, data);
 }
 
-static void git_config_raw(config_fn_t fn, void *data)
-{
-   struct config_options opts = {0};
-
-   opts.respect_includes = 1;
-   if (have_git_dir())
-   opts.git_dir = get_git_common_dir();
-   if (git_config_with_options(fn, data, NULL, ) < 0)
-   /*
-* git_config_with_options() normally returns only
-* zero, as most errors are fatal, and
-* non-fatal potential errors are guarded by "if"
-* statements that are entered only when no error is
-* possible.
-*
-* If we ever encounter a non-fatal error, it means
-* something went really wrong and we should stop
-* immediately.
-*/
-   die(_("unknown error occurred while reading the configuration 
files"));
-}
-
 static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 {
int i, value_index;
@@ -1676,14 +1648,6 @@ void read_early_config(config_fn_t cb, void *data)
strbuf_release();
 }
 
-static void git_config_check_init(void);
-
-void git_config(config_fn_t fn, void *data)
-{
-   git_config_check_init();
-   configset_iter(_config_set, fn, data);
-}
-
 static struct config_set_element *configset_find_element(struct config_set 
*cs, const char *key)
 {
struct config_set_element k;
@@ -1782,7 +1746,7 @@ void git_configset_clear(struct config_set *cs)
cs->list.items = NULL;
 }
 
-static int config_set_callback(const char *key, const char *value, void *cb)
+int config_set_callback(const char *key, const char *value, void *cb)
 {
struct config_set *cs = cb;
configset_add_value(cs, key, value);
@@ -1893,87 +1857,164 @@ int git_configset_get_pathname(struct config_set *cs, 
const char *key, const cha
return 1;
 }
 
-static void git_config_check_init(void)
+/* Functions use to read configuration from a repository */
+static void git_config_check_init(struct repo *repository)
 {
-   if (the_config_set.hash_initialized)
+   if (repository->config && repository->config->hash_initialized)
return;
-   git_configset_init(_config_set);
-   git_config_raw(config_set_callback, _config_set);
+   repo_read_config(repository);
 }
 
-void git_config_clear(void)
+static void repo_config_clear(struct repo *repository)
 {
-   if (!the_config_set.hash_initialized)
+   if (!repository->config || !repository->config->hash_initialized)
return;
-   git_configset_clear(_config_set);
+   git_configset_clear(repository->config);
 }
 
-int git_config_get_value(const char *key, const char **value)
+static void repo_config(struct repo *repository, config_fn_t fn, void *data)
 {
-   git_config_check_init();
-   return git_configset_get_value(_config_set, key, value);
+   git_config_check_init(repository);
+   configset_iter(repository->config, fn, data);
 }
 
-const struct string_list *git_config_get_value_multi(const char *key)
+static int repo_config_get_value(struct repo *repository,
+const char *key, const char **value)
 {
-   git_config_check_init();
-   return git_configset_get_value_multi(_config_set, key);
+   git_config_check_init(repository);
+   return git_configset_get_value(repository->config, key, value);
 }
 
-int git_config_get_string_const(const char *key, const char **dest)
+static const struct string_list *repo_config_get_value_multi(struct repo 
*repository,
+const char *key)
+{
+   git_config_check_init(repository);
+   return git_configset_get_value_multi(repository->config, key);
+}
+
+static int 

[PATCH 04/31] setup: don't perform lazy initialization of repository state

[Brandon Williams]

Under some circumstances (bogus GIT_DIR value or the discovered gitdir
is '.git') 'setup_git_directory()' won't initialize key repository
state.  This leads to inconsistent state after running the setup code.
To account for this inconsistent state, lazy initialization is done once
a caller asks for the repository's gitdir or some other piece of
repository state.  This is confusing and can be error prone.

Instead let's tighten the expected outcome of 'setup_git_directory()'
and ensure that it initializes repository state in all cases that would
have been handled by lazy initialization.

This also lets us drop the requirement to have 'have_git_dir()' check if
the environment variable GIT_DIR was set as that will be handled by the
end of the setup code.

Signed-off-by: Brandon Williams 
---
 cache.h   |  2 ++
 environment.c | 17 -
 setup.c   | 14 ++
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 272f8e021..d41aab82f 100644
--- a/cache.h
+++ b/cache.h
@@ -462,6 +462,8 @@ static inline enum object_type object_type(unsigned int 
mode)
  */
 extern const char * const local_repo_env[];
 
+extern void setup_git_env(void);
+
 /*
  * Returns true iff we have a configured git repository (either via
  * setup_git_directory, or in the environment via $GIT_DIR).
diff --git a/environment.c b/environment.c
index d40b21fb7..a73b08f5d 100644
--- a/environment.c
+++ b/environment.c
@@ -160,7 +160,7 @@ static char *git_path_from_env(const char *envvar, const 
char *git_dir,
return xstrdup(value);
 }
 
-static void setup_git_env(void)
+void setup_git_env(void)
 {
struct strbuf sb = STRBUF_INIT;
const char *gitfile;
@@ -205,28 +205,27 @@ int is_bare_repository(void)
 int have_git_dir(void)
 {
return startup_info->have_repository
-   || git_dir
-   || getenv(GIT_DIR_ENVIRONMENT);
+   || git_dir;
 }
 
 const char *get_git_dir(void)
 {
if (!git_dir)
-   setup_git_env();
+   BUG("git environment hasn't been setup");
return git_dir;
 }
 
 const char *get_git_common_dir(void)
 {
if (!git_dir)
-   setup_git_env();
+   BUG("git environment hasn't been setup");
return git_common_dir;
 }
 
 const char *get_git_namespace(void)
 {
if (!namespace)
-   setup_git_env();
+   BUG("git environment hasn't been setup");
return namespace;
 }
 
@@ -276,7 +275,7 @@ const char *get_git_work_tree(void)
 char *get_object_directory(void)
 {
if (!git_object_dir)
-   setup_git_env();
+   BUG("git environment hasn't been setup");
return git_object_dir;
 }
 
@@ -316,14 +315,14 @@ int odb_pack_keep(const char *name)
 char *get_index_file(void)
 {
if (!git_index_file)
-   setup_git_env();
+   BUG("git environment hasn't been setup");
return git_index_file;
 }
 
 char *get_graft_file(void)
 {
if (!git_graft_file)
-   setup_git_env();
+   BUG("git environment hasn't been setup");
return git_graft_file;
 }
 
diff --git a/setup.c b/setup.c
index e99a82cbe..b2e05145c 100644
--- a/setup.c
+++ b/setup.c
@@ -1063,6 +1063,20 @@ const char *setup_git_directory_gently(int *nongit_ok)
startup_info->have_repository = !nongit_ok || !*nongit_ok;
startup_info->prefix = prefix;
 
+   /*
+* Not all paths through the setup code will call 'set_git_dir()' (which
+* directly sets up the environment) so in order to guarantee that the
+* environment is in a consistent state after setup, explicitly setup
+* the environment if we have a repository.
+*
+* NEEDSWORK: currently we allow bogus GIT_DIR values to be set in some
+* code paths so we also need to explicitly setup the environment if
+* the user has set GIT_DIR.  It may be beneficial to disallow bogus
+* GIT_DIR values at some point in the future.
+*/
+   if (startup_info->have_repository || getenv(GIT_DIR_ENVIRONMENT))
+   setup_git_env();
+
strbuf_release();
strbuf_release();
 
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 23/31] ls-files: convert show_killed_files to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index c37e9de11..f9578cf9f 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -121,7 +121,8 @@ static void show_other_files(struct dir_struct *dir)
}
 }
 
-static void show_killed_files(struct dir_struct *dir)
+static void show_killed_files(const struct index_state *istate,
+ const struct dir_struct *dir)
 {
int i;
for (i = 0; i < dir->nr; i++) {
@@ -135,29 +136,29 @@ static void show_killed_files(struct dir_struct *dir)
/* If ent->name is prefix of an entry in the
 * cache, it will be killed.
 */
-   pos = cache_name_pos(ent->name, ent->len);
+   pos = index_name_pos(istate, ent->name, 
ent->len);
if (0 <= pos)
die("BUG: killed-file %.*s not found",
ent->len, ent->name);
pos = -pos - 1;
-   while (pos < active_nr &&
-  ce_stage(active_cache[pos]))
+   while (pos < istate->cache_nr &&
+  ce_stage(istate->cache[pos]))
pos++; /* skip unmerged */
-   if (active_nr <= pos)
+   if (istate->cache_nr <= pos)
break;
/* pos points at a name immediately after
 * ent->name in the cache.  Does it expect
 * ent->name to be a directory?
 */
-   len = ce_namelen(active_cache[pos]);
+   len = ce_namelen(istate->cache[pos]);
if ((ent->len < len) &&
-   !strncmp(active_cache[pos]->name,
+   !strncmp(istate->cache[pos]->name,
 ent->name, ent->len) &&
-   active_cache[pos]->name[ent->len] == '/')
+   istate->cache[pos]->name[ent->len] == '/')
killed = 1;
break;
}
-   if (0 <= cache_name_pos(ent->name, sp - ent->name)) {
+   if (0 <= index_name_pos(istate, ent->name, sp - 
ent->name)) {
/* If any of the leading directories in
 * ent->name is registered in the cache,
 * ent->name will be killed.
@@ -338,7 +339,7 @@ static void show_files(struct dir_struct *dir)
if (show_others)
show_other_files(dir);
if (show_killed)
-   show_killed_files(dir);
+   show_killed_files(_index, dir);
}
if (show_cached || show_stage) {
for (i = 0; i < active_nr; i++) {
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 31/31] ls-files: use repository object

[Brandon Williams]

Convert ls-files to use a repository struct and recurse submodules
inprocess.

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 211 ++---
 git.c  |   2 +-
 repo.c |   3 +
 repo.h |   1 +
 t/t3007-ls-files-recurse-submodules.sh |  39 ++
 5 files changed, 135 insertions(+), 121 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 6a0302a28..46962815f 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -5,6 +5,7 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "config.h"
 #include "quote.h"
@@ -17,6 +18,7 @@
 #include "pathspec.h"
 #include "run-command.h"
 #include "submodule.h"
+#include "repo.h"
 
 static int abbrev;
 static int show_deleted;
@@ -32,10 +34,8 @@ static int line_terminator = '\n';
 static int debug_mode;
 static int show_eol;
 static int recurse_submodules;
-static struct argv_array submodule_options = ARGV_ARRAY_INIT;
 
 static const char *prefix;
-static const char *super_prefix;
 static int max_prefix_len;
 static int prefix_len;
 static struct pathspec pathspec;
@@ -73,25 +73,12 @@ static void write_eolinfo(const struct index_state *istate,
 
 static void write_name(const char *name)
 {
-   /*
-* Prepend the super_prefix to name to construct the full_name to be
-* written.
-*/
-   struct strbuf full_name = STRBUF_INIT;
-   if (super_prefix) {
-   strbuf_addstr(_name, super_prefix);
-   strbuf_addstr(_name, name);
-   name = full_name.buf;
-   }
-
/*
 * With "--full-name", prefix_len=0; this caller needs to pass
 * an empty string in that case (a NULL is good for "").
 */
write_name_quoted_relative(name, prefix_len ? prefix : NULL,
   stdout, line_terminator);
-
-   strbuf_release(_name);
 }
 
 static const char *get_tag(const struct cache_entry *ce, const char *tag)
@@ -210,101 +197,59 @@ static void show_killed_files(const struct index_state 
*istate,
}
 }
 
-/*
- * Compile an argv_array with all of the options supported by 
--recurse_submodules
- */
-static void compile_submodule_options(const char **argv,
- const struct dir_struct *dir,
- int show_tag)
-{
-   if (line_terminator == '\0')
-   argv_array_push(_options, "-z");
-   if (show_tag)
-   argv_array_push(_options, "-t");
-   if (show_valid_bit)
-   argv_array_push(_options, "-v");
-   if (show_cached)
-   argv_array_push(_options, "--cached");
-   if (show_eol)
-   argv_array_push(_options, "--eol");
-   if (debug_mode)
-   argv_array_push(_options, "--debug");
-
-   /* Add Pathspecs */
-   argv_array_push(_options, "--");
-   for (; *argv; argv++)
-   argv_array_push(_options, *argv);
-}
+static void show_files(struct repo *repo, struct dir_struct *dir);
 
-/**
- * Recursively call ls-files on a submodule
- */
-static void show_gitlink(const struct cache_entry *ce)
+static void show_submodule(const struct repo *superproject,
+  struct dir_struct *dir, const char *path)
 {
-   struct child_process cp = CHILD_PROCESS_INIT;
-   int status;
-   char *dir;
-
-   prepare_submodule_repo_env(_array);
-   argv_array_push(_array, GIT_DIR_ENVIRONMENT);
-
-   if (prefix_len)
-   argv_array_pushf(_array, "%s=%s",
-GIT_TOPLEVEL_PREFIX_ENVIRONMENT,
-prefix);
-   argv_array_pushf(, "--super-prefix=%s%s/",
-super_prefix ? super_prefix : "",
-ce->name);
-   argv_array_push(, "ls-files");
-   argv_array_push(, "--recurse-submodules");
-
-   /* add supported options */
-   argv_array_pushv(, submodule_options.argv);
-
-   cp.git_cmd = 1;
-   dir = mkpathdup("%s/%s", get_git_work_tree(), ce->name);
-   cp.dir = dir;
-   status = run_command();
-   free(dir);
-   if (status)
-   exit(status);
+   struct repo submodule;
+   char *gitdir = mkpathdup("%s/%s", superproject->worktree, path);
+   repo_init(, gitdir);
+
+   repo_read_index();
+   repo_read_gitmodules();
+
+   if (superproject->submodule_prefix)
+   submodule.submodule_prefix = xstrfmt("%s%s/", 
superproject->submodule_prefix, path);
+   else
+   submodule.submodule_prefix = xstrfmt("%s/", path);
+   show_files(, dir);
+
+   repo_clear();
+   free(gitdir);
 }
 
-static void show_ce_entry(const char *tag, const struct cache_entry *ce)
+static void show_ce(struct repo *repo, struct 

[PATCH 25/31] ls-files: convert show_ru_info to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index d00ca7810..2838e2f75 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -293,14 +293,14 @@ static void show_ce_entry(const char *tag, const struct 
cache_entry *ce)
strbuf_release();
 }
 
-static void show_ru_info(void)
+static void show_ru_info(const struct index_state *istate)
 {
struct string_list_item *item;
 
-   if (!the_index.resolve_undo)
+   if (!istate->resolve_undo)
return;
 
-   for_each_string_list_item(item, the_index.resolve_undo) {
+   for_each_string_list_item(item, istate->resolve_undo) {
const char *path = item->string;
struct resolve_undo_info *ui = item->util;
int i, len;
@@ -686,7 +686,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
}
show_files();
if (show_resolve_undo)
-   show_ru_info();
+   show_ru_info(_index);
 
if (ps_matched) {
int bad;
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 17/31] convert: convert convert_to_git_filter_fd to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 convert.c   | 5 +++--
 convert.h   | 3 ++-
 sha1_file.c | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/convert.c b/convert.c
index ff3e72657..824b606fa 100644
--- a/convert.c
+++ b/convert.c
@@ -1109,7 +1109,8 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
return ret | ident_to_git(path, src, len, dst, ca.ident);
 }
 
-void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
+void convert_to_git_filter_fd(const struct index_state *istate,
+ const char *path, int fd, struct strbuf *dst,
  enum safe_crlf checksafe)
 {
struct conv_attrs ca;
@@ -1121,7 +1122,7 @@ void convert_to_git_filter_fd(const char *path, int fd, 
struct strbuf *dst,
if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
die("%s: clean filter '%s' failed", path, ca.drv->name);
 
-   crlf_to_git(_index, path, dst->buf, dst->len, dst, ca.crlf_action, 
checksafe);
+   crlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, 
checksafe);
ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
 }
 
diff --git a/convert.h b/convert.h
index 667b7dfe0..3a813a797 100644
--- a/convert.h
+++ b/convert.h
@@ -52,7 +52,8 @@ static inline int would_convert_to_git(const char *path)
return convert_to_git(path, NULL, 0, NULL, 0);
 }
 /* Precondition: would_convert_to_git_filter_fd(path) == true */
-extern void convert_to_git_filter_fd(const char *path, int fd,
+extern void convert_to_git_filter_fd(const struct index_state *istate,
+const char *path, int fd,
 struct strbuf *dst,
 enum safe_crlf checksafe);
 extern int would_convert_to_git_filter_fd(const char *path);
diff --git a/sha1_file.c b/sha1_file.c
index 44561e0b9..80e9ef3bb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3581,7 +3581,7 @@ static int index_stream_convert_blob(unsigned char *sha1, 
int fd,
assert(path);
assert(would_convert_to_git_filter_fd(path));
 
-   convert_to_git_filter_fd(path, fd, ,
+   convert_to_git_filter_fd(_index, path, fd, ,
 write_object ? safe_crlf : SAFE_CRLF_FALSE);
 
if (write_object)
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 26/31] ls-files: convert ce_excluded to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 2838e2f75..289c6b2a5 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -322,10 +322,11 @@ static void show_ru_info(const struct index_state *istate)
}
 }
 
-static int ce_excluded(struct dir_struct *dir, const struct cache_entry *ce)
+static int ce_excluded(struct dir_struct *dir, struct index_state *istate,
+  const struct cache_entry *ce)
 {
int dtype = ce_to_dtype(ce);
-   return is_excluded(dir, _index, ce->name, );
+   return is_excluded(dir, istate, ce->name, );
 }
 
 static void show_files(struct dir_struct *dir)
@@ -346,7 +347,7 @@ static void show_files(struct dir_struct *dir)
for (i = 0; i < active_nr; i++) {
const struct cache_entry *ce = active_cache[i];
if ((dir->flags & DIR_SHOW_IGNORED) &&
-   !ce_excluded(dir, ce))
+   !ce_excluded(dir, _index, ce))
continue;
if (show_unmerged && !ce_stage(ce))
continue;
@@ -362,7 +363,7 @@ static void show_files(struct dir_struct *dir)
struct stat st;
int err;
if ((dir->flags & DIR_SHOW_IGNORED) &&
-   !ce_excluded(dir, ce))
+   !ce_excluded(dir, _index, ce))
continue;
if (ce->ce_flags & CE_UPDATE)
continue;
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 27/31] ls-files: convert prune_cache to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 289c6b2a5..e2d8fb7f6 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -381,30 +381,31 @@ static void show_files(struct dir_struct *dir)
 /*
  * Prune the index to only contain stuff starting with "prefix"
  */
-static void prune_cache(const char *prefix, size_t prefixlen)
+static void prune_index(struct index_state *istate,
+   const char *prefix, size_t prefixlen)
 {
int pos;
unsigned int first, last;
 
if (!prefix)
return;
-   pos = cache_name_pos(prefix, prefixlen);
+   pos = index_name_pos(istate, prefix, prefixlen);
if (pos < 0)
pos = -pos-1;
first = pos;
-   last = active_nr;
+   last = istate->cache_nr;
while (last > first) {
int next = (last + first) >> 1;
-   const struct cache_entry *ce = active_cache[next];
+   const struct cache_entry *ce = istate->cache[next];
if (!strncmp(ce->name, prefix, prefixlen)) {
first = next+1;
continue;
}
last = next;
}
-   memmove(active_cache, active_cache + pos,
+   memmove(istate->cache, istate->cache + pos,
(last - pos) * sizeof(struct cache_entry *));
-   active_nr = last - pos;
+   istate->cache_nr = last - pos;
 }
 
 static int get_common_prefix_len(const char *common_prefix)
@@ -662,7 +663,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
max_prefix = common_prefix();
max_prefix_len = get_common_prefix_len(max_prefix);
 
-   prune_cache(max_prefix, max_prefix_len);
+   prune_index(_index, max_prefix, max_prefix_len);
 
/* Treat unmatching pathspec elements as errors */
if (pathspec.nr && error_unmatch)
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 24/31] ls-files: convert show_other_files to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index f9578cf9f..d00ca7810 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -109,13 +109,14 @@ static void show_dir_entry(const char *tag, struct 
dir_entry *ent)
write_name(ent->name);
 }
 
-static void show_other_files(struct dir_struct *dir)
+static void show_other_files(const struct index_state *istate,
+const struct dir_struct *dir)
 {
int i;
 
for (i = 0; i < dir->nr; i++) {
struct dir_entry *ent = dir->entries[i];
-   if (!cache_name_is_other(ent->name, ent->len))
+   if (!index_name_is_other(istate, ent->name, ent->len))
continue;
show_dir_entry(tag_other, ent);
}
@@ -337,7 +338,7 @@ static void show_files(struct dir_struct *dir)
dir->flags |= DIR_COLLECT_KILLED_ONLY;
fill_directory(dir, _index, );
if (show_others)
-   show_other_files(dir);
+   show_other_files(_index, dir);
if (show_killed)
show_killed_files(_index, dir);
}
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 30/31] ls-files: factor out tag calculation

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 41 +
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 2849c9a65..6a0302a28 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -94,6 +94,30 @@ static void write_name(const char *name)
strbuf_release(_name);
 }
 
+static const char *get_tag(const struct cache_entry *ce, const char *tag)
+{
+   static char alttag[4];
+
+   if (tag && *tag && show_valid_bit && (ce->ce_flags & CE_VALID)) {
+   memcpy(alttag, tag, 3);
+
+   if (isalpha(tag[0])) {
+   alttag[0] = tolower(tag[0]);
+   } else if (tag[0] == '?') {
+   alttag[0] = '!';
+   } else {
+   alttag[0] = 'v';
+   alttag[1] = tag[0];
+   alttag[2] = ' ';
+   alttag[3] = 0;
+   }
+
+   tag = alttag;
+   }
+
+   return tag;
+}
+
 static void print_debug(const struct cache_entry *ce)
 {
if (debug_mode) {
@@ -264,22 +288,7 @@ static void show_ce_entry(const char *tag, const struct 
cache_entry *ce)
  len, ps_matched,
  S_ISDIR(ce->ce_mode) ||
  S_ISGITLINK(ce->ce_mode))) {
-   if (tag && *tag && show_valid_bit &&
-   (ce->ce_flags & CE_VALID)) {
-   static char alttag[4];
-   memcpy(alttag, tag, 3);
-   if (isalpha(tag[0]))
-   alttag[0] = tolower(tag[0]);
-   else if (tag[0] == '?')
-   alttag[0] = '!';
-   else {
-   alttag[0] = 'v';
-   alttag[1] = tag[0];
-   alttag[2] = ' ';
-   alttag[3] = 0;
-   }
-   tag = alttag;
-   }
+   tag = get_tag(ce, tag);
 
if (!show_stage) {
fputs(tag, stdout);
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 28/31] ls-files: convert show_files to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index e2d8fb7f6..3061af2c5 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -329,7 +329,7 @@ static int ce_excluded(struct dir_struct *dir, struct 
index_state *istate,
return is_excluded(dir, istate, ce->name, );
 }
 
-static void show_files(struct dir_struct *dir)
+static void show_files(struct index_state *istate, struct dir_struct *dir)
 {
int i;
 
@@ -337,17 +337,17 @@ static void show_files(struct dir_struct *dir)
if (show_others || show_killed) {
if (!show_others)
dir->flags |= DIR_COLLECT_KILLED_ONLY;
-   fill_directory(dir, _index, );
+   fill_directory(dir, istate, );
if (show_others)
-   show_other_files(_index, dir);
+   show_other_files(istate, dir);
if (show_killed)
-   show_killed_files(_index, dir);
+   show_killed_files(istate, dir);
}
if (show_cached || show_stage) {
-   for (i = 0; i < active_nr; i++) {
-   const struct cache_entry *ce = active_cache[i];
+   for (i = 0; i < istate->cache_nr; i++) {
+   const struct cache_entry *ce = istate->cache[i];
if ((dir->flags & DIR_SHOW_IGNORED) &&
-   !ce_excluded(dir, _index, ce))
+   !ce_excluded(dir, istate, ce))
continue;
if (show_unmerged && !ce_stage(ce))
continue;
@@ -358,12 +358,12 @@ static void show_files(struct dir_struct *dir)
}
}
if (show_deleted || show_modified) {
-   for (i = 0; i < active_nr; i++) {
-   const struct cache_entry *ce = active_cache[i];
+   for (i = 0; i < istate->cache_nr; i++) {
+   const struct cache_entry *ce = istate->cache[i];
struct stat st;
int err;
if ((dir->flags & DIR_SHOW_IGNORED) &&
-   !ce_excluded(dir, _index, ce))
+   !ce_excluded(dir, istate, ce))
continue;
if (ce->ce_flags & CE_UPDATE)
continue;
@@ -372,7 +372,7 @@ static void show_files(struct dir_struct *dir)
err = lstat(ce->name, );
if (show_deleted && err)
show_ce_entry(tag_removed, ce);
-   if (show_modified && ce_modified(ce, , 0))
+   if (show_modified && ie_modified(istate, ce, , 0))
show_ce_entry(tag_modified, ce);
}
}
@@ -686,7 +686,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
die("ls-files --with-tree is incompatible with -s or 
-u");
overlay_tree_on_index(_index, with_tree, max_prefix);
}
-   show_files();
+   show_files(_index, );
if (show_resolve_undo)
show_ru_info(_index);
 
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 29/31] ls-files: factor out debug info into a function

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 3061af2c5..2849c9a65 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -94,6 +94,19 @@ static void write_name(const char *name)
strbuf_release(_name);
 }
 
+static void print_debug(const struct cache_entry *ce)
+{
+   if (debug_mode) {
+   const struct stat_data *sd = >ce_stat_data;
+
+   printf("  ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
+   printf("  mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
+   printf("  dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
+   printf("  uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
+   printf("  size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
+   }
+}
+
 static void show_dir_entry(const char *tag, struct dir_entry *ent)
 {
int len = max_prefix_len;
@@ -279,15 +292,7 @@ static void show_ce_entry(const char *tag, const struct 
cache_entry *ce)
}
write_eolinfo(_index, ce, ce->name);
write_name(ce->name);
-   if (debug_mode) {
-   const struct stat_data *sd = >ce_stat_data;
-
-   printf("  ctime: %d:%d\n", sd->sd_ctime.sec, 
sd->sd_ctime.nsec);
-   printf("  mtime: %d:%d\n", sd->sd_mtime.sec, 
sd->sd_mtime.nsec);
-   printf("  dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
-   printf("  uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
-   printf("  size: %d\tflags: %x\n", sd->sd_size, 
ce->ce_flags);
-   }
+   print_debug(ce);
}
 
strbuf_release();
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 21/31] ls-files: convert overlay_tree_on_cache to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/commit.c   |  3 ++-
 builtin/ls-files.c | 15 ---
 cache.h|  3 ++-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 805da4915..3d98084fb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -254,7 +254,8 @@ static int list_paths(struct string_list *list, const char 
*with_tree,
 
if (with_tree) {
char *max_prefix = common_prefix(pattern);
-   overlay_tree_on_cache(with_tree, max_prefix ? max_prefix : 
prefix);
+   overlay_tree_on_index(_index, with_tree,
+ max_prefix ? max_prefix : prefix);
free(max_prefix);
}
 
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 620487a77..a5ceeb052 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -432,7 +432,8 @@ static int get_common_prefix_len(const char *common_prefix)
  * that were given from the command line.  We are not
  * going to write this index out.
  */
-void overlay_tree_on_cache(const char *tree_name, const char *prefix)
+void overlay_tree_on_index(struct index_state *istate,
+  const char *tree_name, const char *prefix)
 {
struct tree *tree;
struct object_id oid;
@@ -447,8 +448,8 @@ void overlay_tree_on_cache(const char *tree_name, const 
char *prefix)
die("bad tree-ish %s", tree_name);
 
/* Hoist the unmerged entries up to stage #3 to make room */
-   for (i = 0; i < active_nr; i++) {
-   struct cache_entry *ce = active_cache[i];
+   for (i = 0; i < istate->cache_nr; i++) {
+   struct cache_entry *ce = istate->cache[i];
if (!ce_stage(ce))
continue;
ce->ce_flags |= CE_STAGEMASK;
@@ -461,11 +462,11 @@ void overlay_tree_on_cache(const char *tree_name, const 
char *prefix)
   PATHSPEC_PREFER_CWD, prefix, matchbuf);
} else
memset(, 0, sizeof(pathspec));
-   if (read_tree(tree, 1, , _index))
+   if (read_tree(tree, 1, , istate))
die("unable to read tree entries %s", tree_name);
 
-   for (i = 0; i < active_nr; i++) {
-   struct cache_entry *ce = active_cache[i];
+   for (i = 0; i < istate->cache_nr; i++) {
+   struct cache_entry *ce = istate->cache[i];
switch (ce_stage(ce)) {
case 0:
last_stage0 = ce;
@@ -680,7 +681,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
 */
if (show_stage || show_unmerged)
die("ls-files --with-tree is incompatible with -s or 
-u");
-   overlay_tree_on_cache(with_tree, max_prefix);
+   overlay_tree_on_index(_index, with_tree, max_prefix);
}
show_files();
if (show_resolve_undo)
diff --git a/cache.h b/cache.h
index 02ab5f801..73724a3ad 100644
--- a/cache.h
+++ b/cache.h
@@ -1993,7 +1993,8 @@ extern int ws_blank_line(const char *line, int len, 
unsigned ws_rule);
 #define ws_tab_width(rule) ((rule) & WS_TAB_WIDTH_MASK)
 
 /* ls-files */
-void overlay_tree_on_cache(const char *tree_name, const char *prefix);
+void overlay_tree_on_index(struct index_state *istate,
+  const char *tree_name, const char *prefix);
 
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 20/31] tree: convert read_tree to take an index parameter

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c |  2 +-
 tree.c | 28 ++--
 tree.h |  3 ++-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index f16ce0053..620487a77 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -461,7 +461,7 @@ void overlay_tree_on_cache(const char *tree_name, const 
char *prefix)
   PATHSPEC_PREFER_CWD, prefix, matchbuf);
} else
memset(, 0, sizeof(pathspec));
-   if (read_tree(tree, 1, ))
+   if (read_tree(tree, 1, , _index))
die("unable to read tree entries %s", tree_name);
 
for (i = 0; i < active_nr; i++) {
diff --git a/tree.c b/tree.c
index 603b29ee8..dd69423d9 100644
--- a/tree.c
+++ b/tree.c
@@ -1,3 +1,4 @@
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "cache-tree.h"
 #include "tree.h"
@@ -8,7 +9,11 @@
 
 const char *tree_type = "tree";
 
-static int read_one_entry_opt(const unsigned char *sha1, const char *base, int 
baselen, const char *pathname, unsigned mode, int stage, int opt)
+static int read_one_entry_opt(struct index_state *istate,
+ const unsigned char *sha1,
+ const char *base, int baselen,
+ const char *pathname,
+ unsigned mode, int stage, int opt)
 {
int len;
unsigned int size;
@@ -27,14 +32,15 @@ static int read_one_entry_opt(const unsigned char *sha1, 
const char *base, int b
memcpy(ce->name, base, baselen);
memcpy(ce->name + baselen, pathname, len+1);
hashcpy(ce->oid.hash, sha1);
-   return add_cache_entry(ce, opt);
+   return add_index_entry(istate, ce, opt);
 }
 
 static int read_one_entry(const unsigned char *sha1, struct strbuf *base,
  const char *pathname, unsigned mode, int stage,
  void *context)
 {
-   return read_one_entry_opt(sha1, base->buf, base->len, pathname,
+   struct index_state *istate = context;
+   return read_one_entry_opt(istate, sha1, base->buf, base->len, pathname,
  mode, stage,
  ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);
 }
@@ -47,7 +53,8 @@ static int read_one_entry_quick(const unsigned char *sha1, 
struct strbuf *base,
const char *pathname, unsigned mode, int stage,
void *context)
 {
-   return read_one_entry_opt(sha1, base->buf, base->len, pathname,
+   struct index_state *istate = context;
+   return read_one_entry_opt(istate, sha1, base->buf, base->len, pathname,
  mode, stage,
  ADD_CACHE_JUST_APPEND);
 }
@@ -144,7 +151,8 @@ static int cmp_cache_name_compare(const void *a_, const 
void *b_)
  ce2->name, ce2->ce_namelen, ce_stage(ce2));
 }
 
-int read_tree(struct tree *tree, int stage, struct pathspec *match)
+int read_tree(struct tree *tree, int stage, struct pathspec *match,
+ struct index_state *istate)
 {
read_tree_fn_t fn = NULL;
int i, err;
@@ -164,23 +172,23 @@ int read_tree(struct tree *tree, int stage, struct 
pathspec *match)
 * do it the original slow way, otherwise, append and then
 * sort at the end.
 */
-   for (i = 0; !fn && i < active_nr; i++) {
-   const struct cache_entry *ce = active_cache[i];
+   for (i = 0; !fn && i < istate->cache_nr; i++) {
+   const struct cache_entry *ce = istate->cache[i];
if (ce_stage(ce) == stage)
fn = read_one_entry;
}
 
if (!fn)
fn = read_one_entry_quick;
-   err = read_tree_recursive(tree, "", 0, stage, match, fn, NULL);
+   err = read_tree_recursive(tree, "", 0, stage, match, fn, istate);
if (fn == read_one_entry || err)
return err;
 
/*
 * Sort the cache entry -- we need to nuke the cache tree, though.
 */
-   cache_tree_free(_cache_tree);
-   QSORT(active_cache, active_nr, cmp_cache_name_compare);
+   cache_tree_free(>cache_tree);
+   QSORT(istate->cache, istate->cache_nr, cmp_cache_name_compare);
return 0;
 }
 
diff --git a/tree.h b/tree.h
index 0d4734b94..744e6dc2a 100644
--- a/tree.h
+++ b/tree.h
@@ -34,6 +34,7 @@ extern int read_tree_recursive(struct tree *tree,
   int stage, const struct pathspec *pathspec,
   read_tree_fn_t fn, void *context);
 
-extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec);
+extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec,
+struct index_state *istate);
 
 #endif /* TREE_H */
-- 

[PATCH 22/31] ls-files: convert write_eolinfo to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a5ceeb052..c37e9de11 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -54,17 +54,16 @@ static const char *tag_modified = "";
 static const char *tag_skip_worktree = "";
 static const char *tag_resolve_undo = "";
 
-static void write_eolinfo(const struct cache_entry *ce, const char *path)
+static void write_eolinfo(const struct index_state *istate,
+ const struct cache_entry *ce, const char *path)
 {
-   if (!show_eol)
-   return;
-   else {
+   if (show_eol) {
struct stat st;
const char *i_txt = "";
const char *w_txt = "";
const char *a_txt = get_convert_attr_ascii(path);
if (ce && S_ISREG(ce->ce_mode))
-   i_txt = get_cached_convert_stats_ascii(_index,
+   i_txt = get_cached_convert_stats_ascii(istate,
   ce->name);
if (!lstat(path, ) && S_ISREG(st.st_mode))
w_txt = get_wt_convert_stats_ascii(path);
@@ -106,7 +105,7 @@ static void show_dir_entry(const char *tag, struct 
dir_entry *ent)
return;
 
fputs(tag, stdout);
-   write_eolinfo(NULL, ent->name);
+   write_eolinfo(NULL, NULL, ent->name);
write_name(ent->name);
 }
 
@@ -276,7 +275,7 @@ static void show_ce_entry(const char *tag, const struct 
cache_entry *ce)
   find_unique_abbrev(ce->oid.hash, abbrev),
   ce_stage(ce));
}
-   write_eolinfo(ce, ce->name);
+   write_eolinfo(_index, ce, ce->name);
write_name(ce->name);
if (debug_mode) {
const struct stat_data *sd = >ce_stat_data;
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 19/31] convert: convert renormalize_buffer to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 convert.c | 6 --
 convert.h | 3 ++-
 ll-merge.c| 2 +-
 merge-recursive.c | 4 ++--
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index 5af6fdf3f..7d2a519da 100644
--- a/convert.c
+++ b/convert.c
@@ -1,3 +1,4 @@
+#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "config.h"
 #include "attr.h"
@@ -1166,14 +1167,15 @@ int convert_to_working_tree(const char *path, const 
char *src, size_t len, struc
return convert_to_working_tree_internal(path, src, len, dst, 0);
 }
 
-int renormalize_buffer(const char *path, const char *src, size_t len, struct 
strbuf *dst)
+int renormalize_buffer(const struct index_state *istate, const char *path,
+  const char *src, size_t len, struct strbuf *dst)
 {
int ret = convert_to_working_tree_internal(path, src, len, dst, 1);
if (ret) {
src = dst->buf;
len = dst->len;
}
-   return ret | convert_to_git(_index, path, src, len, dst, 
SAFE_CRLF_RENORMALIZE);
+   return ret | convert_to_git(istate, path, src, len, dst, 
SAFE_CRLF_RENORMALIZE);
 }
 
 /*
diff --git a/convert.h b/convert.h
index 60cb41d6a..cecf59d1a 100644
--- a/convert.h
+++ b/convert.h
@@ -46,7 +46,8 @@ extern int convert_to_git(const struct index_state *istate,
  struct strbuf *dst, enum safe_crlf checksafe);
 extern int convert_to_working_tree(const char *path, const char *src,
   size_t len, struct strbuf *dst);
-extern int renormalize_buffer(const char *path, const char *src, size_t len,
+extern int renormalize_buffer(const struct index_state *istate,
+ const char *path, const char *src, size_t len,
  struct strbuf *dst);
 static inline int would_convert_to_git(const struct index_state *istate,
   const char *path)
diff --git a/ll-merge.c b/ll-merge.c
index 24ff94e1d..b9576efe9 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -340,7 +340,7 @@ static const struct ll_merge_driver 
*find_ll_merge_driver(const char *merge_attr
 static void normalize_file(mmfile_t *mm, const char *path)
 {
struct strbuf strbuf = STRBUF_INIT;
-   if (renormalize_buffer(path, mm->ptr, mm->size, )) {
+   if (renormalize_buffer(_index, path, mm->ptr, mm->size, )) {
free(mm->ptr);
mm->size = strbuf.len;
mm->ptr = strbuf_detach(, NULL);
diff --git a/merge-recursive.c b/merge-recursive.c
index c2494f34f..a4b3858d0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1640,8 +1640,8 @@ static int blob_unchanged(struct merge_options *opt,
 * performed.  Comparison can be skipped if both files are
 * unchanged since their sha1s have already been compared.
 */
-   if (renormalize_buffer(path, o.buf, o.len, ) |
-   renormalize_buffer(path, a.buf, a.len, ))
+   if (renormalize_buffer(_index, path, o.buf, o.len, ) |
+   renormalize_buffer(_index, path, a.buf, a.len, ))
ret = (o.len == a.len && !memcmp(o.buf, a.buf, o.len));
 
 error_return:
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 18/31] convert: convert convert_to_git to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 apply.c | 2 +-
 builtin/blame.c | 2 +-
 combine-diff.c  | 2 +-
 convert.c   | 7 ---
 convert.h   | 8 +---
 diff.c  | 6 +++---
 dir.c   | 2 +-
 sha1_file.c | 4 ++--
 8 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/apply.c b/apply.c
index 87db9618d..8eca54325 100644
--- a/apply.c
+++ b/apply.c
@@ -2268,7 +2268,7 @@ static int read_old_data(struct stat *st, const char 
*path, struct strbuf *buf)
case S_IFREG:
if (strbuf_read_file(buf, path, st->st_size) != st->st_size)
return error(_("unable to open or read %s"), path);
-   convert_to_git(path, buf->buf, buf->len, buf, 0);
+   convert_to_git(_index, path, buf->buf, buf->len, buf, 0);
return 0;
default:
return -1;
diff --git a/builtin/blame.c b/builtin/blame.c
index c0ae49298..317d2ec37 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2384,7 +2384,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
if (strbuf_read(, 0, 0) < 0)
die_errno("failed to read from stdin");
}
-   convert_to_git(path, buf.buf, buf.len, , 0);
+   convert_to_git(_index, path, buf.buf, buf.len, , 0);
origin->file.ptr = buf.buf;
origin->file.size = buf.len;
pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
diff --git a/combine-diff.c b/combine-diff.c
index 2848034fe..74f723af3 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1053,7 +1053,7 @@ static void show_patch_diff(struct combine_diff_path 
*elem, int num_parent,
if (is_file) {
struct strbuf buf = STRBUF_INIT;
 
-   if (convert_to_git(elem->path, result, len, 
, safe_crlf)) {
+   if (convert_to_git(_index, elem->path, 
result, len, , safe_crlf)) {
free(result);
result = strbuf_detach(, );
result_size = len;
diff --git a/convert.c b/convert.c
index 824b606fa..5af6fdf3f 100644
--- a/convert.c
+++ b/convert.c
@@ -1085,7 +1085,8 @@ const char *get_convert_attr_ascii(const char *path)
return "";
 }
 
-int convert_to_git(const char *path, const char *src, size_t len,
+int convert_to_git(const struct index_state *istate,
+  const char *path, const char *src, size_t len,
struct strbuf *dst, enum safe_crlf checksafe)
 {
int ret = 0;
@@ -1101,7 +1102,7 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
src = dst->buf;
len = dst->len;
}
-   ret |= crlf_to_git(_index, path, src, len, dst, ca.crlf_action, 
checksafe);
+   ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, 
checksafe);
if (ret && dst) {
src = dst->buf;
len = dst->len;
@@ -1172,7 +1173,7 @@ int renormalize_buffer(const char *path, const char *src, 
size_t len, struct str
src = dst->buf;
len = dst->len;
}
-   return ret | convert_to_git(path, src, len, dst, SAFE_CRLF_RENORMALIZE);
+   return ret | convert_to_git(_index, path, src, len, dst, 
SAFE_CRLF_RENORMALIZE);
 }
 
 /*
diff --git a/convert.h b/convert.h
index 3a813a797..60cb41d6a 100644
--- a/convert.h
+++ b/convert.h
@@ -41,15 +41,17 @@ extern const char *get_wt_convert_stats_ascii(const char 
*path);
 extern const char *get_convert_attr_ascii(const char *path);
 
 /* returns 1 if *dst was used */
-extern int convert_to_git(const char *path, const char *src, size_t len,
+extern int convert_to_git(const struct index_state *istate,
+ const char *path, const char *src, size_t len,
  struct strbuf *dst, enum safe_crlf checksafe);
 extern int convert_to_working_tree(const char *path, const char *src,
   size_t len, struct strbuf *dst);
 extern int renormalize_buffer(const char *path, const char *src, size_t len,
  struct strbuf *dst);
-static inline int would_convert_to_git(const char *path)
+static inline int would_convert_to_git(const struct index_state *istate,
+  const char *path)
 {
-   return convert_to_git(path, NULL, 0, NULL, 0);
+   return convert_to_git(istate, path, NULL, 0, NULL, 0);
 }
 /* Precondition: would_convert_to_git_filter_fd(path) == true */
 extern void convert_to_git_filter_fd(const struct index_state *istate,
diff --git a/diff.c b/diff.c
index 2f2467c6d..87ed6d6d3 100644
--- a/diff.c
+++ b/diff.c
@@ -2756,7 +2756,7 @@ static int reuse_worktree_file(const char *name, const 
unsigned char 

[PATCH 16/31] convert: convert crlf_to_git to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 convert.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index 574003023..ff3e72657 100644
--- a/convert.c
+++ b/convert.c
@@ -219,13 +219,13 @@ static void check_safe_crlf(const char *path, enum 
crlf_action crlf_action,
}
 }
 
-static int has_cr_in_index(const char *path)
+static int has_cr_in_index(const struct index_state *istate, const char *path)
 {
unsigned long sz;
void *data;
int has_cr;
 
-   data = read_blob_data_from_cache(path, );
+   data = read_blob_data_from_index(istate, path, );
if (!data)
return 0;
has_cr = memchr(data, '\r', sz) != NULL;
@@ -255,7 +255,8 @@ static int will_convert_lf_to_crlf(size_t len, struct 
text_stat *stats,
 
 }
 
-static int crlf_to_git(const char *path, const char *src, size_t len,
+static int crlf_to_git(const struct index_state *istate,
+  const char *path, const char *src, size_t len,
   struct strbuf *buf,
   enum crlf_action crlf_action, enum safe_crlf checksafe)
 {
@@ -287,7 +288,8 @@ static int crlf_to_git(const char *path, const char *src, 
size_t len,
 * unless we want to renormalize in a merge or
 * cherry-pick.
 */
-   if ((checksafe != SAFE_CRLF_RENORMALIZE) && 
has_cr_in_index(path))
+   if ((checksafe != SAFE_CRLF_RENORMALIZE) &&
+   has_cr_in_index(istate, path))
convert_crlf_into_lf = 0;
}
if ((checksafe == SAFE_CRLF_WARN ||
@@ -1099,7 +1101,7 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
src = dst->buf;
len = dst->len;
}
-   ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
+   ret |= crlf_to_git(_index, path, src, len, dst, ca.crlf_action, 
checksafe);
if (ret && dst) {
src = dst->buf;
len = dst->len;
@@ -1119,7 +1121,7 @@ void convert_to_git_filter_fd(const char *path, int fd, 
struct strbuf *dst,
if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
die("%s: clean filter '%s' failed", path, ca.drv->name);
 
-   crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
+   crlf_to_git(_index, path, dst->buf, dst->len, dst, ca.crlf_action, 
checksafe);
ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
 }
 
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 12/31] submodule-config: store the_submodule_cache in the_repository

[Brandon Williams]

Refactor how 'the_submodule_cache' is handled so that it can be stored
inside of a repository object.  Also migrate 'the_submodule_cache' to be
stored in 'the_repository'.

Signed-off-by: Brandon Williams 
---
 repo.c |  6 +
 repo.h |  2 ++
 submodule-config.c | 71 --
 submodule-config.h | 10 
 4 files changed, 71 insertions(+), 18 deletions(-)

diff --git a/repo.c b/repo.c
index c79d29534..13b7d244f 100644
--- a/repo.c
+++ b/repo.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "repo.h"
 #include "config.h"
+#include "submodule-config.h"
 
 /*
  * This may be the wrong place for this.
@@ -166,4 +167,9 @@ void repo_clear(struct repo *repo)
free(repo->index);
repo->index = NULL;
}
+
+   if (repo->submodule_cache) {
+   submodule_cache_free(repo->submodule_cache);
+   repo->submodule_cache = NULL;
+   }
 }
diff --git a/repo.h b/repo.h
index 756cda9e1..ebce2a408 100644
--- a/repo.h
+++ b/repo.h
@@ -3,6 +3,7 @@
 
 struct config_set;
 struct index_state;
+struct submodule_cache;
 
 struct repo {
/* Environment */
@@ -22,6 +23,7 @@ struct repo {
 */
struct config_set *config;
struct index_state *index;
+   struct submodule_cache *submodule_cache;
 
/* Configurations */
unsigned ignore_env:1;
diff --git a/submodule-config.c b/submodule-config.c
index d8f8d5ea3..cd356fec0 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "repo.h"
 #include "config.h"
 #include "submodule-config.h"
 #include "submodule.h"
@@ -15,6 +16,7 @@
 struct submodule_cache {
struct hashmap for_path;
struct hashmap for_name;
+   unsigned initialized:1;
 };
 
 /*
@@ -31,9 +33,6 @@ enum lookup_type {
lookup_path
 };
 
-static struct submodule_cache the_submodule_cache;
-static int is_cache_init;
-
 static int config_path_cmp(const struct submodule_entry *a,
   const struct submodule_entry *b,
   const void *unused)
@@ -50,10 +49,16 @@ static int config_name_cmp(const struct submodule_entry *a,
   hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);
 }
 
-static void cache_init(struct submodule_cache *cache)
+static struct submodule_cache *submodule_cache_alloc(void)
+{
+   return xcalloc(1, sizeof(struct submodule_cache));
+}
+
+static void submodule_cache_init(struct submodule_cache *cache)
 {
hashmap_init(>for_path, (hashmap_cmp_fn) config_path_cmp, 0);
hashmap_init(>for_name, (hashmap_cmp_fn) config_name_cmp, 0);
+   cache->initialized = 1;
 }
 
 static void free_one_config(struct submodule_entry *entry)
@@ -65,11 +70,14 @@ static void free_one_config(struct submodule_entry *entry)
free(entry->config);
 }
 
-static void cache_free(struct submodule_cache *cache)
+static void submodule_cache_clear(struct submodule_cache *cache)
 {
struct hashmap_iter iter;
struct submodule_entry *entry;
 
+   if (!cache->initialized)
+   return;
+
/*
 * We iterate over the name hash here to be symmetric with the
 * allocation of struct submodule entries. Each is allocated by
@@ -81,6 +89,13 @@ static void cache_free(struct submodule_cache *cache)
 
hashmap_free(>for_path, 1);
hashmap_free(>for_name, 1);
+   cache->initialized = 0;
+}
+
+void submodule_cache_free(struct submodule_cache *cache)
+{
+   submodule_cache_clear(cache);
+   free(cache);
 }
 
 static unsigned int hash_sha1_string(const unsigned char *sha1,
@@ -494,43 +509,63 @@ static const struct submodule *config_from(struct 
submodule_cache *cache,
return submodule;
 }
 
-static void ensure_cache_init(void)
+static void submodule_cache_check_init(struct repo *repository)
 {
-   if (is_cache_init)
+   if (repository->submodule_cache && 
repository->submodule_cache->initialized)
return;
 
-   cache_init(_submodule_cache);
-   is_cache_init = 1;
+   if (!repository->submodule_cache)
+   repository->submodule_cache = submodule_cache_alloc();
+
+   submodule_cache_init(repository->submodule_cache);
 }
 
-int parse_submodule_config_option(const char *var, const char *value)
+int submodule_config_option(struct repo *repository,
+   const char *var, const char *value)
 {
struct parse_config_parameter parameter;
-   parameter.cache = _submodule_cache;
+
+   submodule_cache_check_init(repository);
+
+   parameter.cache = repository->submodule_cache;
parameter.treeish_name = NULL;
parameter.gitmodules_sha1 = null_sha1;
parameter.overwrite = 1;
 
-   ensure_cache_init();
return parse_config(var, value, );
 }
 
+int parse_submodule_config_option(const char *var, const char *value)
+{

[PATCH 08/31] environment: store worktree in the_repository

[Brandon Williams]

Migrate 'work_tree' to be stored in 'the_repository'.

Signed-off-by: Brandon Williams 
---
 environment.c | 9 -
 repo.c| 7 +++
 repo.h| 2 ++
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/environment.c b/environment.c
index a0519f4f3..2722ebb9f 100644
--- a/environment.c
+++ b/environment.c
@@ -96,7 +96,6 @@ int ignore_untracked_cache_config;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
-static char *work_tree;
 
 static const char *super_prefix;
 
@@ -220,19 +219,19 @@ void set_git_work_tree(const char *new_work_tree)
 {
if (git_work_tree_initialized) {
new_work_tree = real_path(new_work_tree);
-   if (strcmp(new_work_tree, work_tree))
+   if (strcmp(new_work_tree, the_repository.worktree))
die("internal error: work tree has already been set\n"
"Current worktree: %s\nNew worktree: %s",
-   work_tree, new_work_tree);
+   the_repository.worktree, new_work_tree);
return;
}
git_work_tree_initialized = 1;
-   work_tree = real_pathdup(new_work_tree, 1);
+   repo_set_worktree(_repository, new_work_tree);
 }
 
 const char *get_git_work_tree(void)
 {
-   return work_tree;
+   return the_repository.worktree;
 }
 
 char *get_object_directory(void)
diff --git a/repo.c b/repo.c
index f7c3617a9..789ffdd78 100644
--- a/repo.c
+++ b/repo.c
@@ -84,6 +84,11 @@ void repo_set_gitdir(struct repo *repo, const char *path)
repo_setup_env(repo);
 }
 
+void repo_set_worktree(struct repo *repo, const char *path)
+{
+   repo->worktree = real_pathdup(path, 1);
+}
+
 int repo_init(struct repo *repo, const char *gitdir)
 {
int error = 0;
@@ -121,4 +126,6 @@ int repo_init(struct repo *repo, const char *gitdir)
 void repo_clear(struct repo *repo)
 {
repo_clear_env(repo);
+   free(repo->worktree);
+   repo->worktree = NULL;
 }
diff --git a/repo.h b/repo.h
index 453049e67..95d021049 100644
--- a/repo.h
+++ b/repo.h
@@ -9,6 +9,7 @@ struct repo {
char *index_file;
char *graft_file;
char *namespace;
+   char *worktree;
 
/* Configurations */
unsigned ignore_env:1;
@@ -19,6 +20,7 @@ struct repo {
 extern struct repo the_repository;
 
 extern void repo_set_gitdir(struct repo *repo, const char *path);
+extern void repo_set_worktree(struct repo *repo, const char *path);
 extern int repo_init(struct repo *repo, const char *path);
 extern void repo_clear(struct repo *repo);
 
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 07/31] environment: place key repository state in the_repository

[Brandon Williams]

Migrate 'git_dir', 'git_common_dir', 'git_object_dir', 'git_index_file',
'git_graft_file', and 'namespace' to be stored in 'the_repository'.

Signed-off-by: Brandon Williams 
---
 cache.h   |  1 -
 environment.c | 65 ++-
 path.c| 11 +-
 setup.c   | 17 ++--
 4 files changed, 36 insertions(+), 58 deletions(-)

diff --git a/cache.h b/cache.h
index c0b4c8d83..02ab5f801 100644
--- a/cache.h
+++ b/cache.h
@@ -769,7 +769,6 @@ extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
-extern int git_db_env, git_index_env, git_graft_env, git_common_dir_env;
 
 /*
  * Include broken refs in all ref iterations, which will
diff --git a/environment.c b/environment.c
index 77900b31a..a0519f4f3 100644
--- a/environment.c
+++ b/environment.c
@@ -8,6 +8,7 @@
  * are.
  */
 #include "cache.h"
+#include "repo.h"
 #include "config.h"
 #include "refs.h"
 #include "fmt-merge-msg.h"
@@ -97,14 +98,8 @@ int ignore_untracked_cache_config;
 char *git_work_tree_cfg;
 static char *work_tree;
 
-static const char *namespace;
-
 static const char *super_prefix;
 
-static const char *git_dir, *git_common_dir;
-static char *git_object_dir, *git_index_file, *git_graft_file;
-int git_db_env, git_index_env, git_graft_env, git_common_dir_env;
-
 /*
  * Repository-local GIT_* environment variables; see cache.h for details.
  */
@@ -148,47 +143,16 @@ char *expand_namespace(const char *raw_namespace)
return strbuf_detach(, NULL);
 }
 
-static char *git_path_from_env(const char *envvar, const char *git_dir,
-  const char *path, int *fromenv)
-{
-   const char *value = getenv(envvar);
-   if (!value)
-   return xstrfmt("%s/%s", git_dir, path);
-   if (fromenv)
-   *fromenv = 1;
-   return xstrdup(value);
-}
-
 void setup_git_env(void)
 {
-   struct strbuf sb = STRBUF_INIT;
-   const char *gitfile;
const char *shallow_file;
const char *replace_ref_base;
 
-   git_dir = getenv(GIT_DIR_ENVIRONMENT);
-   if (!git_dir) {
-   if (!startup_info->have_repository)
-   BUG("setup_git_env called without repository");
-   git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
-   }
-   gitfile = read_gitfile(git_dir);
-   git_dir = xstrdup(gitfile ? gitfile : git_dir);
-   if (get_common_dir(, git_dir))
-   git_common_dir_env = 1;
-   git_common_dir = strbuf_detach(, NULL);
-   git_object_dir = git_path_from_env(DB_ENVIRONMENT, git_common_dir,
-  "objects", _db_env);
-   git_index_file = git_path_from_env(INDEX_ENVIRONMENT, git_dir,
-  "index", _index_env);
-   git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, git_common_dir,
-  "info/grafts", _graft_env);
if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
check_replace_refs = 0;
replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
  : "refs/replace/");
-   namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
if (shallow_file)
set_alternate_shallow_file(shallow_file, 0);
@@ -203,28 +167,28 @@ int is_bare_repository(void)
 int have_git_dir(void)
 {
return startup_info->have_repository
-   || git_dir;
+   || the_repository.gitdir;
 }
 
 const char *get_git_dir(void)
 {
-   if (!git_dir)
+   if (!the_repository.gitdir)
BUG("git environment hasn't been setup");
-   return git_dir;
+   return the_repository.gitdir;
 }
 
 const char *get_git_common_dir(void)
 {
-   if (!git_dir)
+   if (!the_repository.commondir)
BUG("git environment hasn't been setup");
-   return git_common_dir;
+   return the_repository.commondir;
 }
 
 const char *get_git_namespace(void)
 {
-   if (!namespace)
+   if (!the_repository.namespace)
BUG("git environment hasn't been setup");
-   return namespace;
+   return the_repository.namespace;
 }
 
 const char *strip_namespace(const char *namespaced_ref)
@@ -273,9 +237,9 @@ const char *get_git_work_tree(void)
 
 char *get_object_directory(void)
 {
-   if (!git_object_dir)
+   if (!the_repository.objectdir)
BUG("git environment hasn't been setup");
-   return git_object_dir;
+   return the_repository.objectdir;
 }
 
 int odb_mkstemp(struct strbuf *template, const char *pattern)
@@ -313,22 +277,23 @@ int odb_pack_keep(const char *name)
 
 char *get_index_file(void)
 {
-   if (!git_index_file)
+   if 

[PATCH 13/31] repo: add repo_read_gitmodules

[Brandon Williams]

Teach the repo object to be able to populate the submodule_cache by
reading the repository's gitmodules file.

Signed-off-by: Brandon Williams 
---
 repo.c | 14 ++
 repo.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/repo.c b/repo.c
index 13b7d244f..6f7b2015f 100644
--- a/repo.c
+++ b/repo.c
@@ -116,6 +116,20 @@ void repo_read_index(struct repo *repo)
die(_("failure reading index"));
 }
 
+static int gitmodules_cb(const char *var, const char *value, void *data)
+{
+   struct repo *repo = data;
+   return submodule_config_option(repo, var, value);
+}
+
+void repo_read_gitmodules(struct repo *repo)
+{
+   char *gitmodules_path = xstrfmt("%s/.gitmodules", repo->worktree);
+
+   git_config_from_file(gitmodules_cb, gitmodules_path, repo);
+   free(gitmodules_path);
+}
+
 int repo_init(struct repo *repo, const char *gitdir)
 {
int error = 0;
diff --git a/repo.h b/repo.h
index ebce2a408..ad0184eaa 100644
--- a/repo.h
+++ b/repo.h
@@ -37,6 +37,7 @@ extern void repo_set_gitdir(struct repo *repo, const char 
*path);
 extern void repo_set_worktree(struct repo *repo, const char *path);
 extern void repo_read_config(struct repo *repo);
 extern void repo_read_index(struct repo *repo);
+extern void repo_read_gitmodules(struct repo *repo);
 extern int repo_init(struct repo *repo, const char *path);
 extern void repo_clear(struct repo *repo);
 
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 03/31] config: don't implicitly use gitdir

[Brandon Williams]

Commit 2185fde56 (config: handle conditional include when $GIT_DIR is
not set up) added a 'git_dir' field to the config_options struct.  Let's
use this option field explicitly all the time instead of occasionally
falling back to calling 'git_pathdup("config")' to get the path to the
local repository configuration.  This allows 'go_git_config_sequence()'
to not implicitly rely on global repository state.

Signed-off-by: Brandon Williams 
---
 builtin/config.c | 2 ++
 config.c | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 753c40a5c..90f49a6ee 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -539,6 +539,8 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
config_options.respect_includes = !given_config_source.file;
else
config_options.respect_includes = respect_includes_opt;
+   if (have_git_dir())
+   config_options.git_dir = get_git_common_dir();
 
if (end_null) {
term = '\0';
diff --git a/config.c b/config.c
index 2390f98e3..ff09b27b8 100644
--- a/config.c
+++ b/config.c
@@ -1548,8 +1548,6 @@ static int do_git_config_sequence(const struct 
config_options *opts,
 
if (opts->git_dir)
repo_config = mkpathdup("%s/config", opts->git_dir);
-   else if (have_git_dir())
-   repo_config = git_pathdup("config");
else
repo_config = NULL;
 
@@ -1613,6 +1611,8 @@ static void git_config_raw(config_fn_t fn, void *data)
struct config_options opts = {0};
 
opts.respect_includes = 1;
+   if (have_git_dir())
+   opts.git_dir = get_git_common_dir();
if (git_config_with_options(fn, data, NULL, ) < 0)
/*
 * git_config_with_options() normally returns only
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 14/31] submodule: convert is_submodule_initialized to work on a repository

[Brandon Williams]

Convert 'is_submodule_initialized()' to take a repository object and
while we're at it, lets rename the function to 'is_submodule_active()'
and remove the NEEDSWORK comment.

Signed-off-by: Brandon Williams 
---
 builtin/grep.c  |  3 ++-
 builtin/submodule--helper.c |  9 +
 config.c| 12 ++--
 config.h|  9 +
 submodule.c | 21 +
 submodule.h |  3 ++-
 6 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 0f4a1e5a3..c3086bc0b 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2006 Junio C Hamano
  */
 #include "cache.h"
+#include "repo.h"
 #include "config.h"
 #include "blob.h"
 #include "tree.h"
@@ -626,7 +627,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
 static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
  const char *filename, const char *path)
 {
-   if (!is_submodule_initialized(path))
+   if (!is_submodule_active(_repository, path))
return 0;
if (!is_submodule_populated_gently(path, NULL)) {
/*
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4dcbfb952..d2d6fa914 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1,4 +1,5 @@
 #include "builtin.h"
+#include "repo.h"
 #include "cache.h"
 #include "config.h"
 #include "parse-options.h"
@@ -280,7 +281,7 @@ static void module_list_active(struct module_list *list)
for (i = 0; i < list->nr; i++) {
const struct cache_entry *ce = list->entries[i];
 
-   if (!is_submodule_initialized(ce->name))
+   if (!is_submodule_active(_repository, ce->name))
continue;
 
ALLOC_GROW(active_modules.entries,
@@ -362,7 +363,7 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
 *
 * Set active flag for the submodule being initialized
 */
-   if (!is_submodule_initialized(path)) {
+   if (!is_submodule_active(_repository, path)) {
strbuf_reset();
strbuf_addf(, "submodule.%s.active", sub->name);
git_config_set_gently(sb.buf, "true");
@@ -817,7 +818,7 @@ static int prepare_to_clone_next_submodule(const struct 
cache_entry *ce,
}
 
/* Check if the submodule has been initialized. */
-   if (!is_submodule_initialized(ce->name)) {
+   if (!is_submodule_active(_repository, ce->name)) {
next_submodule_warn_missing(suc, out, displaypath);
goto cleanup;
}
@@ -1193,7 +1194,7 @@ static int is_active(int argc, const char **argv, const 
char *prefix)
 
gitmodules_config();
 
-   return !is_submodule_initialized(argv[1]);
+   return !is_submodule_active(_repository, argv[1]);
 }
 
 #define SUPPORT_SUPER_PREFIX (1<<0)
diff --git a/config.c b/config.c
index 930333e89..977bba233 100644
--- a/config.c
+++ b/config.c
@@ -1885,8 +1885,8 @@ static int repo_config_get_value(struct repo *repository,
return git_configset_get_value(repository->config, key, value);
 }
 
-static const struct string_list *repo_config_get_value_multi(struct repo 
*repository,
-const char *key)
+const struct string_list *repo_config_get_value_multi(struct repo *repository,
+ const char *key)
 {
git_config_check_init(repository);
return git_configset_get_value_multi(repository->config, key);
@@ -1903,8 +1903,8 @@ static int repo_config_get_string_const(struct repo 
*repository,
return ret;
 }
 
-static int repo_config_get_string(struct repo *repository,
- const char *key, char **dest)
+int repo_config_get_string(struct repo *repository,
+  const char *key, char **dest)
 {
git_config_check_init(repository);
return repo_config_get_string_const(repository, key, (const char 
**)dest);
@@ -1924,8 +1924,8 @@ static int repo_config_get_ulong(struct repo *repository,
return git_configset_get_ulong(repository->config, key, dest);
 }
 
-static int repo_config_get_bool(struct repo *repository,
-   const char *key, int *dest)
+int repo_config_get_bool(struct repo *repository,
+const char *key, int *dest)
 {
git_config_check_init(repository);
return git_configset_get_bool(repository->config, key, dest);
diff --git a/config.h b/config.h
index 4cd9e2915..c4406b728 100644
--- a/config.h
+++ b/config.h
@@ -163,6 +163,15 @@ extern int git_configset_get_bool_or_int(struct config_set 
*cs, const char *key,
 extern int git_configset_get_maybe_bool(struct config_set *cs, const char 
*key, int *dest);
 

[PATCH 15/31] convert: convert get_cached_convert_stats_ascii to take an index

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 builtin/ls-files.c | 3 ++-
 convert.c  | 5 +++--
 convert.h  | 5 -
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index c4357dc30..f16ce0053 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -64,7 +64,8 @@ static void write_eolinfo(const struct cache_entry *ce, const 
char *path)
const char *w_txt = "";
const char *a_txt = get_convert_attr_ascii(path);
if (ce && S_ISREG(ce->ce_mode))
-   i_txt = get_cached_convert_stats_ascii(ce->name);
+   i_txt = get_cached_convert_stats_ascii(_index,
+  ce->name);
if (!lstat(path, ) && S_ISREG(st.st_mode))
w_txt = get_wt_convert_stats_ascii(path);
printf("i/%-5s w/%-5s attr/%-17s\t", i_txt, w_txt, a_txt);
diff --git a/convert.c b/convert.c
index 5f4a4b1f5..574003023 100644
--- a/convert.c
+++ b/convert.c
@@ -135,11 +135,12 @@ static const char *gather_convert_stats_ascii(const char 
*data, unsigned long si
}
 }
 
-const char *get_cached_convert_stats_ascii(const char *path)
+const char *get_cached_convert_stats_ascii(const struct index_state *istate,
+  const char *path)
 {
const char *ret;
unsigned long sz;
-   void *data = read_blob_data_from_cache(path, );
+   void *data = read_blob_data_from_index(istate, path, );
ret = gather_convert_stats_ascii(data, sz);
free(data);
return ret;
diff --git a/convert.h b/convert.h
index 82871a11d..667b7dfe0 100644
--- a/convert.h
+++ b/convert.h
@@ -4,6 +4,8 @@
 #ifndef CONVERT_H
 #define CONVERT_H
 
+struct index_state;
+
 enum safe_crlf {
SAFE_CRLF_FALSE = 0,
SAFE_CRLF_FAIL = 1,
@@ -33,7 +35,8 @@ enum eol {
 };
 
 extern enum eol core_eol;
-extern const char *get_cached_convert_stats_ascii(const char *path);
+extern const char *get_cached_convert_stats_ascii(const struct index_state 
*istate,
+ const char *path);
 extern const char *get_wt_convert_stats_ascii(const char *path);
 extern const char *get_convert_attr_ascii(const char *path);
 
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 11/31] repo: add index_state to struct repo

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 repo.c | 17 +
 repo.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/repo.c b/repo.c
index c67cad5a2..c79d29534 100644
--- a/repo.c
+++ b/repo.c
@@ -104,6 +104,17 @@ void repo_read_config(struct repo *repo)
git_config_with_options(config_set_callback, repo->config, NULL, );
 }
 
+void repo_read_index(struct repo *repo)
+{
+   if (!repo->index)
+   repo->index = xcalloc(1, sizeof(struct index_state));
+   else
+   discard_index(repo->index);
+
+   if (read_index_from(repo->index, repo->index_file) < 0)
+   die(_("failure reading index"));
+}
+
 int repo_init(struct repo *repo, const char *gitdir)
 {
int error = 0;
@@ -149,4 +160,10 @@ void repo_clear(struct repo *repo)
free(repo->config);
repo->config = NULL;
}
+
+   if (repo->index) {
+   discard_index(repo->index);
+   free(repo->index);
+   repo->index = NULL;
+   }
 }
diff --git a/repo.h b/repo.h
index 284452832..756cda9e1 100644
--- a/repo.h
+++ b/repo.h
@@ -2,6 +2,7 @@
 #define REPO_H
 
 struct config_set;
+struct index_state;
 
 struct repo {
/* Environment */
@@ -20,6 +21,7 @@ struct repo {
 * ~/.gitconfig, XDG config file and the global /etc/gitconfig)
 */
struct config_set *config;
+   struct index_state *index;
 
/* Configurations */
unsigned ignore_env:1;
@@ -32,6 +34,7 @@ extern struct repo the_repository;
 extern void repo_set_gitdir(struct repo *repo, const char *path);
 extern void repo_set_worktree(struct repo *repo, const char *path);
 extern void repo_read_config(struct repo *repo);
+extern void repo_read_index(struct repo *repo);
 extern int repo_init(struct repo *repo, const char *path);
 extern void repo_clear(struct repo *repo);
 
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 02/31] config: don't include config.h by default

[Brandon Williams]

Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams 
---
 advice.c | 1 +
 alias.c  | 1 +
 apply.c  | 1 +
 archive-tar.c| 1 +
 archive-zip.c| 1 +
 archive.c| 1 +
 attr.c   | 1 +
 bisect.c | 1 +
 branch.c | 1 +
 builtin/add.c| 1 +
 builtin/am.c | 1 +
 builtin/blame.c  | 1 +
 builtin/branch.c | 1 +
 builtin/cat-file.c   | 1 +
 builtin/check-attr.c | 1 +
 builtin/check-ignore.c   | 1 +
 builtin/check-mailmap.c  | 1 +
 builtin/checkout-index.c | 1 +
 builtin/checkout.c   | 1 +
 builtin/clean.c  | 1 +
 builtin/clone.c  | 1 +
 builtin/column.c | 1 +
 builtin/commit-tree.c| 1 +
 builtin/commit.c | 1 +
 builtin/config.c | 1 +
 builtin/count-objects.c  | 1 +
 builtin/describe.c   | 1 +
 builtin/diff-files.c | 1 +
 builtin/diff-index.c | 1 +
 builtin/diff-tree.c  | 1 +
 builtin/diff.c   | 1 +
 builtin/difftool.c   | 1 +
 builtin/fast-export.c| 1 +
 builtin/fetch.c  | 1 +
 builtin/fmt-merge-msg.c  | 1 +
 builtin/for-each-ref.c   | 1 +
 builtin/fsck.c   | 1 +
 builtin/gc.c | 1 +
 builtin/grep.c   | 1 +
 builtin/hash-object.c| 1 +
 builtin/help.c   | 1 +
 builtin/index-pack.c | 1 +
 builtin/init-db.c| 1 +
 builtin/log.c| 1 +
 builtin/ls-files.c   | 1 +
 builtin/ls-tree.c| 1 +
 builtin/merge-base.c | 1 +
 builtin/merge-file.c | 1 +
 builtin/merge.c  | 1 +
 builtin/mv.c | 1 +
 builtin/name-rev.c   | 1 +
 builtin/notes.c  | 1 +
 builtin/pack-objects.c   | 1 +
 builtin/patch-id.c   | 1 +
 builtin/pull.c   | 1 +
 builtin/push.c   | 1 +
 builtin/read-tree.c  | 1 +
 builtin/rebase--helper.c | 1 +
 builtin/receive-pack.c   | 1 +
 builtin/reflog.c | 1 +
 builtin/remote.c | 1 +
 builtin/repack.c | 1 +
 builtin/replace.c| 1 +
 builtin/rerere.c | 1 +
 builtin/reset.c  | 1 +
 builtin/rev-list.c   | 1 +
 builtin/rev-parse.c  | 1 +
 builtin/revert.c | 1 +
 builtin/rm.c | 1 +
 builtin/send-pack.c  | 1 +
 builtin/shortlog.c   | 1 +
 builtin/show-branch.c| 1 +
 builtin/stripspace.c | 1 +
 builtin/submodule--helper.c  | 1 +
 builtin/symbolic-ref.c   | 1 +
 builtin/tag.c| 1 +
 builtin/unpack-file.c| 1 +
 builtin/unpack-objects.c | 1 +
 builtin/update-index.c   | 1 +
 builtin/update-ref.c | 1 +
 builtin/update-server-info.c | 1 +
 builtin/var.c| 1 +
 builtin/verify-commit.c  | 1 +
 builtin/verify-pack.c| 1 +
 builtin/verify-tag.c | 1 +
 builtin/worktree.c   | 1 +
 builtin/write-tree.c | 1 +
 cache.h  | 1 -
 color.c  | 1 +
 column.c | 1 +
 config.c | 1 +
 connect.c| 1 +
 convert.c| 1 +
 credential-cache--daemon.c   | 1 +
 credential.c | 1 +
 daemon.c | 1 +
 diff.c   | 1 +
 dir.c| 1 +
 environment.c| 1 +
 fast-import.c| 1 +
 fetch-pack.c | 1 +
 git.c| 1 +
 gpg-interface.c  | 1 +
 graph.c  | 1 +
 grep.c   | 1 +
 help.c   | 1 +
 http-backend.c   | 1 +
 http-fetch.c | 1 +
 http.c   | 1 +
 ident.c  | 1 +
 imap-send.c  | 1 +
 ll-merge.c   | 1 +
 log-tree.c   | 1 +
 mailinfo.c   | 1 +
 merge-recursive.c| 1 +
 notes-utils.c| 1 +
 notes.c  | 1 +
 pager.c  | 1 +
 parse-options.c  | 1 +
 pathspec.c   | 1 +
 

[PATCH 00/31] repository object

[Brandon Williams]

Given the vast interest expressed when I sent out my RFC series I decided it
would be worth it to invest more time to making a repository object a reality.

This series is an extension of the last series I sent out (in that ls-files is
converted to working on submodules in-process using repository objects instead
of spawning a child process to do the work).  The big difference from the RFC
series is that I went through and did the work to migrate key repository state
from global variables in 'environment.c' to being stored in a repository object
itself.  I migrated the bits of state that seemed reasonable for this series,
there is still a lot of global state which could be migrated in the future.

I do think that we need to be slightly cautious about moving global state into
the repository object though, I don't want 'struct repo' to simply become a
kitchen sink where everything gets dumped.  But this is just a warning for the
future.

Since this is a v1 I'm fairly certain that it still has a lot of rough edges
(like I think I need to write better commit messages, and we should probably
have more comments documenting object fields/contract) but I want to get the
review process started sooner rather than later since I'm sure people will have
opinions (e.g. should it be called 'struct repo' or 'struct repository'?!).

So here it is!  Thank you so much for taking the time review this, any and all
comments would be appreciated.

- Brandon

Patches [01-14] -> Introducing the Repository object and migrating some state
   to be stored in 'the_repository'.

Patches [15-31] -> Converting ls-files to use 'struct repo' in order to recurse
   submodules in-process.

Brandon Williams (31):
  config: create config.h
  config: don't include config.h by default
  config: don't implicitly use gitdir
  setup: don't perform lazy initialization of repository state
  environment: remove namespace_len variable
  repo: introduce the repository object
  environment: place key repository state in the_repository
  environment: store worktree in the_repository
  setup: add comment indicating a hack
  config: migrate the_configset to the_repository
  repo: add index_state to struct repo
  submodule-config: store the_submodule_cache in the_repository
  repo: add repo_read_gitmodules
  submodule: convert is_submodule_initialized to work on a repository
  convert: convert get_cached_convert_stats_ascii to take an index
  convert: convert crlf_to_git to take an index
  convert: convert convert_to_git_filter_fd to take an index
  convert: convert convert_to_git to take an index
  convert: convert renormalize_buffer to take an index
  tree: convert read_tree to take an index parameter
  ls-files: convert overlay_tree_on_cache to take an index
  ls-files: convert write_eolinfo to take an index
  ls-files: convert show_killed_files to take an index
  ls-files: convert show_other_files to take an index
  ls-files: convert show_ru_info to take an index
  ls-files: convert ce_excluded to take an index
  ls-files: convert prune_cache to take an index
  ls-files: convert show_files to take an index
  ls-files: factor out debug info into a function
  ls-files: factor out tag calculation
  ls-files: use repository object

 Makefile   |   1 +
 advice.c   |   1 +
 alias.c|   1 +
 apply.c|   3 +-
 archive-tar.c  |   1 +
 archive-zip.c  |   1 +
 archive.c  |   1 +
 attr.c |   1 +
 bisect.c   |   1 +
 branch.c   |   1 +
 builtin/add.c  |   1 +
 builtin/am.c   |   1 +
 builtin/blame.c|   3 +-
 builtin/branch.c   |   1 +
 builtin/cat-file.c |   1 +
 builtin/check-attr.c   |   1 +
 builtin/check-ignore.c |   1 +
 builtin/check-mailmap.c|   1 +
 builtin/checkout-index.c   |   1 +
 builtin/checkout.c |   1 +
 builtin/clean.c|   1 +
 builtin/clone.c|   1 +
 builtin/column.c   |   1 +
 builtin/commit-tree.c  |   1 +
 builtin/commit.c   |   4 +-
 builtin/config.c   |   3 +
 builtin/count-objects.c|   1 +
 builtin/describe.c |   1 +
 builtin/diff-files.c   |   1 +
 builtin/diff-index.c   |   1 +
 builtin/diff-tree.c|   1 +
 builtin/diff.c |   1 +
 builtin/difftool.c |   1 +
 builtin/fast-export.c  |   1 +
 builtin/fetch.c|   1 +
 builtin/fmt-merge-msg.c|   

[PATCH 06/31] repo: introduce the repository object

[Brandon Williams]

Introduce the repository object 'struct repo' which can be used hold all
state pertaining to a git repository.

The aim of object-ifying the repository is to (1) make the code base
more readable and easier to reason about and (2) allow for working on
multiple repositories, specifically submodules, within the same process.

TODO: Add more motivating points for adding a repository object?

Signed-off-by: Brandon Williams 
---
 Makefile  |   1 +
 cache.h   |   1 +
 environment.c |   2 +-
 repo.c| 124 ++
 repo.h|  25 
 5 files changed, 152 insertions(+), 1 deletion(-)
 create mode 100644 repo.c
 create mode 100644 repo.h

diff --git a/Makefile b/Makefile
index 2ed6db728..3d3d556ef 100644
--- a/Makefile
+++ b/Makefile
@@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
+LIB_OBJS += repo.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
 LIB_OBJS += revision.o
diff --git a/cache.h b/cache.h
index d41aab82f..c0b4c8d83 100644
--- a/cache.h
+++ b/cache.h
@@ -483,6 +483,7 @@ extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
 extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
 extern int get_common_dir(struct strbuf *sb, const char *gitdir);
+extern char *expand_namespace(const char *raw_namespace);
 extern const char *get_git_namespace(void);
 extern const char *strip_namespace(const char *namespaced_ref);
 extern const char *get_super_prefix(void);
diff --git a/environment.c b/environment.c
index e035f6372..77900b31a 100644
--- a/environment.c
+++ b/environment.c
@@ -127,7 +127,7 @@ const char * const local_repo_env[] = {
NULL
 };
 
-static char *expand_namespace(const char *raw_namespace)
+char *expand_namespace(const char *raw_namespace)
 {
struct strbuf buf = STRBUF_INIT;
struct strbuf **components, **c;
diff --git a/repo.c b/repo.c
new file mode 100644
index 0..f7c3617a9
--- /dev/null
+++ b/repo.c
@@ -0,0 +1,124 @@
+#include "cache.h"
+#include "repo.h"
+
+/*
+ * This may be the wrong place for this.
+ * It may be better to go in env.c or setup for the time being?
+ */
+struct repo the_repository;
+
+static char *git_path_from_env(const char *envvar, const char *git_dir,
+  const char *path, int fromenv)
+{
+   if (fromenv) {
+   const char *value = getenv(envvar);
+   if (value)
+   return xstrdup(value);
+   }
+
+   return xstrfmt("%s/%s", git_dir, path);
+}
+
+static int find_common_dir(struct strbuf *sb, const char *gitdir, int fromenv)
+{
+   if (fromenv) {
+   const char *value = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+   if (value) {
+   strbuf_addstr(sb, value);
+   return 1;
+   }
+   }
+
+   return get_common_dir_noenv(sb, gitdir);
+}
+
+/* called after setting gitdir */
+static void repo_setup_env(struct repo *repo)
+{
+   struct strbuf sb = STRBUF_INIT;
+
+   if (!repo->gitdir)
+   BUG("gitdir wasn't set before setting up the environment");
+
+   repo->different_commondir = find_common_dir(, repo->gitdir,
+   !repo->ignore_env);
+   repo->commondir = strbuf_detach(, NULL);
+   repo->objectdir = git_path_from_env(DB_ENVIRONMENT, repo->commondir,
+   "objects", !repo->ignore_env);
+   repo->index_file = git_path_from_env(INDEX_ENVIRONMENT, repo->gitdir,
+"index", !repo->ignore_env);
+   repo->graft_file = git_path_from_env(GRAFT_ENVIRONMENT, repo->commondir,
+"info/grafts", !repo->ignore_env);
+   repo->namespace = expand_namespace(repo->ignore_env ? NULL :
+  getenv(GIT_NAMESPACE_ENVIRONMENT));
+}
+
+static void repo_clear_env(struct repo *repo)
+{
+   free(repo->gitdir);
+   repo->gitdir = NULL;
+   free(repo->commondir);
+   repo->commondir = NULL;
+   free(repo->objectdir);
+   repo->objectdir = NULL;
+   free(repo->index_file);
+   repo->index_file = NULL;
+   free(repo->graft_file);
+   repo->graft_file = NULL;
+   free(repo->namespace);
+   repo->namespace = NULL;
+}
+
+void repo_set_gitdir(struct repo *repo, const char *path)
+{
+   const char *gitfile = read_gitfile(path);
+
+   /*
+* NEEDSWORK: Eventually we want to be able to free gitdir and the rest
+* of the environment before reinitializing it again, but we have some
+* crazy code paths where we try to set gitdir with the current gitdir
+* and we don't want to free gitdir before copying the passed in value.
+*/
+   repo->gitdir = xstrdup(gitfile ? 

[PATCH 05/31] environment: remove namespace_len variable

[Brandon Williams]

Use 'skip_prefix' instead of 'starts_with' so that we can drop the need
to keep around 'namespace_len'.

Signed-off-by: Brandon Williams 
---
 environment.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/environment.c b/environment.c
index a73b08f5d..e035f6372 100644
--- a/environment.c
+++ b/environment.c
@@ -98,7 +98,6 @@ char *git_work_tree_cfg;
 static char *work_tree;
 
 static const char *namespace;
-static size_t namespace_len;
 
 static const char *super_prefix;
 
@@ -190,7 +189,6 @@ void setup_git_env(void)
git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
  : "refs/replace/");
namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
-   namespace_len = strlen(namespace);
shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
if (shallow_file)
set_alternate_shallow_file(shallow_file, 0);
@@ -231,9 +229,10 @@ const char *get_git_namespace(void)
 
 const char *strip_namespace(const char *namespaced_ref)
 {
-   if (!starts_with(namespaced_ref, get_git_namespace()))
-   return NULL;
-   return namespaced_ref + namespace_len;
+   const char *out;
+   if (skip_prefix(namespaced_ref, get_git_namespace(), ))
+   return out;
+   return NULL;
 }
 
 const char *get_super_prefix(void)
-- 
2.13.0.506.g27d5fe0cd-goog

[PATCH 01/31] config: create config.h

[Brandon Williams]

Move all config related declarations from cache.h to a new config.h
header file.  This makes cache.h smaller and allows for the opportunity
in a following patch to only include config.h when needed.

Signed-off-by: Brandon Williams 
---
 cache.h  | 190 +
 config.h | 194 +++
 2 files changed, 195 insertions(+), 189 deletions(-)
 create mode 100644 config.h

diff --git a/cache.h b/cache.h
index ae4c45d37..a27fee458 100644
--- a/cache.h
+++ b/cache.h
@@ -11,6 +11,7 @@
 #include "string-list.h"
 #include "pack-revindex.h"
 #include "hash.h"
+#include "config.h"
 
 #ifndef platform_SHA_CTX
 /*
@@ -1866,188 +1867,9 @@ extern int packed_object_info(struct packed_git *pack, 
off_t offset, struct obje
 /* Dumb servers support */
 extern int update_server_info(int);
 
-/* git_config_parse_key() returns these negated: */
-#define CONFIG_INVALID_KEY 1
-#define CONFIG_NO_SECTION_OR_NAME 2
-/* git_config_set_gently(), git_config_set_multivar_gently() return the above 
or these: */
-#define CONFIG_NO_LOCK -1
-#define CONFIG_INVALID_FILE 3
-#define CONFIG_NO_WRITE 4
-#define CONFIG_NOTHING_SET 5
-#define CONFIG_INVALID_PATTERN 6
-#define CONFIG_GENERIC_ERROR 7
-
-#define CONFIG_REGEX_NONE ((void *)1)
-
-struct git_config_source {
-   unsigned int use_stdin:1;
-   const char *file;
-   const char *blob;
-};
-
-enum config_origin_type {
-   CONFIG_ORIGIN_BLOB,
-   CONFIG_ORIGIN_FILE,
-   CONFIG_ORIGIN_STDIN,
-   CONFIG_ORIGIN_SUBMODULE_BLOB,
-   CONFIG_ORIGIN_CMDLINE
-};
-
-struct config_options {
-   unsigned int respect_includes : 1;
-   const char *git_dir;
-};
-
-typedef int (*config_fn_t)(const char *, const char *, void *);
-extern int git_default_config(const char *, const char *, void *);
-extern int git_config_from_file(config_fn_t fn, const char *, void *);
-extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
-   const char *name, const char *buf, 
size_t len, void *data);
-extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
-const unsigned char *sha1, void *data);
-extern void git_config_push_parameter(const char *text);
-extern int git_config_from_parameters(config_fn_t fn, void *data);
-extern void read_early_config(config_fn_t cb, void *data);
-extern void git_config(config_fn_t fn, void *);
-extern int git_config_with_options(config_fn_t fn, void *,
-  struct git_config_source *config_source,
-  const struct config_options *opts);
-extern int git_parse_ulong(const char *, unsigned long *);
-extern int git_parse_maybe_bool(const char *);
-extern int git_config_int(const char *, const char *);
-extern int64_t git_config_int64(const char *, const char *);
-extern unsigned long git_config_ulong(const char *, const char *);
-extern ssize_t git_config_ssize_t(const char *, const char *);
-extern int git_config_bool_or_int(const char *, const char *, int *);
-extern int git_config_bool(const char *, const char *);
-extern int git_config_maybe_bool(const char *, const char *);
-extern int git_config_string(const char **, const char *, const char *);
-extern int git_config_pathname(const char **, const char *, const char *);
-extern int git_config_set_in_file_gently(const char *, const char *, const 
char *);
-extern void git_config_set_in_file(const char *, const char *, const char *);
-extern int git_config_set_gently(const char *, const char *);
-extern void git_config_set(const char *, const char *);
-extern int git_config_parse_key(const char *, char **, int *);
-extern int git_config_key_is_valid(const char *key);
-extern int git_config_set_multivar_gently(const char *, const char *, const 
char *, int);
-extern void git_config_set_multivar(const char *, const char *, const char *, 
int);
-extern int git_config_set_multivar_in_file_gently(const char *, const char *, 
const char *, const char *, int);
-extern void git_config_set_multivar_in_file(const char *, const char *, const 
char *, const char *, int);
-extern int git_config_rename_section(const char *, const char *);
-extern int git_config_rename_section_in_file(const char *, const char *, const 
char *);
-extern const char *git_etc_gitconfig(void);
-extern int git_env_bool(const char *, int);
-extern unsigned long git_env_ulong(const char *, unsigned long);
-extern int git_config_system(void);
-extern int config_error_nonbool(const char *);
-#if defined(__GNUC__)
-#define config_error_nonbool(s) (config_error_nonbool(s), const_error())
-#endif
 extern const char *get_log_output_encoding(void);
 extern const char *get_commit_output_encoding(void);
 
-extern int git_config_parse_parameter(const char *, config_fn_t fn, void 
*data);
-
-enum config_scope {
-   CONFIG_SCOPE_UNKNOWN = 0,
-   

Re: bug: `git log --grep ... --invert-grep --author=...` negates / ignores --author

[Jeff King]

On Wed, May 31, 2017 at 08:08:54PM +0200, Ævar Arnfjörð Bjarmason wrote:

> $ git log --grep=bar --author=Ævar --pretty=format:%an -100 origin/pu
> |sort|uniq -c|sort -nr
> 5 Ævar Arnfjörð Bjarmason
> 
> $ git log --author=Ævar --pretty=format:%an -100 origin/pu |sort|uniq
> -c|sort -nr
> 100 Ævar Arnfjörð Bjarmason
> 
> $ git log --grep=bar --invert-grep --author=Ævar --pretty=format:%an
> -100 origin/pu |sort|uniq -c|sort -nr
>  78 Junio C Hamano
>  14 Jeff King
>   2 Andreas Heiduk
>   1 Sahil Dua
>   1 Rikard Falkeborn
>   1 Johannes Sixt
>   1 Johannes Schindelin
>   1 Ben Peart
>   1 Ævar Arnfjörð Bjarmason
> 
> That last command should only find my commits, but instead --author is
> discarded.

I would have thought that the last command wouldn't find _any_ of your
commits. Don't we consider --author a greppable pattern and invert it?

By itself:

  $ git log --author=Ævar --invert-grep --format=%an -100 origin/pu |
sort | uniq -c | sort -rn
   79 Junio C Hamano
8 Stefan Beller
8 Jeff King
1 Sahil Dua
1 Rikard Falkeborn
1 Johannes Sixt
1 Johannes Schindelin
1 Andreas Heiduk

So that makes sense from the "--author is invertable" world-view.

But I'm puzzled that you show up at all when --grep=bar is added (and
moreover, that we get _one_ commit from you, not the 5 found in your
initial command). That seems like a bug.

-Peff

[PATCH v1] doc: rewrite description for rev-parse --short

[Andreas Heiduk]

`git rev-parse --short` is not a generic modifier but just a variant
of `--verify` and considers the given length only as a suggestion to
ensure uniqueness.

Signed-off-by: Andreas Heiduk 
---
 Documentation/config.txt|  1 +
 Documentation/git-rev-parse.txt | 12 ++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 43d830ee3..3256a3344 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -883,6 +883,7 @@ core.abbrev::
computed based on the approximate number of packed objects
in your repository, which hopefully is enough for
abbreviated object names to stay unique for some time.
+   The minimum length is 4.
 
 add.ignoreErrors::
 add.ignore-errors (deprecated)::
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index c40c47044..b1293f24b 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -126,6 +126,12 @@ can be used.
'git diff-{asterisk}'). In contrast to the `--sq-quote` option,
the command input is still interpreted as usual.
 
+--short[=length]::
+   Same as `--verify` but shortens the object name to a unique
+   prefix with at least `length` characters. The minimum length
+   is 4, the default is the effective value of the `core.abbrev`
+   configuration variable (see linkgit:git-config[1]).
+
 --not::
When showing object names, prefix them with '{caret}' and
strip '{caret}' prefix from the object names that already have
@@ -136,12 +142,6 @@ can be used.
The option core.warnAmbiguousRefs is used to select the strict
abbreviation mode.
 
---short::
---short=number::
-   Instead of outputting the full SHA-1 values of object names try to
-   abbreviate them to a shorter unique name. When no length is specified
-   7 is used. The minimum length is 4.
-
 --symbolic::
Usually the object names are output in SHA-1 form (with
possible '{caret}' prefix); this option makes them output in a
-- 
2.13.0

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Jeff King]

On Wed, May 31, 2017 at 05:29:20PM -0400, Jeff King wrote:

> Or did you mean that diff_tree_paths() could now take an actual
> array-of-struct rather than an array-of-pointer-to-struct? That would
> drop the "parents_oid" array entirely. I think that's actually
> orthogonal to this change (the same could have been done with the
> original sha1 array), but would be a nice cleanup on top.

I took a quick look at this, but it doesn't work. This caller
(find_paths_multitree) would be happy to just pass the existing
"parents" array.

But if we change the interface to diff_tree_paths(), we also have to
change ll_diff_tree_paths() to match. And that function is called from
emit_path(), which really does assemble a list of pointers to
non-adjacent bits of memory. So it would still have to allocate, and
would now have to copy whole oids rather than just pointers.

But even worse, it seems to leave some entries as NULL. We'd need some
sentinel value (like the null sha1) to replace that.

So I think the code is already in its simplest form.

-Peff

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Jeff King]

On Wed, May 31, 2017 at 11:24:33AM -0700, Stefan Beller wrote:

> On Tue, May 30, 2017 at 10:31 AM, Brandon Williams  wrote:
> >
> > Signed-off-by: Brandon Williams 
> > ---
> >  combine-diff.c | 10 +-
> >  diff.h |  4 ++--
> >  tree-diff.c| 63 
> > +-
> >  3 files changed, 39 insertions(+), 38 deletions(-)
> >
> > diff --git a/combine-diff.c b/combine-diff.c
> > index 04c4ae856..ec9d93044 100644
> > --- a/combine-diff.c
> > +++ b/combine-diff.c
> > @@ -1364,22 +1364,22 @@ static struct combine_diff_path 
> > *find_paths_multitree(
> > struct diff_options *opt)
> >  {
> > int i, nparent = parents->nr;
> > -   const unsigned char **parents_sha1;
> > +   const struct object_id **parents_oid;
> > struct combine_diff_path paths_head;
> > struct strbuf base;
> >
> > -   ALLOC_ARRAY(parents_sha1, nparent);
> > +   ALLOC_ARRAY(parents_oid, nparent);
> > for (i = 0; i < nparent; i++)
> > -   parents_sha1[i] = parents->oid[i].hash;
> > +   parents_oid[i] = >oid[i];
> 
> I have the impression that we could get away with one layer less
> of indirection. Previously we had a heap allocated array (*) of (char*),
> now we'd have a an array (*) of pointers(*) of the oid struct, that
> is a (char[]) essentially. Maybe I am just confused?

I don't think so. We always could have allocated the original as an
array of 20-byte chunks. It's syntactically less awkward in C when that
20-byte chunk is wrapped in a struct like object_id. But fundamentally
I think we don't need to be making a copy of the oid. We're pointing to
the existing copy in the "parents" array.

Or did you mean that diff_tree_paths() could now take an actual
array-of-struct rather than an array-of-pointer-to-struct? That would
drop the "parents_oid" array entirely. I think that's actually
orthogonal to this change (the same could have been done with the
original sha1 array), but would be a nice cleanup on top.

-Peff

Re: Coloring

[Samuel Lijin]

On Wed, May 31, 2017 at 5:10 PM, Irving Rabin  wrote:
>
> Thanks Jeff, my problem has been resolved by Samuel Lijin.
> My terminal settings didn't set bold which remained white. I fixed it
> and my problem was gone!

Specifically, Irving's terminal rendered bold text as white. No bug here :)

> This issue is closed. Is there any way to retire it?

That's pretty much it.

> Irving Rabin
> Software Developer @Edmodo
> 408-242-1299
>
>
>
>
>
> On Wed, May 31, 2017 at 2:04 PM, Jeff King  wrote:
> > On Wed, May 31, 2017 at 11:33:31AM -0700, Irving Rabin wrote:
> >
> >> Specifically, if the field is supposed to be white, it doesn't mean it
> >> should be literally 0xFF. It should be the color that I have
> >> configured as White color for my console emulator.
> >>
> >> I like light-screen terminals, and I configure my ANSI colors in the
> >> way that they are clearly visible on the background and clearly
> >> distinct between themselves. In my terminal settings background is
> >> light-yellow, Black is black, Yellow is brown, Red is dark red,
> >> Magenta is purple and White is dark gray. I set it once and I can use
> >> it everywhere - all Unix commands work correctly, I can edit
> >> highlighted source code in Vim, and all my color settings are
> >> respected.
> >
> > Git outputs ANSI color codes, which are interpreted by your terminal.
> > You _can_ configure Git to send 24-bit color codes if your terminal
> > supports it, but by default it uses the traditional set of limited color
> > and attribute codes.
> >
> > What does running the following snippet in your shell look like?
> >
> > -- >8 --
> >
> > while read name code; do
> > printf '\033[%sm%s\033[m\n' "$code" "$name"
> > done <<-\EOF
> > normal
> > bold 1
> > red 31
> > green 32
> > yellow 33
> > blue 34
> > magenta 35
> > cyan 36
> > bold-red 1;31
> > bold-green 1;32
> > bold-yellow 1;33
> > bold-blue 1;34
> > bold-magenta 1;35
> > bold-cyan 1;36
> > EOF
> >
> > -- 8< --
> >
> > If any of the colors are not what you expect, is there a pattern? E.g.,
> > I wouldn't be surprised if "bold" shows up as bright white. In many
> > modern terminal emulators, the bold variants need to be configured
> > separately from the non-bold ones, and default to lighter variants of
> > their non-bold counterparts. The solution there would be to check your
> > terminal emulator config.
> >
> > If it does all look as you'd expect, try adding "| less -R" to the end of
> > the "done <<-\EOF" line. Most of Git's output goes through that pager
> > (though I _think_ it's mostly just passing through the ANSI codes, so it
> > wouldn't have any effect).
> >
> > -Peff

Re: persistent-https, url insteadof, and `git submodule`

[Jeff King]

On Wed, May 31, 2017 at 04:23:49PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > It really is an issue of the user knowing about the problem (and how to
> > solve it), and I don't think we can get around that securely. So better
> > documentation probably is the right solution.
> >
> > I'll see if I can cook something up.
> 
> I was going to say: A way to have our cake & eat it too here would be
> to just support *.insteadOfRegex, i.e.
> "url.persistent-https://.insteadOfRegex="^https://;.
> 
> But in this case, even if we can just do un-anchored string
> replacement, isn't a way around this just to do
> "url.persistent-https://.insteadOf=https://; & untaint & document that
> you should do that?

I think we already do the second form, and that's what Elliott ran into.
The problem is that it is not clear if "persistent-https" is safe to use
for submodules. _We_ know that it is because we know what that remote
does, but Git doesn't know that. You would not necessarily want:

  [url "ext::ssh-wrapper "]
  insteadOf  = "ssh://"

to kick in for a submodule. That's a fairly insane thing to be doing,
but the point is that we don't know if the rewritten protocol is ready
to handle "tainted" URLs that come from an untrusted submodule file.

-Peff

Re: [PATCH 6.5?/8] version: move --build-options to a test helper

[Jeff King]

On Wed, May 31, 2017 at 07:51:20PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Thanks both. It makes sense to discard this patch.
> 
> I wasn't sure whether anyone really cared about this, and thought a
> patch was a better starting point of discussion.

I will never complain about somebody starting a discussion with a patch
as long as they don't mind if it gets shot down. :)

Thanks for raising the point. I do think it was worth thinking about.

-Peff

Re: Coloring

[Irving Rabin]

Thanks Jeff, my problem has been resolved by Samuel Lijin.
My terminal settings didn't set bold which remained white. I fixed it
and my problem was gone!
This issue is closed. Is there any way to retire it?
Irving Rabin
Software Developer @Edmodo
408-242-1299





On Wed, May 31, 2017 at 2:04 PM, Jeff King  wrote:
> On Wed, May 31, 2017 at 11:33:31AM -0700, Irving Rabin wrote:
>
>> Specifically, if the field is supposed to be white, it doesn't mean it
>> should be literally 0xFF. It should be the color that I have
>> configured as White color for my console emulator.
>>
>> I like light-screen terminals, and I configure my ANSI colors in the
>> way that they are clearly visible on the background and clearly
>> distinct between themselves. In my terminal settings background is
>> light-yellow, Black is black, Yellow is brown, Red is dark red,
>> Magenta is purple and White is dark gray. I set it once and I can use
>> it everywhere - all Unix commands work correctly, I can edit
>> highlighted source code in Vim, and all my color settings are
>> respected.
>
> Git outputs ANSI color codes, which are interpreted by your terminal.
> You _can_ configure Git to send 24-bit color codes if your terminal
> supports it, but by default it uses the traditional set of limited color
> and attribute codes.
>
> What does running the following snippet in your shell look like?
>
> -- >8 --
>
> while read name code; do
> printf '\033[%sm%s\033[m\n' "$code" "$name"
> done <<-\EOF
> normal
> bold 1
> red 31
> green 32
> yellow 33
> blue 34
> magenta 35
> cyan 36
> bold-red 1;31
> bold-green 1;32
> bold-yellow 1;33
> bold-blue 1;34
> bold-magenta 1;35
> bold-cyan 1;36
> EOF
>
> -- 8< --
>
> If any of the colors are not what you expect, is there a pattern? E.g.,
> I wouldn't be surprised if "bold" shows up as bright white. In many
> modern terminal emulators, the bold variants need to be configured
> separately from the non-bold ones, and default to lighter variants of
> their non-bold counterparts. The solution there would be to check your
> terminal emulator config.
>
> If it does all look as you'd expect, try adding "| less -R" to the end of
> the "done <<-\EOF" line. Most of Git's output goes through that pager
> (though I _think_ it's mostly just passing through the ANSI codes, so it
> wouldn't have any effect).
>
> -Peff

Re: Coloring

[Jeff King]

On Wed, May 31, 2017 at 11:33:31AM -0700, Irving Rabin wrote:

> Specifically, if the field is supposed to be white, it doesn't mean it
> should be literally 0xFF. It should be the color that I have
> configured as White color for my console emulator.
> 
> I like light-screen terminals, and I configure my ANSI colors in the
> way that they are clearly visible on the background and clearly
> distinct between themselves. In my terminal settings background is
> light-yellow, Black is black, Yellow is brown, Red is dark red,
> Magenta is purple and White is dark gray. I set it once and I can use
> it everywhere - all Unix commands work correctly, I can edit
> highlighted source code in Vim, and all my color settings are
> respected.

Git outputs ANSI color codes, which are interpreted by your terminal.
You _can_ configure Git to send 24-bit color codes if your terminal
supports it, but by default it uses the traditional set of limited color
and attribute codes.

What does running the following snippet in your shell look like?

-- >8 --

while read name code; do
printf '\033[%sm%s\033[m\n' "$code" "$name"
done <<-\EOF
normal
bold 1
red 31
green 32
yellow 33
blue 34
magenta 35
cyan 36
bold-red 1;31
bold-green 1;32
bold-yellow 1;33
bold-blue 1;34
bold-magenta 1;35
bold-cyan 1;36
EOF

-- 8< --

If any of the colors are not what you expect, is there a pattern? E.g.,
I wouldn't be surprised if "bold" shows up as bright white. In many
modern terminal emulators, the bold variants need to be configured
separately from the non-bold ones, and default to lighter variants of
their non-bold counterparts. The solution there would be to check your
terminal emulator config.

If it does all look as you'd expect, try adding "| less -R" to the end of
the "done <<-\EOF" line. Most of Git's output goes through that pager
(though I _think_ it's mostly just passing through the ANSI codes, so it
wouldn't have any effect).

-Peff

Re: [Bug] git branch -v has problems with carriage returns

[Stefan Beller]

On Tue, May 30, 2017 at 10:32 PM, Atousa Duprat  wrote:
> Here is my first attempt at fixing the issue.

Cool you're looking into this. :)

>
> There are two problems in ref-filter.c:
>
> First, copy_subject() has been modified to turn '\n' into a space and
> every other ascii control character to be ignored.
>
> Second, find_subpos() doesn't realize that a line that only contains a
> '\r\n' is a blank line – at least when using crlf convention.
> I have changed things so that a sequence of either '\n' or "\r\n"
> separate the subject from the body of the commit message.
> I am not looking at the crlf setting because it doesn't seem like a
> useful distinction – when one would we ever care for \r\n not to be a
> blank line?  But it could be done...
>
> Both fixes are minimal, but it feels like they are a issues with the
> specific encoding.  Does git mandate ascii or utf-8 commit messages?
> If not, there may be a larger issue here with encodings and line-end
> conventions at the very least in ref-filter.c
> Guidance would be appreciated for how to deal with this issue...
>
> Patch attached.
>

Please read Documentation/SubmittingPatches
(tl;dr:
(a) please sign your patch, read https://developercertificate.org/
(b) if possible please send patches inline instead of attached)

> diff --git a/ref-filter.c b/ref-filter.c
> index 3a640448f..bc573f481 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -836,11 +836,15 @@ static const char *copy_email(const char *buf)
>  static char *copy_subject(const char *buf, unsigned long len)
>  {
>  char *r = xmemdupz(buf, len);
> -int i;
> +int i, j;
>
> -for (i = 0; i < len; i++)
> +for (i = 0, j = 0; i < len; i++, j++)
>  if (r[i] == '\n')
> -r[i] = ' ';
> +r[j] = ' ';
> +else if (r[i] < 32)
> +j--; // skip ascii control characters that are not '\n'

/*
 * Our comment style uses the other way,
 * as it is compatible with more compilers, still.
 */

This seems to solve a different problem than the carriage return
discussed? So it could go into a separate patch.


> +else r[j] = r[i];
> +r[j]=0;
>
>  return r;
>  }
> @@ -956,9 +960,12 @@ static void find_subpos(const char *buf, unsigned long 
> sz,
>  eol++;
>  buf = eol;
>  }
> +

stray new line?

>  /* skip any empty lines */
>  while (*buf == '\n')
>  buf++;
> +while (*buf == '\r' && *(buf+1) == '\n')
> +buf += 2;

This first skips LF empty lines and then skips CRLF empty
lines. What if they are mixed? I'd think if we extend the
empty line detection we'd want to robust to such as well,
so maybe

while (*buf == '\r' || *buf == '\n')
buf++;

Maybe this is a bit too greedy?

Re: [PATCH 2/2] rebase: turn on progress option by default for format-patch

[Stefan Beller]

On Wed, May 31, 2017 at 12:46 PM, Kevin Willford  wrote:

>
> I thought about that and certainly could do it but I have found it nice to 
> have the number of patches that are generated in the output even for a small 
> number or commits.  For example when I run a `git rebase master` and expect 
> there to be only 2 commits, the message "Generating patch: 100% (2/2), done." 
>  Gives me that good feeling that I did it right and didn't mess something up. 
>  I'm good either way though.
>

Oh I see, that number matching makes sense.
Though by reading the code, I have the impression
that the final value would not change. So if it would take
a really long time for these 2 patches, you'd still get the
(2/2), to know it was 2 patches indeed.

When it goes quickly, I'd be more concerned about
screen real estate, driving off the recent history from
the screen.

Also we maybe want to have
 "Generating patches: 100% (2/2), done."
plural, as (when using the delay)
it is more likely to kick in for more than just one patch?

Sorry, if this comes across as bike shedding.
It's meant as food for thought. :)

Thanks,
Stefan

RE: [PATCH 2/2] rebase: turn on progress option by default for format-patch

[Kevin Willford]

> -Original Message-
> From: Stefan Beller [mailto:sbel...@google.com]
> Sent: Wednesday, May 31, 2017 1:09 PM
> To: Kevin Willford 
> Cc: git@vger.kernel.org; Junio C Hamano ; Kevin
> Willford 
> Subject: Re: [PATCH 2/2] rebase: turn on progress option by default for
> format-patch
> 
> On Wed, May 31, 2017 at 8:04 AM, Kevin Willford 
> wrote:
> > This change passes the progress option of format-patch by default and
> > passes the -q --quiet option through to the format-patch call so that
> > it is respected as well.
> 
> This is not conflicting with Johannes rewrite of rebase in C?
> (rebase is a huge beast IIUC)

I will check with Johannes and see what possible conflicts there could be.
Since these are flags that get passed to the format-patch code, it shouldn't 
take much to put it in the C code as well.

> 
> When passing the progress option by default to formatting patches, maybe
> we should use start_progress_delay in the previous patch instead to omit
> the progress for short lived patch formatting sessions?
> (say a delay of one second?)
> 

I thought about that and certainly could do it but I have found it nice to have 
the number of patches that are generated in the output even for a small number 
or commits.  For example when I run a `git rebase master` and expect there to 
be only 2 commits, the message "Generating patch: 100% (2/2), done."  Gives me 
that good feeling that I did it right and didn't mess something up.  I'm good 
either way though.

> Thanks,
> Stefan
> 
> >
> > Signed-off-by: Kevin Willford 
> > ---
> >  git-rebase--am.sh | 5 +++--
> >  git-rebase.sh | 2 ++
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-rebase--am.sh b/git-rebase--am.sh index
> > 375239341f..ab2be30abf 100644
> > --- a/git-rebase--am.sh
> > +++ b/git-rebase--am.sh
> > @@ -51,8 +51,9 @@ then
> >  else
> > rm -f "$GIT_DIR/rebased-patches"
> >
> > -   git format-patch -k --stdout --full-index --cherry-pick 
> > --right-only \
> > -   --src-prefix=a/ --dst-prefix=b/ --no-renames 
> > --no-cover-letter \
> > +   git format-patch $git_format_patch_opt -k --stdout --full-index \
> > +   --cherry-pick --right-only --src-prefix=a/ --dst-prefix=b/ \
> > +   --no-renames --no-cover-letter --progress \
> > "$revisions" ${restrict_revision+^$restrict_revision} \
> > >"$GIT_DIR/rebased-patches"
> > ret=$?
> > diff --git a/git-rebase.sh b/git-rebase.sh index
> > db1deed846..b72e319ac9 100755
> > --- a/git-rebase.sh
> > +++ b/git-rebase.sh
> > @@ -73,6 +73,7 @@ test "$(git config --bool rebase.stat)" = true &&
> > diffstat=t  autostash="$(git config --bool rebase.autostash || echo false)"
> >  fork_point=auto
> >  git_am_opt=
> > +git_format_patch_opt=
> >  rebase_root=
> >  force_rebase=
> >  allow_rerere_autoupdate=
> > @@ -308,6 +309,7 @@ do
> > --quiet)
> > GIT_QUIET=t
> > git_am_opt="$git_am_opt -q"
> > +   git_format_patch_opt="$git_format_patch_opt -q"
> > verbose=
> > diffstat=
> > ;;
> > --
> > 2.13.0.92.g73a4ce6a77
> >

RE: [PATCH 1/2] format-patch: have progress option while generating patches

[Kevin Willford]

> -Original Message-
> From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On
> Behalf Of Stefan Beller
> Sent: Wednesday, May 31, 2017 12:40 PM
> To: Kevin Willford 
> Cc: git@vger.kernel.org; Junio C Hamano ; Kevin
> Willford 
> Subject: Re: [PATCH 1/2] format-patch: have progress option while
> generating patches
> 
> On Wed, May 31, 2017 at 8:04 AM, Kevin Willford 
> wrote:
> > When generating patches for the rebase command if the user does not
> > realize the branch they are rebasing onto is thousands of commits
> > different there is no progress indication after initial rewinding
> > message.
> >
> > This patch allows a progress option to be passed to format-patch so
> > that the user can be informed the progress of generating the patch.
> > This option will then be used by the rebase command when calling
> > format-patch.
> 
> After reading the code, I was looking for some explanation on the underlying
> assumptions, such as:
> 
>   The progress meter as presented in this patch assumes the thousands of
>   patches to have a fine granularity as well as assuming to require all the
>   same amount of work/time for each, such that a steady progress bar
>   is achieved.
> 
>   We do not want to estimate the time for each patch based e.g.
>   on their size or number of touched files (or parents) as that is too
>   expensive for just a progress meter.
> 

Sounds good.  I will add some explanation to the commit message.

> 
> >
> > Signed-off-by: Kevin Willford 
> > ---
> >  Documentation/git-format-patch.txt |  8 
> >  builtin/log.c  | 10 ++
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/Documentation/git-format-patch.txt
> > b/Documentation/git-format-patch.txt
> > index c890328b02..ee5f99f606 100644
> > --- a/Documentation/git-format-patch.txt
> > +++ b/Documentation/git-format-patch.txt
> > @@ -23,6 +23,7 @@ SYNOPSIS
> >[(--reroll-count|-v) ]
> >[--to=] [--cc=]
> >[--[no-]cover-letter] [--quiet] [--notes[=]]
> > +  [--progress]
> >[]
> >[  |  ]
> >
> > @@ -260,6 +261,7 @@ you can use `--suffix=-patch` to get `0001-
> description-of-my-change-patch`.
> >  -q::
> >  --quiet::
> > Do not print the names of the generated files to standard output.
> > +   Progress is not reported to the standard error stream.
> >
> >  --no-binary::
> > Do not output contents of changes in binary files, instead @@
> > -283,6 +285,12 @@ you can use `--suffix=-patch` to get `0001-description-
> of-my-change-patch`.
> > range are always formatted as creation patches, independently
> > of this flag.
> >
> > +--progress::
> > +   Progress status is reported on the standard error stream
> > +   by default when it is attached to a terminal, unless -q
> > +   is specified. This flag forces progress status even if the
> > +   standard error stream is not directed to a terminal.
> > +
> >  CONFIGURATION
> >  -
> >  You can specify extra mail header lines to be added to each message,
> > diff --git a/builtin/log.c b/builtin/log.c index
> > 631fbc984f..02c50431b6 100644
> > --- a/builtin/log.c
> > +++ b/builtin/log.c
> > @@ -26,6 +26,7 @@
> >  #include "version.h"
> >  #include "mailmap.h"
> >  #include "gpg-interface.h"
> > +#include "progress.h"
> >
> >  /* Set a default date-time format for git log ("log.date" config
> > variable) */  static const char *default_date_mode = NULL; @@ -1409,6
> > +1410,8 @@ int cmd_format_patch(int argc, const char **argv, const char
> *prefix)
> > char *branch_name = NULL;
> > char *base_commit = NULL;
> > struct base_tree_info bases;
> > +   int show_progress = 0;
> > +   struct progress *progress = NULL;
> >
> > const struct option builtin_format_patch_options[] = {
> > { OPTION_CALLBACK, 'n', "numbered", , NULL,
> > @@ -1480,6 +1483,8 @@ int cmd_format_patch(int argc, const char **argv,
> const char *prefix)
> > OPT_FILENAME(0, "signature-file", _file,
> > N_("add a signature from a file")),
> > OPT__QUIET(, N_("don't print the patch
> > filenames")),
> > +   OPT_BOOL(0, "progress", _progress,
> > +N_("show progress")),
> > OPT_END()
> > };
> >
> > @@ -1739,8 +1744,12 @@ int cmd_format_patch(int argc, const char
> **argv, const char *prefix)
> > start_number--;
> > }
> > rev.add_signoff = do_signoff;
> > +
> > +   if (show_progress && !quiet)
> > +   progress = start_progress(_("Generating patch"),
> > + total);
> > while (0 <= --nr) {
> > int shown;
> > +   display_progress(progress, 

Git Daemon on Windows fatal error.

[Hector Santos]

Hi, I am relatively new to GIT (coming from CVS and SVN) and I am 
trying to setup "Git Daemon" on windows.


I got it working for Local network communications:

d:\local\wc5\testgit>git clone git://localhost/http clone10
Cloning into 'clone10'...
remote: Counting objects: 526, done.
remote: Compressing objects: 100% (520/520), done.
Receiving objects: 100% (526/526), 1.38 MiB | 0 bytes/s, done.
remote: Total 526 (delta 81), reused 0 (delta 0)
Resolving deltas: 100% (81/81), done.

but it fails over the wire when using the public host domain:

d:\local\wc5\testgit>git clone git://public.example.dom/http clone11
Cloning into 'clone11'...
remote: Counting objects: 526, done.
remote: Compressing objects: 100% (520/520), done.
remote: Total 526 (delta 81), reused 0 (delta 0)
fatal: read error: Invalid argument
fatal: early EOF
fatal: index-pack failed

Sometimes its a different initial fatal error but generally the same.  
Once or twice, a repeat MAY work, but often not.


Short of digging into the git source code, I did as much research 
online and tried the various config options suggestions, changing the 
packet size, etc, to no avail.


To me, this seems like a "Socket Half Close" problem.   If anyone is 
aware of what appears to be a long time "known" problem, and have a 
real solution, it would be greatly appreciated.   Otherwise, I am very 
interesting in exploring the Half Close solution as I've seen similar 
behavior in other internet hosting servers in the past.   A simple 
closesocket() wrapper funciton did the trick:



// HalfCloseSocket() performs a TCP Half Close by calling shutdown()
// which signals the remote that no more data is going to be
// sent (FIN signal). HalfCloseSocket() then goes into a
// recv() loop to wait for the remote to acknowledge the close.
// This acknowledgment comes as a recv() return value
// of zero (less).

BOOL HalfCloseSocket(SOCKET socket)
{
if (shutdown(socket,SD_SENT) != 0) {
return FALSE;
}
int ret = 0;
int msecs = 10; // poor man sanity check
char buf[8*1024];
while ((ret = recv(socket, buf,sizeof(buf),0)) > 0) {
buf[0] = 0;
buf[1] = 0;
msecs--;
if (msecs == 0) break;
}
return closesocket(socket);
}

While I rather not get into the source, I am willing to explore the 
effort if there is no other option.


Thanks for any input you can provide

Hector Santos
Santronics Software, Inc.

--
HLS

Re: [PATCH 2/2] rebase: turn on progress option by default for format-patch

[Stefan Beller]

On Wed, May 31, 2017 at 8:04 AM, Kevin Willford  wrote:
> This change passes the progress option of format-patch by
> default and passes the -q --quiet option through to the
> format-patch call so that it is respected as well.

This is not conflicting with Johannes rewrite of rebase in C?
(rebase is a huge beast IIUC)

When passing the progress option by default to formatting patches,
maybe we should use start_progress_delay in the previous patch instead
to omit the progress for short lived patch formatting sessions?
(say a delay of one second?)

Thanks,
Stefan

>
> Signed-off-by: Kevin Willford 
> ---
>  git-rebase--am.sh | 5 +++--
>  git-rebase.sh | 2 ++
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 375239341f..ab2be30abf 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -51,8 +51,9 @@ then
>  else
> rm -f "$GIT_DIR/rebased-patches"
>
> -   git format-patch -k --stdout --full-index --cherry-pick --right-only \
> -   --src-prefix=a/ --dst-prefix=b/ --no-renames 
> --no-cover-letter \
> +   git format-patch $git_format_patch_opt -k --stdout --full-index \
> +   --cherry-pick --right-only --src-prefix=a/ --dst-prefix=b/ \
> +   --no-renames --no-cover-letter --progress \
> "$revisions" ${restrict_revision+^$restrict_revision} \
> >"$GIT_DIR/rebased-patches"
> ret=$?
> diff --git a/git-rebase.sh b/git-rebase.sh
> index db1deed846..b72e319ac9 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -73,6 +73,7 @@ test "$(git config --bool rebase.stat)" = true && diffstat=t
>  autostash="$(git config --bool rebase.autostash || echo false)"
>  fork_point=auto
>  git_am_opt=
> +git_format_patch_opt=
>  rebase_root=
>  force_rebase=
>  allow_rerere_autoupdate=
> @@ -308,6 +309,7 @@ do
> --quiet)
> GIT_QUIET=t
> git_am_opt="$git_am_opt -q"
> +   git_format_patch_opt="$git_format_patch_opt -q"
> verbose=
> diffstat=
> ;;
> --
> 2.13.0.92.g73a4ce6a77
>

Re: Coloring

[Samuel Lijin]

On Wed, May 31, 2017 at 2:33 PM, Irving Rabin  wrote:
>
> Folks, I am reporting an issue with coloring of the output of Git
> commands, like status, diff, etc.
>
> Specifically, if the field is supposed to be white, it doesn't mean it
> should be literally 0xFF. It should be the color that I have
> configured as White color for my console emulator.
>
> I like light-screen terminals, and I configure my ANSI colors in the
> way that they are clearly visible on the background and clearly
> distinct between themselves. In my terminal settings background is
> light-yellow, Black is black, Yellow is brown, Red is dark red,
> Magenta is purple and White is dark gray. I set it once and I can use
> it everywhere - all Unix commands work correctly, I can edit
> highlighted source code in Vim, and all my color settings are
> respected.

Can you elaborate on how it is that you redefine your terminal color
scheme? There are multiple levels at which you can do that, which will
have some bearing on the answer.

> However Git behaves differently. When I run git diff, some of the
> output is literally white on light yellow background. It is like "we
> know what is White, so we ignore your settings". And it is quite
> irritating.
>
> Is there a way to make Git respect terminal settings and not to
> override them with absolute colors? If so, please advise. If not, then
> I guess it is a bug to fix, right?
>
> Thanks,
> Irving Rabin
> Software Developer @Edmodo
> 408-242-1299

Re: [PATCH 2/3] rebase: Add tests for console output

[Phillip Wood]

On 31/05/17 11:42, Phillip Wood wrote:

From: Phillip Wood 

Check the console output when using --autostash and the stash applies
cleanly is what we expect. To avoid this test depending on commit and
stash hashes it uses sed to replace them with XXX. The sed script also
replaces carriage returns in the output with '\r' to avoid embedded
'^M's in the expected output files. Unfortunately this means we still
end up with an embedded '^M' in the sed script which may not be
preserved when sending this. The last line of the sed script should be
+s/^M/\\r/g


Thinking about this it might be better to create the sed script with 
printf when running the test



Signed-off-by: Phillip Wood 
---
  t/t3420-rebase-autostash.sh  | 10 +-
  t/t3420/expected-success-am  |  6 ++
  t/t3420/expected-success-interactive |  4 
  t/t3420/expected-success-merge   | 30 ++
  t/t3420/remove-ids.sed   |  6 ++
  5 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 
ab8a63e8d6dc643b28eb0c74ba3f032b7532226f..886be63c6d13e1ac4197a1b185659fb3d7d7eb26
 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -53,12 +53,20 @@ testrebase() {
git checkout -b rebased-feature-branch feature-branch &&
test_when_finished git branch -D rebased-feature-branch &&
echo dirty >>file3 &&
-   git rebase$type unrelated-onto-branch &&
+   git rebase$type unrelated-onto-branch >tmp 2>&1 &&
grep unrelated file4 &&
grep dirty file3 &&
git checkout feature-branch
'
  
+	test_expect_success "rebase$type --autostash: check output" '

+   suffix=${type#\ -} && suffix=${suffix:--am} &&
+   sed -f $TEST_DIRECTORY/t3420/remove-ids.sed tmp \
+   >actual-success$suffix &&
+   test_cmp $TEST_DIRECTORY/t3420/expected-success$suffix \
+   actual-success$suffix
+   '
+
test_expect_success "rebase$type: dirty index, non-conflicting rebase" '
test_config rebase.autostash true &&
git reset --hard &&
diff --git a/t/t3420/expected-success-am b/t/t3420/expected-success-am
new file mode 100644
index 
..c18ded04f703ed2aa83d5e62589a908d0a44cf7e
--- /dev/null
+++ b/t/t3420/expected-success-am
@@ -0,0 +1,6 @@
+Created autostash: XXX
+HEAD is now at XXX third commit
+First, rewinding head to replay your work on top of it...
+Applying: second commit
+Applying: third commit
+Applied autostash.
diff --git a/t/t3420/expected-success-interactive 
b/t/t3420/expected-success-interactive
new file mode 100644
index 
..b31f71c95ddc9c18ce9956c1aadf53cedd966801
--- /dev/null
+++ b/t/t3420/expected-success-interactive
@@ -0,0 +1,4 @@
+Created autostash: XXX
+HEAD is now at XXX third commit
+Rebasing (1/2)\rRebasing (2/2)\rSuccessfully rebased and updated 
refs/heads/rebased-feature-branch.
+Applied autostash.
diff --git a/t/t3420/expected-success-merge b/t/t3420/expected-success-merge
new file mode 100644
index 
..66386f7cb5242a255d9cc64aad741e651ec7ec1e
--- /dev/null
+++ b/t/t3420/expected-success-merge
@@ -0,0 +1,30 @@
+Created autostash: XXX
+HEAD is now at XXX third commit
+First, rewinding head to replay your work on top of it...
+Merging unrelated-onto-branch with HEAD~1
+Merging:
+XXX unrelated commit
+XXX second commit
+found 1 common ancestor:
+XXX initial commit
+[detached HEAD XXX] second commit
+ Author: A U Thor 
+ Date: Thu Apr 7 15:14:13 2005 -0700
+ 2 files changed, 2 insertions(+)
+ create mode 100644 file1
+ create mode 100644 file2
+Committed: 0001 second commit
+Merging unrelated-onto-branch with HEAD~0
+Merging:
+XXX second commit
+XXX third commit
+found 1 common ancestor:
+XXX second commit
+[detached HEAD XXX] third commit
+ Author: A U Thor 
+ Date: Thu Apr 7 15:15:13 2005 -0700
+ 1 file changed, 1 insertion(+)
+ create mode 100644 file3
+Committed: 0002 third commit
+All done.
+Applied autostash.
diff --git a/t/t3420/remove-ids.sed b/t/t3420/remove-ids.sed
new file mode 100644
index 
..9e9048b02bd04d287461543d85db0bb715b89f8c
--- /dev/null
+++ b/t/t3420/remove-ids.sed
@@ -0,0 +1,6 @@
+s/^\(Created autostash: \)[0-9a-f]\{6,\}$/\1XXX/
+s/^\(HEAD is now at \)[0-9a-f]\{6,\}\( .* commit\)$/\1XXX\2/
+s/^[0-9a-f]\{6,\}\( .* commit\)$/XXX\1/
+s/\(detached HEAD \)[0-9a-f]\{6,\}/\1XXX/
+s/\(could not apply \)[0-9a-f]\{6,\}/\1XXX/g
+s//\\r/g

Re: What's cooking in git.git (May 2017, #08; Mon, 29)

[Ævar Arnfjörð Bjarmason]

On Mon, May 29, 2017 at 8:23 AM, Junio C Hamano  wrote:
> * ab/pcre-v2 (2017-05-26) 7 commits
>  - grep: add support for PCRE v2
>  - grep: un-break building with PCRE < 8.20
>  - grep: un-break building with PCRE < 8.32
>  - grep: add support for the PCRE v1 JIT API
>  - log: add -P as a synonym for --perl-regexp
>  - grep: skip pthreads overhead when using one thread
>  - grep: don't redundantly compile throwaway patterns under threading
>  (this branch uses ab/grep-preparatory-cleanup.)
>
>  Update "perl-compatible regular expression" support to enable JIT
>  and also allow linking with the newer PCRE v2 library.
>
>  Will merge to 'next'.

First a general question: When you say "will merge" in these E-Mails,
that means "before the next what's cooking in ~7 days, right? I.e. if
there's no issues in a given series does the pu->next->master cycle
take 3wks?

Anyway, the PCRE v1 set of JIT patches break builds on PCRE compiled
with --disable-jit, which is apparently Johannes's Windows
configuration.

This on top fixes it:

diff --git a/grep.h b/grep.h
index 246f6695c1..6cb21a3544 100644
--- a/grep.h
+++ b/grep.h
@@ -3,7 +3,7 @@
 #include "color.h"
 #ifdef USE_LIBPCRE1
 #include 
-#ifdef PCRE_CONFIG_JIT
+#ifdef SLJIT_CONFIG_AUTO
 #if PCRE_MAJOR >= 8 && PCRE_MINOR >= 32
 #define GIT_PCRE1_USE_JIT
 #endif

But I'm waiting on the pcre-dev list to answer my "is this use of an
internal macro really the least insane way to check for this, because
there seems to be no other way?" question:
https://lists.exim.org/lurker/message/20170531.184405.9d7b128f.en.html

I'm also going to fix this up:

diff --git a/grep.c b/grep.c
index e4c1ead104..d0bf37858a 100644
--- a/grep.c
+++ b/grep.c
@@ -502,9 +502,7 @@ static void compile_pcre2_pattern(struct grep_pat
*p, const struct grep_opt *opt
pcre2_config(PCRE2_CONFIG_JIT, >pcre2_jit_on);
if (p->pcre2_jit_on == 1) {
jitret = pcre2_jit_compile(p->pcre2_pattern,
PCRE2_JIT_COMPLETE);
-   if (!jitret)
-   p->pcre2_jit_on = 1;
-   else
+   if (jitret)
die("Couldn't JIT the PCRE2 pattern '%s', got
'%d'\n", p->pattern, jitret);
p->pcre2_jit_stack = pcre2_jit_stack_create(1, 1024 *
1024, NULL);
if (!p->pcre2_jit_stack)


And a perf test in a commit message that makes no sense since I split
up the PCRE v1 patch.

Re: [PATCH v4 00/10] The final building block for a faster rebase -i

[Ævar Arnfjörð Bjarmason]

On Tue, May 30, 2017 at 10:22 PM, Ævar Arnfjörð Bjarmason
 wrote:
> On Tue, May 30, 2017 at 5:44 PM, Johannes Schindelin
>  wrote:
>> pu does not build for me:
>>
>> 2017-05-30T11:38:50.0089681Z libgit.a(grep.o): In function `pcre1match':
>> 2017-05-30T11:38:50.0289250Z .../grep.c:411: undefined reference to 
>> `__imp_pcre_jit_exec'
>> 2017-05-30T11:38:50.0329160Z collect2.exe: error: ld returned 1 exit
>> status
>
> Ouch, looks like I've missed some spot in my pcre1 jit series. What's
> the PCRE version you have? This is somehow doing the wrong thing with
> this bit in grep.h:
>
> #include 
> #ifdef PCRE_CONFIG_JIT
> #if PCRE_MAJOR >= 8 && PCRE_MINOR >= 32
> #define GIT_PCRE1_USE_JIT
> [...]

I've found what the problem is myself. I'll submit a new version of
the series that fixes this.

Re: [PATCH 1/2] format-patch: have progress option while generating patches

[Stefan Beller]

On Wed, May 31, 2017 at 8:04 AM, Kevin Willford  wrote:
> When generating patches for the rebase command if the user does
> not realize the branch they are rebasing onto is thousands of
> commits different there is no progress indication after initial
> rewinding message.
>
> This patch allows a progress option to be passed to format-patch
> so that the user can be informed the progress of generating the
> patch.  This option will then be used by the rebase command when
> calling format-patch.

After reading the code, I was looking for some explanation
on the underlying assumptions, such as:

  The progress meter as presented in this patch assumes the thousands of
  patches to have a fine granularity as well as assuming to require all the
  same amount of work/time for each, such that a steady progress bar
  is achieved.

  We do not want to estimate the time for each patch based e.g.
  on their size or number of touched files (or parents) as that is too
  expensive for just a progress meter.


>
> Signed-off-by: Kevin Willford 
> ---
>  Documentation/git-format-patch.txt |  8 
>  builtin/log.c  | 10 ++
>  2 files changed, 18 insertions(+)
>
> diff --git a/Documentation/git-format-patch.txt 
> b/Documentation/git-format-patch.txt
> index c890328b02..ee5f99f606 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -23,6 +23,7 @@ SYNOPSIS
>[(--reroll-count|-v) ]
>[--to=] [--cc=]
>[--[no-]cover-letter] [--quiet] [--notes[=]]
> +  [--progress]
>[]
>[  |  ]
>
> @@ -260,6 +261,7 @@ you can use `--suffix=-patch` to get 
> `0001-description-of-my-change-patch`.
>  -q::
>  --quiet::
> Do not print the names of the generated files to standard output.
> +   Progress is not reported to the standard error stream.
>
>  --no-binary::
> Do not output contents of changes in binary files, instead
> @@ -283,6 +285,12 @@ you can use `--suffix=-patch` to get 
> `0001-description-of-my-change-patch`.
> range are always formatted as creation patches, independently
> of this flag.
>
> +--progress::
> +   Progress status is reported on the standard error stream
> +   by default when it is attached to a terminal, unless -q
> +   is specified. This flag forces progress status even if the
> +   standard error stream is not directed to a terminal.
> +
>  CONFIGURATION
>  -
>  You can specify extra mail header lines to be added to each message,
> diff --git a/builtin/log.c b/builtin/log.c
> index 631fbc984f..02c50431b6 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -26,6 +26,7 @@
>  #include "version.h"
>  #include "mailmap.h"
>  #include "gpg-interface.h"
> +#include "progress.h"
>
>  /* Set a default date-time format for git log ("log.date" config variable) */
>  static const char *default_date_mode = NULL;
> @@ -1409,6 +1410,8 @@ int cmd_format_patch(int argc, const char **argv, const 
> char *prefix)
> char *branch_name = NULL;
> char *base_commit = NULL;
> struct base_tree_info bases;
> +   int show_progress = 0;
> +   struct progress *progress = NULL;
>
> const struct option builtin_format_patch_options[] = {
> { OPTION_CALLBACK, 'n', "numbered", , NULL,
> @@ -1480,6 +1483,8 @@ int cmd_format_patch(int argc, const char **argv, const 
> char *prefix)
> OPT_FILENAME(0, "signature-file", _file,
> N_("add a signature from a file")),
> OPT__QUIET(, N_("don't print the patch filenames")),
> +   OPT_BOOL(0, "progress", _progress,
> +N_("show progress")),
> OPT_END()
> };
>
> @@ -1739,8 +1744,12 @@ int cmd_format_patch(int argc, const char **argv, 
> const char *prefix)
> start_number--;
> }
> rev.add_signoff = do_signoff;
> +
> +   if (show_progress && !quiet)
> +   progress = start_progress(_("Generating patch"), total);
> while (0 <= --nr) {
> int shown;
> +   display_progress(progress, total - nr);
> commit = list[nr];
> rev.nr = total - nr + (start_number - 1);
> /* Make the second and subsequent mails replies to the first 
> */
> @@ -1805,6 +1814,7 @@ int cmd_format_patch(int argc, const char **argv, const 
> char *prefix)
> if (!use_stdout)
> fclose(rev.diffopt.file);
> }
> +   stop_progress();
> free(list);
> free(branch_name);
> string_list_clear(_to, 0);
> --
> 2.13.0.92.g73a4ce6a77
>

Coloring

[Irving Rabin]

Folks, I am reporting an issue with coloring of the output of Git
commands, like status, diff, etc.

Specifically, if the field is supposed to be white, it doesn't mean it
should be literally 0xFF. It should be the color that I have
configured as White color for my console emulator.

I like light-screen terminals, and I configure my ANSI colors in the
way that they are clearly visible on the background and clearly
distinct between themselves. In my terminal settings background is
light-yellow, Black is black, Yellow is brown, Red is dark red,
Magenta is purple and White is dark gray. I set it once and I can use
it everywhere - all Unix commands work correctly, I can edit
highlighted source code in Vim, and all my color settings are
respected.

However Git behaves differently. When I run git diff, some of the
output is literally white on light yellow background. It is like "we
know what is White, so we ignore your settings". And it is quite
irritating.

Is there a way to make Git respect terminal settings and not to
override them with absolute colors? If so, please advise. If not, then
I guess it is a bug to fix, right?

Thanks,
Irving Rabin
Software Developer @Edmodo
408-242-1299

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Stefan Beller]

On Tue, May 30, 2017 at 10:31 AM, Brandon Williams  wrote:
>
> Signed-off-by: Brandon Williams 
> ---
>  combine-diff.c | 10 +-
>  diff.h |  4 ++--
>  tree-diff.c| 63 
> +-
>  3 files changed, 39 insertions(+), 38 deletions(-)
>
> diff --git a/combine-diff.c b/combine-diff.c
> index 04c4ae856..ec9d93044 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -1364,22 +1364,22 @@ static struct combine_diff_path *find_paths_multitree(
> struct diff_options *opt)
>  {
> int i, nparent = parents->nr;
> -   const unsigned char **parents_sha1;
> +   const struct object_id **parents_oid;
> struct combine_diff_path paths_head;
> struct strbuf base;
>
> -   ALLOC_ARRAY(parents_sha1, nparent);
> +   ALLOC_ARRAY(parents_oid, nparent);
> for (i = 0; i < nparent; i++)
> -   parents_sha1[i] = parents->oid[i].hash;
> +   parents_oid[i] = >oid[i];

I have the impression that we could get away with one layer less
of indirection. Previously we had a heap allocated array (*) of (char*),
now we'd have a an array (*) of pointers(*) of the oid struct, that
is a (char[]) essentially. Maybe I am just confused?