[PATCH 4/7] dir: hide untracked contents of untracked dirs

[Samuel Lijin]

When we taught read_directory_recursive() to recurse into untracked
directories in search of ignored files given DIR_SHOW_IGNORED_TOO, that
had the side effect of teaching it to collect the untracked contents of
untracked directories. It does not make sense to return these, so we
teach read_directory() to strip dir->entries of any such untracked
contents.
---
 dir.c | 44 
 1 file changed, 44 insertions(+)

diff --git a/dir.c b/dir.c
index 25cb9eadf..f0ddb4608 100644
--- a/dir.c
+++ b/dir.c
@@ -2075,6 +2075,50 @@ int read_directory(struct dir_struct *dir, const char 
*path,
read_directory_recursive(dir, path, len, untracked, 0, 
pathspec);
QSORT(dir->entries, dir->nr, cmp_name);
QSORT(dir->ignored, dir->ignored_nr, cmp_name);
+
+   // if collecting ignored files, never consider a directory containing
+   // ignored files to be untracked
+   if (dir->flags & DIR_SHOW_IGNORED_TOO) {
+   int i, j, nr_removed = 0;
+
+   // remove from dir->entries untracked contents of untracked dirs
+   for (i = 0; i < dir->nr; i++) {
+   if (!dir->entries[i])
+   continue;
+
+   for (j = i + 1; j < dir->nr; j++) {
+   if (!dir->entries[j])
+   continue;
+   if (check_contains(dir->entries[i], 
dir->entries[j])) {
+   nr_removed++;
+   free(dir->entries[j]);
+   dir->entries[j] = NULL;
+   }
+   else {
+   break;
+   }
+   }
+   }
+
+   // strip dir->entries of NULLs
+   if (nr_removed) {
+   for (i = 0;;) {
+   while (i < dir->nr && dir->entries[i])
+   i++;
+   if (i == dir->nr)
+   break;
+   j = i;
+   while (j < dir->nr && !dir->entries[j])
+   j++;
+   if (j == dir->nr)
+   break;
+   dir->entries[i] = dir->entries[j];
+   dir->entries[j] = NULL;
+   }
+   dir->nr -= nr_removed;
+   }
+   }
+
if (dir->untracked) {
static struct trace_key trace_untracked_stats = 
TRACE_KEY_INIT(UNTRACKED_STATS);
trace_printf_key(_untracked_stats,
-- 
2.12.2

[PATCH 0/7] Keep git clean -d from inadvertently removing ignored files

[Samuel Lijin]

This patch series fixes the bug where git clean -d culls directories containing
only untracked and ignored files, by first teaching read_directory() and
read_directory_recursive() to search "untracked" directories (read: directories
*treated* as untracked because they only contain untracked and ignored files)
for ignored contents, and then teaching cmd_clean() to skip untracked
directories containing ignored files.

This does, however, introduce a breaking change in the behavior of git status:
when invoked with --ignored, git status will now return ignored files in an
untracked directory, whereas previously it would not.

First patches to the actual C code that I'm sending out! :D Looking forward to
feedback - the changes I made in read_directory_recursive() and read_directory()
feel a bit hacky, but I'm not sure how to get around that.

Samuel Lijin (7):
  t7300: skip untracked dirs containing ignored files
  dir: recurse into untracked dirs for ignored files
  dir: add method to check if a dir_entry lexically contains another
  dir: hide untracked contents of untracked dirs
  dir: change linkage of cmp_name() and check_contains()
  builtin/clean: teach clean -d to skip dirs containing ignored files
  t7061: check for ignored file in untracked dir

 builtin/clean.c| 24 --
 dir.c  | 61 --
 dir.h  |  3 +++
 t/t7061-wtstatus-ignore.sh |  1 +
 t/t7300-clean.sh   | 10 
 5 files changed, 95 insertions(+), 4 deletions(-)

-- 
2.12.2

[PATCH 3/7] dir: add method to check if a dir_entry lexically contains another

[Samuel Lijin]

Introduce a method that allows us to check if one dir_entry corresponds
to a path which contains the path corresponding to another dir_entry.
---
 dir.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/dir.c b/dir.c
index 6bd0350e9..25cb9eadf 100644
--- a/dir.c
+++ b/dir.c
@@ -1852,6 +1852,14 @@ static int cmp_name(const void *p1, const void *p2)
return name_compare(e1->name, e1->len, e2->name, e2->len);
 }
 
+// check if *out lexically contains *in
+static int check_contains(const struct dir_entry *out, const struct dir_entry 
*in)
+{
+   return (out->len < in->len) &&
+   (out->name[out->len - 1] == '/') &&
+   !memcmp(out->name, in->name, out->len);
+}
+
 static int treat_leading_path(struct dir_struct *dir,
  const char *path, int len,
  const struct pathspec *pathspec)
-- 
2.12.2

[PATCH 6/7] builtin/clean: teach clean -d to skip dirs containing ignored files

[Samuel Lijin]

There is an implicit assumption that a directory containing only
untracked and ignored files should itself be considered untracked. This
makes sense in use cases where we're asking if a directory should be
added to the git database, but not when we're asking if a directory can
be safely removed from the working tree; as a result, clean -d would
assume that an "untracked" directory containing ignored files could be
deleted.

To get around this, we teach clean -d to collect ignored files and skip
over so-called "untracked" directories if they contain any ignored
files.
---
 builtin/clean.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index d861f836a..368e19427 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -859,7 +859,7 @@ static void interactive_main_loop(void)
 
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
-   int i, res;
+   int i, j, res;
int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
@@ -911,6 +911,9 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
  " refusing to clean"));
}
 
+   if (remove_directories)
+   dir.flags |= DIR_SHOW_IGNORED_TOO;
+
if (force > 1)
rm_flags = 0;
 
@@ -932,7 +935,7 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
 
fill_directory(, );
 
-   for (i = 0; i < dir.nr; i++) {
+   for (j = i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
int matches = 0;
struct stat st;
@@ -954,10 +957,27 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
matches != MATCHED_EXACTLY)
continue;
 
+   // skip any dir.entries which contains a dir.ignored
+   for (; j < dir.ignored_nr; j++) {
+   if (cmp_name([i],
+   [j]) < 0)
+   break;
+   }
+   if ((j < dir.ignored_nr) &&
+   check_contains(dir.entries[i], dir.ignored[j])) 
{
+   continue;
+   }
+
rel = relative_path(ent->name, prefix, );
string_list_append(_list, rel);
}
 
+   for (i = 0; i < dir.nr; i++)
+   free(dir.entries[i]);
+
+   for (i = 0; i < dir.ignored_nr; i++)
+   free(dir.ignored[i]);
+
if (interactive && del_list.nr > 0)
interactive_main_loop();
 
-- 
2.12.2

[PATCH 7/7] t7061: check for ignored file in untracked dir

[Samuel Lijin]

---
 t/t7061-wtstatus-ignore.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index cdc0747bf..fc6013ba3 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -9,6 +9,7 @@ cat >expected <<\EOF
 ?? actual
 ?? expected
 ?? untracked/
+!! untracked/ignored
 EOF
 
 test_expect_success 'status untracked directory with --ignored' '
-- 
2.12.2

[PATCH 2/7] dir: recurse into untracked dirs for ignored files

[Samuel Lijin]

We consider directories containing only untracked and ignored files to
be themselves untracked, which in the usual case means we don't have to
search these directories. This is problematic when we want to collect
ignored files with DIR_SHOW_IGNORED_TOO, though, so we teach
read_directory_recursive() to recurse into untracked directories to find
the ignored files they contain when DIR_SHOW_IGNORED_TOO is set. This
has the side effect of also collecting all untracked files in untracked
directories as well.
---
 dir.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index f451bfa48..6bd0350e9 100644
--- a/dir.c
+++ b/dir.c
@@ -1784,7 +1784,12 @@ static enum path_treatment 
read_directory_recursive(struct dir_struct *dir,
dir_state = state;
 
/* recurse into subdir if instructed by treat_path */
-   if (state == path_recurse) {
+   if ((state == path_recurse) ||
+   ((get_dtype(cdir.de, path.buf, path.len) == 
DT_DIR) &&
+(state == path_untracked) &&
+(dir->flags & DIR_SHOW_IGNORED_TOO))
+   )
+   {
struct untracked_cache_dir *ud;
ud = lookup_untracked(dir->untracked, untracked,
  path.buf + baselen,
-- 
2.12.2

[PATCH 5/7] dir: change linkage of cmp_name() and check_contains()

[Samuel Lijin]

---
 dir.c | 4 ++--
 dir.h | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index f0ddb4608..91103b561 100644
--- a/dir.c
+++ b/dir.c
@@ -1844,7 +1844,7 @@ static enum path_treatment 
read_directory_recursive(struct dir_struct *dir,
return dir_state;
 }
 
-static int cmp_name(const void *p1, const void *p2)
+int cmp_name(const void *p1, const void *p2)
 {
const struct dir_entry *e1 = *(const struct dir_entry **)p1;
const struct dir_entry *e2 = *(const struct dir_entry **)p2;
@@ -1853,7 +1853,7 @@ static int cmp_name(const void *p1, const void *p2)
 }
 
 // check if *out lexically contains *in
-static int check_contains(const struct dir_entry *out, const struct dir_entry 
*in)
+int check_contains(const struct dir_entry *out, const struct dir_entry *in)
 {
return (out->len < in->len) &&
(out->name[out->len - 1] == '/') &&
diff --git a/dir.h b/dir.h
index bf23a470a..1ddd8b611 100644
--- a/dir.h
+++ b/dir.h
@@ -326,6 +326,9 @@ static inline int dir_path_match(const struct dir_entry 
*ent,
  has_trailing_dir);
 }
 
+int cmp_name(const void *p1, const void *p2);
+int check_contains(const struct dir_entry *out, const struct dir_entry *in);
+
 void untracked_cache_invalidate_path(struct index_state *, const char *);
 void untracked_cache_remove_from_index(struct index_state *, const char *);
 void untracked_cache_add_to_index(struct index_state *, const char *);
-- 
2.12.2

[PATCH 1/7] t7300: skip untracked dirs containing ignored files

[Samuel Lijin]

If git sees a directory which contains only untracked and ignored
files, clean -d should not remove that directory.
---
 t/t7300-clean.sh | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
index b89fd2a6a..948a455e8 100755
--- a/t/t7300-clean.sh
+++ b/t/t7300-clean.sh
@@ -653,4 +653,14 @@ test_expect_success 'git clean -d respects pathspecs 
(pathspec is prefix of dir)
test_path_is_dir foobar
 '
 
+test_expect_success 'git clean -d skips untracked dirs containing ignored 
files' '
+   echo /foo/bar >.gitignore &&
+   rm -rf foo &&
+   mkdir -p foo &&
+   touch foo/bar &&
+   git clean -df &&
+   test_path_is_file foo/bar &&
+   test_path_is_dir foo
+'
+
 test_done
-- 
2.12.2

Su cuenta bancaria esta bloqueada

[BAC Credomatic]

Su cuenta bancaria esta bloqueada. Para desbloquear tu cuenta, haz click aqui: 
http://baccredomaatic.com

Re: Git checkout issue - deleting file without apparent reason

[Jeff King]

On Tue, May 02, 2017 at 09:33:02PM -0400, Paul van Wichen wrote:

> We are having a strange issue that we haven't been able to pin down.
> Scenario: master branch and feature branch both have a specific file.
> 1. Master checked out.
> 2. git status show no changes / clean staging area.
> 3. Checkout feature branch .
> 4. git status show no changes / clean staging area.
> 5. Checkout master again.
> 6. git status shows that a file has been deleted (i.e. the file was
> removed from the file system and the staging area shows it as
> deleted).
> 
> The file exists in both the feature branch and the master branch. As
> best as we can tell, the file is identical on both commits.
> The issue occurs on multiple platforms - tested on Windows and OS X.
> It only occurs for 1 specific file.

Just a guess, but might there be another file in the repository whose
name differs only in case? On a case-insensitive filesystem that can
cause gremlins like this, because the filesystem cannot represent all of
the states in the git tree.

-Peff

Re: Git checkout issue - deleting file without apparent reason

[Bryan Turner]

On Tue, May 2, 2017 at 6:33 PM, Paul van Wichen
 wrote:
> Hi,
>
> We are having a strange issue that we haven't been able to pin down.
> Scenario: master branch and feature branch both have a specific file.
> 1. Master checked out.
> 2. git status show no changes / clean staging area.
> 3. Checkout feature branch .
> 4. git status show no changes / clean staging area.
> 5. Checkout master again.
> 6. git status shows that a file has been deleted (i.e. the file was
> removed from the file system and the staging area shows it as
> deleted).
>
> The file exists in both the feature branch and the master branch. As
> best as we can tell, the file is identical on both commits.
> The issue occurs on multiple platforms - tested on Windows and OS X.
> It only occurs for 1 specific file.
>
> Based on the activity of the file, nothing stands out as unusual.
>
> How we go about troubleshooting this and determining the cause?
>
> Thanks,
> Paul van Wichen.

Git checkout issue - deleting file without apparent reason

[Paul van Wichen]

Hi,

We are having a strange issue that we haven't been able to pin down.
Scenario: master branch and feature branch both have a specific file.
1. Master checked out.
2. git status show no changes / clean staging area.
3. Checkout feature branch .
4. git status show no changes / clean staging area.
5. Checkout master again.
6. git status shows that a file has been deleted (i.e. the file was
removed from the file system and the staging area shows it as
deleted).

The file exists in both the feature branch and the master branch. As
best as we can tell, the file is identical on both commits.
The issue occurs on multiple platforms - tested on Windows and OS X.
It only occurs for 1 specific file.

Based on the activity of the file, nothing stands out as unusual.

How we go about troubleshooting this and determining the cause?

Thanks,
Paul van Wichen.

Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

[Liam Beguin]

Hi Johannes, 

On Tue, 2017-05-02 at 17:48 +0200, Johannes Schindelin wrote:
> Hi Liam,
> 
> On Tue, 2 May 2017, Liam Beguin wrote:
> 
> > Add the 'rebase.abbreviateCommands' configuration option to allow `git
> > rebase -i` to default to the single-letter command-names in the todo
> > list.
> > 
> > Using single-letter command-names can present two benefits.  First, it
> > makes it easier to change the action since you only need to replace a
> > single character (i.e.: in vim "r" instead of
> > "ciw").  Second, using this with a large enough value of
> > 'core.abbrev' enables the lines of the todo list to remain aligned
> > making the files easier to read.
> > 
> > Changes from v1 to v2:
> >  - Improve Documentation and commit message
> > 
> > Changes from v2 to v3:
> >  - Transform a single patch into a series
> >  - change option name from 'rebase.abbrevCmd' to 'rebase.abbreviateCommands'
> >  - abbreviate all commands (not just pick)
> >  - teach `git rebase -i --autosquash` to recognise single-letter 
> > command-names
> >  - move rebase configuration documentation to 
> > Documentation/rebase-config.txt
> >  - update Documentation to use the preferred naming for the todo list
> >  - update Documentation and commit messages according to feedback
> 
> Thank you for this pleasant read. It is an excellent contribution, and the
> way you communicate what you changed and why is very welcome.
> 

Thanks! and thank you for the support and help.

> I offered a couple of comments, my biggest one probably being that this
> patch series is crossing paths with my patch series that tries to move
> more functionality out of the git-rebase--interactive.sh script into the
> git-rebase--helper that is written in C, closely followed by my suggestion
> to fold at least part of the functionality into the SHA-1
> collapsing/expanding.
> 

I've seen a few messages about this migration, but I'm not yet sure I understand
the difference between the shell and the C implementations. Is the C version 
going
to replace 'git-rebase--interactive.sh'?

> If your patch series "wins", I can easily forward-port your changes to the
> rebase-i-extra branch, but it may actually make sense to build on top of
> the rebase-i-extra branch to begin with. If you agree: I pushed the
> proposed change to the `rebase-i-extra+abbrev` branch at
> https://github.com/dscho/git.
> 

If 'git-rebase--interactive.sh' is bound to be replaced, I could
just shrink this to the Documentation cleanup (patches 4 and 5)
and rework the rest on top of your new implementation.

> I look forward to see this story unfold!
> 
> Ciao,
> Johannes

Thanks, 
Liam

Re: [PATCH v3 3/6] rebase -i: add short command-name in --autosquash

[Liam Beguin]

Hi Johannes,

On Tue, 2017-05-02 at 17:34 +0200, Johannes Schindelin wrote:
> Hi Liam,
> 
> On Tue, 2 May 2017, Liam Beguin wrote:
> 
> > teach `git rebase -i` to recognise short command-names when using the
> > '--autosquash' option. This allows commit with titles beginning with
> > "s! ..." and "f! ..." to be treated the same way as "squash! ..." and
> > "fixup! ..." respectively.
> 
> As the recommended way to generate those commits is by using the
> --fixup/--squash options of git-commit, and as there is *a much higher*
> chance of false positives when using a very short tell-tale such as `f!`
> (which could be an abbreviation for an expletive, likewise `s!`), I do not
> think we will want this change.
> 
> Let's keep handling just fixup!/squash!
> 
> Ciao,
> Johannes

I was not quite sure about this change. My guess was that since --autosquash
needs the whole commit title to find a match, the short version had little
probability of generating a false positive. I thought it made sense to include
the change in this series, but I understand why it's probably not a good idea
to take it. I'll remove it in the next series.

Thanks, 
Liam

[PATCH 18/24] cache.h: drop add_to_cache

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/commit.c | 2 +-
 cache.h  | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index fa962c4f86..ac4fe97d36 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -287,7 +287,7 @@ static void add_remove_files(struct string_list *list)
continue;
 
if (!lstat(p->string, )) {
-   if (add_to_cache(p->string, , 0))
+   if (add_to_index(_index, p->string, , 0))
die(_("updating files failed"));
} else
remove_file_from_index(_index, p->string);
diff --git a/cache.h b/cache.h
index 88c6f8a7df..bc49defc27 100644
--- a/cache.h
+++ b/cache.h
@@ -354,7 +354,6 @@ extern void free_name_hash(struct index_state *istate);
 
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
-#define add_to_cache(path, st, flags) add_to_index(_index, (path), (st), 
(flags))
 #define add_file_to_cache(path, flags) add_file_to_index(_index, (path), 
(flags))
 #define chmod_cache_entry(ce, flip) chmod_index_entry(_index, (ce), (flip))
 #define refresh_cache(flags) refresh_index(_index, (flags), NULL, NULL, 
NULL)
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 16/24] cache.h: drop rename_cache_entry_at

[Stefan Beller]

One define was unused, the other occurs just once. Easy patch.

Signed-off-by: Stefan Beller 
---
 builtin/mv.c | 2 +-
 cache.h  | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 40e40bb098..1a3c61994c 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -287,7 +287,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
pos = index_name_pos(_index, src, strlen(src));
assert(pos >= 0);
if (!show_only)
-   rename_cache_entry_at(pos, dst);
+   rename_index_entry_at(_index, pos, dst);
}
 
if (gitmodules_modified)
diff --git a/cache.h b/cache.h
index 54c1bbf0d1..4c12ff265e 100644
--- a/cache.h
+++ b/cache.h
@@ -354,8 +354,6 @@ extern void free_name_hash(struct index_state *istate);
 
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
-#define rename_cache_entry_at(pos, new_name) rename_index_entry_at(_index, 
(pos), (new_name))
-#define remove_cache_entry_at(pos) remove_index_entry_at(_index, (pos))
 #define remove_file_from_cache(path) remove_file_from_index(_index, (path))
 #define add_to_cache(path, st, flags) add_to_index(_index, (path), (st), 
(flags))
 #define add_file_to_cache(path, flags) add_file_to_index(_index, (path), 
(flags))
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 17/24] cache.h: drop remove_file_from_cache

[Stefan Beller]

coccinelle patch:
@@ expression path; @@
-remove_file_from_cache(path)
+remove_file_from_index(_index, path)

Additionally drop the define from cache.h manually.

Signed-off-by: Stefan Beller 
---
 apply.c|  4 ++--
 builtin/commit.c   |  2 +-
 builtin/reset.c|  2 +-
 builtin/rm.c   |  2 +-
 builtin/update-index.c | 10 +-
 cache.h|  1 -
 merge-recursive.c  | 16 
 7 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/apply.c b/apply.c
index 66d4969e72..8a61f19d03 100644
--- a/apply.c
+++ b/apply.c
@@ -4230,7 +4230,7 @@ static int build_fake_ancestor(struct apply_state *state, 
struct patch *list)
  static int remove_file(struct apply_state *state, struct patch *patch, int 
rmdir_empty)
  {
 if (state->update_index) {
-if (remove_file_from_cache(patch->old_name) < 0)
+if (remove_file_from_index(_index, patch->old_name) < 0)
 return error(_("unable to remove %s from index"), 
patch->old_name);
 }
 if (!state->cached) {
@@ -4418,7 +4418,7 @@ static int add_conflicted_stages_file(struct apply_state 
*state,
ce_size = cache_entry_size(namelen);
mode = patch->new_mode ? patch->new_mode : (S_IFREG | 0644);
 
-   remove_file_from_cache(patch->new_name);
+   remove_file_from_index(_index, patch->new_name);
for (stage = 1; stage < 4; stage++) {
if (is_null_oid(>threeway_stage[stage - 1]))
continue;
diff --git a/builtin/commit.c b/builtin/commit.c
index ef12ea3991..fa962c4f86 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -290,7 +290,7 @@ static void add_remove_files(struct string_list *list)
if (add_to_cache(p->string, , 0))
die(_("updating files failed"));
} else
-   remove_file_from_cache(p->string);
+   remove_file_from_index(_index, p->string);
}
 }
 
diff --git a/builtin/reset.c b/builtin/reset.c
index 0e19d6e8d5..90c56b46f2 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -125,7 +125,7 @@ static void update_index_from_diff(struct diff_queue_struct 
*q,
struct cache_entry *ce;
 
if (is_missing && !intent_to_add) {
-   remove_file_from_cache(one->path);
+   remove_file_from_index(_index, one->path);
continue;
}
 
diff --git a/builtin/rm.c b/builtin/rm.c
index c77c941ef0..f479100298 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -343,7 +343,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!quiet)
printf("rm '%s'\n", path);
 
-   if (remove_file_from_cache(path))
+   if (remove_file_from_index(_index, path))
die(_("git rm: unable to remove %s"), path);
}
 
diff --git a/builtin/update-index.c b/builtin/update-index.c
index e0738f74bf..8c4911e920 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -243,7 +243,7 @@ static int remove_one_path(const char *path)
 {
if (!allow_remove)
return error("%s: does not exist and --remove not passed", 
path);
-   if (remove_file_from_cache(path))
+   if (remove_file_from_index(_index, path))
return error("%s: cannot remove from the index", path);
return 0;
 }
@@ -376,7 +376,7 @@ static int process_path(const char *path)
 * so updating it does not make sense.
 * On the other hand, removing it from index should work
 */
-   if (allow_remove && remove_file_from_cache(path))
+   if (allow_remove && remove_file_from_index(_index, path))
return error("%s: cannot remove from the index", path);
return 0;
}
@@ -459,7 +459,7 @@ static void update_one(const char *path)
}
 
if (force_remove) {
-   if (remove_file_from_cache(path))
+   if (remove_file_from_index(_index, path))
die("git update-index: unable to remove %s", path);
report("remove '%s'", path);
return;
@@ -541,7 +541,7 @@ static void read_index_info(int nul_term_line)
 
if (!mode) {
/* mode == 0 means there is no such path -- remove */
-   if (remove_file_from_cache(path_name))
+   if (remove_file_from_index(_index, path_name))
die("git update-index: unable to remove %s",
ptr);
}
@@ -658,7 +658,7 @@ static int unresolve_one(const char *path)
goto free_return;
}
 
-   remove_file_from_cache(path);
+   remove_file_from_index(_index, path);
  

[PATCH 14/24] cache.h: drop cache_name_pos

[Stefan Beller]

coccinelle patch:
@@ expression name, namelen; @@
-cache_name_pos(name, namelen)
+index_name_pos(_index, name, namelen)

Additionally manual editing:
 * drop the define from cache.h.

Signed-off-by: Stefan Beller 
---
 apply.c  |  9 +
 builtin/blame.c  |  4 ++--
 builtin/checkout-index.c |  2 +-
 builtin/checkout.c   |  2 +-
 builtin/ls-files.c   |  7 ---
 builtin/merge-index.c|  2 +-
 builtin/mv.c |  8 
 builtin/rm.c |  4 ++--
 builtin/update-index.c   | 10 +-
 cache.h  |  1 -
 diff.c   |  2 +-
 dir.c|  8 
 merge-recursive.c|  4 ++--
 pathspec.c   |  2 +-
 rerere.c |  2 +-
 sha1_name.c  |  6 +++---
 submodule.c  |  2 +-
 wt-status.c  |  4 ++--
 18 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/apply.c b/apply.c
index eb8eaeabec..bb1cd77c66 100644
--- a/apply.c
+++ b/apply.c
@@ -3496,7 +3496,7 @@ static int load_current(struct apply_state *state,
if (!patch->is_new)
die("BUG: patch to %s is not a creation", patch->old_name);
 
-   pos = cache_name_pos(name, strlen(name));
+   pos = index_name_pos(_index, name, strlen(name));
if (pos < 0)
return error(_("%s: does not exist in index"), name);
ce = the_index.cache[pos];
@@ -3665,7 +3665,8 @@ static int check_preimage(struct apply_state *state,
}
 
if (state->check_index && !previous) {
-   int pos = cache_name_pos(old_name, strlen(old_name));
+   int pos = index_name_pos(_index, old_name,
+strlen(old_name));
if (pos < 0) {
if (patch->is_new < 0)
goto is_new;
@@ -3721,7 +3722,7 @@ static int check_to_create(struct apply_state *state,
struct stat nst;
 
if (state->check_index &&
-   cache_name_pos(new_name, strlen(new_name)) >= 0 &&
+   index_name_pos(_index, new_name, strlen(new_name)) >= 0 &&
!ok_if_exists)
return EXISTS_IN_INDEX;
if (state->cached)
@@ -3998,7 +3999,7 @@ static int get_current_oid(struct apply_state *state, 
const char *path,
 
if (read_apply_cache(state) < 0)
return -1;
-   pos = cache_name_pos(path, strlen(path));
+   pos = index_name_pos(_index, path, strlen(path));
if (pos < 0)
return -1;
oidcpy(oid, _index.cache[pos]->oid);
diff --git a/builtin/blame.c b/builtin/blame.c
index b47aae25d4..c71d9a3340 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2239,7 +2239,7 @@ static void verify_working_tree_path(struct commit 
*work_tree, const char *path)
return;
}
 
-   pos = cache_name_pos(path, strlen(path));
+   pos = index_name_pos(_index, path, strlen(path));
if (pos >= 0)
; /* path is in the index */
else if (-1 - pos < the_index.cache_nr &&
@@ -2399,7 +2399,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
 
len = strlen(path);
if (!mode) {
-   int pos = cache_name_pos(path, len);
+   int pos = index_name_pos(_index, path, len);
if (0 <= pos)
mode = the_index.cache[pos]->ce_mode;
else
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 1c3dcc1a8b..e8fc24b2ce 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -46,7 +46,7 @@ static void write_tempfile_record(const char *name, const 
char *prefix)
 static int checkout_file(const char *name, const char *prefix)
 {
int namelen = strlen(name);
-   int pos = cache_name_pos(name, namelen);
+   int pos = index_name_pos(_index, name, namelen);
int has_same_name = 0;
int did_checkout = 0;
int errs = 0;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a6cd8c0f37..039d3d296b 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -107,7 +107,7 @@ static int update_some(const unsigned char *sha1, struct 
strbuf *base,
 * entry in place. Whether it is UPTODATE or not, checkout_entry will
 * do the right thing.
 */
-   pos = cache_name_pos(ce->name, ce->ce_namelen);
+   pos = index_name_pos(_index, ce->name, ce->ce_namelen);
if (pos >= 0) {
struct cache_entry *old = the_index.cache[pos];
if (ce->ce_mode == old->ce_mode &&
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 6f7ecec1b0..3507490d3e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -134,7 +134,8 @@ static void show_killed_files(struct dir_struct *dir)
/* If ent->name is prefix of an entry in the
 * cache, it 

[PATCH 19/24] cache.h: drop add_file_to_cache

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 cache.h | 1 -
 rerere.c| 2 +-
 submodule.c | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index bc49defc27..a9b059913e 100644
--- a/cache.h
+++ b/cache.h
@@ -354,7 +354,6 @@ extern void free_name_hash(struct index_state *istate);
 
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
-#define add_file_to_cache(path, flags) add_file_to_index(_index, (path), 
(flags))
 #define chmod_cache_entry(ce, flip) chmod_index_entry(_index, (ce), (flip))
 #define refresh_cache(flags) refresh_index(_index, (flags), NULL, NULL, 
NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(_index, (ce), (st), 
(options))
diff --git a/rerere.c b/rerere.c
index b6d84b8461..0ada0ef247 100644
--- a/rerere.c
+++ b/rerere.c
@@ -712,7 +712,7 @@ static void update_paths(struct string_list *update)
 
for (i = 0; i < update->nr; i++) {
struct string_list_item *item = >items[i];
-   if (add_file_to_cache(item->string, 0))
+   if (add_file_to_index(_index, item->string, 0))
exit(128);
fprintf(stderr, "Staged '%s' using previous resolution.\n",
item->string);
diff --git a/submodule.c b/submodule.c
index 148194831d..6587bc0d84 100644
--- a/submodule.c
+++ b/submodule.c
@@ -119,7 +119,7 @@ int remove_path_from_gitmodules(const char *path)
 
 void stage_updated_gitmodules(void)
 {
-   if (add_file_to_cache(".gitmodules", 0))
+   if (add_file_to_index(_index, ".gitmodules", 0))
die(_("staging updated .gitmodules failed"));
 }
 
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 21/24] cache.h: drop refresh_cache

[Stefan Beller]

coccinelle patch:
@@ expression flags; @@
-refresh_cache(flags)
+refresh_index(_index, flags, NULL, NULL, NULL)

Additionally drop the define from cache.h manually.

This is a commit where I think the macro expansion is maybe silly, but
for reasons outlined in the first patch of the series, we still follow
through.

Signed-off-by: Stefan Beller 
---
 builtin/am.c   | 4 ++--
 builtin/checkout.c | 2 +-
 builtin/commit.c   | 6 +++---
 builtin/diff.c | 3 ++-
 builtin/merge.c| 8 
 builtin/update-index.c | 3 ++-
 cache.h| 1 -
 merge.c| 2 +-
 wt-status.c| 2 +-
 9 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 38fc4566f7..a7388ec4e0 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1122,7 +1122,7 @@ static void refresh_and_write_cache(void)
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
-   refresh_cache(REFRESH_QUIET);
+   refresh_index(_index, REFRESH_QUIET, NULL, NULL, NULL);
if (write_locked_index(_index, lock_file, COMMIT_LOCK))
die(_("unable to write index file"));
 }
@@ -1977,7 +1977,7 @@ static int fast_forward_to(struct tree *head, struct tree 
*remote, int reset)
lock_file = xcalloc(1, sizeof(struct lock_file));
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
 
-   refresh_cache(REFRESH_QUIET);
+   refresh_index(_index, REFRESH_QUIET, NULL, NULL, NULL);
 
memset(, 0, sizeof(opts));
opts.head_idx = 1;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 34ec9f7e32..49a8b3a7ff 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -509,7 +509,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
 
setup_unpack_trees_porcelain(, "checkout");
 
-   refresh_cache(REFRESH_QUIET);
+   refresh_index(_index, REFRESH_QUIET, NULL, NULL, NULL);
 
if (unmerged_index(_index)) {
error(_("you need to resolve your current index 
first"));
diff --git a/builtin/commit.c b/builtin/commit.c
index ac4fe97d36..4b95ff181b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -328,7 +328,7 @@ static void refresh_cache_or_die(int refresh_flags)
 * refresh_flags contains REFRESH_QUIET, so the only errors
 * are for unmerged entries.
 */
-   if (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))
+   if (refresh_index(_index, refresh_flags | REFRESH_IN_PORCELAIN, 
NULL, NULL, NULL))
die_resolve_conflict("commit");
 }
 
@@ -470,7 +470,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
 
hold_locked_index(_lock, LOCK_DIE_ON_ERROR);
add_remove_files();
-   refresh_cache(REFRESH_QUIET);
+   refresh_index(_index, REFRESH_QUIET, NULL, NULL, NULL);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_locked_index(_index, _lock, CLOSE_LOCK))
die(_("unable to write new_index file"));
@@ -482,7 +482,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
 
create_base_index(current_head);
add_remove_files();
-   refresh_cache(REFRESH_QUIET);
+   refresh_index(_index, REFRESH_QUIET, NULL, NULL, NULL);
 
if (write_locked_index(_index, _lock, CLOSE_LOCK))
die(_("unable to write temporary index file"));
diff --git a/builtin/diff.c b/builtin/diff.c
index 1efd0d6b61..b11b94b214 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -211,7 +211,8 @@ static void refresh_index_quietly(void)
return;
discard_index(_index);
read_index(_index);
-   refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
+   refresh_index(_index, REFRESH_QUIET | REFRESH_UNMERGED, NULL,
+ NULL, NULL);
update_index_if_able(_index, lock_file);
 }
 
diff --git a/builtin/merge.c b/builtin/merge.c
index b023107d26..4e219cb9e8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -324,7 +324,7 @@ static void restore_state(const struct object_id *head,
run_command_v_opt(args, RUN_GIT_CMD);
 
strbuf_release();
-   refresh_cache(REFRESH_QUIET);
+   refresh_index(_index, REFRESH_QUIET, NULL, NULL, NULL);
 }
 
 /* This is called when no merge was necessary. */
@@ -639,7 +639,7 @@ static int try_merge_strategy(const char *strategy, struct 
commit_list *common,
const char *head_arg = "HEAD";
 
hold_locked_index(, LOCK_DIE_ON_ERROR);
-   refresh_cache(REFRESH_QUIET);
+   refresh_index(_index, REFRESH_QUIET, NULL, NULL, NULL);
if (the_index.cache_changed &&
write_locked_index(_index, , COMMIT_LOCK))
return error(_("Unable to write index."));
@@ -786,7 +786,7 @@ static int merge_trivial(struct commit *head, struct 

[PATCH 15/24] cache.h: drop add_cache_entry

[Stefan Beller]

coccinelle patch
-add_cache_entry(ce, option)
+add_index_entry(_index, ce, option)

Additionally drop the define from cache.h manually.

Signed-off-by: Stefan Beller 
---
 apply.c| 4 ++--
 builtin/blame.c| 3 ++-
 builtin/checkout.c | 3 ++-
 builtin/reset.c| 3 ++-
 builtin/update-index.c | 8 
 cache.h| 1 -
 merge-recursive.c  | 4 ++--
 tree.c | 2 +-
 8 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/apply.c b/apply.c
index bb1cd77c66..66d4969e72 100644
--- a/apply.c
+++ b/apply.c
@@ -4284,7 +4284,7 @@ static int build_fake_ancestor(struct apply_state *state, 
struct patch *list)
   "for newly created file %s"), path);
}
}
-   if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+   if (add_index_entry(_index, ce, ADD_CACHE_OK_TO_ADD) < 0) {
free(ce);
return error(_("unable to add cache entry for %s"), path);
}
@@ -4428,7 +4428,7 @@ static int add_conflicted_stages_file(struct apply_state 
*state,
ce->ce_flags = create_ce_flags(stage);
ce->ce_namelen = namelen;
oidcpy(>oid, >threeway_stage[stage - 1]);
-   if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
+   if (add_index_entry(_index, ce, ADD_CACHE_OK_TO_ADD) < 0) {
free(ce);
return error(_("unable to add cache entry for %s"),
 patch->new_name);
diff --git a/builtin/blame.c b/builtin/blame.c
index c71d9a3340..5140015cc0 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2413,7 +2413,8 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
ce->ce_flags = create_ce_flags(0);
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);
-   add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
+   add_index_entry(_index, ce,
+   ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
 
cache_tree_invalidate_path(_index, path);
 
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 039d3d296b..34ec9f7e32 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -118,7 +118,8 @@ static int update_some(const unsigned char *sha1, struct 
strbuf *base,
}
}
 
-   add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
+   add_index_entry(_index, ce,
+   ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
return 0;
 }
 
diff --git a/builtin/reset.c b/builtin/reset.c
index f8073b1caa..0e19d6e8d5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -138,7 +138,8 @@ static void update_index_from_diff(struct diff_queue_struct 
*q,
ce->ce_flags |= CE_INTENT_TO_ADD;
set_object_name_for_intent_to_add_entry(ce);
}
-   add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | 
ADD_CACHE_OK_TO_REPLACE);
+   add_index_entry(_index, ce,
+   ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
}
 }
 
diff --git a/builtin/update-index.c b/builtin/update-index.c
index d7a117c674..e0738f74bf 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -286,7 +286,7 @@ static int add_one_path(const struct cache_entry *old, 
const char *path, int len
}
option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
-   if (add_cache_entry(ce, option))
+   if (add_index_entry(_index, ce, option))
return error("%s: cannot add to the index - missing --add 
option?", path);
return 0;
 }
@@ -416,7 +416,7 @@ static int add_cacheinfo(unsigned int mode, const struct 
object_id *oid,
ce->ce_flags |= CE_VALID;
option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
-   if (add_cache_entry(ce, option))
+   if (add_index_entry(_index, ce, option))
return error("%s: cannot add to the index - missing --add 
option?",
 path);
report("add '%s'", path);
@@ -659,12 +659,12 @@ static int unresolve_one(const char *path)
}
 
remove_file_from_cache(path);
-   if (add_cache_entry(ce_2, ADD_CACHE_OK_TO_ADD)) {
+   if (add_index_entry(_index, ce_2, ADD_CACHE_OK_TO_ADD)) {
error("%s: cannot add our version to the index.", path);
ret = -1;
goto free_return;
}
-   if (!add_cache_entry(ce_3, ADD_CACHE_OK_TO_ADD))
+   if (!add_index_entry(_index, ce_3, ADD_CACHE_OK_TO_ADD))
return 0;
error("%s: cannot add their version to the index.", path);
ret = -1;
diff --git a/cache.h b/cache.h
index a18ebf263a..54c1bbf0d1 100644
--- 

[PATCH 23/24] cache.h: drop ce_match_stat

[Stefan Beller]

coccinelle patch:
@@ expression ce, st, options; @@
-ce_match_stat(ce, st, options)
+ie_match_stat(_index, ce, st, options)

Additionally drop the define from cache.h manually.

Note that there is an empty define section in cache.h now.
The cleanup of that is done in a later patch.

Signed-off-by: Stefan Beller 
---
 apply.c| 3 ++-
 builtin/rm.c   | 2 +-
 builtin/update-index.c | 2 +-
 cache.h| 1 -
 check-racy.c   | 4 ++--
 diff-lib.c | 2 +-
 diff.c | 2 +-
 entry.c| 3 ++-
 submodule.c| 2 +-
 9 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/apply.c b/apply.c
index 8a61f19d03..46bc5a20b9 100644
--- a/apply.c
+++ b/apply.c
@@ -3364,7 +3364,8 @@ static int verify_index_match(const struct cache_entry 
*ce, struct stat *st)
return -1;
return 0;
}
-   return ce_match_stat(ce, st, 
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
+   return ie_match_stat(_index, ce, st,
+CE_MATCH_IGNORE_VALID | 
CE_MATCH_IGNORE_SKIP_WORKTREE);
 }
 
 #define SUBMODULE_PATCH_WITHOUT_INDEX 1
diff --git a/builtin/rm.c b/builtin/rm.c
index f479100298..51b64f2bae 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -163,7 +163,7 @@ static int check_local_mod(struct object_id *head, int 
index_only)
 * Is the index different from the file in the work tree?
 * If it's a submodule, is its work tree modified?
 */
-   if (ce_match_stat(ce, , 0) ||
+   if (ie_match_stat(_index, ce, , 0) ||
(S_ISGITLINK(ce->ce_mode) &&
 bad_to_remove_submodule(ce->name,
SUBMODULE_REMOVAL_DIE_ON_ERROR |
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 9cbd346f95..042f4c94cf 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -268,7 +268,7 @@ static int add_one_path(const struct cache_entry *old, 
const char *path, int len
struct cache_entry *ce;
 
/* Was the old index entry already up-to-date? */
-   if (old && !ce_stage(old) && !ce_match_stat(old, st, 0))
+   if (old && !ce_stage(old) && !ie_match_stat(_index, old, st, 0))
return 0;
 
size = cache_entry_size(len);
diff --git a/cache.h b/cache.h
index c34fc4fd40..f2a45eda9a 100644
--- a/cache.h
+++ b/cache.h
@@ -354,7 +354,6 @@ extern void free_name_hash(struct index_state *istate);
 
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
-#define ce_match_stat(ce, st, options) ie_match_stat(_index, (ce), (st), 
(options))
 #endif
 
 enum object_type {
diff --git a/check-racy.c b/check-racy.c
index 6599ae84cf..485d7ab8f8 100644
--- a/check-racy.c
+++ b/check-racy.c
@@ -16,9 +16,9 @@ int main(int ac, char **av)
continue;
}
 
-   if (ce_match_stat(ce, , 0))
+   if (ie_match_stat(_index, ce, , 0))
dirty++;
-   else if (ce_match_stat(ce, , CE_MATCH_RACY_IS_DIRTY))
+   else if (ie_match_stat(_index, ce, , 
CE_MATCH_RACY_IS_DIRTY))
racy++;
else
clean++;
diff --git a/diff-lib.c b/diff-lib.c
index de59ec0459..4ca3ce9c90 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -69,7 +69,7 @@ static int match_stat_with_submodule(struct diff_options 
*diffopt,
 struct stat *st, unsigned ce_option,
 unsigned *dirty_submodule)
 {
-   int changed = ce_match_stat(ce, st, ce_option);
+   int changed = ie_match_stat(_index, ce, st, ce_option);
if (S_ISGITLINK(ce->ce_mode)) {
unsigned orig_flags = diffopt->flags;
if (!DIFF_OPT_TST(diffopt, OVERRIDE_SUBMODULE_CONFIG))
diff --git a/diff.c b/diff.c
index f2ee40fe21..bd1478f6c9 100644
--- a/diff.c
+++ b/diff.c
@@ -2782,7 +2782,7 @@ static int reuse_worktree_file(const char *name, const 
unsigned char *sha1, int
 * If ce matches the file in the work tree, we can reuse it.
 */
if (ce_uptodate(ce) ||
-   (!lstat(name, ) && !ce_match_stat(ce, , 0)))
+   (!lstat(name, ) && !ie_match_stat(_index, ce, , 0)))
return 1;
 
return 0;
diff --git a/entry.c b/entry.c
index d2b512da90..d3a34c9cc4 100644
--- a/entry.c
+++ b/entry.c
@@ -266,7 +266,8 @@ int checkout_entry(struct cache_entry *ce,
 
if (!check_path(path.buf, path.len, , state->base_dir_len)) {
const struct submodule *sub;
-   unsigned changed = ce_match_stat(ce, , 
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
+   unsigned changed = ie_match_stat(_index, ce, ,
+CE_MATCH_IGNORE_VALID | 
CE_MATCH_IGNORE_SKIP_WORKTREE);
/*
 * Needs 

[PATCH 24/24] cache.h: retire NO_THE_INDEX_COMPATIBILITY_MACROS

[Stefan Beller]

The NO_THE_INDEX_COMPATIBILITY_MACROS pre processor setting was introduced
in 4aab5b46f4 (Make read-cache.c "the_index" free., 2007-04-01), stating:

This makes all low-level functions defined in read-cache.c to
take an explicit index_state structure as their first parameter,
to specify which index to work on.

The reasoning is very vague, maybe indicating that having the index
specified to work on is easier to for the new reader to understand what
is going on.

All preceding patches worked on retiring functions that do not take an
explicit index as to where to perform its work. Spelling out the reasons
why we want to specify the index at each call:

1)  Ease of understanding:
   The recent patches dropped a lot of macros that may confuse new people
   diving into the code base.
2a) Spelling out global state explicitly:
   Anything you do in submodule land today needs to spawn new processes in
   the submodule. This is cumbersome and not performant. So we may want to
   have an abstraction of a repo, i.e. all repository state in one struct.
   That way we can open a submodule in-process and perform the required
   actions without spawning a process. The road to this future is a long
   road, and we have to start somewhere. Exposing the global state seems
   like a good starter point.
2b) Spelling out global state explicitly (II): (Jeff Hostetler wrote)
   In addition to (eventually) allowing multiple repos be open at the same
   time for submodules, it would also help with various multi-threading
   efforts.  For example, we have loops that do a
   "for (k = 0, k < active_nr; k++) {...}"  There is no visual clue in that
   code that it references "the_index" and therefore should be subject to
   the same locking.  Granted, this is a trivial example, but goes to the
   argument that the code has lots of subtle global variables and macros
   that make it difficult to reason about the code.

   This step would help un-hide this.

Signed-off-by: Stefan Beller 
---
 attr.c | 1 -
 cache.h| 4 
 name-hash.c| 1 -
 read-cache.c   | 1 -
 unpack-trees.c | 1 -
 5 files changed, 8 deletions(-)

diff --git a/attr.c b/attr.c
index 7e2134471c..3decf432b8 100644
--- a/attr.c
+++ b/attr.c
@@ -7,7 +7,6 @@
  * an insanely large number of attributes.
  */
 
-#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "exec_cmd.h"
 #include "attr.h"
diff --git a/cache.h b/cache.h
index f2a45eda9a..f4b7b8621c 100644
--- a/cache.h
+++ b/cache.h
@@ -352,10 +352,6 @@ extern void add_name_hash(struct index_state *istate, 
struct cache_entry *ce);
 extern void remove_name_hash(struct index_state *istate, struct cache_entry 
*ce);
 extern void free_name_hash(struct index_state *istate);
 
-
-#ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
-#endif
-
 enum object_type {
OBJ_BAD = -1,
OBJ_NONE = 0,
diff --git a/name-hash.c b/name-hash.c
index 39309efb7f..02870c1d19 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -5,7 +5,6 @@
  *
  * Copyright (C) 2008 Linus Torvalds
  */
-#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 
 struct dir_entry {
diff --git a/read-cache.c b/read-cache.c
index 6681376f5b..e0c6f3ca11 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3,7 +3,6 @@
  *
  * Copyright (C) Linus Torvalds, 2005
  */
-#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "tempfile.h"
 #include "lockfile.h"
diff --git a/unpack-trees.c b/unpack-trees.c
index 3dd8f60fc1..41956c4b37 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1,4 +1,3 @@
-#define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "dir.h"
 #include "tree.h"
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 09/24] cache.h: drop resolve_undo_clear

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/checkout.c | 2 +-
 builtin/merge.c| 2 +-
 builtin/read-tree.c| 2 +-
 builtin/update-index.c | 2 +-
 cache.h| 1 -
 merge.c| 2 +-
 6 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2328a475ea..abcc45a74f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -491,7 +491,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
if (read_index_preload(_index, NULL) < 0)
return error(_("index file corrupt"));
 
-   resolve_undo_clear();
+   resolve_undo_clear_index(_index);
if (opts->force) {
ret = reset_tree(new->commit->tree, opts, 1, writeout_error);
if (ret)
diff --git a/builtin/merge.c b/builtin/merge.c
index c27c806ac1..b023107d26 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1191,7 +1191,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
else
die(_("You have not concluded your cherry-pick 
(CHERRY_PICK_HEAD exists)."));
}
-   resolve_undo_clear();
+   resolve_undo_clear_index(_index);
 
if (verbosity < 0)
show_diffstat = 0;
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 0bcf021ead..61f5f6f028 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -199,7 +199,7 @@ int cmd_read_tree(int argc, const char **argv, const char 
*unused_prefix)
die("You need to resolve your current index first");
stage = opts.merge = 1;
}
-   resolve_undo_clear();
+   resolve_undo_clear_index(_index);
 
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
diff --git a/builtin/update-index.c b/builtin/update-index.c
index b8458016f0..c9f06169c0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -796,7 +796,7 @@ static int chmod_callback(const struct option *opt,
 static int resolve_undo_clear_callback(const struct option *opt,
const char *arg, int unset)
 {
-   resolve_undo_clear();
+   resolve_undo_clear_index(_index);
return 0;
 }
 
diff --git a/cache.h b/cache.h
index 8a2dc393dc..abf1474034 100644
--- a/cache.h
+++ b/cache.h
@@ -371,7 +371,6 @@ extern void free_name_hash(struct index_state *istate);
 #define cache_dir_exists(name, namelen) index_dir_exists(_index, (name), 
(namelen))
 #define cache_file_exists(name, namelen, igncase) 
index_file_exists(_index, (name), (namelen), (igncase))
 #define cache_name_is_other(name, namelen) index_name_is_other(_index, 
(name), (namelen))
-#define resolve_undo_clear() resolve_undo_clear_index(_index)
 #endif
 
 enum object_type {
diff --git a/merge.c b/merge.c
index 748305031e..06509a6df2 100644
--- a/merge.c
+++ b/merge.c
@@ -39,7 +39,7 @@ int try_merge_command(const char *strategy, size_t xopts_nr,
discard_cache();
if (read_index(_index) < 0)
die(_("failed to read the cache"));
-   resolve_undo_clear();
+   resolve_undo_clear_index(_index);
 
return ret;
 }
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 08/24] cache.h: drop unmerge_cache[_entry_at]

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/update-index.c | 2 +-
 cache.h| 2 --
 rerere.c   | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8667c48446..b8458016f0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -613,7 +613,7 @@ static int unresolve_one(const char *path)
pos = cache_name_pos(path, namelen);
if (0 <= pos) {
/* already merged */
-   pos = unmerge_cache_entry_at(pos);
+   pos = unmerge_index_entry_at(_index, pos);
if (pos < the_index.cache_nr) {
const struct cache_entry *ce = the_index.cache[pos];
if (ce_stage(ce) &&
diff --git a/cache.h b/cache.h
index d078e88c3f..8a2dc393dc 100644
--- a/cache.h
+++ b/cache.h
@@ -372,8 +372,6 @@ extern void free_name_hash(struct index_state *istate);
 #define cache_file_exists(name, namelen, igncase) 
index_file_exists(_index, (name), (namelen), (igncase))
 #define cache_name_is_other(name, namelen) index_name_is_other(_index, 
(name), (namelen))
 #define resolve_undo_clear() resolve_undo_clear_index(_index)
-#define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
-#define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
 #endif
 
 enum object_type {
diff --git a/rerere.c b/rerere.c
index b9b39a959e..03218166ab 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1116,7 +1116,7 @@ int rerere_forget(struct pathspec *pathspec)
 * recover the original conflicted state and then
 * find the conflicted paths.
 */
-   unmerge_cache(pathspec);
+   unmerge_index(_index, pathspec);
find_conflict();
for (i = 0; i < conflict.nr; i++) {
struct string_list_item *it = [i];
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 20/24] cache.h: drop chmod_cache_entry

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/add.c  | 2 +-
 builtin/update-index.c | 2 +-
 cache.h| 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index f6d71b10d0..288b1f5bb3 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -40,7 +40,7 @@ static void chmod_pathspec(struct pathspec *pathspec, int 
force_mode)
if (pathspec && !ce_path_match(ce, pathspec, NULL))
continue;
 
-   if (chmod_cache_entry(ce, force_mode) < 0)
+   if (chmod_index_entry(_index, ce, force_mode) < 0)
fprintf(stderr, "cannot chmod '%s'", ce->name);
}
 }
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8c4911e920..f1c52a5531 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -432,7 +432,7 @@ static void chmod_path(char flip, const char *path)
if (pos < 0)
goto fail;
ce = the_index.cache[pos];
-   if (chmod_cache_entry(ce, flip) < 0)
+   if (chmod_index_entry(_index, ce, flip) < 0)
goto fail;
 
report("chmod %cx '%s'", flip, path);
diff --git a/cache.h b/cache.h
index a9b059913e..07ad23d912 100644
--- a/cache.h
+++ b/cache.h
@@ -354,7 +354,6 @@ extern void free_name_hash(struct index_state *istate);
 
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
-#define chmod_cache_entry(ce, flip) chmod_index_entry(_index, (ce), (flip))
 #define refresh_cache(flags) refresh_index(_index, (flags), NULL, NULL, 
NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(_index, (ce), (st), 
(options))
 #define ce_modified(ce, st, options) ie_modified(_index, (ce), (st), 
(options))
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 22/24] cache.h: drop ce_modified

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/ls-files.c | 2 +-
 cache.h| 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 3507490d3e..89fac7ddf5 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -369,7 +369,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(_index, ce, , 
0))
show_ce_entry(tag_modified, ce);
}
}
diff --git a/cache.h b/cache.h
index 23c9c1697c..c34fc4fd40 100644
--- a/cache.h
+++ b/cache.h
@@ -355,7 +355,6 @@ extern void free_name_hash(struct index_state *istate);
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
 #define ce_match_stat(ce, st, options) ie_match_stat(_index, (ce), (st), 
(options))
-#define ce_modified(ce, st, options) ie_modified(_index, (ce), (st), 
(options))
 #endif
 
 enum object_type {
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 13/24] cache.h: drop is_cache_unborn(), discard_cache(), unmerged_cache()

[Stefan Beller]

cocci semantic patch:
@@ @@
-is_cache_unborn()
+is_index_unborn(_index)
@@ @@
-discard_cache()
+discard_index(_index)
@@ @@
-unmerged_cache()
+unmerged_index(_index)

Additionally the defines in cache.h were removed manually.

Signed-off-by: Stefan Beller 
---
 builtin/am.c|  8 
 builtin/blame.c |  2 +-
 builtin/checkout.c  |  4 ++--
 builtin/commit.c| 10 +-
 builtin/diff.c  |  2 +-
 builtin/pull.c  |  2 +-
 builtin/read-tree.c |  2 +-
 builtin/reset.c |  2 +-
 cache.h |  3 ---
 merge-recursive.c   |  8 
 merge.c |  2 +-
 sequencer.c |  4 ++--
 t/helper/test-lazy-init-name-hash.c | 10 +-
 t/helper/test-read-cache.c  |  2 +-
 wt-status.c |  2 +-
 15 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index bb0927fbcc..38fc4566f7 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1556,7 +1556,7 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
 
if (index_file) {
/* Reload index as apply_all_patches() will have modified it. */
-   discard_cache();
+   discard_index(_index);
read_index_from(_index, index_file);
}
 
@@ -1599,7 +1599,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
if (build_fake_ancestor(state, index_path))
return error("could not build fake ancestor");
 
-   discard_cache();
+   discard_index(_index);
read_index_from(_index, index_path);
 
if (write_index_as_tree(orig_tree.hash, _index, index_path, 0, 
NULL))
@@ -1632,7 +1632,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
 
say(state, stdout, _("Falling back to patching base and 3-way 
merge..."));
 
-   discard_cache();
+   discard_index(_index);
read_index(_index);
 
/*
@@ -1938,7 +1938,7 @@ static void am_resolve(struct am_state *state)
die_user_resolve(state);
}
 
-   if (unmerged_cache()) {
+   if (unmerged_index(_index)) {
printf_ln(_("You still have unmerged paths in your index.\n"
"Did you forget to use 'git add'?"));
die_user_resolve(state);
diff --git a/builtin/blame.c b/builtin/blame.c
index cbb7c1fd9d..b47aae25d4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2394,7 +2394,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
 * bits; we are not going to write this index out -- we just
 * want to run "diff-index --cached".
 */
-   discard_cache();
+   discard_index(_index);
read_index(_index);
 
len = strlen(path);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index abcc45a74f..a6cd8c0f37 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -510,13 +510,13 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
 
refresh_cache(REFRESH_QUIET);
 
-   if (unmerged_cache()) {
+   if (unmerged_index(_index)) {
error(_("you need to resolve your current index 
first"));
return 1;
}
 
/* 2-way merge to the new branch */
-   topts.initial_checkout = is_cache_unborn();
+   topts.initial_checkout = is_index_unborn(_index);
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
diff --git a/builtin/commit.c b/builtin/commit.c
index 687e7c8a3a..ef12ea3991 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -301,7 +301,7 @@ static void create_base_index(const struct commit 
*current_head)
struct tree_desc t;
 
if (!current_head) {
-   discard_cache();
+   discard_index(_index);
return;
}
 
@@ -369,7 +369,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
else
unsetenv(INDEX_ENVIRONMENT);
 
-   discard_cache();
+   discard_index(_index);
read_index_from(_index, get_lock_file_path(_lock));
if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
if (reopen_lock_file(_lock) < 0)
@@ -464,7 +464,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
if (list_paths(, !current_head ? NULL : "HEAD", prefix, 
))
exit(1);
 
-   discard_cache();
+   discard_index(_index);
if (read_index(_index) < 0)
die(_("cannot read the 

[PATCH 06/24] unpack-trees.c: rename parameter 'the_index'

[Stefan Beller]

As "the_index" is already a global variable, we do not want to confuse
the local variable with the global variable.

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

diff --git a/unpack-trees.c b/unpack-trees.c
index aa15111fef..3dd8f60fc1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1199,7 +1199,7 @@ static int clear_ce_flags(struct cache_entry **cache, int 
nr,
  * Set/Clear CE_NEW_SKIP_WORKTREE according to $GIT_DIR/info/sparse-checkout
  */
 static void mark_new_skip_worktree(struct exclude_list *el,
-  struct index_state *the_index,
+  struct index_state *index,
   int select_flag, int skip_wt_flag)
 {
int i;
@@ -1208,8 +1208,8 @@ static void mark_new_skip_worktree(struct exclude_list 
*el,
 * 1. Pretend the narrowest worktree: only unmerged entries
 * are checked out
 */
-   for (i = 0; i < the_index->cache_nr; i++) {
-   struct cache_entry *ce = the_index->cache[i];
+   for (i = 0; i < index->cache_nr; i++) {
+   struct cache_entry *ce = index->cache[i];
 
if (select_flag && !(ce->ce_flags & select_flag))
continue;
@@ -1224,7 +1224,7 @@ static void mark_new_skip_worktree(struct exclude_list 
*el,
 * 2. Widen worktree according to sparse-checkout file.
 * Matched entries will have skip_wt_flag cleared (i.e. "in")
 */
-   clear_ce_flags(the_index->cache, the_index->cache_nr,
+   clear_ce_flags(index->cache, index->cache_nr,
   select_flag, skip_wt_flag, el);
 }
 
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 12/24] cache.h: drop cache_dir_exists

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 cache.h | 1 -
 dir.c   | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 85a85f8b96..4b8e8c31fe 100644
--- a/cache.h
+++ b/cache.h
@@ -368,7 +368,6 @@ extern void free_name_hash(struct index_state *istate);
 #define refresh_cache(flags) refresh_index(_index, (flags), NULL, NULL, 
NULL)
 #define ce_match_stat(ce, st, options) ie_match_stat(_index, (ce), (st), 
(options))
 #define ce_modified(ce, st, options) ie_modified(_index, (ce), (st), 
(options))
-#define cache_dir_exists(name, namelen) index_dir_exists(_index, (name), 
(namelen))
 #endif
 
 enum object_type {
diff --git a/dir.c b/dir.c
index 0327832e53..63edaec2ef 100644
--- a/dir.c
+++ b/dir.c
@@ -1266,7 +1266,7 @@ static enum exist_status 
directory_exists_in_index_icase(const char *dirname, in
 {
struct cache_entry *ce;
 
-   if (cache_dir_exists(dirname, len))
+   if (index_dir_exists(_index, dirname, len))
return index_directory;
 
ce = index_file_exists(_index, dirname, len, ignore_case);
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 11/24] cache.h: drop cache_file_exists

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 apply.c   | 2 +-
 cache.h   | 1 -
 dir.c | 9 +
 merge-recursive.c | 3 ++-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/apply.c b/apply.c
index 159e039a18..eb8eaeabec 100644
--- a/apply.c
+++ b/apply.c
@@ -3810,7 +3810,7 @@ static int path_is_beyond_symlink_1(struct apply_state 
*state, struct strbuf *na
if (state->check_index) {
struct cache_entry *ce;
 
-   ce = cache_file_exists(name->buf, name->len, 
ignore_case);
+   ce = index_file_exists(_index, name->buf, 
name->len, ignore_case);
if (ce && S_ISLNK(ce->ce_mode))
return 1;
} else {
diff --git a/cache.h b/cache.h
index 5de8ab4e69..85a85f8b96 100644
--- a/cache.h
+++ b/cache.h
@@ -369,7 +369,6 @@ extern void free_name_hash(struct index_state *istate);
 #define ce_match_stat(ce, st, options) ie_match_stat(_index, (ce), (st), 
(options))
 #define ce_modified(ce, st, options) ie_modified(_index, (ce), (st), 
(options))
 #define cache_dir_exists(name, namelen) index_dir_exists(_index, (name), 
(namelen))
-#define cache_file_exists(name, namelen, igncase) 
index_file_exists(_index, (name), (namelen), (igncase))
 #endif
 
 enum object_type {
diff --git a/dir.c b/dir.c
index d5e1c462bb..0327832e53 100644
--- a/dir.c
+++ b/dir.c
@@ -1235,7 +1235,7 @@ static struct dir_entry *dir_entry_new(const char 
*pathname, int len)
 
 static struct dir_entry *dir_add_name(struct dir_struct *dir, const char 
*pathname, int len)
 {
-   if (cache_file_exists(pathname, len, ignore_case))
+   if (index_file_exists(_index, pathname, len, ignore_case))
return NULL;
 
ALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);
@@ -1269,7 +1269,7 @@ static enum exist_status 
directory_exists_in_index_icase(const char *dirname, in
if (cache_dir_exists(dirname, len))
return index_directory;
 
-   ce = cache_file_exists(dirname, len, ignore_case);
+   ce = index_file_exists(_index, dirname, len, ignore_case);
if (ce && S_ISGITLINK(ce->ce_mode))
return index_gitdir;
 
@@ -1460,7 +1460,7 @@ static int get_index_dtype(const char *path, int len)
int pos;
const struct cache_entry *ce;
 
-   ce = cache_file_exists(path, len, 0);
+   ce = index_file_exists(_index, path, len, 0);
if (ce) {
if (!ce_uptodate(ce))
return DT_UNKNOWN;
@@ -1522,7 +1522,8 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
  int dtype, struct dirent *de)
 {
int exclude;
-   int has_path_in_index = !!cache_file_exists(path->buf, path->len, 
ignore_case);
+   int has_path_in_index = !!index_file_exists(_index, path->buf,
+   path->len, ignore_case);
 
if (dtype == DT_UNKNOWN)
dtype = get_dtype(de, path->buf, path->len);
diff --git a/merge-recursive.c b/merge-recursive.c
index 57ca250c88..b8b3a153f1 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -623,7 +623,8 @@ static int remove_file(struct merge_options *o, int clean,
if (update_working_directory) {
if (ignore_case) {
struct cache_entry *ce;
-   ce = cache_file_exists(path, strlen(path), ignore_case);
+   ce = index_file_exists(_index, path,
+  strlen(path), ignore_case);
if (ce && ce_stage(ce) == 0)
return 0;
}
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 10/24] cache.h: drop cache_name_is_other

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/clean.c| 2 +-
 builtin/ls-files.c | 2 +-
 cache.h| 1 -
 dir.c  | 2 +-
 wt-status.c| 4 ++--
 5 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 9bdefca6dc..c6aacbb0f0 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -938,7 +938,7 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
struct stat st;
const char *rel;
 
-   if (!cache_name_is_other(ent->name, ent->len))
+   if (!index_name_is_other(_index, ent->name, ent->len))
continue;
 
if (pathspec.nr)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index edcad6e8e1..6f7ecec1b0 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -114,7 +114,7 @@ static void show_other_files(struct dir_struct *dir)
 
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(_index, ent->name, ent->len))
continue;
show_dir_entry(tag_other, ent);
}
diff --git a/cache.h b/cache.h
index abf1474034..5de8ab4e69 100644
--- a/cache.h
+++ b/cache.h
@@ -370,7 +370,6 @@ extern void free_name_hash(struct index_state *istate);
 #define ce_modified(ce, st, options) ie_modified(_index, (ce), (st), 
(options))
 #define cache_dir_exists(name, namelen) index_dir_exists(_index, (name), 
(namelen))
 #define cache_file_exists(name, namelen, igncase) 
index_file_exists(_index, (name), (namelen), (igncase))
-#define cache_name_is_other(name, namelen) index_name_is_other(_index, 
(name), (namelen))
 #endif
 
 enum object_type {
diff --git a/dir.c b/dir.c
index 8abad1b969..d5e1c462bb 100644
--- a/dir.c
+++ b/dir.c
@@ -1244,7 +1244,7 @@ static struct dir_entry *dir_add_name(struct dir_struct 
*dir, const char *pathna
 
 struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char 
*pathname, int len)
 {
-   if (!cache_name_is_other(pathname, len))
+   if (!index_name_is_other(_index, pathname, len))
return NULL;
 
ALLOC_GROW(dir->ignored, dir->ignored_nr+1, dir->ignored_alloc);
diff --git a/wt-status.c b/wt-status.c
index 750ed28b49..ff0e70a25a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -669,7 +669,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 
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(_index, ent->name, ent->len) &&
dir_path_match(ent, >pathspec, 0, NULL))
string_list_insert(>untracked, ent->name);
free(ent);
@@ -677,7 +677,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 
for (i = 0; i < dir.ignored_nr; i++) {
struct dir_entry *ent = dir.ignored[i];
-   if (cache_name_is_other(ent->name, ent->len) &&
+   if (index_name_is_other(_index, ent->name, ent->len) &&
dir_path_match(ent, >pathspec, 0, NULL))
string_list_insert(>ignored, ent->name);
free(ent);
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 05/24] cache.h: drop read_cache_unmerged()

[Stefan Beller]

@@ @@
-read_cache_unmerged()
+read_index_unmerged(_index)

Additionally drop the define from cache.h manually.

Signed-off-by: Stefan Beller 
---
 builtin/am.c| 2 +-
 builtin/merge.c | 2 +-
 builtin/pull.c  | 2 +-
 builtin/read-tree.c | 2 +-
 builtin/reset.c | 2 +-
 cache.h | 1 -
 sequencer.c | 2 +-
 7 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index cb3e4dff63..bb0927fbcc 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2053,7 +2053,7 @@ static int clean_index(const struct object_id *head, 
const struct object_id *rem
if (!remote_tree)
return error(_("Could not parse object '%s'."), 
oid_to_hex(remote));
 
-   read_cache_unmerged();
+   read_index_unmerged(_index);
 
if (fast_forward_to(head_tree, head_tree, 1))
return -1;
diff --git a/builtin/merge.c b/builtin/merge.c
index 4d4c56050c..c27c806ac1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1170,7 +1170,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
goto done;
}
 
-   if (read_cache_unmerged())
+   if (read_index_unmerged(_index))
die_resolve_conflict("merge");
 
if (file_exists(git_path_merge_head())) {
diff --git a/builtin/pull.c b/builtin/pull.c
index dd1a4a94e4..42578cee05 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -788,7 +788,7 @@ int cmd_pull(int argc, const char **argv, const char 
*prefix)
 
git_config(git_pull_config, NULL);
 
-   if (read_cache_unmerged())
+   if (read_index_unmerged(_index))
die_resolve_conflict("pull");
 
if (file_exists(git_path_merge_head()))
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index f997814933..0bcf021ead 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -195,7 +195,7 @@ int cmd_read_tree(int argc, const char **argv, const char 
*unused_prefix)
 */
 
if (opts.reset || opts.merge || opts.prefix) {
-   if (read_cache_unmerged() && (opts.prefix || opts.merge))
+   if (read_index_unmerged(_index) && (opts.prefix || 
opts.merge))
die("You need to resolve your current index first");
stage = opts.merge = 1;
}
diff --git a/builtin/reset.c b/builtin/reset.c
index 03c5498d6e..4a4eb723dd 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -66,7 +66,7 @@ static int reset_index(const struct object_id *oid, int 
reset_type, int quiet)
opts.reset = 1;
}
 
-   read_cache_unmerged();
+   read_index_unmerged(_index);
 
if (reset_type == KEEP) {
struct object_id head_oid;
diff --git a/cache.h b/cache.h
index a66ae97fb7..9b94339573 100644
--- a/cache.h
+++ b/cache.h
@@ -355,7 +355,6 @@ extern void free_name_hash(struct index_state *istate);
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
 #define is_cache_unborn() is_index_unborn(_index)
-#define read_cache_unmerged() read_index_unmerged(_index)
 #define discard_cache() discard_index(_index)
 #define unmerged_cache() unmerged_index(_index)
 #define cache_name_pos(name, namelen) 
index_name_pos(_index,(name),(namelen))
diff --git a/sequencer.c b/sequencer.c
index 9409b65aaa..f20e05fe60 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -349,7 +349,7 @@ static struct tree *empty_tree(void)
 
 static int error_dirty_index(struct replay_opts *opts)
 {
-   if (read_cache_unmerged())
+   if (read_index_unmerged(_index))
return error_resolve_conflict(_(action_name(opts)));
 
error(_("your local changes would be overwritten by %s."),
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 07/24] cache.h: drop read_blob_data_from_cache

[Stefan Beller]

This was done without cocinelle, as we only have 2 occurrences.

Signed-off-by: Stefan Beller 
---
 cache.h   | 1 -
 convert.c | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 9b94339573..d078e88c3f 100644
--- a/cache.h
+++ b/cache.h
@@ -374,7 +374,6 @@ extern void free_name_hash(struct index_state *istate);
 #define resolve_undo_clear() resolve_undo_clear_index(_index)
 #define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
 #define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
-#define read_blob_data_from_cache(path, sz) 
read_blob_data_from_index(_index, (path), (sz))
 #endif
 
 enum object_type {
diff --git a/convert.c b/convert.c
index 8d652bf27c..cf29280ae1 100644
--- a/convert.c
+++ b/convert.c
@@ -137,7 +137,7 @@ const char *get_cached_convert_stats_ascii(const char *path)
 {
const char *ret;
unsigned long sz;
-   void *data = read_blob_data_from_cache(path, );
+   void *data = read_blob_data_from_index(_index, path, );
ret = gather_convert_stats_ascii(data, sz);
free(data);
return ret;
@@ -222,7 +222,7 @@ static int has_cr_in_index(const char *path)
void *data;
int has_cr;
 
-   data = read_blob_data_from_cache(path, );
+   data = read_blob_data_from_index(_index, path, );
if (!data)
return 0;
has_cr = memchr(data, '\r', sz) != NULL;
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 02/24] cache.h: drop active_* macros

[Stefan Beller]

Based on the coccinelle patch:

@@ @@
-active_cache
+the_index.cache
@@ @@
-active_nr
+the_index.cache_nr
@@ @@
-active_alloc
+the_index.cache_alloc
@@ @@
-active_cache_changed
+the_index.cache_changed
@@ @@
-active_cache_tree
+the_index.cache_tree

Additional manual editing:
* drop the macros from cache.h
* fix the whitespace issue that apply complained about in
  builtin/checkout.c
* builtin/commit.c had an occurrence of active_cache_tree->sha1, which
  was not picked up with the coccinelle patch.
* diff.c and t/t2107-update-index-basic.sh referenced
  active_cache[_changed] in comments, fix them up.
* ative_nr was referenced in comments in read-cache.c and
  builtin/update-index.c, fix them.

Signed-off-by: Stefan Beller 
---
 apply.c  |  6 ++---
 builtin/add.c|  6 ++---
 builtin/am.c |  6 ++---
 builtin/blame.c  |  6 ++---
 builtin/checkout-index.c |  8 +++
 builtin/checkout.c   | 49 
 builtin/commit.c | 20 
 builtin/fsck.c   | 12 +-
 builtin/grep.c   |  8 +++
 builtin/ls-files.c   | 36 ++---
 builtin/merge-index.c| 10 
 builtin/merge.c  | 12 +-
 builtin/mv.c | 10 
 builtin/read-tree.c  |  2 +-
 builtin/rm.c | 16 ++---
 builtin/submodule--helper.c  |  8 +++
 builtin/update-index.c   | 48 ---
 cache.h  |  6 -
 check-racy.c |  4 ++--
 diff-lib.c   |  6 ++---
 diff.c   |  8 +++
 dir.c| 20 
 merge-recursive.c| 28 +++
 pathspec.c   | 14 ++--
 read-cache.c |  2 +-
 rerere.c | 26 ++---
 revision.c   | 18 +++
 sequencer.c  | 19 
 sha1_name.c  | 14 ++--
 submodule.c  | 12 +-
 t/helper/test-dump-cache-tree.c  |  2 +-
 t/helper/test-scrap-cache-tree.c |  2 +-
 t/t2107-update-index-basic.sh|  2 +-
 tree.c   |  8 +++
 wt-status.c  | 12 +-
 35 files changed, 232 insertions(+), 234 deletions(-)

diff --git a/apply.c b/apply.c
index 82701d6580..ae1b659b68 100644
--- a/apply.c
+++ b/apply.c
@@ -3499,7 +3499,7 @@ static int load_current(struct apply_state *state,
pos = cache_name_pos(name, strlen(name));
if (pos < 0)
return error(_("%s: does not exist in index"), name);
-   ce = active_cache[pos];
+   ce = the_index.cache[pos];
if (lstat(name, )) {
if (errno != ENOENT)
return error_errno("%s", name);
@@ -3671,7 +3671,7 @@ static int check_preimage(struct apply_state *state,
goto is_new;
return error(_("%s: does not exist in index"), 
old_name);
}
-   *ce = active_cache[pos];
+   *ce = the_index.cache[pos];
if (stat_ret < 0) {
if (checkout_target(_index, *ce, st))
return -1;
@@ -4001,7 +4001,7 @@ static int get_current_oid(struct apply_state *state, 
const char *path,
pos = cache_name_pos(path, strlen(path));
if (pos < 0)
return -1;
-   oidcpy(oid, _cache[pos]->oid);
+   oidcpy(oid, _index.cache[pos]->oid);
return 0;
 }
 
diff --git a/builtin/add.c b/builtin/add.c
index ab6d04e8db..f6d71b10d0 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -34,8 +34,8 @@ static void chmod_pathspec(struct pathspec *pathspec, int 
force_mode)
 {
int i;
 
-   for (i = 0; i < active_nr; i++) {
-   struct cache_entry *ce = active_cache[i];
+   for (i = 0; i < the_index.cache_nr; i++) {
+   struct cache_entry *ce = the_index.cache[i];
 
if (pathspec && !ce_path_match(ce, pathspec, NULL))
continue;
@@ -458,7 +458,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
unplug_bulk_checkin();
 
 finish:
-   if (active_cache_changed) {
+   if (the_index.cache_changed) {
if (write_locked_index(_index, _file, COMMIT_LOCK))
die(_("Unable to write new index file"));
}
diff --git a/builtin/am.c b/builtin/am.c
index 4405d7307c..c6a679d8e1 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1155,12 +1155,12 @@ static int index_has_changes(struct strbuf *sb)
diff_flush();
return DIFF_OPT_TST(, 

[PATCH 04/24] cache.h: drop read_cache_preload(pathspec)

[Stefan Beller]

coccinelle patch:
@@ expression E; @@
-read_cache_preload(E)
+read_index_preload(_index, E)

Additionally manual editing:
* drop the define from cache.h.
* builtin/{commit,describe}.c were not picked up as we have NULL and
  the address of an expression. Converted them manually.
* builtin/diff{-files,-index}.c error messages converted as well.

Signed-off-by: Stefan Beller 
---
 builtin/checkout.c | 4 ++--
 builtin/commit.c   | 4 ++--
 builtin/describe.c | 2 +-
 builtin/diff-files.c   | 4 ++--
 builtin/diff-index.c   | 4 ++--
 builtin/diff.c | 8 
 builtin/update-index.c | 2 +-
 cache.h| 1 -
 8 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0aac616ad6..2328a475ea 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -295,7 +295,7 @@ static int checkout_paths(const struct checkout_opts *opts,
lock_file = xcalloc(1, sizeof(struct lock_file));
 
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
-   if (read_cache_preload(>pathspec) < 0)
+   if (read_index_preload(_index, >pathspec) < 0)
return error(_("index file corrupt"));
 
if (opts->source_tree)
@@ -488,7 +488,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
-   if (read_cache_preload(NULL) < 0)
+   if (read_index_preload(_index, NULL) < 0)
return error(_("index file corrupt"));
 
resolve_undo_clear();
diff --git a/builtin/commit.c b/builtin/commit.c
index 65a04ac199..687e7c8a3a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -346,7 +346,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
   PATHSPEC_PREFER_FULL,
   prefix, argv);
 
-   if (read_cache_preload() < 0)
+   if (read_index_preload(_index, ) < 0)
die(_("index file corrupt"));
 
if (interactive) {
@@ -1377,7 +1377,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
   PATHSPEC_PREFER_FULL,
   prefix, argv);
 
-   read_cache_preload();
+   read_index_preload(_index, );
refresh_index(_index, REFRESH_QUIET|REFRESH_UNMERGED, , 
NULL, NULL);
 
fd = hold_locked_index(_lock, 0);
diff --git a/builtin/describe.c b/builtin/describe.c
index a5cd8c513f..0229458ac6 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -531,7 +531,7 @@ int cmd_describe(int argc, const char **argv, const char 
*prefix)
static struct lock_file index_lock;
int fd;
 
-   read_cache_preload(NULL);
+   read_index_preload(_index, NULL);
refresh_index(_index, 
REFRESH_QUIET|REFRESH_UNMERGED,
  NULL, NULL, NULL);
fd = hold_locked_index(_lock, 0);
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 15c61fd8d1..d400d8c1fc 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -62,8 +62,8 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
(rev.diffopt.output_format & DIFF_FORMAT_PATCH))
rev.combine_merges = rev.dense_combined_merges = 1;
 
-   if (read_cache_preload() < 0) {
-   perror("read_cache_preload");
+   if (read_index_preload(_index, ) < 0) {
+   perror("read_index_preload");
return -1;
}
result = run_diff_files(, options);
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 49fd64d4ce..3fbe33a90a 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -44,8 +44,8 @@ int cmd_diff_index(int argc, const char **argv, const char 
*prefix)
usage(diff_cache_usage);
if (!cached) {
setup_work_tree();
-   if (read_cache_preload() < 0) {
-   perror("read_cache_preload");
+   if (read_index_preload(_index, ) < 0) {
+   perror("read_index_preload");
return -1;
}
} else if (read_index(_index) < 0) {
diff --git a/builtin/diff.c b/builtin/diff.c
index ed9edb2d0c..0ae33bce2b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -144,8 +144,8 @@ static int builtin_diff_index(struct rev_info *revs,
usage(builtin_diff_usage);
if (!cached) {
setup_work_tree();
-   if (read_cache_preload(>diffopt.pathspec) < 0) {
-   perror("read_cache_preload");
+   if (read_index_preload(_index, >diffopt.pathspec) < 
0) {
+   perror("read_index_preload");
return -1;
}
} else if 

[PATCH 03/24] cache.h: drop read_cache_from

[Stefan Beller]

coccinelle patch:
@@ expression E; @@
-read_cache_from(E)
+read_index_from(_index, E)

additionally drop the define from cache.h manually

Signed-off-by: Stefan Beller 
---
 apply.c  | 2 +-
 builtin/am.c | 4 ++--
 builtin/commit.c | 6 +++---
 cache.h  | 1 -
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/apply.c b/apply.c
index ae1b659b68..159e039a18 100644
--- a/apply.c
+++ b/apply.c
@@ -3985,7 +3985,7 @@ static int check_patch_list(struct apply_state *state, 
struct patch *patch)
 static int read_apply_cache(struct apply_state *state)
 {
if (state->index_file)
-   return read_cache_from(state->index_file);
+   return read_index_from(_index, state->index_file);
else
return read_index(_index);
 }
diff --git a/builtin/am.c b/builtin/am.c
index c6a679d8e1..cb3e4dff63 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1557,7 +1557,7 @@ static int run_apply(const struct am_state *state, const 
char *index_file)
if (index_file) {
/* Reload index as apply_all_patches() will have modified it. */
discard_cache();
-   read_cache_from(index_file);
+   read_index_from(_index, index_file);
}
 
return 0;
@@ -1600,7 +1600,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
return error("could not build fake ancestor");
 
discard_cache();
-   read_cache_from(index_path);
+   read_index_from(_index, index_path);
 
if (write_index_as_tree(orig_tree.hash, _index, index_path, 0, 
NULL))
return error(_("Repository lacks necessary blobs to fall back 
on 3-way merge."));
diff --git a/builtin/commit.c b/builtin/commit.c
index 01d298c836..65a04ac199 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -370,7 +370,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
unsetenv(INDEX_ENVIRONMENT);
 
discard_cache();
-   read_cache_from(get_lock_file_path(_lock));
+   read_index_from(_index, get_lock_file_path(_lock));
if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
if (reopen_lock_file(_lock) < 0)
die(_("unable to write index file"));
@@ -489,7 +489,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
 
discard_cache();
ret = get_lock_file_path(_lock);
-   read_cache_from(ret);
+   read_index_from(_index, ret);
return ret;
 }
 
@@ -949,7 +949,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
 * the editor and after we invoke run_status above.
 */
discard_cache();
-   read_cache_from(index_file);
+   read_index_from(_index, index_file);
if (update_main_cache_tree(0)) {
error(_("Error building trees"));
return 0;
diff --git a/cache.h b/cache.h
index 4e913d1346..6abf48dcc3 100644
--- a/cache.h
+++ b/cache.h
@@ -354,7 +354,6 @@ extern void free_name_hash(struct index_state *istate);
 
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
-#define read_cache_from(path) read_index_from(_index, (path))
 #define read_cache_preload(pathspec) read_index_preload(_index, (pathspec))
 #define is_cache_unborn() is_index_unborn(_index)
 #define read_cache_unmerged() read_index_unmerged(_index)
-- 
2.13.0.rc1.39.ga6db8bfa24

[PATCH 01/24] cache.h: drop read_cache()

[Stefan Beller]

This patch is produced via coccinelle using this semantic patch:

@@ @@
-read_cache()
+read_index(_index)

Additional manual editing:
* drop define in cache.h
* a comment in builtin/check-ignore.c and read-cache.c were
  converted
* builtin/diff.c: fix error message referencing the function.

Signed-off-by: Stefan Beller 
---
 apply.c  |  2 +-
 builtin/add.c|  4 ++--
 builtin/am.c |  2 +-
 builtin/blame.c  |  4 ++--
 builtin/check-attr.c |  2 +-
 builtin/check-ignore.c   |  4 ++--
 builtin/checkout-index.c |  2 +-
 builtin/clean.c  |  2 +-
 builtin/commit.c |  4 ++--
 builtin/diff-index.c |  2 +-
 builtin/diff.c   |  6 +++---
 builtin/fsck.c   |  2 +-
 builtin/grep.c   |  2 +-
 builtin/ls-files.c   |  2 +-
 builtin/merge-index.c|  2 +-
 builtin/mv.c |  2 +-
 builtin/reset.c  |  2 +-
 builtin/rev-parse.c  |  2 +-
 builtin/rm.c |  2 +-
 builtin/submodule--helper.c  |  2 +-
 builtin/update-index.c   |  2 +-
 cache.h  |  1 -
 check-racy.c |  2 +-
 diff.c   |  2 +-
 merge-recursive.c|  2 +-
 merge.c  |  2 +-
 read-cache.c |  2 +-
 rerere.c |  6 +++---
 revision.c   |  4 ++--
 sequencer.c  |  6 +++---
 sha1_name.c  |  2 +-
 submodule.c  |  4 ++--
 t/helper/test-dump-cache-tree.c  |  2 +-
 t/helper/test-dump-untracked-cache.c |  2 +-
 t/helper/test-lazy-init-name-hash.c  | 10 +-
 t/helper/test-read-cache.c   |  2 +-
 t/helper/test-scrap-cache-tree.c |  2 +-
 37 files changed, 52 insertions(+), 53 deletions(-)

diff --git a/apply.c b/apply.c
index e6dbab26ad..82701d6580 100644
--- a/apply.c
+++ b/apply.c
@@ -3987,7 +3987,7 @@ static int read_apply_cache(struct apply_state *state)
if (state->index_file)
return read_cache_from(state->index_file);
else
-   return read_cache();
+   return read_index(_index);
 }
 
 /* This function tries to read the object name from the current index */
diff --git a/builtin/add.c b/builtin/add.c
index 9f53f020d0..ab6d04e8db 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -205,7 +205,7 @@ static int edit_patch(int argc, const char **argv, const 
char *prefix)
 
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 
-   if (read_cache() < 0)
+   if (read_index(_index) < 0)
die(_("Could not read the index"));
 
init_revisions(, prefix);
@@ -376,7 +376,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
return 0;
}
 
-   if (read_cache() < 0)
+   if (read_index(_index) < 0)
die(_("index file corrupt"));
 
/*
diff --git a/builtin/am.c b/builtin/am.c
index a95dd8b4e6..4405d7307c 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1633,7 +1633,7 @@ static int fall_back_threeway(const struct am_state 
*state, const char *index_pa
say(state, stdout, _("Falling back to patching base and 3-way 
merge..."));
 
discard_cache();
-   read_cache();
+   read_index(_index);
 
/*
 * This is not so wrong. Depending on which base we picked, orig_tree
diff --git a/builtin/blame.c b/builtin/blame.c
index 07506a3e45..59955208fd 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
unsigned mode;
struct strbuf msg = STRBUF_INIT;
 
-   read_cache();
+   read_index(_index);
time();
commit = alloc_commit_node();
commit->object.parsed = 1;
@@ -2395,7 +2395,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
 * want to run "diff-index --cached".
 */
discard_cache();
-   read_cache();
+   read_index(_index);
 
len = strlen(path);
if (!mode) {
diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 4d01ca0c8b..9cc3675d62 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -114,7 +114,7 @@ int cmd_check_attr(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, check_attr_options,
 check_attr_usage, PARSE_OPT_KEEP_DASHDASH);
 
-   if (read_cache() < 0) {
+   if (read_index(_index) < 0) {
die("invalid cache");
}
 
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c

[PATCH 00/24] Retire NO_THE_INDEX_COMPATIBILITY_MACROS

[Stefan Beller]

This is the follow up to [1], but actually completed now.

The reasoning why this series is a good idea is in the commit message of the
last patch, quoted here:

The NO_THE_INDEX_COMPATIBILITY_MACROS pre processor setting was introduced
in 4aab5b46f4 (Make read-cache.c "the_index" free., 2007-04-01), stating:

This makes all low-level functions defined in read-cache.c to
take an explicit index_state structure as their first parameter,
to specify which index to work on.

The reasoning is very vague, maybe indicating that having the index
specified to work on is easier to for the new reader to understand what
is going on.

All preceding patches worked on retiring functions that do not take an
explicit index as to where to perform its work. Spelling out the reasons
why we want to specify the index at each call:

1)  Ease of understanding:
   The recent patches dropped a lot of macros that may confuse new people
   diving into the code base.
2a) Spelling out global state explicitly:
   Anything you do in submodule land today needs to spawn new processes in
   the submodule. This is cumbersome and not performant. So we may want to
   have an abstraction of a repo, i.e. all repository state in one struct.
   That way we can open a submodule in-process and perform the required
   actions without spawning a process. The road to this future is a long
   road, and we have to start somewhere. Exposing the global state seems
   like a good starter point.
2b) Spelling out global state explicitly (II): (Jeff Hostetler wrote)
   In addition to (eventually) allowing multiple repos be open at the same
   time for submodules, it would also help with various multi-threading
   efforts.  For example, we have loops that do a
   "for (k = 0, k < active_nr; k++) {...}"  There is no visual clue in that
   code that it references "the_index" and therefore should be subject to
   the same locking.  Granted, this is a trivial example, but goes to the
   argument that the code has lots of subtle global variables and macros
   that make it difficult to reason about the code.

   This step would help un-hide this.
   

Review as well as critique if this is the right approach to an endgame with
less globals splattered throughout the codebase is welcome.

Thanks,
Stefan

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

Stefan Beller (24):
  cache.h: drop read_cache()
  cache.h: drop active_* macros
  cache.h: drop read_cache_from
  cache.h: drop read_cache_preload(pathspec)
  cache.h: drop read_cache_unmerged()
  unpack-trees.c: rename parameter 'the_index'
  cache.h: drop read_blob_data_from_cache
  cache.h: drop unmerge_cache[_entry_at]
  cache.h: drop resolve_undo_clear
  cache.h: drop cache_name_is_other
  cache.h: drop cache_file_exists
  cache.h: drop cache_dir_exists
  cache.h: drop is_cache_unborn(), discard_cache(), unmerged_cache()
  cache.h: drop cache_name_pos
  cache.h: drop add_cache_entry
  cache.h: drop rename_cache_entry_at
  cache.h: drop remove_file_from_cache
  cache.h: drop add_to_cache
  cache.h: drop add_file_to_cache
  cache.h: drop chmod_cache_entry
  cache.h: drop refresh_cache
  cache.h: drop ce_modified
  cache.h: drop ce_match_stat
  cache.h: retire NO_THE_INDEX_COMPATIBILITY_MACROS

 apply.c  | 32 +++--
 attr.c   |  1 -
 builtin/add.c| 12 ++---
 builtin/am.c | 26 +--
 builtin/blame.c  | 19 
 builtin/check-attr.c |  2 +-
 builtin/check-ignore.c   |  4 +-
 builtin/checkout-index.c | 12 ++---
 builtin/checkout.c   | 66 +-
 builtin/clean.c  |  4 +-
 builtin/commit.c | 52 ++---
 builtin/describe.c   |  2 +-
 builtin/diff-files.c |  4 +-
 builtin/diff-index.c |  6 +--
 builtin/diff.c   | 19 
 builtin/fsck.c   | 14 +++---
 builtin/grep.c   | 10 ++--
 builtin/ls-files.c   | 49 +--
 builtin/merge-index.c| 14 +++---
 builtin/merge.c  | 24 +-
 builtin/mv.c | 22 -
 builtin/pull.c   |  4 +-
 builtin/read-tree.c  |  8 ++--
 builtin/reset.c  | 11 +++--
 builtin/rev-parse.c  |  2 +-
 builtin/rm.c | 26 +--
 builtin/submodule--helper.c  | 10 ++--
 builtin/update-index.c   | 91 +++-
 cache.h  | 35 --
 check-racy.c | 10 ++--
 convert.c|  4 +-
 diff-lib.c   |  8 ++--
 diff.c   | 14 +++---
 dir.c  

Re: [PATCH 0/5] Abide by our own rules regarding line endings

[Jonathan Nieder]

Hi,

Johannes Schindelin wrote:

> Over the past decade, there have been a couple of attempts to remedy the
[...]

I'm intentionally skimming this cover letter, since anything important
it says needs to also be in the commit messages.

[...]
> Without these fixes, Git will fail to build and pass the test suite, as
> can be verified even on Linux using this cadence:
>
>   git config core.autocrlf true
>   rm .git/index && git stash
>   make DEVELOPER=1 -j15 test

This should go in a commit message (or perhaps in all of them).

[...]
> Johannes Schindelin (5):
>   Fix build with core.autocrlf=true
>   git-new-workdir: mark script as LF-only
>   completion: mark bash script as LF-only
>   Fix the remaining tests that failed with core.autocrlf=true
>   t4051: mark supporting files as requiring LF-only line endings

With or without that change,
Reviewed-by: Jonathan Nieder 

The t/README bit in patch 4/5 is sad (I want to be able to open
t/README using an old version of Notepad) but changing that test to
use another file seems out of scope for what this series does.

Thanks,
Jonathan

Re: Proposal for missing blob support in Git repos

[Jonathan Tan]

On 05/02/2017 11:32 AM, Ævar Arnfjörð Bjarmason wrote:

On Tue, May 2, 2017 at 7:21 PM, Jonathan Tan  wrote:

On Mon, May 1, 2017 at 6:41 PM, Junio C Hamano  wrote:

Jonathan Tan  writes:


On 05/01/2017 04:29 PM, Junio C Hamano wrote:

Jonathan Tan  writes:


Thanks for your comments. If you're referring to the codepath
involving write_sha1_file() (for example, builtin/hash-object ->
index_fd or builtin/unpack-objects), that is fine because
write_sha1_file() invokes freshen_packed_object() and
freshen_loose_object() directly to check if the object already exists
(and thus does not invoke the new mechanism in this patch).


Is that a good thing, though?  It means that you an attacker can
feed one version to the remote object store your "grab blob" hook
gets the blobs from, and have you add a colliding object locally,
and the usual "are we recording the same object as existing one?"
check is bypassed.


If I understand this correctly, what you mean is the situation where
the hook adds an object to the local repo, overriding another object
of the same name?


No.

write_sha1_file() pays attention to objects already in the local
object store to avoid hash collisions that can be used to replace a
known-to-be-good object and that is done as a security measure.
What I am reading in your response was that this new mechanism
bypasses that, and I was wondering if that is a good thing.


Oh, what I meant was that write_sha1_file() bypasses the new
mechanism, not that the new mechanism bypasses the checks in
write_sha1_file().

To be clear, here's what happens when write_sha1_file() is invoked
(before and after this patch - this patch does not affect
write_sha1_file at all):
1. (some details omitted)
2. call freshen_packed_object
3, call freshen_loose_object if necessary
4. write object (if freshen_packed_object and freshen_loose_object do
not both return 0)

Nothing changes in this patch (whether a hook is defined or not).


But don't the semantics change in the sense that before
core.missingBlobCommand you couldn't write a new blob SHA1 that was
already part of your history,


Strictly speaking, you can already do this if you don't have the blob in 
your local repo (for example, with shallow clones - you likely wouldn't 
have blobs pointed to by historical commits outside whatever depth is set).


> whereas with this change

write_sha1_file() might write what it considers to be a new blob, but
it's actually colliding with an existing blob, but write_sha1_file()
doesn't know that because knowing would involve asking the hook to
fetch the blob?


Yes, this might happen.

I see the semantics as "don't write what you already have", where "have" 
means what you have in local storage, but if you extend "have" to what 
upstream has, then yes, you're right that this changes (ignoring shallow 
clones).


This does remove a resistance that we have against hash collision (in 
that normally we would have the correct object for a given hash and can 
resist other servers trying to introduce a wrong object, but now that is 
no longer the case), but I think it's better than consulting the hook 
whenever you want to write anything (which is also a change in semantics 
in that you're consulting an external source whenever you're writing an 
object, besides the performance implications).



And here's what happens when has_sha1_file (or another function listed
in the commit message) is invoked:
1. check for existence of packed object of the requested name
2. check for existence of loose object of the requested name
3. check again for existence of packed object of the requested name
4. if a hook is defined, invoke the hook and repeat 1-3

Here, in step 4, the hook could do whatever it wants to the repository.


This might be a bit of early bikeshedding, but then again the lack of
early bikeshedding tends to turn into standards.

Wouldn't it be better to name this core.missingObjectCommand & have
the hook take a list on stdin like:

[]

And have the hook respond:

[]

I.e. what you'd do now is send this to the hook:

1  blobmissing

And the hook would respond:

1  ok

But this leaves open the door addressing this potential edge case with
writing new blobs in the future, i.e. write_sha1_file() could call it
as:

1  blobnew

And the hook could either respond immediately as:

1  ok

If it's in some #YOLO mode where it's not going to check for colliding
blobs over the network, or alternatively & ask the parent repo if it
has those blobs, and if so print:

1  collision

Or something like that.

This also enables future lazy loading of trees/commits from the same
API, and for the hook to respond out-of-order to the input it gets as
it can, since each request is prefixed with an incrementing request
id.


My initial thought is that it would be better to extend hook support by 
adding 

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 10:51 PM, Kevin Daudt  wrote:
> On Tue, May 02, 2017 at 08:52:21PM +0200, Ęvar Arnfjörš Bjarmason wrote:
>>
>>  * Due to the bizarro existing semantics of the configure script noted
>> upthread if you have a git build script that does --with-libpcre & you
>> have libpcre1 installed, it'll link to it, but now since
>> --with-libpcre defaults to libpcre2 it'll silently skip linking to it
>> if you don't have it installed.
>>
>
> Case in point: The Archlinux git-git aur package[0] (community maintained,
> latest git version) does run ./configure without --with-libpcre, but
>  requests it from make with USE_LIBPCRE=1.
>
> I noticed when trying git grep -P which then failed.
>
> [0]: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=git-git

Whatever's going on there is unrelated to the issue I'm talking about,
but if that's producing a package where -P doesn't work that looks
like a bug in the Makefile.

The way --with-libpcre works now is that it second guesses you, i.e.
it'll put USE_LIBPCRE=YesPlease into config.mak.autogen (sourced by
the Makefile) only if you supply --with-libpcre *and* it can find a
working libpcre on the system, otherwise it silently ignores you.

Whatever you supply to the configure script it'll be overridden by
Makefile arguments if present, but even if there was another bug where
./configure arguments took precedence over Makefile arguments I don't
see how it would apply here, in this case nothing libpcre related will
be written to config.mak.autogen.

So I must be missing something. I don't see how that package build
doesn't either fail at compile time because it doesn't have pcre, or
work, in which case the -P option will work.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Kevin Daudt]

On Tue, May 02, 2017 at 08:52:21PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
>  * Due to the bizarro existing semantics of the configure script noted
> upthread if you have a git build script that does --with-libpcre & you
> have libpcre1 installed, it'll link to it, but now since
> --with-libpcre defaults to libpcre2 it'll silently skip linking to it
> if you don't have it installed.
> 

Case in point: The Archlinux git-git aur package[0] (community maintained,
latest git version) does run ./configure without --with-libpcre, but
 requests it from make with USE_LIBPCRE=1.

I noticed when trying git grep -P which then failed.

[0]: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=git-git

Re: Reference help

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 9:33 PM, Desjardin, Donald
 wrote:
> Sorry if this is not the place for this.
>
> I'm looking for any reference to potential problems when updating a git 
> client (say from 1.7.N to 1.8.N) with old workspaces.
>
> The scenario is this:
> Lots of developers use a single machine for work
> They have lots of workspaces created using the old client
> We want to upgrade to the new client
>
> Are there any potential problems just upgrading the client and NOT re-cloning 
> their workspaces (or stashing or committing or pushing)?
>
> Subversion had/has some feature that could tell that the workspace was 
> created using an older client and you could magically run something to update 
> the workspace.
>
> I'm not above telling all my developers to commit/push to a feature branch, 
> remove the workspaces and re-clone as needed on a flag-day, but I'd like to 
> know if I have to.
>
> If there is any documentation that talks about this (I know 1.7 is old).

Git, unlike Subversion has used the same stable underlying storage
format for all of its history. So you certainly don't need to migrate
these checkouts when you upgrade git, they'll just work.

There are several things that changed in the UI, some in incompatible
ways, most prominently the behavior of "git push", but it's unlikely
anyone will be affected by that beyond some minor annoyance.

You can see announcements of major changes in git's release notes,
which you can browse here:
https://github.com/git/git/tree/master/Documentation/RelNotes

E.g. 1.8.0.txt in that directory will have notes for upgrading from
1.7.x to 1.8.0.

Note that both 1.7.0 and 1.8.0 are really old, from 2010 & 2012
respectively. Although it'll be a bigger jump you might want to
consider updating to the most recent release directly, currently
2.12.2, this'll be more changes at once, but you won't have to go from
1.7 to 1.8, 1.8 to 1.9 etc.

Reference help

[Desjardin, Donald]

Sorry if this is not the place for this.

I'm looking for any reference to potential problems when updating a git client 
(say from 1.7.N to 1.8.N) with old workspaces.

The scenario is this:
Lots of developers use a single machine for work
They have lots of workspaces created using the old client
We want to upgrade to the new client

Are there any potential problems just upgrading the client and NOT re-cloning 
their workspaces (or stashing or committing or pushing)?

Subversion had/has some feature that could tell that the workspace was created 
using an older client and you could magically run something to update the 
workspace.

I'm not above telling all my developers to commit/push to a feature branch, 
remove the workspaces and re-clone as needed on a flag-day, but I'd like to 
know if I have to.

If there is any documentation that talks about this (I know 1.7 is old).

Thanks,

Don Desjardin

Re: [PATCHv2 0/3] Some submodule bugfixes

[Brandon Williams]

On 05/02, Stefan Beller wrote:
> v2:
>  * I dropped the RFC patches for reattaching HEAD as that is not the main
>concern of this series.
>  * patch1 is just about dropping cp1 to reusing cp, instead of additionally
>"fixing" mem leaks as finish_command does that for us
>  * reordered the patches, former patch 3 now as patch 2 (avoid auto-discovery
>in new working tree manipulator code)
>  * Regarding former patch2, I wrote:
> > Junio wrote:
> >> Sounds good (if only because this makes it similar to other
> >> codepaths).
> >>
> >> Is this something whose breakage before the patch is easily
> >> demonstrated with a test?
> 
> > I'll try to come up with a test in a reroll.
>  
> done
> 
> Thanks,
> Stefan

Changes look good.

> 
> 
> Stefan Beller (3):
>   submodule_move_head: reuse child_process structure for futher commands
>   submodule: avoid auto-discovery in new working tree manipulator code
>   submodule: properly recurse for read-tree and checkout
> 
>  submodule.c| 21 +++--
>  t/lib-submodule-update.sh  |  7 +--
>  t/t1013-read-tree-submodule.sh |  1 -
>  t/t2013-checkout-submodule.sh  |  1 -
>  4 files changed, 12 insertions(+), 18 deletions(-)
> 
> -- 
> 2.13.0.rc1.19.g083305f9b1
> 

-- 
Brandon Williams

[PATCHv2 2/3] submodule: avoid auto-discovery in new working tree manipulator code

[Stefan Beller]

All commands that are run in a submodule, are run in a correct setup,
there is no need to prepare the environment without setting the GIT_DIR
variable. By setting the GIT_DIR variable we fix issues as discussed in
10f5c52656 (submodule: avoid auto-discovery in
prepare_submodule_repo_env(), 2016-09-01)

Signed-off-by: Stefan Beller 
---
 submodule.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/submodule.c b/submodule.c
index a2377ce019..b0141a66dd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1363,7 +1363,7 @@ static int submodule_has_dirty_index(const struct 
submodule *sub)
 {
struct child_process cp = CHILD_PROCESS_INIT;
 
-   prepare_submodule_repo_env_no_git_dir(_array);
+   prepare_submodule_repo_env(_array);
 
cp.git_cmd = 1;
argv_array_pushl(, "diff-index", "--quiet",
@@ -1380,7 +1380,7 @@ static int submodule_has_dirty_index(const struct 
submodule *sub)
 static void submodule_reset_index(const char *path)
 {
struct child_process cp = CHILD_PROCESS_INIT;
-   prepare_submodule_repo_env_no_git_dir(_array);
+   prepare_submodule_repo_env(_array);
 
cp.git_cmd = 1;
cp.no_stdin = 1;
@@ -1438,7 +1438,7 @@ int submodule_move_head(const char *path,
}
}
 
-   prepare_submodule_repo_env_no_git_dir(_array);
+   prepare_submodule_repo_env(_array);
 
cp.git_cmd = 1;
cp.no_stdin = 1;
-- 
2.13.0.rc1.19.g083305f9b1

[PATCHv2 3/3] submodule: properly recurse for read-tree and checkout

[Stefan Beller]

We forgot to prepare the submodule env, which is only a problem for
nested submodules. See 2e5d6503bd (ls-files: fix recurse-submodules
with nested submodules, 2017-04-13) for further explanation.

To come up with a proper test for this, we'd need to look at nested
submodules just as in that given commit. It turns out we're lucky
and these tests already exist, but are marked as failing. We need
to pass `--recurse-submodules` to read-tree additionally to make
these tests pass. Passing that flag alone would not make the tests
pass, such that this covers testing for the bug fix of the submodule
env as well.

Signed-off-by: Stefan Beller 
---
 submodule.c| 3 ++-
 t/lib-submodule-update.sh  | 7 +--
 t/t1013-read-tree-submodule.sh | 1 -
 t/t2013-checkout-submodule.sh  | 1 -
 4 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/submodule.c b/submodule.c
index b0141a66dd..b3ae642f29 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1446,7 +1446,7 @@ int submodule_move_head(const char *path,
 
argv_array_pushf(, "--super-prefix=%s%s/",
get_super_prefix_or_empty(), path);
-   argv_array_pushl(, "read-tree", NULL);
+   argv_array_pushl(, "read-tree", "--recurse-submodules", NULL);
 
if (flags & SUBMODULE_MOVE_HEAD_DRY_RUN)
argv_array_push(, "-n");
@@ -1474,6 +1474,7 @@ int submodule_move_head(const char *path,
cp.no_stdin = 1;
cp.dir = path;
 
+   prepare_submodule_repo_env(_array);
argv_array_pushl(, "update-ref", "HEAD", new, 
NULL);
 
if (run_command()) {
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index fb4f7b014e..2c17826e95 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -787,11 +787,6 @@ test_submodule_switch_recursing () {
then
RESULTDS=failure
fi
-   RESULTR=success
-   if test "$KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED" = 1
-   then
-   RESULTR=failure
-   fi
RESULTOI=success
if test "$KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED" = 1
then
@@ -1003,7 +998,7 @@ test_submodule_switch_recursing () {
'
 
# recursing deeper than one level doesn't work yet.
-   test_expect_$RESULTR "$command: modified submodule updates submodule 
recursively" '
+   test_expect_success "$command: modified submodule updates submodule 
recursively" '
prolog &&
reset_work_tree_to_interested add_nested_sub &&
(
diff --git a/t/t1013-read-tree-submodule.sh b/t/t1013-read-tree-submodule.sh
index de1ba02dc5..7019d0a04f 100755
--- a/t/t1013-read-tree-submodule.sh
+++ b/t/t1013-read-tree-submodule.sh
@@ -5,7 +5,6 @@ test_description='read-tree can handle submodules'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
 
-KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1
 KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
 KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1
 
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index e8f70b806f..aa35223369 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -64,7 +64,6 @@ test_expect_success '"checkout " honors 
submodule.*.ignore from .git/
 '
 
 KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1
-KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1
 test_submodule_switch_recursing "git checkout --recurse-submodules"
 
 test_submodule_forced_switch_recursing "git checkout -f --recurse-submodules"
-- 
2.13.0.rc1.19.g083305f9b1

[PATCHv2 0/3] Some submodule bugfixes

[Stefan Beller]

v2:
 * I dropped the RFC patches for reattaching HEAD as that is not the main
   concern of this series.
 * patch1 is just about dropping cp1 to reusing cp, instead of additionally
   "fixing" mem leaks as finish_command does that for us
 * reordered the patches, former patch 3 now as patch 2 (avoid auto-discovery
   in new working tree manipulator code)
 * Regarding former patch2, I wrote:
> Junio wrote:
>> Sounds good (if only because this makes it similar to other
>> codepaths).
>>
>> Is this something whose breakage before the patch is easily
>> demonstrated with a test?

> I'll try to come up with a test in a reroll.
 
done

Thanks,
Stefan


Stefan Beller (3):
  submodule_move_head: reuse child_process structure for futher commands
  submodule: avoid auto-discovery in new working tree manipulator code
  submodule: properly recurse for read-tree and checkout

 submodule.c| 21 +++--
 t/lib-submodule-update.sh  |  7 +--
 t/t1013-read-tree-submodule.sh |  1 -
 t/t2013-checkout-submodule.sh  |  1 -
 4 files changed, 12 insertions(+), 18 deletions(-)

-- 
2.13.0.rc1.19.g083305f9b1

[PATCHv2 1/3] submodule_move_head: reuse child_process structure for futher commands

[Stefan Beller]

We do not need to declare another struct child_process, but we can just
reuse the existing `cp` struct.

Signed-off-by: Stefan Beller 
---
 submodule.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/submodule.c b/submodule.c
index d3299e29c0..a2377ce019 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1468,15 +1468,15 @@ int submodule_move_head(const char *path,
 
if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
if (new) {
-   struct child_process cp1 = CHILD_PROCESS_INIT;
+   child_process_init();
/* also set the HEAD accordingly */
-   cp1.git_cmd = 1;
-   cp1.no_stdin = 1;
-   cp1.dir = path;
+   cp.git_cmd = 1;
+   cp.no_stdin = 1;
+   cp.dir = path;
 
-   argv_array_pushl(, "update-ref", "HEAD", new, 
NULL);
+   argv_array_pushl(, "update-ref", "HEAD", new, 
NULL);
 
-   if (run_command()) {
+   if (run_command()) {
ret = -1;
goto out;
}
-- 
2.13.0.rc1.19.g083305f9b1

Re: [PATCH v2 5/6] submodule: improve submodule_has_commits

[Brandon Williams]

On 05/02, Brandon Williams wrote:
> On 05/02, Stefan Beller wrote:
> > On Tue, May 2, 2017 at 10:25 AM, Brandon Williams  wrote:
> > > On 05/01, Stefan Beller wrote:
> > >> On Mon, May 1, 2017 at 6:02 PM, Brandon Williams  
> > >> wrote:
> > >> > +
> > >> > +   if (capture_command(, , GIT_MAX_HEXSZ + 1) || 
> > >> > out.len)
> > >>
> > >> eh, I gave too much and self-contradicting feedback here earlier,
> > >> ideally I'd like to review this to be similar as:
> > >>
> > >> if (capture_command(, , GIT_MAX_HEXSZ + 1)
> > >> die("cannot capture git-rev-list in submodule '%s', sub->path);
> > >
> > > This wouldn't really work because if you provide a SHA1 to rev-list
> > > which it isn't able to find then it returns a non-zero exit code which
> > > would cause this to die, which isn't the desired behavior.
> > 
> > Oh. In that case, why do we even check for its stdout?
> > (from rev-lists man page)
> >--quiet
> >Don’t print anything to standard output. This form is primarily
> >meant to allow the caller to test the exit status to see if a 
> > range
> >of objects is fully connected (or not). It is faster than
> >redirecting stdout to /dev/null as the output does not have to be
> >formatted.
> > 
> 
> Sounds good, Let me take a look at using --quiet

>From our offline discussion --quiet doesn't do what we want here.  We
are checking (1) if the commit exists and (2) if it is reachable.  Using
--quiet would only satisfy (1)

> 
> > >
> > > I feel like you're making this a little too complicated, as all I'm
> > > doing is shuffling around already existing logic.  I understand the want
> > > to make things more robust but this seems unnecessarily complex.
> > 
> > ok. I was just giving my thoughts on how I would approach it.
> > 
> > Thanks,
> > Stefan
> 
> -- 
> Brandon Williams

-- 
Brandon Williams

Re: [PATCH v2 5/6] submodule: improve submodule_has_commits

[Brandon Williams]

On 05/02, Stefan Beller wrote:
> On Tue, May 2, 2017 at 10:25 AM, Brandon Williams  wrote:
> > On 05/01, Stefan Beller wrote:
> >> On Mon, May 1, 2017 at 6:02 PM, Brandon Williams  wrote:
> >> > +
> >> > +   if (capture_command(, , GIT_MAX_HEXSZ + 1) || 
> >> > out.len)
> >>
> >> eh, I gave too much and self-contradicting feedback here earlier,
> >> ideally I'd like to review this to be similar as:
> >>
> >> if (capture_command(, , GIT_MAX_HEXSZ + 1)
> >> die("cannot capture git-rev-list in submodule '%s', sub->path);
> >
> > This wouldn't really work because if you provide a SHA1 to rev-list
> > which it isn't able to find then it returns a non-zero exit code which
> > would cause this to die, which isn't the desired behavior.
> 
> Oh. In that case, why do we even check for its stdout?
> (from rev-lists man page)
>--quiet
>Don’t print anything to standard output. This form is primarily
>meant to allow the caller to test the exit status to see if a range
>of objects is fully connected (or not). It is faster than
>redirecting stdout to /dev/null as the output does not have to be
>formatted.
> 

Sounds good, Let me take a look at using --quiet

> >
> > I feel like you're making this a little too complicated, as all I'm
> > doing is shuffling around already existing logic.  I understand the want
> > to make things more robust but this seems unnecessarily complex.
> 
> ok. I was just giving my thoughts on how I would approach it.
> 
> Thanks,
> Stefan

-- 
Brandon Williams

Re: [PATCH v2 00/53] object_id part 8

[Brandon Williams]

On 05/01, brian m. carlson wrote:
> This is the eighth series of patches to convert unsigned char [20] to
> struct object_id.  This series converts lookup_commit, lookup_blob,
> lookup_tree, lookup_tag, and finally parse_object to struct object_id.
> 
> A small number of functions have temporaries inserted during the
> conversion in order to allow conversion of functions that still need to
> take unsigned char *; they are removed either later in the series or
> will be in a future series.
> 
> This series can be fetched from the object-id-part8 branch from either
> of the follwing:
> 
> https://github.com/bk2204/git
> https://git.crustytoothpaste.net/git/bmc/git.git

I've looked through the series and overall everything looks good.

-- 
Brandon Williams

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 6:05 PM, Johannes Schindelin
 wrote:
> Hi Ævar,
>
> On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
>>  wrote:
>> >
>> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
>> >
>> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>> >>  - SQUASH???
>> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
>> >>  - grep: add support for PCRE v2
>> >>  - grep: add support for the PCRE v1 JIT API
>> >>  - perf: add a performance comparison test of grep -E and -P
>> >>  - grep: change the internal PCRE code & header names to be PCRE1
>> >>  - grep: change the internal PCRE macro names to be PCRE1
>> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>> >>  - log: add -P as a synonym for --perl-regexp
>> >>  - log: add exhaustive tests for pattern style options & config
>> >>  - grep: add a test for backreferences in PCRE patterns
>> >>  - Makefile & configure: reword outdated comment about PCRE
>> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>> >>  - grep: remove redundant regflags assignment under PCRE
>> >>  - grep: submodule-related case statements should die if new fields are 
>> >> added
>> >>  - grep: add tests for grep pattern types being passed to submodules
>> >>  - grep: amend submodule recursion test in preparation for rx engine 
>> >> testing
>> >>
>> >>  PCRE2, which has an API different from and incompatible with PCRE,
>> >>  can now be chosen to support "grep -P -e ''" and friends.
>> >
>> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
>> > variation of this error:
>> >
>> > CC blob.o
>> > In file included from revision.h:5:0,
>> >  from bisect.c:4:
>> > grep.h:16:19: fatal error: pcre2.h: No such file or directory
>> >  #include 
>> >^
>> > compilation terminated.
>> >
>> > Maybe this can be fixed before hitting `next`?
>>
>> This will be due to a combination of the build machine not having pcre
>> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
>> default PCRE implementation" patch, which makes v2 the default for
>> USE_LIBPCRE=YesPlease.
>>
>> Is it easy to install v2 on these build machines? Alternatively that
>> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
>> to use v1, but as explained in that commit I think it makes sense to
>> make v2 the default.
>
> Let me put it this way: Installing PCRE v1 in MSYS2 is as easy as
>
> pacman -Sy mingw-w64-x86_64-pcre
>
> To install PCRE v2, you would have to copy-edit
> https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-pcre/PKGBUILD,
> make sure that it builds by running it through
>
> makepkg-mingw -s
>
> possibly initializing a Git repository in the
> mingw-w64-pcre/src/${_realname}-${pkgver}/ directory, patching the source
> until it builds, committing the changes, adding them as patch files to the
> same directory as the new PKGBUILD, adjusting the PKGBUILD file to list
> the patch files with their checksums and to add the commands to apply
> them.
>
> Then (and this is a one time cost, fortunately) initializing two packages
> on BinTray (which we use to serve the Pacman packages of Git for Windows),
> then build and upload the packages.
>
> In short, PCRE v2 would be slightly (ahem, ahem) more involved than PCRE
> v1.
>
> I cannot imagine that MSYS2 is the only environment with that issue.

I think it's worth it to copy/paste the commit message where I made
this change, since we're having this discussion in this thread:

Makefile & configure: make PCRE v2 the default PCRE implementation

Change the USE_LIBPCRE=YesPlease & --with-libpcre flags to the
Makefile & configure script, respectively, to mean use PCRE v2, not
PCRE v1.

The legacy library previously available via those options is still
available on request via USE_LIBPCRE1=YesPlease or
--with-libpcre1. The existing USE_LIBPCRE2=YesPlease & --with-libpcre2
still explicitly ask for v2.

The v2 PCRE is stable & end-user compatible, all this change does is
change the default. Someone building a new git is likely to also have
packaged PCRE v2 sometime in the last 2 years since it was released.
If not they can choose to use the legacy v2 library by making the
trivial s/USE_LIBPCRE/USE_LIBPCRE1/ change, or package up PCRE v2.

New releases of PCRE v2 are already faster than PCRE v1, and not only
is all significant development is happening on v2, but bugs in
reported against v1 have started getting WONTFIX'd asking users to
just upgrade to v2.

So it makes 

Re: CYGWIN git cannot install python packages with pip

[ankostis]

Note that MSYS2 Git-12.2.1 also has no such problem.

On 2 May 2017 at 20:08, ankostis  wrote:
> On Windows, with Cygwin-git 02.12.2-1 the python command:
>
> pip install git+https://github.com/...`
>
> fails to work with the following error:
>
> ...
>
> Git-2.8.3 had no such problem.
>
> Any ideas?

Re: Proposal for missing blob support in Git repos

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 7:21 PM, Jonathan Tan  wrote:
> On Mon, May 1, 2017 at 6:41 PM, Junio C Hamano  wrote:
>> Jonathan Tan  writes:
>>
>>> On 05/01/2017 04:29 PM, Junio C Hamano wrote:
 Jonathan Tan  writes:

> Thanks for your comments. If you're referring to the codepath
> involving write_sha1_file() (for example, builtin/hash-object ->
> index_fd or builtin/unpack-objects), that is fine because
> write_sha1_file() invokes freshen_packed_object() and
> freshen_loose_object() directly to check if the object already exists
> (and thus does not invoke the new mechanism in this patch).

 Is that a good thing, though?  It means that you an attacker can
 feed one version to the remote object store your "grab blob" hook
 gets the blobs from, and have you add a colliding object locally,
 and the usual "are we recording the same object as existing one?"
 check is bypassed.
>>>
>>> If I understand this correctly, what you mean is the situation where
>>> the hook adds an object to the local repo, overriding another object
>>> of the same name?
>>
>> No.
>>
>> write_sha1_file() pays attention to objects already in the local
>> object store to avoid hash collisions that can be used to replace a
>> known-to-be-good object and that is done as a security measure.
>> What I am reading in your response was that this new mechanism
>> bypasses that, and I was wondering if that is a good thing.
>
> Oh, what I meant was that write_sha1_file() bypasses the new
> mechanism, not that the new mechanism bypasses the checks in
> write_sha1_file().
>
> To be clear, here's what happens when write_sha1_file() is invoked
> (before and after this patch - this patch does not affect
> write_sha1_file at all):
> 1. (some details omitted)
> 2. call freshen_packed_object
> 3, call freshen_loose_object if necessary
> 4. write object (if freshen_packed_object and freshen_loose_object do
> not both return 0)
>
> Nothing changes in this patch (whether a hook is defined or not).

But don't the semantics change in the sense that before
core.missingBlobCommand you couldn't write a new blob SHA1 that was
already part of your history, whereas with this change
write_sha1_file() might write what it considers to be a new blob, but
it's actually colliding with an existing blob, but write_sha1_file()
doesn't know that because knowing would involve asking the hook to
fetch the blob?

> And here's what happens when has_sha1_file (or another function listed
> in the commit message) is invoked:
> 1. check for existence of packed object of the requested name
> 2. check for existence of loose object of the requested name
> 3. check again for existence of packed object of the requested name
> 4. if a hook is defined, invoke the hook and repeat 1-3
>
> Here, in step 4, the hook could do whatever it wants to the repository.

This might be a bit of early bikeshedding, but then again the lack of
early bikeshedding tends to turn into standards.

Wouldn't it be better to name this core.missingObjectCommand & have
the hook take a list on stdin like:

[]

And have the hook respond:

[]

I.e. what you'd do now is send this to the hook:

1  blobmissing

And the hook would respond:

1  ok

But this leaves open the door addressing this potential edge case with
writing new blobs in the future, i.e. write_sha1_file() could call it
as:

1  blobnew

And the hook could either respond immediately as:

1  ok

If it's in some #YOLO mode where it's not going to check for colliding
blobs over the network, or alternatively & ask the parent repo if it
has those blobs, and if so print:

1  collision

Or something like that.

This also enables future lazy loading of trees/commits from the same
API, and for the hook to respond out-of-order to the input it gets as
it can, since each request is prefixed with an incrementing request
id.

Re: [PATCH v3 14/25] setup_discovered_git_dir(): plug memory leak

[Jeff King]

On Tue, May 02, 2017 at 10:20:29AM -0700, Stefan Beller wrote:

> > -   gitdir = real_pathdup(gitdir, 1);
> > +   gitdir = to_free = real_pathdup(gitdir, 1);
> > if (chdir(cwd->buf))
> > die_errno("Could not come back to cwd");
> 
> As the original motivation was to shut up Coverity, this may not
> accomplish that goal, as in the path of taking the die_errno, we do not
> free `to_free`. But that is ok as the actual goal is to hav no memleaks
> in the good case. A memleak just before a die is no big deal.

I think Coverity understands our NORETURN attributes, so this should be
fine (and if it doesn't, then we should fix that in the model file; but
from the general results I've seen, it does).

-Peff

Re: [PATCH v2 03/53] Convert struct cache_tree to use struct object_id

[Brandon Williams]

On 05/01, brian m. carlson wrote:
> Convert the sha1 member of struct cache_tree to struct object_id by
> changing the definition and applying the following semantic patch, plus
> the standard object_id transforms:
> 
> @@
> struct cache_tree E1;
> @@
> - E1.sha1
> + E1.oid.hash
> 
> @@
> struct cache_tree *E1;
> @@
> - E1->sha1
> + E1->oid.hash
> 
> Fix up one reference to active_cache_tree which was not automatically
> caught by Coccinelle.  These changes are prerequisites for converting
> parse_object.
> 
> Signed-off-by: brian m. carlson 
> ---
>  builtin/commit.c|  2 +-
>  builtin/fsck.c  |  4 ++--
>  cache-tree.c| 31 ---
>  cache-tree.h|  3 ++-
>  merge-recursive.c   |  2 +-
>  revision.c  |  2 +-
>  sequencer.c |  3 ++-
>  t/helper/test-dump-cache-tree.c |  4 ++--
>  8 files changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 1d805f5da..8685c888f 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1758,7 +1758,7 @@ int cmd_commit(int argc, const char **argv, const char 
> *prefix)
>   append_merge_tag_headers(parents, );
>   }
>  
> - if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1,
> + if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->oid.hash,
>parents, oid.hash, author_ident.buf, sign_commit, 
> extra)) {
>   rollback_index_files();
>   die(_("failed to write commit object"));
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index b5e13a455..c40e14de6 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -599,10 +599,10 @@ static int fsck_cache_tree(struct cache_tree *it)
>   fprintf(stderr, "Checking cache tree\n");
>  
>   if (0 <= it->entry_count) {
> - struct object *obj = parse_object(it->sha1);
> + struct object *obj = parse_object(it->oid.hash);
>   if (!obj) {
>   error("%s: invalid sha1 pointer in cache-tree",
> -   sha1_to_hex(it->sha1));
> +   oid_to_hex(>oid));
>   errors_found |= ERROR_REFS;
>   return 1;
>   }
> diff --git a/cache-tree.c b/cache-tree.c
> index 345ea3596..35d507ed7 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -225,7 +225,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
>   int i;
>   if (!it)
>   return 0;
> - if (it->entry_count < 0 || !has_sha1_file(it->sha1))
> + if (it->entry_count < 0 || !has_sha1_file(it->oid.hash))
>   return 0;
>   for (i = 0; i < it->subtree_nr; i++) {
>   if (!cache_tree_fully_valid(it->down[i]->cache_tree))
> @@ -253,7 +253,7 @@ static int update_one(struct cache_tree *it,
>  
>   *skip_count = 0;
>  
> - if (0 <= it->entry_count && has_sha1_file(it->sha1))
> + if (0 <= it->entry_count && has_sha1_file(it->oid.hash))
>   return it->entry_count;
>  
>   /*
> @@ -340,7 +340,7 @@ static int update_one(struct cache_tree *it,
>   die("cache-tree.c: '%.*s' in '%s' not found",
>   entlen, path + baselen, path);
>   i += sub->count;
> - sha1 = sub->cache_tree->sha1;
> + sha1 = sub->cache_tree->oid.hash;
>   mode = S_IFDIR;
>   contains_ita = sub->cache_tree->entry_count < 0;
>   if (contains_ita) {
> @@ -402,12 +402,13 @@ static int update_one(struct cache_tree *it,
>   unsigned char sha1[20];
>   hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
>   if (has_sha1_file(sha1))
> - hashcpy(it->sha1, sha1);
> + hashcpy(it->oid.hash, sha1);
>   else
>   to_invalidate = 1;
>   } else if (dryrun)
> - hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);
> - else if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) {
> + hash_sha1_file(buffer.buf, buffer.len, tree_type,
> +it->oid.hash);
> + else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
> it->oid.hash)) {
>   strbuf_release();
>   return -1;
>   }
> @@ -417,7 +418,7 @@ static int update_one(struct cache_tree *it,
>  #if DEBUG
>   fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
>   it->entry_count, it->subtree_nr,
> - sha1_to_hex(it->sha1));
> + oid_to_hex(>oid));
>  #endif
>   return i;
>  }
> @@ -457,14 +458,14 @@ static void write_one(struct strbuf *buffer, struct 
> cache_tree *it,
>   if (0 <= it->entry_count)
>  

CYGWIN git cannot install python packages with pip

[ankostis]

On Windows, with Cygwin-git 02.12.2-1 the python command:

pip install git+https://github.com/...`

fails to work with the following error:

$ pip install git+https://github.com/ipython/traitlets/
Collecting git+https://github.com/ipython/traitlets/
Cloning https://github.com/ipython/traitlets/ to c:\users\username\appdata
\local\temp\pip-kjwxq_oy-build
fatal: Invalid path '/cygdrive/d/Work/ALLINONES2/co2mpas_AIO-v1.6.0/CO2MPA
S/C:\Users\username\AppData\Local\Temp\pip-kjwxq_oy-build': No such file or di
rectory
git clone -q https://github.com/ipython/traitlets/ C:\Users\user\AppData\L
ocal\Temp\pip-kjwxq_oy-build" failed with error code 128 in None



Git-2.8.3 had no such problem.

Any ideas?

Re: [PATCH v2 02/53] Clean up outstanding object_id transforms.

[Brandon Williams]

On 05/01, brian m. carlson wrote:
> The semantic patch for standard object_id transforms found two
> outstanding places where we could make a transformation automatically.
> Apply these changes.
> 
> Signed-off-by: brian m. carlson 
> ---
>  builtin/diff.c | 2 +-
>  reflog-walk.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/diff.c b/builtin/diff.c
> index d184aafab..a25b4e4ae 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -408,7 +408,7 @@ int cmd_diff(int argc, const char **argv, const char 
> *prefix)
>   } else if (obj->type == OBJ_BLOB) {
>   if (2 <= blobs)
>   die(_("more than two blobs given: '%s'"), name);
> - hashcpy(blob[blobs].oid.hash, obj->oid.hash);
> + oidcpy([blobs].oid, >oid);
>   blob[blobs].name = name;
>   blob[blobs].mode = entry->mode;
>   blobs++;
> diff --git a/reflog-walk.c b/reflog-walk.c
> index 99679f582..c8fdf051d 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -241,7 +241,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, 
> struct commit *commit)
>   logobj = parse_object(reflog->ooid.hash);
>   } while (commit_reflog->recno && (logobj && logobj->type != 
> OBJ_COMMIT));
>  
> - if (!logobj && commit_reflog->recno >= 0 && 
> is_null_sha1(reflog->ooid.hash)) {
> + if (!logobj && commit_reflog->recno >= 0 && is_null_oid(>ooid)) 
> {

Not relevant to this series but I was confused for a second seeing
'ooid' as I have no clue what that means :)

>   /* a root commit, but there are still more entries to show */
>   reflog = _reflog->reflogs->items[commit_reflog->recno];
>   logobj = parse_object(reflog->noid.hash);

-- 
Brandon Williams

Re: [PATCH v2 5/6] submodule: improve submodule_has_commits

[Stefan Beller]

On Tue, May 2, 2017 at 10:25 AM, Brandon Williams  wrote:
> On 05/01, Stefan Beller wrote:
>> On Mon, May 1, 2017 at 6:02 PM, Brandon Williams  wrote:
>> > +
>> > +   if (capture_command(, , GIT_MAX_HEXSZ + 1) || 
>> > out.len)
>>
>> eh, I gave too much and self-contradicting feedback here earlier,
>> ideally I'd like to review this to be similar as:
>>
>> if (capture_command(, , GIT_MAX_HEXSZ + 1)
>> die("cannot capture git-rev-list in submodule '%s', sub->path);
>
> This wouldn't really work because if you provide a SHA1 to rev-list
> which it isn't able to find then it returns a non-zero exit code which
> would cause this to die, which isn't the desired behavior.

Oh. In that case, why do we even check for its stdout?
(from rev-lists man page)
   --quiet
   Don’t print anything to standard output. This form is primarily
   meant to allow the caller to test the exit status to see if a range
   of objects is fully connected (or not). It is faster than
   redirecting stdout to /dev/null as the output does not have to be
   formatted.

>
> I feel like you're making this a little too complicated, as all I'm
> doing is shuffling around already existing logic.  I understand the want
> to make things more robust but this seems unnecessarily complex.

ok. I was just giving my thoughts on how I would approach it.

Thanks,
Stefan

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Ævar Arnfjörð Bjarmason]

On Tue, May 2, 2017 at 7:43 PM, Brandon Williams  wrote:
> On 05/02, Ævar Arnfjörð Bjarmason wrote:
>> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
>>  wrote:
>> > Hi Ævar,
>> >
>> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
>> >
>> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
>> >>  - SQUASH???
>> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
>> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
>> >>  - grep: add support for PCRE v2
>> >>  - grep: add support for the PCRE v1 JIT API
>> >>  - perf: add a performance comparison test of grep -E and -P
>> >>  - grep: change the internal PCRE code & header names to be PCRE1
>> >>  - grep: change the internal PCRE macro names to be PCRE1
>> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
>> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
>> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
>> >>  - log: add -P as a synonym for --perl-regexp
>> >>  - log: add exhaustive tests for pattern style options & config
>> >>  - grep: add a test for backreferences in PCRE patterns
>> >>  - Makefile & configure: reword outdated comment about PCRE
>> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
>> >>  - grep: remove redundant regflags assignment under PCRE
>> >>  - grep: submodule-related case statements should die if new fields are 
>> >> added
>> >>  - grep: add tests for grep pattern types being passed to submodules
>> >>  - grep: amend submodule recursion test in preparation for rx engine 
>> >> testing
>> >>
>> >>  PCRE2, which has an API different from and incompatible with PCRE,
>> >>  can now be chosen to support "grep -P -e ''" and friends.
>> >
>> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
>> > variation of this error:
>> >
>> > CC blob.o
>> > In file included from revision.h:5:0,
>> >  from bisect.c:4:
>> > grep.h:16:19: fatal error: pcre2.h: No such file or directory
>> >  #include 
>> >^
>> > compilation terminated.
>> >
>> > Maybe this can be fixed before hitting `next`?
>>
>> This will be due to a combination of the build machine not having pcre
>> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
>> default PCRE implementation" patch, which makes v2 the default for
>> USE_LIBPCRE=YesPlease.
>>
>> Is it easy to install v2 on these build machines? Alternatively that
>> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
>> to use v1, but as explained in that commit I think it makes sense to
>> make v2 the default.
>
> Shouldn't the Makefile check for the existence of PCREv2 before making
> it the default?

We can't check for the existence of anything in the Makefile, that's
what the configure script is for.

The configure script does the "right" thing with pcre v2.

The "right" thing being the bizarro pre-existing behavior of "oh you
asked for PCRE? Well let me check if it's on the system, if it's not
I'll just silently undo what you asked for". Which is crazy, but was
there before my patches, so I didn't change it.

I.e. instead of the more sane behavior of opting the user into an
optional library if it's found on the system, and producing an error
if you supply --with-that-library but don't have thatlibrary.so.

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Brandon Williams]

On 05/02, Ævar Arnfjörð Bjarmason wrote:
> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
>  wrote:
> > Hi Ævar,
> >
> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
> >
> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
> >>  - SQUASH???
> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
> >>  - grep: add support for PCRE v2
> >>  - grep: add support for the PCRE v1 JIT API
> >>  - perf: add a performance comparison test of grep -E and -P
> >>  - grep: change the internal PCRE code & header names to be PCRE1
> >>  - grep: change the internal PCRE macro names to be PCRE1
> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
> >>  - log: add -P as a synonym for --perl-regexp
> >>  - log: add exhaustive tests for pattern style options & config
> >>  - grep: add a test for backreferences in PCRE patterns
> >>  - Makefile & configure: reword outdated comment about PCRE
> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
> >>  - grep: remove redundant regflags assignment under PCRE
> >>  - grep: submodule-related case statements should die if new fields are 
> >> added
> >>  - grep: add tests for grep pattern types being passed to submodules
> >>  - grep: amend submodule recursion test in preparation for rx engine 
> >> testing
> >>
> >>  PCRE2, which has an API different from and incompatible with PCRE,
> >>  can now be chosen to support "grep -P -e ''" and friends.
> >
> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
> > variation of this error:
> >
> > CC blob.o
> > In file included from revision.h:5:0,
> >  from bisect.c:4:
> > grep.h:16:19: fatal error: pcre2.h: No such file or directory
> >  #include 
> >^
> > compilation terminated.
> >
> > Maybe this can be fixed before hitting `next`?
> 
> This will be due to a combination of the build machine not having pcre
> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
> default PCRE implementation" patch, which makes v2 the default for
> USE_LIBPCRE=YesPlease.
> 
> Is it easy to install v2 on these build machines? Alternatively that
> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
> to use v1, but as explained in that commit I think it makes sense to
> make v2 the default.

Shouldn't the Makefile check for the existence of PCREv2 before making
it the default?

-- 
Brandon Williams

Re: [PATCH v2 5/6] submodule: improve submodule_has_commits

[Brandon Williams]

On 05/01, Stefan Beller wrote:
> On Mon, May 1, 2017 at 6:02 PM, Brandon Williams  wrote:
> > +
> > +   if (capture_command(, , GIT_MAX_HEXSZ + 1) || 
> > out.len)
> 
> eh, I gave too much and self-contradicting feedback here earlier,
> ideally I'd like to review this to be similar as:
> 
> if (capture_command(, , GIT_MAX_HEXSZ + 1)
> die("cannot capture git-rev-list in submodule '%s', sub->path);

This wouldn't really work because if you provide a SHA1 to rev-list
which it isn't able to find then it returns a non-zero exit code which
would cause this to die, which isn't the desired behavior.

> 
> if (out.len)
> has_commit = 0;
> 
> instead as that does not have a silent error. (though it errs
> on the safe side, so maybe it is not to bad.)
> 
> I could understand if the callers do not want to have
> `submodule_has_commits` die()-ing on them, so maybe
> 
> if (capture_command(, , GIT_MAX_HEXSZ + 1) {
> warning("cannot capture git-rev-list in submodule '%s', sub->path);
> has_commit = -1;
> /* this would require auditing all callers and handling -1 though */
> }
> 
> if (out.len)
> has_commit = 0;
> 
> As the comment eludes, we'd then have
>  0 -> has no commits
>  1 -> has commits
> -1 -> error
> 
> So to group (error || has_no_commits), we could write
> 
> if (submodule_has_commits(..) <= 0)
> 
> which is awkward. So maybe we can rename the function
> to misses_submodule_commits instead, as then we could
> flip the return value as well and have
> 
>  0 -> has commits
>  1 -> has no commits
> -1 -> error
> 
> and the lazy invoker could just go with
> 
> if (!misses_submodule_commits(..))
> proceed();
> else
> die("missing submodule commits or errors; I don't care");
> 
> whereas the careful invoker could go with
> 
> switch (misses_submodule_commits(..)) {
> case 0:
> proceed(); break;
> case 1:
> pull_magic_trick(); break;
> case -1:
> make_errors_go_away_and_retry(); break;
> }

I feel like you're making this a little too complicated, as all I'm
doing is shuffling around already existing logic.  I understand the want
to make things more robust but this seems unnecessarily complex.

> ---
> On the longer term plan:
> As you wrote about costs. Maybe instead of invoking rev-list,
> we could try to have this in-core as a first try-out for
> "classified-repos", looking at refs.h there is e.g.
> 
> int for_each_ref_submodule(const char *submodule_path,
>   each_ref_fn fn, void *cb_data);
> 
> which we could use to obtain all submodule refs and then
> use the revision walking machinery to find out ourselves if
> we have or do not have the commits. (As we loaded the
> odb of the submodule, this would *just work*, building one
> kludgy hack upon the next.)
> 
> Thanks,
> Stefan

-- 
Brandon Williams

Re: [PATCH v3 14/25] setup_discovered_git_dir(): plug memory leak

[Stefan Beller]

On Tue, May 2, 2017 at 9:02 AM, Johannes Schindelin
 wrote:
> The setup_explicit_git_dir() function does not take custody of the string
> passed as first parameter; we have to release it if we turned the value of
> git_dir into an absolute path.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  setup.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 0320a9ad14c..e3f7699a902 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -703,11 +703,16 @@ static const char *setup_discovered_git_dir(const char 
> *gitdir,
>
> /* --work-tree is set without --git-dir; use discovered one */
> if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
> +   char *to_free = NULL;
> +   const char *ret;
> +
> if (offset != cwd->len && !is_absolute_path(gitdir))
> -   gitdir = real_pathdup(gitdir, 1);
> +   gitdir = to_free = real_pathdup(gitdir, 1);
> if (chdir(cwd->buf))
> die_errno("Could not come back to cwd");

As the original motivation was to shut up Coverity, this may not
accomplish that goal, as in the path of taking the die_errno, we do not
free `to_free`. But that is ok as the actual goal is to hav no memleaks
in the good case. A memleak just before a die is no big deal.

> -   return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
> +   ret = setup_explicit_git_dir(gitdir, cwd, nongit_ok);
> +   free(to_free);
> +   return ret;

And we free our pathdup from above, makes sense.

Thanks,
Stefan

Re: [PATCH 0/5] Start of a journey: drop NO_THE_INDEX_COMPATIBILITY_MACROS

[Stefan Beller]

On Tue, May 2, 2017 at 8:35 AM, Jeff Hostetler  wrote:
> You may also want to look at unpack-trees.c : mark_new_skip_worktree().
> It has a local variable named "the_index" in the argument list.
> You may want to rename this to avoid confusion.

Thanks for bringing this up. I just made a local commit,
to be sent out with v2.

>
> Thank you for bringing this up and making this proposal.
> I started a similar effort internally last fall, but
> stopped because of the footprint size.
>

Yeah, I also have a bad feeling about the foot print, which
is why I asked if now is a good time to go with such a series.


> In addition to (eventually) allowing multiple repos be open at
> the same time for submodules, it would also help with various
> multi-threading efforts.  For example, we have loops that do a
> "for (k = 0, k < active_nr; k++) {...}"  There is no visual clue
> in that code that it references "the_index" and therefore should
> be subject to the same locking.  Granted, this is a trivial example,
> but goes to the argument that the code has lots of subtle global
> variables and macros that make it difficult to reason about the
> code.
>
> This step would help un-hide this.

Thanks for pointing out the actual underlying reason, that I was trying
to formulate. I'll borrow these lines for future cover letters.

Thanks,
Stefan

[PATCH v3] l10n: de.po: update German translation

[Ralf Thielow]

Translate 96 new messages came from git.pot update in dfc182b (l10n:
git.pot: v2.13.0 round 1 (96 new, 37 removed)).

Signed-off-by: Ralf Thielow 
---
 po/de.po | 323 ---
 1 file changed, 142 insertions(+), 181 deletions(-)

diff --git a/po/de.po b/po/de.po
index b92c4fe11..8272b06b5 100644
--- a/po/de.po
+++ b/po/de.po
@@ -866,9 +866,9 @@ msgid "Argument not supported for format '%s': -%d"
 msgstr "Argument für Format '%s' nicht unterstützt: -%d"
 
 #: attr.c:212
-#, fuzzy, c-format
+#, c-format
 msgid "%.*s is not a valid attribute name"
-msgstr "'%s' ist kein gültiger Name für ein Remote-Repository"
+msgstr "%.*s ist kein gültiger Attributname"
 
 #: attr.c:408
 msgid ""
@@ -1260,6 +1260,8 @@ msgstr "Speicher verbraucht"
 #: config.c:191
 msgid "relative config include conditionals must come from files"
 msgstr ""
+"Bedingungen für das Einbinden von Konfigurationen aus relativen Pfaden muss\n"
+"aus Dateien kommen."
 
 #: config.c:701
 #, c-format
@@ -1379,12 +1381,12 @@ msgstr "Ungültiger %s: '%s'"
 #: config.c:1952
 #, c-format
 msgid "unknown core.untrackedCache value '%s'; using 'keep' default value"
-msgstr ""
+msgstr "Unbekannter Wert '%s' in core.untrackedCache; benutze Standardwert 
'keep'"
 
 #: config.c:1978
 #, c-format
 msgid "splitIndex.maxPercentChange value '%d' should be between 0 and 100"
-msgstr ""
+msgstr "Der Wert '%d' von splitIndex.maxPercentChange sollte zwischen 0 und 
100 liegen."
 
 #: config.c:1989
 #, c-format
@@ -1645,9 +1647,9 @@ msgstr ""
 "für dieses Verzeichnis deaktiviert."
 
 #: dir.c:2776 dir.c:2781
-#, fuzzy, c-format
+#, c-format
 msgid "could not create directories for %s"
-msgstr "Konnte Verzeichnis '%s' nicht erstellen."
+msgstr "Konnte Verzeichnisse für '%s' nicht erstellen."
 
 #: dir.c:2806
 #, c-format
@@ -1655,9 +1657,9 @@ msgid "could not migrate git directory from '%s' to '%s'"
 msgstr "Konnte Git-Verzeichnis nicht von '%s' nach '%s' migrieren."
 
 #: entry.c:280
-#, fuzzy, c-format
+#, c-format
 msgid "could not stat file '%s'"
-msgstr "konnte Datei '%s' nicht erstellen"
+msgstr "konnte Datei '%s' nicht lesen"
 
 #: fetch-pack.c:249
 msgid "git fetch-pack: expected shallow list"
@@ -1827,14 +1829,14 @@ msgid "no matching remote head"
 msgstr "kein übereinstimmender Remote-Branch"
 
 #: fetch-pack.c:1147
-#, fuzzy, c-format
+#, c-format
 msgid "no such remote ref %s"
-msgstr "Kein solches Remote-Repository: %s"
+msgstr "keine solche Remote-Referenz %s"
 
 #: fetch-pack.c:1150
 #, c-format
 msgid "Server does not allow request for unadvertised object %s"
-msgstr ""
+msgstr "Der Server erlaubt keine Anfrage für nicht angebotenes Objekt %s."
 
 #: gpg-interface.c:185
 msgid "gpg failed to sign the data"
@@ -1961,31 +1963,31 @@ msgstr ""
 
 #: ident.c:367
 msgid "no email was given and auto-detection is disabled"
-msgstr ""
+msgstr "keine E-Mail angegeben und automatische Erkennung ist deaktiviert"
 
 #: ident.c:372
-#, fuzzy, c-format
+#, c-format
 msgid "unable to auto-detect email address (got '%s')"
-msgstr "Fehler: konnte keine gültige Adresse aus %s extrahieren\n"
+msgstr "Konnte die E-Mail-Adresse nicht automatisch erkennen ('%s' erhalten)"
 
 #: ident.c:382
 msgid "no name was given and auto-detection is disabled"
-msgstr ""
+msgstr "kein Name angegeben und automatische Erkennung ist deaktiviert"
 
 #: ident.c:388
-#, fuzzy, c-format
+#, c-format
 msgid "unable to auto-detect name (got '%s')"
-msgstr "konnte \"Tree\"-Objekt (%s) nicht lesen"
+msgstr "konnte Namen nicht automatisch erkennen ('%s' erhalten)"
 
 #: ident.c:396
 #, c-format
 msgid "empty ident name (for <%s>) not allowed"
-msgstr ""
+msgstr "Leerer Name in Identifikation (für <%s>) nicht erlaubt."
 
 #: ident.c:402
 #, c-format
 msgid "name consists only of disallowed characters: %s"
-msgstr ""
+msgstr "Name besteht nur aus nicht erlaubten Zeichen: %s"
 
 #: ident.c:417 builtin/commit.c:611
 #, c-format
@@ -2102,13 +2104,11 @@ msgstr ""
 "im Arbeitsbereich gelassen."
 
 #: merge-recursive.c:1097
-#, fuzzy, c-format
+#, c-format
 msgid ""
 "CONFLICT (%s/delete): %s deleted in %s and %s to %s in %s. Version %s of %s "
 "left in tree."
-msgstr ""
-"KONFLIKT (%s/löschen): %s gelöscht in %s und %s in %s. Stand %s von %s wurde "
-"im Arbeitsbereich gelassen."
+msgstr "KONFLIKT (%s/löschen): %s gelöscht in %s und %s nach %s in %s. Stand 
%s von %s wurde im Arbeitsbereich gelassen."
 
 #: merge-recursive.c:1104
 #, c-format
@@ -2120,13 +2120,11 @@ msgstr ""
 "im Arbeitsbereich bei %s gelassen."
 
 #: merge-recursive.c:1109
-#, fuzzy, c-format
+#, c-format
 msgid ""
 "CONFLICT (%s/delete): %s deleted in %s and %s to %s in %s. Version %s of %s "
 "left in tree at %s."
-msgstr ""
-"KONFLIKT (%s/löschen): %s gelöscht in %s und %s in %s. Stand %s von %s wurde "
-"im Arbeitsbereich bei %s gelassen."
+msgstr "KONFLIKT (%s/löschen): %s gelöscht in %s und %s nach %s in %s. Stand 
%s von %s wurde im Arbeitsbereich bei %s 

Re: [PATCH v2] l10n: de.po: update German translation

[Ralf Thielow]

2017-05-01 14:19 GMT+02:00 Simon Ruderich :
> On Sun, Apr 30, 2017 at 09:11:49PM +0200, Ralf Thielow wrote:
>>  #: config.c:1952
>>  #, c-format
>>  msgid "unknown core.untrackedCache value '%s'; using 'keep' default value"
>> -msgstr ""
>> +msgstr "Unbekannter Wert '%s' in core.untrackedCache; benutze Stardardwert 
>> 'keep'"
>  
> Standardwert
>
>>  #: entry.c:280
>> -#, fuzzy, c-format
>> +#, c-format
>>  msgid "could not stat file '%s'"
>> -msgstr "konnte Datei '%s' nicht erstellen"
>> +msgstr "konnte Datei '%s' nicht lesen"
>
> Read is not quite stat (there are situations where you can stat
> but not read a file), but I can't think of a better translation.
>
>>  #: builtin/describe.c:551
>> -#, fuzzy
>>  msgid "--broken is incompatible with commit-ishes"
>> -msgstr "Die Option --dirty kann nicht mit Commits verwendet werden."
>> +msgstr "Die Option --broken kann nicht nit Commits verwendet werden."
>   ^^^
> mit
>
>>  #: builtin/tag.c:312
>>  msgid "tag: tagging "
>> -msgstr ""
>> +msgstr "Tag: tagge "
>
> Lowercase Tag because it's used as command prefix? In other
> places in this patch the lowercase version was used for commands.
>
> Regards
> Simon
> --

Thanks!

> + privacy is necessary
> + using gnupg http://gnupg.org
> + public key id: 0x92FEFDB7E44C32F9

Re: [PATCH v2 1/1] t0027: tests are not expensive; remove t0025

[Johannes Schindelin]

Hi Bögi,

On Tue, 2 May 2017, tbo...@web.de wrote:

> From: Torsten Bögershausen 
> 
> The purpose of t0027 is to test all CRLF related conversions at
> "git checkout" and "git add".
> 
> Running t0027 under Git for Windows takes 3-4 minutes, so the whole script
> had been marked as "EXPENSIVE".
> 
> The source code for "Git for Windows" overrides this since 2014:
> "t0027 is marked expensive, but really, for MinGW we want to run these
> tests always."

Indeed.

> Recent "stress" tests show that t0025 if flaky, reported by Lars Schneider,
> larsxschnei...@gmail.com
> 
> All tests from t0025 are covered in t0027 already, so that t0025 can be
> retiered:

s/retiered/retired/

> The execution time for t0027 is 14 seconds under Linux,
> and 63 seconds under Mac Os X.
> And in case you ask, things are not going significantly faster using a SSD
> instead of a spinning disk.
> 
> Signed-off-by: Torsten Bögershausen 

Thank you for this patch.

Apart from the tyop, would it be possible to fix the formatting to look
less strange? (Unless you use this to transport a super-secret message
steganographically to an alien planet or some such, of course.)

Ciao,
Dscho

Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #01; Mon, 1)

[Johannes Schindelin]

Hi Ævar,

On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:

> On Tue, May 2, 2017 at 2:09 PM, Johannes Schindelin
>  wrote:
> >
> > On Sun, 30 Apr 2017, Junio C Hamano wrote:
> >
> >> * ab/grep-pcre-v2 (2017-04-25) 20 commits
> >>  - SQUASH???
> >>  - Makefile & configure: make PCRE v2 the default PCRE implementation
> >>  - grep: remove support for concurrent use of both PCRE v1 & v2
> >>  - grep: add support for PCRE v2
> >>  - grep: add support for the PCRE v1 JIT API
> >>  - perf: add a performance comparison test of grep -E and -P
> >>  - grep: change the internal PCRE code & header names to be PCRE1
> >>  - grep: change the internal PCRE macro names to be PCRE1
> >>  - test-lib: rename the LIBPCRE prerequisite to PCRE
> >>  - grep: make grep.patternType=[pcre|pcre1] a synonym for "perl"
> >>  - grep & rev-list doc: stop promising libpcre for --perl-regexp
> >>  - log: add -P as a synonym for --perl-regexp
> >>  - log: add exhaustive tests for pattern style options & config
> >>  - grep: add a test for backreferences in PCRE patterns
> >>  - Makefile & configure: reword outdated comment about PCRE
> >>  - grep: remove redundant `regflags &= ~REG_EXTENDED` assignments
> >>  - grep: remove redundant regflags assignment under PCRE
> >>  - grep: submodule-related case statements should die if new fields are 
> >> added
> >>  - grep: add tests for grep pattern types being passed to submodules
> >>  - grep: amend submodule recursion test in preparation for rx engine 
> >> testing
> >>
> >>  PCRE2, which has an API different from and incompatible with PCRE,
> >>  can now be chosen to support "grep -P -e ''" and friends.
> >
> > FWIW for quite a couple of recent builds, `pu` fails on Windows with a
> > variation of this error:
> >
> > CC blob.o
> > In file included from revision.h:5:0,
> >  from bisect.c:4:
> > grep.h:16:19: fatal error: pcre2.h: No such file or directory
> >  #include 
> >^
> > compilation terminated.
> >
> > Maybe this can be fixed before hitting `next`?
> 
> This will be due to a combination of the build machine not having pcre
> v2 (but having v1) & my "Makefile & configure: make PCRE v2 the
> default PCRE implementation" patch, which makes v2 the default for
> USE_LIBPCRE=YesPlease.
> 
> Is it easy to install v2 on these build machines? Alternatively that
> patch could be ejected out of pu, or you could USE_LIBPCRE1=YesPlease
> to use v1, but as explained in that commit I think it makes sense to
> make v2 the default.

Let me put it this way: Installing PCRE v1 in MSYS2 is as easy as

pacman -Sy mingw-w64-x86_64-pcre

To install PCRE v2, you would have to copy-edit
https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-pcre/PKGBUILD,
make sure that it builds by running it through

makepkg-mingw -s

possibly initializing a Git repository in the
mingw-w64-pcre/src/${_realname}-${pkgver}/ directory, patching the source
until it builds, committing the changes, adding them as patch files to the
same directory as the new PKGBUILD, adjusting the PKGBUILD file to list
the patch files with their checksums and to add the commands to apply
them.

Then (and this is a one time cost, fortunately) initializing two packages
on BinTray (which we use to serve the Pacman packages of Git for Windows),
then build and upload the packages.

In short, PCRE v2 would be slightly (ahem, ahem) more involved than PCRE
v1.

I cannot imagine that MSYS2 is the only environment with that issue.

Ciao,
Dscho

[PATCH v3 23/25] name-rev: avoid leaking memory in the `deref` case

[Johannes Schindelin]

When the `name_rev()` function is asked to dereference the tip name, it
allocates memory. But when it turns out that another tip already
described the commit better than the current one, we forgot to release
the memory.

Pointed out by Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/name-rev.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 92a5d8a5d26..e7a3fe7ee70 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -28,6 +28,7 @@ static void name_rev(struct commit *commit,
struct rev_name *name = (struct rev_name *)commit->util;
struct commit_list *parents;
int parent_number = 1;
+   char *to_free = NULL;
 
parse_commit(commit);
 
@@ -35,7 +36,7 @@ static void name_rev(struct commit *commit,
return;
 
if (deref) {
-   tip_name = xstrfmt("%s^0", tip_name);
+   tip_name = to_free = xstrfmt("%s^0", tip_name);
 
if (generation)
die("generation: %d, but deref?", generation);
@@ -53,8 +54,10 @@ static void name_rev(struct commit *commit,
name->taggerdate = taggerdate;
name->generation = generation;
name->distance = distance;
-   } else
+   } else {
+   free(to_free);
return;
+   }
 
for (parents = commit->parents;
parents;
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 25/25] submodule_uses_worktrees(): plug memory leak

[Johannes Schindelin]

There is really no reason why we would need to hold onto the allocated
string longer than necessary.

Reported by Coverity.

Signed-off-by: Johannes Schindelin 
---
 worktree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/worktree.c b/worktree.c
index bae787cf8d7..89a81b13de3 100644
--- a/worktree.c
+++ b/worktree.c
@@ -399,6 +399,7 @@ int submodule_uses_worktrees(const char *path)
 
/* The env would be set for the superproject. */
get_common_dir_noenv(, submodule_gitdir);
+   free(submodule_gitdir);
 
/*
 * The check below is only known to be good for repository format
@@ -418,7 +419,6 @@ int submodule_uses_worktrees(const char *path)
/* See if there is any file inside the worktrees directory. */
dir = opendir(sb.buf);
strbuf_release();
-   free(submodule_gitdir);
 
if (!dir)
return 0;
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 19/25] line-log: avoid memory leak

[Johannes Schindelin]

Discovered by Coverity.

Signed-off-by: Johannes Schindelin 
---
 line-log.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/line-log.c b/line-log.c
index a23b910471b..b9087814b8c 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1125,6 +1125,7 @@ static int process_ranges_ordinary_commit(struct rev_info 
*rev, struct commit *c
changed = process_all_files(_range, rev, , range);
if (parent)
add_line_range(rev, parent, parent_range);
+   free_line_log_data(parent_range);
return changed;
 }
 
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 20/25] shallow: avoid memory leak

[Johannes Schindelin]

Reported by Coverity.

Signed-off-by: Johannes Schindelin 
---
 shallow.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/shallow.c b/shallow.c
index 25b6db989bf..f9370961f99 100644
--- a/shallow.c
+++ b/shallow.c
@@ -473,11 +473,15 @@ static void paint_down(struct paint_info *info, const 
unsigned char *sha1,
struct commit_list *head = NULL;
int bitmap_nr = (info->nr_bits + 31) / 32;
size_t bitmap_size = st_mult(sizeof(uint32_t), bitmap_nr);
-   uint32_t *tmp = xmalloc(bitmap_size); /* to be freed before return */
-   uint32_t *bitmap = paint_alloc(info);
struct commit *c = lookup_commit_reference_gently(sha1, 1);
+   uint32_t *tmp; /* to be freed before return */
+   uint32_t *bitmap;
+
if (!c)
return;
+
+   tmp = xmalloc(bitmap_size);
+   bitmap = paint_alloc(info);
memset(bitmap, 0, bitmap_size);
bitmap[id / 32] |= (1U << (id % 32));
commit_list_insert(c, );
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 24/25] show_worktree(): plug memory leak

[Johannes Schindelin]

The buffer allocated by shorten_unambiguous_ref() needs to be released.

Discovered by Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/worktree.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 1722a9bdc2a..ff5dfd2b102 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -414,9 +414,11 @@ static void show_worktree(struct worktree *wt, int 
path_maxlen, int abbrev_len)
find_unique_abbrev(wt->head_sha1, 
DEFAULT_ABBREV));
if (wt->is_detached)
strbuf_addstr(, "(detached HEAD)");
-   else if (wt->head_ref)
-   strbuf_addf(, "[%s]", 
shorten_unambiguous_ref(wt->head_ref, 0));
-   else
+   else if (wt->head_ref) {
+   char *ref = shorten_unambiguous_ref(wt->head_ref, 0);
+   strbuf_addf(, "[%s]", ref);
+   free(ref);
+   } else
strbuf_addstr(, "(error)");
}
printf("%s\n", sb.buf);
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 21/25] add_reflog_for_walk: avoid memory leak

[Johannes Schindelin]

We free()d the `log` buffer when dwim_log() returned 1, but not when it
returned a larger value (which meant that it still allocated the buffer
but we simply ignored it).

While in the vicinity, make sure that the `reflogs` structure as well as
the `branch` variable are released properly, too.

Identified by Coverity.

Signed-off-by: Johannes Schindelin 
---
 reflog-walk.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index 99679f58255..c63eb1a3fd7 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -183,7 +183,11 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
if (!reflogs || reflogs->nr == 0) {
struct object_id oid;
char *b;
-   if (dwim_log(branch, strlen(branch), oid.hash, ) == 
1) {
+   int ret = dwim_log(branch, strlen(branch),
+  oid.hash, );
+   if (ret > 1)
+   free(b);
+   else if (ret == 1) {
if (reflogs) {
free(reflogs->ref);
free(reflogs);
@@ -193,17 +197,27 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
reflogs = read_complete_reflog(branch);
}
}
-   if (!reflogs || reflogs->nr == 0)
+   if (!reflogs || reflogs->nr == 0) {
+   if (reflogs) {
+   free(reflogs->ref);
+   free(reflogs);
+   }
+   free(branch);
return -1;
+   }
string_list_insert(>complete_reflogs, branch)->util
= reflogs;
}
+   free(branch);
 
commit_reflog = xcalloc(1, sizeof(struct commit_reflog));
if (recno < 0) {
commit_reflog->recno = get_reflog_recno_by_time(reflogs, 
timestamp);
if (commit_reflog->recno < 0) {
-   free(branch);
+   if (reflogs) {
+   free(reflogs->ref);
+   free(reflogs);
+   }
free(commit_reflog);
return -1;
}
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 22/25] remote: plug memory leak in match_explicit()

[Johannes Schindelin]

The `guess_ref()` returns an allocated buffer of which `make_linked_ref()`
does not take custody (`alloc_ref()` makes a copy), therefore we need to
release the buffer afterwards.

Noticed via Coverity.

Signed-off-by: Johannes Schindelin 
---
 remote.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/remote.c b/remote.c
index 801137c72eb..72b4591b983 100644
--- a/remote.c
+++ b/remote.c
@@ -1191,9 +1191,10 @@ static int match_explicit(struct ref *src, struct ref 
*dst,
else if (is_null_oid(_src->new_oid))
error("unable to delete '%s': remote ref does not 
exist",
  dst_value);
-   else if ((dst_guess = guess_ref(dst_value, matched_src)))
+   else if ((dst_guess = guess_ref(dst_value, matched_src))) {
matched_dst = make_linked_ref(dst_guess, dst_tail);
-   else
+   free(dst_guess);
+   } else
error("unable to push to unqualified destination: %s\n"
  "The destination refspec neither matches an "
  "existing ref on the remote nor\n"
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 18/25] receive-pack: plug memory leak in update()

[Johannes Schindelin]

Reported via Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/receive-pack.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f96834f42c9..48c07ddb659 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -986,7 +986,8 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
 {
const char *name = cmd->ref_name;
struct strbuf namespaced_name_buf = STRBUF_INIT;
-   const char *namespaced_name, *ret;
+   static char *namespaced_name;
+   const char *ret;
struct object_id *old_oid = >old_oid;
struct object_id *new_oid = >new_oid;
 
@@ -997,6 +998,7 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
}
 
strbuf_addf(_name_buf, "%s%s", get_git_namespace(), name);
+   free(namespaced_name);
namespaced_name = strbuf_detach(_name_buf, NULL);
 
if (is_ref_checked_out(namespaced_name)) {
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 10/25] cat-file: fix memory leak

[Johannes Schindelin]

Discovered by Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/cat-file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1890d7a6390..9af863e7915 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -165,6 +165,7 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
die("git cat-file %s: bad file", obj_name);
 
write_or_die(1, buf, size);
+   free(buf);
return 0;
 }
 
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 12/25] split_commit_in_progress(): fix memory leak

[Johannes Schindelin]

Reported via Coverity.

Signed-off-by: Johannes Schindelin 
---
 wt-status.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index 0a6e16dbe0f..1f3f6bcb980 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1088,8 +1088,13 @@ static int split_commit_in_progress(struct wt_status *s)
char *rebase_orig_head = 
read_line_from_git_path("rebase-merge/orig-head");
 
if (!head || !orig_head || !rebase_amend || !rebase_orig_head ||
-   !s->branch || strcmp(s->branch, "HEAD"))
+   !s->branch || strcmp(s->branch, "HEAD")) {
+   free(head);
+   free(orig_head);
+   free(rebase_amend);
+   free(rebase_orig_head);
return split_in_progress;
+   }
 
if (!strcmp(rebase_amend, rebase_orig_head)) {
if (strcmp(head, rebase_amend))
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 14/25] setup_discovered_git_dir(): plug memory leak

[Johannes Schindelin]

The setup_explicit_git_dir() function does not take custody of the string
passed as first parameter; we have to release it if we turned the value of
git_dir into an absolute path.

Signed-off-by: Johannes Schindelin 
---
 setup.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index 0320a9ad14c..e3f7699a902 100644
--- a/setup.c
+++ b/setup.c
@@ -703,11 +703,16 @@ static const char *setup_discovered_git_dir(const char 
*gitdir,
 
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
+   char *to_free = NULL;
+   const char *ret;
+
if (offset != cwd->len && !is_absolute_path(gitdir))
-   gitdir = real_pathdup(gitdir, 1);
+   gitdir = to_free = real_pathdup(gitdir, 1);
if (chdir(cwd->buf))
die_errno("Could not come back to cwd");
-   return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
+   ret = setup_explicit_git_dir(gitdir, cwd, nongit_ok);
+   free(to_free);
+   return ret;
}
 
/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 15/25] pack-redundant: plug memory leak

[Johannes Schindelin]

Identified via Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/pack-redundant.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 72c815844dd..cb1df1c7614 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -442,6 +442,7 @@ static void minimize(struct pack_list **min)
/* return if there are no objects missing from the unique set */
if (missing->size == 0) {
*min = unique;
+   free(missing);
return;
}
 
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 17/25] fast-export: avoid leaking memory in handle_tag()

[Johannes Schindelin]

Reported by, you guessed it, Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/fast-export.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index e0220630d00..64617ad8e36 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -734,6 +734,7 @@ static void handle_tag(const char *name, struct tag *tag)
 oid_to_hex(>object.oid));
case DROP:
/* Ignore this tag altogether */
+   free(buf);
return;
case REWRITE:
if (tagged->type != OBJ_COMMIT) {
@@ -765,6 +766,7 @@ static void handle_tag(const char *name, struct tag *tag)
   (int)(tagger_end - tagger), tagger,
   tagger == tagger_end ? "" : "\n",
   (int)message_size, (int)message_size, message ? message : "");
+   free(buf);
 }
 
 static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name)
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 11/25] checkout: fix memory leak

[Johannes Schindelin]

This change addresses part of the NEEDSWORK comment above the code,
therefore the comment needs to be adjusted, too.

Discovered via Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/checkout.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfa5419f335..5faea3a05fa 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -235,14 +235,14 @@ static int checkout_merged(int pos, const struct checkout 
*state)
/*
 * NEEDSWORK:
 * There is absolutely no reason to write this as a blob object
-* and create a phony cache entry just to leak.  This hack is
-* primarily to get to the write_entry() machinery that massages
-* the contents to work-tree format and writes out which only
-* allows it for a cache entry.  The code in write_entry() needs
-* to be refactored to allow us to feed a 
-* instead of a cache entry.  Such a refactoring would help
-* merge_recursive as well (it also writes the merge result to the
-* object database even when it may contain conflicts).
+* and create a phony cache entry.  This hack is primarily to get
+* to the write_entry() machinery that massages the contents to
+* work-tree format and writes out which only allows it for a
+* cache entry.  The code in write_entry() needs to be refactored
+* to allow us to feed a  instead of a cache
+* entry.  Such a refactoring would help merge_recursive as well
+* (it also writes the merge result to the object database even
+* when it may contain conflicts).
 */
if (write_sha1_file(result_buf.ptr, result_buf.size,
blob_type, oid.hash))
@@ -251,6 +251,7 @@ static int checkout_merged(int pos, const struct checkout 
*state)
if (!ce)
die(_("make_cache_entry failed for path '%s'"), path);
status = checkout_entry(ce, state, NULL);
+   free(ce);
return status;
 }
 
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 16/25] mktree: plug memory leaks reported by Coverity

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin 
---
 builtin/mktree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/mktree.c b/builtin/mktree.c
index de9b40fc63b..da0fd8cd706 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -72,7 +72,7 @@ static void mktree_line(char *buf, size_t len, int 
nul_term_line, int allow_miss
unsigned mode;
enum object_type mode_type; /* object type derived from mode */
enum object_type obj_type; /* object type derived from sha */
-   char *path;
+   char *path, *to_free = NULL;
unsigned char sha1[20];
 
ptr = buf;
@@ -102,7 +102,7 @@ static void mktree_line(char *buf, size_t len, int 
nul_term_line, int allow_miss
struct strbuf p_uq = STRBUF_INIT;
if (unquote_c_style(_uq, path, NULL))
die("invalid quoting");
-   path = strbuf_detach(_uq, NULL);
+   path = to_free = strbuf_detach(_uq, NULL);
}
 
/*
@@ -136,6 +136,7 @@ static void mktree_line(char *buf, size_t len, int 
nul_term_line, int allow_miss
}
 
append_to_tree(mode, sha1, path);
+   free(to_free);
 }
 
 int cmd_mktree(int ac, const char **av, const char *prefix)
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 13/25] setup_bare_git_dir(): help static analysis

[Johannes Schindelin]

Coverity reported a memory leak in this function. However, it can only
be called once, as setup_git_directory() changes global state and hence
is not reentrant.

Mark the variable as static to indicate that this is a singleton.

Signed-off-by: Johannes Schindelin 
---
 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.c b/setup.c
index 0309c278218..0320a9ad14c 100644
--- a/setup.c
+++ b/setup.c
@@ -748,7 +748,7 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, 
int offset,
 
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
-   const char *gitdir;
+   static const char *gitdir;
 
gitdir = offset == cwd->len ? "." : xmemdupz(cwd->buf, offset);
if (chdir(cwd->buf))
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 09/25] mailinfo & mailsplit: check for EOF while parsing

[Johannes Schindelin]

While POSIX states that it is okay to pass EOF to isspace() (and it seems
to be implied that EOF should *not* be treated as whitespace), and also to
pass EOF to ungetc() (which seems to be intended to fail without buffering
the character), it is much better to handle these cases explicitly. Not
only does it reduce head-scratching (and helps static analysis avoid
reporting false positives), it also lets us handle files containing
nothing but whitespace by erroring out.

Reported via Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/mailsplit.c | 10 ++
 mailinfo.c  |  9 -
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 30681681c13..664400b8169 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -232,6 +232,16 @@ static int split_mbox(const char *file, const char *dir, 
int allow_bare,
 
do {
peek = fgetc(f);
+   if (peek == EOF) {
+   if (f == stdin)
+   /* empty stdin is OK */
+   ret = skip;
+   else {
+   fclose(f);
+   error(_("empty mbox: '%s'"), file);
+   }
+   goto out;
+   }
} while (isspace(peek));
ungetc(peek, f);
 
diff --git a/mailinfo.c b/mailinfo.c
index 68037758f2f..f92cb9f729c 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -882,7 +882,10 @@ static int read_one_header_line(struct strbuf *line, FILE 
*in)
for (;;) {
int peek;
 
-   peek = fgetc(in); ungetc(peek, in);
+   peek = fgetc(in);
+   if (peek == EOF)
+   break;
+   ungetc(peek, in);
if (peek != ' ' && peek != '\t')
break;
if (strbuf_getline_lf(, in))
@@ -1099,6 +1102,10 @@ int mailinfo(struct mailinfo *mi, const char *msg, const 
char *patch)
 
do {
peek = fgetc(mi->input);
+   if (peek == EOF) {
+   fclose(cmitmsg);
+   return error("empty patch: '%s'", patch);
+   }
} while (isspace(peek));
ungetc(peek, mi->input);
 
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 05/25] git_config_rename_section_in_file(): avoid resource leak

[Johannes Schindelin]

In case of errors, we really want the file descriptor to be closed.

Discovered by a Coverity scan.

Signed-off-by: Johannes Schindelin 
---
 config.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index b4a3205da32..a30056ec7e9 100644
--- a/config.c
+++ b/config.c
@@ -2621,7 +2621,7 @@ int git_config_rename_section_in_file(const char 
*config_filename,
struct lock_file *lock;
int out_fd;
char buf[1024];
-   FILE *config_file;
+   FILE *config_file = NULL;
struct stat st;
 
if (new_name && !section_name_is_ok(new_name)) {
@@ -2703,11 +2703,14 @@ int git_config_rename_section_in_file(const char 
*config_filename,
}
}
fclose(config_file);
+   config_file = NULL;
 commit_and_out:
if (commit_lock_file(lock) < 0)
ret = error_errno("could not write config file %s",
  config_filename);
 out:
+   if (config_file)
+   fclose(config_file);
rollback_lock_file(lock);
 out_no_rollback:
free(filename_buf);
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 08/25] status: close file descriptor after reading git-rebase-todo

[Johannes Schindelin]

Reported via Coverity.

Signed-off-by: Johannes Schindelin 
---
 wt-status.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/wt-status.c b/wt-status.c
index 03754849626..0a6e16dbe0f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1168,6 +1168,7 @@ static int read_rebase_todolist(const char *fname, struct 
string_list *lines)
abbrev_sha1_in_line();
string_list_append(lines, line.buf);
}
+   fclose(f);
return 0;
 }
 
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 07/25] difftool: address a couple of resource/memory leaks

[Johannes Schindelin]

This change plugs a couple of memory leaks and makes sure that the file
descriptor is closed in run_dir_diff().

Spotted by Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/difftool.c | 33 +++--
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/builtin/difftool.c b/builtin/difftool.c
index 1354d0e4625..b9a892f2693 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -226,6 +226,7 @@ static void changed_files(struct hashmap *result, const 
char *index_path,
hashmap_entry_init(entry, strhash(buf.buf));
hashmap_add(result, entry);
}
+   fclose(fp);
if (finish_command(_files))
die("diff-files did not exit properly");
strbuf_release(_env);
@@ -439,8 +440,10 @@ static int run_dir_diff(const char *extcmd, int symlinks, 
const char *prefix,
}
 
if (lmode && status != 'C') {
-   if (checkout_path(lmode, , src_path, ))
-   return error("could not write '%s'", src_path);
+   if (checkout_path(lmode, , src_path, )) {
+   ret = error("could not write '%s'", src_path);
+   goto finish;
+   }
}
 
if (rmode && !S_ISLNK(rmode)) {
@@ -456,9 +459,12 @@ static int run_dir_diff(const char *extcmd, int symlinks, 
const char *prefix,
hashmap_add(_tree_dups, entry);
 
if (!use_wt_file(workdir, dst_path, )) {
-   if (checkout_path(rmode, , dst_path, 
))
-   return error("could not write '%s'",
-dst_path);
+   if (checkout_path(rmode, , dst_path,
+ )) {
+   ret = error("could not write '%s'",
+   dst_path);
+   goto finish;
+   }
} else if (!is_null_oid()) {
/*
 * Changes in the working tree need special
@@ -473,10 +479,12 @@ static int run_dir_diff(const char *extcmd, int symlinks, 
const char *prefix,
ADD_CACHE_JUST_APPEND);
 
add_path(, rdir_len, dst_path);
-   if (ensure_leading_directories(rdir.buf))
-   return error("could not create "
-"directory for '%s'",
-dst_path);
+   if (ensure_leading_directories(rdir.buf)) {
+   ret = error("could not create "
+   "directory for '%s'",
+   dst_path);
+   goto finish;
+   }
add_path(, wtdir_len, dst_path);
if (symlinks) {
if (symlink(wtdir.buf, rdir.buf)) {
@@ -497,13 +505,15 @@ static int run_dir_diff(const char *extcmd, int symlinks, 
const char *prefix,
}
}
 
+   fclose(fp);
+   fp = NULL;
if (finish_command()) {
ret = error("error occurred running diff --raw");
goto finish;
}
 
if (!i)
-   return 0;
+   goto finish;
 
/*
 * Changes to submodules require special treatment.This loop writes a
@@ -626,6 +636,9 @@ static int run_dir_diff(const char *extcmd, int symlinks, 
const char *prefix,
exit_cleanup(tmpdir, rc);
 
 finish:
+   if (fp)
+   fclose(fp);
+
free(lbase_dir);
free(rbase_dir);
strbuf_release();
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 06/25] get_mail_commit_oid(): avoid resource leak

[Johannes Schindelin]

When we fail to read, or parse, the file, we still want to close the file
descriptor and release the strbuf.

Reported via Coverity.

Signed-off-by: Johannes Schindelin 
---
 builtin/am.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index a95dd8b4e6c..9c5c2c778e2 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1351,19 +1351,16 @@ static int get_mail_commit_oid(struct object_id 
*commit_id, const char *mail)
struct strbuf sb = STRBUF_INIT;
FILE *fp = xfopen(mail, "r");
const char *x;
+   int ret = 0;
 
-   if (strbuf_getline_lf(, fp))
-   return -1;
-
-   if (!skip_prefix(sb.buf, "From ", ))
-   return -1;
-
-   if (get_oid_hex(x, commit_id) < 0)
-   return -1;
+   if (strbuf_getline_lf(, fp) ||
+   !skip_prefix(sb.buf, "From ", ) ||
+   get_oid_hex(x, commit_id) < 0)
+   ret = -1;
 
strbuf_release();
fclose(fp);
-   return 0;
+   return ret;
 }
 
 /**
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 04/25] add_commit_patch_id(): avoid allocating memory unnecessarily

[Johannes Schindelin]

It would appear that we allocate (and forget to release) memory if the
patch ID is not even defined.

Reported by the Coverity tool.

Signed-off-by: Johannes Schindelin 
---
 patch-ids.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/patch-ids.c b/patch-ids.c
index fa8f11de826..92eba7a059e 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -99,11 +99,12 @@ struct patch_id *has_commit_patch_id(struct commit *commit,
 struct patch_id *add_commit_patch_id(struct commit *commit,
 struct patch_ids *ids)
 {
-   struct patch_id *key = xcalloc(1, sizeof(*key));
+   struct patch_id *key;
 
if (!patch_id_defined(commit))
return NULL;
 
+   key = xcalloc(1, sizeof(*key));
if (init_patch_id_entry(key, commit, ids)) {
free(key);
return NULL;
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 03/25] winansi: avoid buffer overrun

[Johannes Schindelin]

When we could not convert the UTF-8 sequence into Unicode for writing to
the Console, we should not try to write an insanely-long sequence of
invalid wide characters (mistaking the negative return value for an
unsigned length).

Reported by Coverity.

Signed-off-by: Johannes Schindelin 
---
 compat/winansi.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/compat/winansi.c b/compat/winansi.c
index fd6910746c8..861b79d8c31 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -135,6 +135,11 @@ static void write_console(unsigned char *str, size_t len)
 
/* convert utf-8 to utf-16 */
int wlen = xutftowcsn(wbuf, (char*) str, ARRAY_SIZE(wbuf), len);
+   if (wlen < 0) {
+   wchar_t *err = L"[invalid]";
+   WriteConsoleW(console, err, wcslen(err), , NULL);
+   return;
+   }
 
/* write directly to console */
WriteConsoleW(console, wbuf, wlen, , NULL);
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 02/25] winansi: avoid use of uninitialized value

[Johannes Schindelin]

When stdout is not connected to a Win32 console, we incorrectly used an
uninitialized value for the "plain" character attributes.

Detected by Coverity.

Signed-off-by: Johannes Schindelin 
---
 compat/winansi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/compat/winansi.c b/compat/winansi.c
index 793420f9d0d..fd6910746c8 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -105,6 +105,8 @@ static int is_console(int fd)
if (!fd) {
if (!GetConsoleMode(hcon, ))
return 0;
+   sbi.wAttributes = FOREGROUND_BLUE | FOREGROUND_GREEN |
+   FOREGROUND_RED;
} else if (!GetConsoleScreenBufferInfo(hcon, ))
return 0;
 
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v3 00/25] Address a couple of issues identified by Coverity

[Johannes Schindelin]

I recently registered the git-for-windows fork with Coverity to ensure
that even the Windows-specific patches get some static analysis love.

While at it, I squashed a couple of obvious issues in the part that is
not Windows-specific.

Note: while this patch series squashes some of those issues, the
remaining issues are not necessarily all false positives (e.g. Coverity
getting fooled by our FLEX_ARRAY trick into believing that the last
member of the struct is indeed a 0 or 1 size array) or intentional (e.g.
builtins not releasing memory because exit() is called right after
returning from the function that leaks memory).

Notable examples of the remaining issues are:

- a couple of callers of shorten_unambiguous_ref() assume that they do
  not have to release the memory returned from that function, often
  assigning the pointer to a `const` variable that typically does not
  hold a pointer that needs to be free()d. My hunch is that we will want
  to convert that function to have a fixed number of static buffers and
  use those in a round robin fashion à la sha1_to_hex().

- pack-redundant.c seems to have hard-to-reason-about code paths that
  may eventually leak memory. Essentially, the custody of the allocated
  memory is not clear at all.

- fast-import.c calls strbuf_detach() on the command_buf without any
  obvious rationale. Turns out that *some* code paths assign
  command_buf.buf to a `recent_command` which releases the buffer later.
  However, from a cursory look it appears to me as if there are some
  other code paths that skip that assignment, effectively causing a memory
  leak once strbuf_detach() is called.

Sadly, I lack the time to pursue those remaining issues further.

Changes since v2:

- renamed the `p` variables introduced by the patch series to the more
  explanatory `to_free`.

- reworded incorrect commit message claiming that
  setup_discovered_git_dir() was using a static variable that is
  actually a singleton

- reverted a code move that would have resulted in accessing
  uninitialized data of callers of mailinfo() that do not die() right
  away but clean up faithfully


Johannes Schindelin (25):
  mingw: avoid memory leak when splitting PATH
  winansi: avoid use of uninitialized value
  winansi: avoid buffer overrun
  add_commit_patch_id(): avoid allocating memory unnecessarily
  git_config_rename_section_in_file(): avoid resource leak
  get_mail_commit_oid(): avoid resource leak
  difftool: address a couple of resource/memory leaks
  status: close file descriptor after reading git-rebase-todo
  mailinfo & mailsplit: check for EOF while parsing
  cat-file: fix memory leak
  checkout: fix memory leak
  split_commit_in_progress(): fix memory leak
  setup_bare_git_dir(): help static analysis
  setup_discovered_git_dir(): plug memory leak
  pack-redundant: plug memory leak
  mktree: plug memory leaks reported by Coverity
  fast-export: avoid leaking memory in handle_tag()
  receive-pack: plug memory leak in update()
  line-log: avoid memory leak
  shallow: avoid memory leak
  add_reflog_for_walk: avoid memory leak
  remote: plug memory leak in match_explicit()
  name-rev: avoid leaking memory in the `deref` case
  show_worktree(): plug memory leak
  submodule_uses_worktrees(): plug memory leak

 builtin/am.c | 15 ++-
 builtin/cat-file.c   |  1 +
 builtin/checkout.c   | 17 +
 builtin/difftool.c   | 33 +++--
 builtin/fast-export.c|  2 ++
 builtin/mailsplit.c  | 10 ++
 builtin/mktree.c |  5 +++--
 builtin/name-rev.c   |  7 +--
 builtin/pack-redundant.c |  1 +
 builtin/receive-pack.c   |  4 +++-
 builtin/worktree.c   |  8 +---
 compat/mingw.c   |  4 +++-
 compat/winansi.c |  7 +++
 config.c |  5 -
 line-log.c   |  1 +
 mailinfo.c   |  9 -
 patch-ids.c  |  3 ++-
 reflog-walk.c| 20 +---
 remote.c |  5 +++--
 setup.c  | 11 ---
 shallow.c|  8 ++--
 worktree.c   |  2 +-
 wt-status.c  |  8 +++-
 23 files changed, 135 insertions(+), 51 deletions(-)


base-commit: d2bbb7c2bcf6e77ebfcabf4e12110fe6d5c91de6
Published-As: https://github.com/dscho/git/releases/tag/coverity-v3
Fetch-It-Via: git fetch https://github.com/dscho/git coverity-v3

Interdiff vs v2:

 diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
 index 9b3efc8e987..664400b8169 100644
 --- a/builtin/mailsplit.c
 +++ b/builtin/mailsplit.c
 @@ -232,9 +232,18 @@ static int split_mbox(const char *file, const char *dir, 
int allow_bare,
  
do {
peek = fgetc(f);
 +  if (peek == EOF) {
 +  if (f == stdin)
 +  /* empty stdin is OK */
 +  ret = skip;
 +  else {
 +  

[PATCH v3 01/25] mingw: avoid memory leak when splitting PATH

[Johannes Schindelin]

In the (admittedly, concocted) case that PATH consists only of colons, we
would leak the duplicated string.

Signed-off-by: Johannes Schindelin 
---
 compat/mingw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 3fbfda5978b..fe0e3ccd248 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -961,8 +961,10 @@ static char **get_path_split(void)
++n;
}
}
-   if (!n)
+   if (!n) {
+   free(envpath);
return NULL;
+   }
 
ALLOC_ARRAY(path, n + 1);
p = envpath;
-- 
2.12.2.windows.2.800.gede8f145e06

[PATCH v2 1/1] t0027: tests are not expensive; remove t0025

[tboegi]

From: Torsten Bögershausen 

The purpose of t0027 is to test all CRLF related conversions at
"git checkout" and "git add".

Running t0027 under Git for Windows takes 3-4 minutes, so the whole script
had been marked as "EXPENSIVE".

The source code for "Git for Windows" overrides this since 2014:
"t0027 is marked expensive, but really, for MinGW we want to run these
tests always."

Recent "stress" tests show that t0025 if flaky, reported by Lars Schneider,
larsxschnei...@gmail.com

All tests from t0025 are covered in t0027 already, so that t0025 can be
retiered:
The execution time for t0027 is 14 seconds under Linux,
and 63 seconds under Mac Os X.
And in case you ask, things are not going significantly faster using a SSD
instead of a spinning disk.

Signed-off-by: Torsten Bögershausen 
---
 t/t0025-crlf-auto.sh | 181 ---
 t/t0027-auto-crlf.sh |   6 --
 2 files changed, 187 deletions(-)
 delete mode 100755 t/t0025-crlf-auto.sh

diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
deleted file mode 100755
index 89826c5..000
--- a/t/t0025-crlf-auto.sh
+++ /dev/null
@@ -1,181 +0,0 @@
-#!/bin/sh
-
-test_description='CRLF conversion'
-
-. ./test-lib.sh
-
-has_cr() {
-   tr '\015' Q <"$1" | grep Q >/dev/null
-}
-
-test_expect_success setup '
-
-   git config core.autocrlf false &&
-
-   for w in Hello world how are you; do echo $w; done >LFonly &&
-   for w in I am very very fine thank you; do echo ${w}Q; done | q_to_cr 
>CRLFonly &&
-   for w in Oh here is a QNUL byte how alarming; do echo ${w}; done | 
q_to_nul >LFwithNUL &&
-   git add . &&
-
-   git commit -m initial &&
-
-   LFonly=$(git rev-parse HEAD:LFonly) &&
-   CRLFonly=$(git rev-parse HEAD:CRLFonly) &&
-   LFwithNUL=$(git rev-parse HEAD:LFwithNUL) &&
-
-   echo happy.
-'
-
-test_expect_success 'default settings cause no changes' '
-
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   git read-tree --reset -u HEAD &&
-
-   ! has_cr LFonly &&
-   has_cr CRLFonly &&
-   LFonlydiff=$(git diff LFonly) &&
-   CRLFonlydiff=$(git diff CRLFonly) &&
-   LFwithNULdiff=$(git diff LFwithNUL) &&
-   test -z "$LFonlydiff" -a -z "$CRLFonlydiff" -a -z "$LFwithNULdiff"
-'
-
-test_expect_success 'crlf=true causes a CRLF file to be normalized' '
-
-   # Backwards compatibility check
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   echo "CRLFonly crlf" > .gitattributes &&
-   git read-tree --reset -u HEAD &&
-
-   # Note, "normalized" means that git will normalize it if added
-   has_cr CRLFonly &&
-   CRLFonlydiff=$(git diff CRLFonly) &&
-   test -n "$CRLFonlydiff"
-'
-
-test_expect_success 'text=true causes a CRLF file to be normalized' '
-
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   echo "CRLFonly text" > .gitattributes &&
-   git read-tree --reset -u HEAD &&
-
-   # Note, "normalized" means that git will normalize it if added
-   has_cr CRLFonly &&
-   CRLFonlydiff=$(git diff CRLFonly) &&
-   test -n "$CRLFonlydiff"
-'
-
-test_expect_success 'eol=crlf gives a normalized file CRLFs with 
autocrlf=false' '
-
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   git config core.autocrlf false &&
-   echo "LFonly eol=crlf" > .gitattributes &&
-   git read-tree --reset -u HEAD &&
-
-   has_cr LFonly &&
-   LFonlydiff=$(git diff LFonly) &&
-   test -z "$LFonlydiff"
-'
-
-test_expect_success 'eol=crlf gives a normalized file CRLFs with 
autocrlf=input' '
-
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   git config core.autocrlf input &&
-   echo "LFonly eol=crlf" > .gitattributes &&
-   git read-tree --reset -u HEAD &&
-
-   has_cr LFonly &&
-   LFonlydiff=$(git diff LFonly) &&
-   test -z "$LFonlydiff"
-'
-
-test_expect_success 'eol=lf gives a normalized file LFs with autocrlf=true' '
-
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   git config core.autocrlf true &&
-   echo "LFonly eol=lf" > .gitattributes &&
-   git read-tree --reset -u HEAD &&
-
-   ! has_cr LFonly &&
-   LFonlydiff=$(git diff LFonly) &&
-   test -z "$LFonlydiff"
-'
-
-test_expect_success 'autocrlf=true does not normalize CRLF files' '
-
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   git config core.autocrlf true &&
-   git read-tree --reset -u HEAD &&
-
-   has_cr LFonly &&
-   has_cr CRLFonly &&
-   LFonlydiff=$(git diff LFonly) &&
-   CRLFonlydiff=$(git diff CRLFonly) &&
-   LFwithNULdiff=$(git diff LFwithNUL) &&
-   test -z "$LFonlydiff" -a -z "$CRLFonlydiff" -a -z "$LFwithNULdiff"
-'
-
-test_expect_success 'text=auto, autocrlf=true does not normalize CRLF files' '
-
-   rm -f .gitattributes tmp LFonly CRLFonly LFwithNUL &&
-   git config core.autocrlf true &&
-  

Re: Could GIT manage revision headers embedded in code ?

[Johannes Schindelin]

Hi Eric,

On Tue, 2 May 2017, Delanoe Eric wrote:

> Could this kind of "keyword expansion" feature be added into GIT ?

Back in the days, it was decided that this should be part of the "export
as zip" functionality, see the `export-subst` feature in git-archive's and
gitattributes' documentation.

In reality, what most projects do is to generate a header or some such as
part of their build process. Git itself may serve as a case in point:

https://github.com/git/git/blob/master/GIT-VERSION-GEN

used in

https://github.com/git/git/blob/v2.12.2/Makefile#L390-L392

Ciao,
Johannes

Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

[Johannes Schindelin]

Hi Liam,

On Tue, 2 May 2017, Liam Beguin wrote:

> Add the 'rebase.abbreviateCommands' configuration option to allow `git
> rebase -i` to default to the single-letter command-names in the todo
> list.
> 
> Using single-letter command-names can present two benefits.  First, it
> makes it easier to change the action since you only need to replace a
> single character (i.e.: in vim "r" instead of
> "ciw").  Second, using this with a large enough value of
> 'core.abbrev' enables the lines of the todo list to remain aligned
> making the files easier to read.
> 
> Changes from v1 to v2:
>  - Improve Documentation and commit message
> 
> Changes from v2 to v3:
>  - Transform a single patch into a series
>  - change option name from 'rebase.abbrevCmd' to 'rebase.abbreviateCommands'
>  - abbreviate all commands (not just pick)
>  - teach `git rebase -i --autosquash` to recognise single-letter command-names
>  - move rebase configuration documentation to Documentation/rebase-config.txt
>  - update Documentation to use the preferred naming for the todo list
>  - update Documentation and commit messages according to feedback

Thank you for this pleasant read. It is an excellent contribution, and the
way you communicate what you changed and why is very welcome.

I offered a couple of comments, my biggest one probably being that this
patch series is crossing paths with my patch series that tries to move
more functionality out of the git-rebase--interactive.sh script into the
git-rebase--helper that is written in C, closely followed by my suggestion
to fold at least part of the functionality into the SHA-1
collapsing/expanding.

If your patch series "wins", I can easily forward-port your changes to the
rebase-i-extra branch, but it may actually make sense to build on top of
the rebase-i-extra branch to begin with. If you agree: I pushed the
proposed change to the `rebase-i-extra+abbrev` branch at
https://github.com/dscho/git.

I look forward to see this story unfold!

Ciao,
Johannes

Re: [PATCH v3 0/6] rebase -i: add config to abbreviate command-names

[Johannes Schindelin]

Hi,


On Tue, 2 May 2017, Ævar Arnfjörð Bjarmason wrote:

> I locally rebased this into just 3 patches, i.e. in this sequence:
> 
> - Documentation: move rebase.* config variables to a separate 
> rebase-config.txt
> - Documentation: use preferred name for the 'todo list' script
> - *all the rest of this squashed*

I think that makes a lot of sense. (I would drop the part about f!/s!, as
I pointed out, though)

Ciao,
Johannes