[PATCH 013/194] sha1_file: add repository argument to sha1_loose_object_info

[Stefan Beller]

Add a repository argument to allow the sha1_loose_object_info caller
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 24832c4b32..514f59c390 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1152,9 +1152,10 @@ int parse_sha1_header(const char *hdr, unsigned long 
*sizep)
return parse_sha1_header_extended(hdr, , 0);
 }
 
-static int sha1_loose_object_info(const unsigned char *sha1,
- struct object_info *oi,
- int flags)
+#define sha1_loose_object_info(r, s, o, f) sha1_loose_object_info_##r(s, o, f)
+static int sha1_loose_object_info_the_repository(const unsigned char *sha1,
+struct object_info *oi,
+int flags)
 {
int status = 0;
unsigned long mapsize;
@@ -1267,7 +1268,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
 
if (!find_pack_entry(real, )) {
/* Most likely it's a loose object. */
-   if (!sha1_loose_object_info(real, oi, flags))
+   if (!sha1_loose_object_info(the_repository, real, oi, flags))
return 0;
 
/* Not a loose object; someone else may have just packed it. */
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 018/194] pack: add repository argument to rearrange_packed_git

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow the rearrange_packed_git caller to
be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 packfile.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packfile.c b/packfile.c
index 58473660cd..52cf9182c4 100644
--- a/packfile.c
+++ b/packfile.c
@@ -857,7 +857,8 @@ static int sort_pack(const void *a_, const void *b_)
return -1;
 }
 
-static void rearrange_packed_git(void)
+#define rearrange_packed_git(r) rearrange_packed_git_##r()
+static void rearrange_packed_git_the_repository(void)
 {
the_repository->objects.packed_git = llist_mergesort(
the_repository->objects.packed_git, get_next_packed_git,
@@ -883,7 +884,7 @@ void prepare_packed_git(void)
prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(the_repository, alt->path, 0);
-   rearrange_packed_git();
+   rearrange_packed_git(the_repository);
prepare_packed_git_mru();
the_repository->objects.packed_git_initialized = 1;
 }
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 010/194] sha1_file: add repository argument to stat_sha1_file

[Stefan Beller]

Add a repository argument to allow the stat_sha1_file caller to be
more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d9f9046f31..6abdb8c488 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -875,8 +875,9 @@ int git_open_cloexec(const char *name, int flags)
  * Note that it may point to static storage and is only valid until another
  * call to sha1_file_name(), etc.
  */
-static int stat_sha1_file(const unsigned char *sha1, struct stat *st,
- const char **path)
+#define stat_sha1_file(r, s, st, p) stat_sha1_file_##r(s, st, p)
+static int stat_sha1_file_the_repository(const unsigned char *sha1,
+struct stat *st, const char **path)
 {
struct alternate_object_database *alt;
 
@@ -1174,7 +1175,7 @@ static int sha1_loose_object_info(const unsigned char 
*sha1,
if (!oi->typep && !oi->typename && !oi->sizep && !oi->contentp) {
const char *path;
struct stat st;
-   if (stat_sha1_file(sha1, , ) < 0)
+   if (stat_sha1_file(the_repository, sha1, , ) < 0)
return -1;
if (oi->disk_sizep)
*oi->disk_sizep = st.st_size;
@@ -1373,7 +1374,7 @@ void *read_sha1_file_extended(const unsigned char *sha1,
die("replacement %s not found for %s",
sha1_to_hex(repl), sha1_to_hex(sha1));
 
-   if (!stat_sha1_file(repl, , ))
+   if (!stat_sha1_file(the_repository, repl, , ))
die("loose object %s (stored in %s) is corrupt",
sha1_to_hex(repl), path);
 
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 012/194] sha1_file: add repository argument to map_sha1_file_1

[Stefan Beller]

Add a repository argument to allow the map_sha1_file_1 caller to be
more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 9fa7243907..24832c4b32 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -931,9 +931,10 @@ static int open_sha1_file_the_repository(const unsigned 
char *sha1,
  * Map the loose object at "path" if it is not NULL, or the path found by
  * searching for a loose object named "sha1".
  */
-static void *map_sha1_file_1(const char *path,
-const unsigned char *sha1,
-unsigned long *size)
+#define map_sha1_file_1(r, p, s, si) map_sha1_file_1_##r(p, s, si)
+static void *map_sha1_file_1_the_repository(const char *path,
+   const unsigned char *sha1,
+   unsigned long *size)
 {
void *map;
int fd;
@@ -962,7 +963,7 @@ static void *map_sha1_file_1(const char *path,
 
 void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
 {
-   return map_sha1_file_1(NULL, sha1, size);
+   return map_sha1_file_1(the_repository, NULL, sha1, size);
 }
 
 static int unpack_sha1_short_header(git_zstream *stream,
@@ -2174,7 +2175,7 @@ int read_loose_object(const char *path,
 
*contents = NULL;
 
-   map = map_sha1_file_1(path, NULL, );
+   map = map_sha1_file_1(the_repository, path, NULL, );
if (!map) {
error_errno("unable to mmap %s", path);
goto out;
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 014/194] object-store: add repository argument to prepare_alt_odb

[Stefan Beller]

Add a repository argument to allow prepare_alt_odb callers to be more
specific about which repository to handle. This is a small mechanical
change; it doesn't change the implementation to handle repositories
other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

While at it, move the declaration to object-store.h, where it should
be easier to find.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 builtin/fsck.c |  2 +-
 cache.h|  1 -
 object-store.h |  3 +++
 packfile.c |  2 +-
 sha1_file.c| 13 +++--
 sha1_name.c|  3 ++-
 6 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index d4d249c2ed..00ec8eecf0 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -695,7 +695,7 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
} else {
fsck_object_dir(get_object_directory());
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list;
alt; alt = alt->next)
fsck_object_dir(alt->path);
diff --git a/cache.h b/cache.h
index 459fd01dbb..70518e24ce 100644
--- a/cache.h
+++ b/cache.h
@@ -1592,7 +1592,6 @@ struct alternate_object_database {
 
char path[FLEX_ARRAY];
 };
-extern void prepare_alt_odb(void);
 extern char *compute_alternate_path(const char *path, struct strbuf *err);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 extern int foreach_alt_odb(alt_odb_fn, void*);
diff --git a/object-store.h b/object-store.h
index 99f77d10cd..e749c952d5 100644
--- a/object-store.h
+++ b/object-store.h
@@ -54,4 +54,7 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
 };
 
+#define prepare_alt_odb(r) prepare_alt_odb_##r()
+extern void prepare_alt_odb_the_repository(void);
+
 #endif /* OBJECT_STORE_H */
diff --git a/packfile.c b/packfile.c
index 31c4ea54ae..fbb2385bad 100644
--- a/packfile.c
+++ b/packfile.c
@@ -879,7 +879,7 @@ void prepare_packed_git(void)
if (the_repository->objects.packed_git_initialized)
return;
prepare_packed_git_one(get_object_directory(), 1);
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
diff --git a/sha1_file.c b/sha1_file.c
index 514f59c390..779bfd4079 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -23,6 +23,7 @@
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
 #include "repository.h"
+#include "object-store.h"
 #include "streaming.h"
 #include "dir.h"
 #include "mru.h"
@@ -585,7 +586,7 @@ void add_to_alternates_memory(const char *reference)
 * Make sure alternates are initialized, or else our entry may be
 * overwritten when they are.
 */
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
 
link_alt_odb_entries(the_repository, reference,
 '\n', NULL, 0);
@@ -671,7 +672,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
struct alternate_object_database *ent;
int r = 0;
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (ent = the_repository->objects.alt_odb_list; ent; ent = ent->next) {
r = fn(ent, cb);
if (r)
@@ -680,7 +681,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
return r;
 }
 
-void prepare_alt_odb(void)
+void prepare_alt_odb_the_repository(void)
 {
const char *alt;
 
@@ -729,7 +730,7 @@ static int check_and_freshen_local(const unsigned char 
*sha1, int freshen)
 static int check_and_freshen_nonlocal(const unsigned char *sha1, int freshen)
 {
struct alternate_object_database *alt;
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
const char *path = alt_sha1_path(alt, sha1);
if (check_and_freshen_file(path, freshen))
@@ -885,7 +886,7 @@ static int stat_sha1_file_the_repository(const unsigned 
char *sha1,
if (!lstat(*path, st))
return 0;
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
errno = ENOENT;
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
*path = alt_sha1_path(alt, sha1);
@@ -914,7 +915,7 @@ static int open_sha1_file_the_repository(const unsigned 
char *sha1,
return fd;
most_interesting_errno = errno;
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
*path = 

[PATCH 017/194] pack: add repository argument to prepare_packed_git_one

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow the prepare_packed_git_one caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 packfile.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/packfile.c b/packfile.c
index 0f63ec79c8..58473660cd 100644
--- a/packfile.c
+++ b/packfile.c
@@ -726,7 +726,8 @@ static void report_pack_garbage(struct string_list *list)
report_helper(list, seen_bits, first, list->nr);
 }
 
-static void prepare_packed_git_one(char *objdir, int local)
+#define prepare_packed_git_one(r, o, l) prepare_packed_git_one_##r(o, l)
+static void prepare_packed_git_one_the_repository(char *objdir, int local)
 {
struct strbuf path = STRBUF_INIT;
size_t dirnamelen;
@@ -878,10 +879,10 @@ void prepare_packed_git(void)
 
if (the_repository->objects.packed_git_initialized)
return;
-   prepare_packed_git_one(get_object_directory(), 1);
+   prepare_packed_git_one(the_repository, get_object_directory(), 1);
prepare_alt_odb(the_repository);
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
-   prepare_packed_git_one(alt->path, 0);
+   prepare_packed_git_one(the_repository, alt->path, 0);
rearrange_packed_git();
prepare_packed_git_mru();
the_repository->objects.packed_git_initialized = 1;
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 016/194] pack: add repository argument to install_packed_git

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow install_packed_git callers to be
more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 fast-import.c | 2 +-
 http.c| 3 ++-
 packfile.c| 4 ++--
 packfile.h| 3 ++-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 1685fe59a2..67550584da 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1038,7 +1038,7 @@ static void end_packfile(void)
if (!new_p)
die("core git rejected index %s", idx_name);
all_packs[pack_id] = new_p;
-   install_packed_git(new_p);
+   install_packed_git(the_repository, new_p);
free(idx_name);
 
/* Print the boundary */
diff --git a/http.c b/http.c
index ab989b88dd..979d9b3e46 100644
--- a/http.c
+++ b/http.c
@@ -1,5 +1,6 @@
 #include "git-compat-util.h"
 #include "http.h"
+#include "repository.h"
 #include "config.h"
 #include "object-store.h"
 #include "pack.h"
@@ -2068,7 +2069,7 @@ int finish_http_pack_request(struct http_pack_request 
*preq)
return -1;
}
 
-   install_packed_git(p);
+   install_packed_git(the_repository, p);
free(tmp_idx);
return 0;
 }
diff --git a/packfile.c b/packfile.c
index fbb2385bad..0f63ec79c8 100644
--- a/packfile.c
+++ b/packfile.c
@@ -671,7 +671,7 @@ struct packed_git *add_packed_git(const char *path, size_t 
path_len, int local)
return p;
 }
 
-void install_packed_git(struct packed_git *pack)
+void install_packed_git_the_repository(struct packed_git *pack)
 {
if (pack->pack_fd != -1)
pack_open_fds++;
@@ -773,7 +773,7 @@ static void prepare_packed_git_one(char *objdir, int local)
 * corresponding .pack file that we can map.
 */
(p = add_packed_git(path.buf, path.len, local)) != 
NULL)
-   install_packed_git(p);
+   install_packed_git(the_repository, p);
}
 
if (!report_garbage)
diff --git a/packfile.h b/packfile.h
index 0cdeb54dcd..7e6959d440 100644
--- a/packfile.h
+++ b/packfile.h
@@ -34,7 +34,8 @@ extern void (*report_garbage)(unsigned seen_bits, const char 
*path);
 
 extern void prepare_packed_git(void);
 extern void reprepare_packed_git(void);
-extern void install_packed_git(struct packed_git *pack);
+#define install_packed_git(r, p) install_packed_git_##r(p)
+extern void install_packed_git_the_repository(struct packed_git *pack);
 
 /*
  * Give a rough count of objects in the repository. This sacrifices accuracy
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 011/194] sha1_file: add repository argument to open_sha1_file

[Stefan Beller]

Add a repository argument to allow the open_sha1_file caller to be
more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

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

diff --git a/sha1_file.c b/sha1_file.c
index 6abdb8c488..9fa7243907 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -900,7 +900,9 @@ static int stat_sha1_file_the_repository(const unsigned 
char *sha1,
  * Like stat_sha1_file(), but actually open the object and return the
  * descriptor. See the caveats on the "path" parameter above.
  */
-static int open_sha1_file(const unsigned char *sha1, const char **path)
+#define open_sha1_file(r, s, p) open_sha1_file_##r(s, p)
+static int open_sha1_file_the_repository(const unsigned char *sha1,
+const char **path)
 {
int fd;
struct alternate_object_database *alt;
@@ -939,7 +941,7 @@ static void *map_sha1_file_1(const char *path,
if (path)
fd = git_open(path);
else
-   fd = open_sha1_file(sha1, );
+   fd = open_sha1_file(the_repository, sha1, );
map = NULL;
if (fd >= 0) {
struct stat st;
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 019/194] pack: add repository argument to prepare_packed_git_mru

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow the prepare_packed_git_mru caller
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 packfile.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packfile.c b/packfile.c
index 52cf9182c4..b4ace96f0c 100644
--- a/packfile.c
+++ b/packfile.c
@@ -865,7 +865,8 @@ static void rearrange_packed_git_the_repository(void)
set_next_packed_git, sort_pack);
 }
 
-static void prepare_packed_git_mru(void)
+#define prepare_packed_git_mru(r) prepare_packed_git_mru_##r()
+static void prepare_packed_git_mru_the_repository(void)
 {
struct packed_git *p;
 
@@ -885,7 +886,7 @@ void prepare_packed_git(void)
for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(the_repository, alt->path, 0);
rearrange_packed_git(the_repository);
-   prepare_packed_git_mru();
+   prepare_packed_git_mru(the_repository);
the_repository->objects.packed_git_initialized = 1;
 }
 
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 020/194] pack: add repository argument to prepare_packed_git

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow prepare_packed_git_mru callers to
be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

Callers adapted using contrib/coccinelle/packed_git.cocci.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 builtin/count-objects.c |  2 +-
 builtin/fsck.c  |  2 +-
 builtin/gc.c|  2 +-
 builtin/pack-objects.c  |  2 +-
 builtin/pack-redundant.c|  2 +-
 contrib/coccinelle/packed_git.cocci |  4 
 fast-import.c   |  2 +-
 http-backend.c  |  2 +-
 pack-bitmap.c   |  2 +-
 packfile.c  | 10 +-
 packfile.h  |  3 ++-
 server-info.c   |  2 +-
 sha1_name.c |  4 ++--
 13 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 843a7d7d7e..e340b3e3c3 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -123,7 +123,7 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
if (!the_repository->objects.packed_git)
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
for (p = the_repository->objects.packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 00ec8eecf0..0e78f4ba72 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -705,7 +705,7 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
uint32_t total = 0, count = 0;
struct progress *progress = NULL;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
 
if (show_progress) {
for (p = the_repository->objects.packed_git; p;
diff --git a/builtin/gc.c b/builtin/gc.c
index 6970f325e5..00a3b402a8 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -174,7 +174,7 @@ static int too_many_packs(void)
if (gc_auto_pack_limit <= 0)
return 0;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
for (cnt = 0, p = the_repository->objects.packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 2ee8b69238..d42df2a2b9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3121,7 +3121,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
if (progress && all_progress_implied)
progress = 2;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
if (ignore_packed_keep) {
struct packed_git *p;
for (p = the_repository->objects.packed_git; p; p = p->next)
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 55462bc826..83d2395dae 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -631,7 +631,7 @@ int cmd_pack_redundant(int argc, const char **argv, const 
char *prefix)
break;
}
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
 
if (load_all_packs)
load_all();
diff --git a/contrib/coccinelle/packed_git.cocci 
b/contrib/coccinelle/packed_git.cocci
index da317a51a9..7554f4773c 100644
--- a/contrib/coccinelle/packed_git.cocci
+++ b/contrib/coccinelle/packed_git.cocci
@@ -5,3 +5,7 @@
 @@ @@
 - packed_git_mru
 + the_repository->objects.packed_git_mru
+
+@@ @@
+- prepare_packed_git()
++ prepare_packed_git(the_repository)
diff --git a/fast-import.c b/fast-import.c
index 67550584da..39cd0b7540 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3471,7 +3471,7 @@ int cmd_main(int argc, const char **argv)
rc_free[i].next = _free[i + 1];
rc_free[cmd_save - 1].next = NULL;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
start_packfile();
set_die_routine(die_nicely);
set_checkpoint_signal();
diff --git a/http-backend.c b/http-backend.c
index 4d93126c15..4950078c93 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -519,7 +519,7 @@ static void get_info_packs(struct strbuf *hdr, char *arg)
size_t cnt = 0;
 
select_getanyfile(hdr);
-   

[PATCH 008/194] sha1_file: add repository argument to read_info_alternates

[Stefan Beller]

Add a repository argument to allow the read_info_alternates caller to
be more specific about which repository to read from. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commit, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 1e17246f5c..acb00b9680 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -393,7 +393,9 @@ static int alt_odb_usable_the_repository(struct strbuf 
*path,
  * SHA1, an extra slash for the first level indirection, and the
  * terminating NUL.
  */
-static void read_info_alternates(const char * relative_base, int depth);
+#define read_info_alternates(r, rb, d) read_info_alternates_##r(rb, d)
+static void read_info_alternates_the_repository(const char *relative_base,
+   int depth);
 #define link_alt_odb_entry(r, e, rb, d, n) link_alt_odb_entry_##r(e, rb, d, n)
 static int link_alt_odb_entry_the_repository(const char *entry,
const char *relative_base, int depth, const char *normalized_objdir)
@@ -434,7 +436,7 @@ static int link_alt_odb_entry_the_repository(const char 
*entry,
ent->next = NULL;
 
/* recursively add alternates */
-   read_info_alternates(pathbuf.buf, depth + 1);
+   read_info_alternates(the_repository, pathbuf.buf, depth + 1);
 
strbuf_release();
return 0;
@@ -500,7 +502,8 @@ static void link_alt_odb_entries(const char *alt, int sep,
strbuf_release();
 }
 
-static void read_info_alternates(const char * relative_base, int depth)
+static void read_info_alternates_the_repository(const char *relative_base,
+   int depth)
 {
char *path;
struct strbuf buf = STRBUF_INIT;
@@ -684,7 +687,7 @@ void prepare_alt_odb(void)
_repository->objects.alt_odb_list;
link_alt_odb_entries(alt, PATH_SEP, NULL, 0);
 
-   read_info_alternates(get_object_directory(), 0);
+   read_info_alternates(the_repository, get_object_directory(), 0);
 }
 
 /* Returns 1 if we have successfully freshened the file, 0 otherwise. */
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 007/194] sha1_file: add repository argument to link_alt_odb_entry

[Stefan Beller]

Add a repository argument to allow the link_alt_odb_entry caller to be
more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commit, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index ca002a272d..1e17246f5c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -394,8 +394,9 @@ static int alt_odb_usable_the_repository(struct strbuf 
*path,
  * terminating NUL.
  */
 static void read_info_alternates(const char * relative_base, int depth);
-static int link_alt_odb_entry(const char *entry, const char *relative_base,
-   int depth, const char *normalized_objdir)
+#define link_alt_odb_entry(r, e, rb, d, n) link_alt_odb_entry_##r(e, rb, d, n)
+static int link_alt_odb_entry_the_repository(const char *entry,
+   const char *relative_base, int depth, const char *normalized_objdir)
 {
struct alternate_object_database *ent;
struct strbuf pathbuf = STRBUF_INIT;
@@ -492,7 +493,8 @@ static void link_alt_odb_entries(const char *alt, int sep,
alt = parse_alt_odb_entry(alt, sep, );
if (!entry.len)
continue;
-   link_alt_odb_entry(entry.buf, relative_base, depth, 
objdirbuf.buf);
+   link_alt_odb_entry(the_repository, entry.buf,
+  relative_base, depth, objdirbuf.buf);
}
strbuf_release();
strbuf_release();
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 009/194] sha1_file: add repository argument to link_alt_odb_entries

[Stefan Beller]

Add a repository argument to allow the link_alt_odb_entries caller to
be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 sha1_file.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index acb00b9680..d9f9046f31 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -471,8 +471,12 @@ static const char *parse_alt_odb_entry(const char *string,
return end;
 }
 
-static void link_alt_odb_entries(const char *alt, int sep,
-const char *relative_base, int depth)
+#define link_alt_odb_entries(r, a, s, rb, d) \
+   link_alt_odb_entries_##r(a, s, rb, d)
+static void link_alt_odb_entries_the_repository(const char *alt,
+   int sep,
+   const char *relative_base,
+   int depth)
 {
struct strbuf objdirbuf = STRBUF_INIT;
struct strbuf entry = STRBUF_INIT;
@@ -515,7 +519,7 @@ static void read_info_alternates_the_repository(const char 
*relative_base,
return;
}
 
-   link_alt_odb_entries(buf.buf, '\n', relative_base, depth);
+   link_alt_odb_entries(the_repository, buf.buf, '\n', relative_base, 
depth);
strbuf_release();
free(path);
 }
@@ -569,7 +573,8 @@ void add_to_alternates_file(const char *reference)
if (commit_lock_file())
die_errno("unable to move new alternates file into 
place");
if (the_repository->objects.alt_odb_tail)
-   link_alt_odb_entries(reference, '\n', NULL, 0);
+   link_alt_odb_entries(the_repository, reference,
+'\n', NULL, 0);
}
free(alts);
 }
@@ -582,7 +587,8 @@ void add_to_alternates_memory(const char *reference)
 */
prepare_alt_odb();
 
-   link_alt_odb_entries(reference, '\n', NULL, 0);
+   link_alt_odb_entries(the_repository, reference,
+'\n', NULL, 0);
 }
 
 /*
@@ -685,7 +691,8 @@ void prepare_alt_odb(void)
 
the_repository->objects.alt_odb_tail =
_repository->objects.alt_odb_list;
-   link_alt_odb_entries(alt, PATH_SEP, NULL, 0);
+   link_alt_odb_entries(the_repository, alt,
+PATH_SEP, NULL, 0);
 
read_info_alternates(the_repository, get_object_directory(), 0);
 }
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 004/194] pack: move prepare_packed_git_run_once to object store

[Stefan Beller]

From: Jonathan Nieder 

Each repository's object store can be initialized independently, so
they must not share a run_once variable.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 object-store.h | 8 +++-
 packfile.c | 7 +++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/object-store.h b/object-store.h
index e2a59a0611..14129fbba1 100644
--- a/object-store.h
+++ b/object-store.h
@@ -15,8 +15,14 @@ struct object_store {
 
struct alternate_object_database *alt_odb_list;
struct alternate_object_database **alt_odb_tail;
+
+   /*
+* Whether packed_git has already been populated with this repository's
+* packs.
+*/
+   unsigned packed_git_initialized : 1;
 };
-#define OBJECT_STORE_INIT { NULL, MRU_INIT, NULL, NULL }
+#define OBJECT_STORE_INIT { NULL, MRU_INIT, NULL, NULL, 0 }
 
 struct packed_git {
struct packed_git *next;
diff --git a/packfile.c b/packfile.c
index 11b0e6613c..177aa219f9 100644
--- a/packfile.c
+++ b/packfile.c
@@ -873,12 +873,11 @@ static void prepare_packed_git_mru(void)
mru_append(_repository->objects.packed_git_mru, p);
 }
 
-static int prepare_packed_git_run_once = 0;
 void prepare_packed_git(void)
 {
struct alternate_object_database *alt;
 
-   if (prepare_packed_git_run_once)
+   if (the_repository->objects.packed_git_initialized)
return;
prepare_packed_git_one(get_object_directory(), 1);
prepare_alt_odb();
@@ -886,13 +885,13 @@ void prepare_packed_git(void)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
prepare_packed_git_mru();
-   prepare_packed_git_run_once = 1;
+   the_repository->objects.packed_git_initialized = 1;
 }
 
 void reprepare_packed_git(void)
 {
approximate_object_count_valid = 0;
-   prepare_packed_git_run_once = 0;
+   the_repository->objects.packed_git_initialized = 0;
prepare_packed_git();
 }
 
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 006/194] sha1_file: add repository argument to alt_odb_usable

[Stefan Beller]

Add a repository argument to allow the alt_odb_usable caller to be
more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.

Since the implementation does not yet work with other repositories,
use a wrapper macro to enforce that the caller passes in
the_repository as the first argument. It would be more appealing to
use BUILD_ASSERT_OR_ZERO to enforce this, but that doesn't work
because it requires a compile-time constant and common compilers like
gcc 4.8.4 do not consider "r == the_repository" a compile-time
constant.

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

diff --git a/sha1_file.c b/sha1_file.c
index 2826d5d6ed..ca002a272d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -350,7 +350,9 @@ static const char *alt_sha1_path(struct 
alternate_object_database *alt,
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
-static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
+#define alt_odb_usable(r, p, n) alt_odb_usable_##r(p, n)
+static int alt_odb_usable_the_repository(struct strbuf *path,
+const char *normalized_objdir)
 {
struct alternate_object_database *alt;
 
@@ -418,7 +420,7 @@ static int link_alt_odb_entry(const char *entry, const char 
*relative_base,
while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
strbuf_setlen(, pathbuf.len - 1);
 
-   if (!alt_odb_usable(, normalized_objdir)) {
+   if (!alt_odb_usable(the_repository, , normalized_objdir)) {
strbuf_release();
return -1;
}
-- 
2.15.1.433.g936d1b9894.dirty

[PATCH 005/194] pack: move approximate object count to object store

[Stefan Beller]

From: Jonathan Nieder 

The approximate_object_count() function maintains a rough count of
objects in a repository to estimate how long object name abbreviates
should be.  Object names are scoped to a repository and the
appropriate length may differ by repository, so the object count
should not be global.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 object-store.h | 10 +-
 packfile.c | 11 +--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/object-store.h b/object-store.h
index 14129fbba1..99f77d10cd 100644
--- a/object-store.h
+++ b/object-store.h
@@ -16,13 +16,21 @@ struct object_store {
struct alternate_object_database *alt_odb_list;
struct alternate_object_database **alt_odb_tail;
 
+   /*
+* A fast, rough count of the number of objects in the repository.
+* These two fields are not meant for direct access. Use
+* approximate_object_count() instead.
+*/
+   unsigned long approximate_object_count;
+   unsigned approximate_object_count_valid : 1;
+
/*
 * Whether packed_git has already been populated with this repository's
 * packs.
 */
unsigned packed_git_initialized : 1;
 };
-#define OBJECT_STORE_INIT { NULL, MRU_INIT, NULL, NULL, 0 }
+#define OBJECT_STORE_INIT { NULL, MRU_INIT, NULL, NULL, 0, 0, 0 }
 
 struct packed_git {
struct packed_git *next;
diff --git a/packfile.c b/packfile.c
index 177aa219f9..31c4ea54ae 100644
--- a/packfile.c
+++ b/packfile.c
@@ -793,8 +793,6 @@ static void prepare_packed_git_one(char *objdir, int local)
strbuf_release();
 }
 
-static int approximate_object_count_valid;
-
 /*
  * Give a fast, rough count of the number of objects in the repository. This
  * ignores loose objects completely. If you have a lot of them, then either
@@ -804,8 +802,8 @@ static int approximate_object_count_valid;
  */
 unsigned long approximate_object_count(void)
 {
-   static unsigned long count;
-   if (!approximate_object_count_valid) {
+   if (!the_repository->objects.approximate_object_count_valid) {
+   unsigned long count;
struct packed_git *p;
 
prepare_packed_git();
@@ -815,8 +813,9 @@ unsigned long approximate_object_count(void)
continue;
count += p->num_objects;
}
+   the_repository->objects.approximate_object_count = count;
}
-   return count;
+   return the_repository->objects.approximate_object_count;
 }
 
 static void *get_next_packed_git(const void *p)
@@ -890,7 +889,7 @@ void prepare_packed_git(void)
 
 void reprepare_packed_git(void)
 {
-   approximate_object_count_valid = 0;
+   the_repository->objects.approximate_object_count_valid = 0;
the_repository->objects.packed_git_initialized = 0;
prepare_packed_git();
 }
-- 
2.15.1.433.g936d1b9894.dirty

[RFC PATCH 000/194] Moving global state into the repository object

[Stefan Beller]

This series moves a lot of global state into the repository struct.
It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0)
It can be found at https://github.com/stefanbeller/git/tree/object-store

Motivation for this series:
* Easier to reason about code when all state is stored in one place,
  for example for multithreading
* Easier to move to processing submodules in-process instead of
  calling a processes for the submodule handling.
  The last patch demonstrates this.

Usually when approaching large refactoring series, a lot of tedious review
work is involved and yet it is hard to be convinced the series did everything
right.

When reviewing this series in particular it is hard to be convinced if
any function that is converted to take the repository as an argument
is fully converted or if there are any hidden dependencies that are not
obvious from code review.

This is why this series tries to be reviewer friendly by utilizing
machine assistance as much as possible. There are 3 types of patches
in this series:

(A) : add repository argument to 
  This sort of patch is just adding the repository as an argument to that
  function. It assumes the given repository is `the_repository` and has
  a compile time check for that assertion using a preprocessor trick.
  As did a compile check after each commit of the series, I don't expect
  the review burden on these patches to be high. Review on these patches
  is mostly checking for formatting errors or if I sneak in malicious
  code -- if you're inclined to believe that.

(B) : allow  to handle arbitrary repositories
  This sort of patch is touching code inside the given function only,
  usually replacing all occurrences of `the_repository` by the argument
  `r`. Here the review is critical: Did I miss any function that relies
  on state of `the_repository` ?
  This series was developed by converting all functions of
  packfile/sha1-file/commit/etc using (A); after the demo patch
  was possible, all patches that did (A), but not (B) were deleted.
  Therefore I was confident at time of writing that patch that
  the conversion of a function which doesn't take a repository argument
  is okay.

(C) other patches, such as moving code around,
demoing this series in the last patch

This approach was chosen as I think it is review friendly, despite the
huge number of patches.

A weakness of this approach is the buildup of a large series, which ignores
the ongoing development. Rebasing that series turned out to be ok, but merging
it with confidence is an issue.

This series was developed partially as a pair programming exercise with
Jonathan Nieder and Brandon Williams.

The idea with the #define trick to separate the two very separate issues
of touching a lot of code and reasoning about its correctness (hidden deps)
is mostly from Jonathan Tan.

Any suggestions welcome!

Thanks,
Stefan

Brandon Williams (4):
  sha1_file: allow add_to_alternates_file to handle arbitrary
repositories
  commit: convert commit_graft_pos() to handle arbitrary repositories
  commit: convert register_commit_graft to handle arbitrary repositories
  commit: convert read_graft_file to handle arbitrary repositories

Jonathan Nieder (50):
  object-store: move packed_git and packed_git_mru to object store
  pack: move prepare_packed_git_run_once to object store
  pack: move approximate object count to object store
  pack: add repository argument to install_packed_git
  pack: add repository argument to prepare_packed_git_one
  pack: add repository argument to rearrange_packed_git
  pack: add repository argument to prepare_packed_git_mru
  pack: add repository argument to prepare_packed_git
  pack: add repository argument to reprepare_packed_git
  pack: add repository argument to sha1_file_name
  pack: add repository argument to map_sha1_file
  sha1_file: allow alt_odb_usable to handle arbitrary repositories
  pack: allow install_packed_git to handle arbitrary repositories
  pack: allow rearrange_packed_git to handle arbitrary repositories
  pack: allow prepare_packed_git_mru to handle arbitrary repositories
  pack: allow prepare_packed_git_one to handle arbitrary repositories
  pack: allow prepare_packed_git to handle arbitrary repositories
  pack: allow reprepare_packed_git to handle arbitrary repositories
  pack: allow sha1_file_name to handle arbitrary repositories
  pack: allow stat_sha1_file to handle arbitrary repositories
  pack: allow open_sha1_file to handle arbitrary repositories
  pack: allow map_sha1_file_1 to handle arbitrary repositories
  pack: allow map_sha1_file to handle arbitrary repositories
  pack: allow sha1_loose_object_info to handle arbitrary repositories
  sha1_file: add repository argument to sha1_object_info_extended
  object-store: move alternates API to new alternates.h
  object-store: move loose object functions to new loose-object.h
  pack: move struct pack_window and pack_entry to packfile.h
  object-store: move replace_objects back to 

[PATCH 002/194] object-store: move alt_odb_list and alt_odb_tail to object store

[Stefan Beller]

In a process with multiple repositories open, alternates should be
associated to a single repository and not shared globally. Move
alt_odb_list and alt_odb_tail into the_repository and adjust callers
to reflect this.

No functional change intended.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 builtin/fsck.c |  4 +++-
 cache.h|  4 ++--
 object-store.h |  6 +-
 packfile.c |  3 ++-
 sha1_file.c| 25 -
 sha1_name.c|  3 ++-
 6 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 04846d46f9..1048255da1 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "repository.h"
 #include "config.h"
 #include "commit.h"
 #include "tree.h"
@@ -694,7 +695,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
fsck_object_dir(get_object_directory());
 
prepare_alt_odb();
-   for (alt = alt_odb_list; alt; alt = alt->next)
+   for (alt = the_repository->objects.alt_odb_list;
+   alt; alt = alt->next)
fsck_object_dir(alt->path);
 
if (check_full) {
diff --git a/cache.h b/cache.h
index d8b975a571..918c2f15b4 100644
--- a/cache.h
+++ b/cache.h
@@ -1573,7 +1573,7 @@ extern int has_dirs_only_path(const char *name, int len, 
int prefix_len);
 extern void schedule_dir_for_removal(const char *name, int len);
 extern void remove_scheduled_dirs(void);
 
-extern struct alternate_object_database {
+struct alternate_object_database {
struct alternate_object_database *next;
 
/* see alt_scratch_buf() */
@@ -1591,7 +1591,7 @@ extern struct alternate_object_database {
struct oid_array loose_objects_cache;
 
char path[FLEX_ARRAY];
-} *alt_odb_list;
+};
 extern void prepare_alt_odb(void);
 extern char *compute_alternate_path(const char *path, struct strbuf *err);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
diff --git a/object-store.h b/object-store.h
index 05722cdde0..ace3efbfab 100644
--- a/object-store.h
+++ b/object-store.h
@@ -1,8 +1,12 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include "cache.h"
+
 struct object_store {
+   struct alternate_object_database *alt_odb_list;
+   struct alternate_object_database **alt_odb_tail;
 };
-#define OBJECT_STORE_INIT {}
+#define OBJECT_STORE_INIT { NULL, NULL }
 
 #endif /* OBJECT_STORE_H */
diff --git a/packfile.c b/packfile.c
index 4a5fe7ab18..d61076faaf 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "mru.h"
 #include "pack.h"
+#include "repository.h"
 #include "dir.h"
 #include "mergesort.h"
 #include "packfile.h"
@@ -880,7 +881,7 @@ void prepare_packed_git(void)
return;
prepare_packed_git_one(get_object_directory(), 1);
prepare_alt_odb();
-   for (alt = alt_odb_list; alt; alt = alt->next)
+   for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
prepare_packed_git_mru();
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac650..2826d5d6ed 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -22,6 +22,7 @@
 #include "pack-revindex.h"
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
+#include "repository.h"
 #include "streaming.h"
 #include "dir.h"
 #include "mru.h"
@@ -346,9 +347,6 @@ static const char *alt_sha1_path(struct 
alternate_object_database *alt,
return buf->buf;
 }
 
-struct alternate_object_database *alt_odb_list;
-static struct alternate_object_database **alt_odb_tail;
-
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
@@ -368,7 +366,7 @@ static int alt_odb_usable(struct strbuf *path, const char 
*normalized_objdir)
 * Prevent the common mistake of listing the same
 * thing twice, or object directory itself.
 */
-   for (alt = alt_odb_list; alt; alt = alt->next) {
+   for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
if (!fspathcmp(path->buf, alt->path))
return 0;
}
@@ -428,8 +426,8 @@ static int link_alt_odb_entry(const char *entry, const char 
*relative_base,
ent = alloc_alt_odb(pathbuf.buf);
 
/* add the alternate entry */
-   *alt_odb_tail = ent;
-   alt_odb_tail = &(ent->next);
+   *the_repository->objects.alt_odb_tail = ent;
+   the_repository->objects.alt_odb_tail = &(ent->next);
ent->next = NULL;
 
/* recursively add alternates */
@@ -563,7 +561,7 @@ void add_to_alternates_file(const char *reference)
fprintf_or_die(out, "%s\n", reference);
if (commit_lock_file())
die_errno("unable to move new alternates file into 

[PATCH 003/194] object-store: move packed_git and packed_git_mru to object store

[Stefan Beller]

From: Jonathan Nieder 

In a process with multiple repositories open, packfile accessors
should be associated to a single repository and not shared globally.
Move packed_git and packed_git_mru into the_repository and adjust
callers to reflect this.

Patch generated by

 1. Moving the struct packed_git declaration to object-store.h
and packed_git, packed_git_mru globals to struct object_store.

 2. Applying the semantic patch contrib/coccinelle/packed_git.cocci
to adjust callers.

 3. Applying line wrapping fixes from "make style" to break the
resulting long lines.

 4. Adding missing #includes of repository.h and object-store.h
where needed.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 builtin/count-objects.c |  6 --
 builtin/fsck.c  |  7 +--
 builtin/gc.c|  4 +++-
 builtin/index-pack.c|  1 +
 builtin/pack-objects.c  | 21 
 builtin/pack-redundant.c|  6 --
 builtin/receive-pack.c  |  1 +
 cache.h | 28 --
 contrib/coccinelle/packed_git.cocci |  7 +++
 fast-import.c   |  6 --
 http-backend.c  |  6 --
 http-push.c |  1 +
 http-walker.c   |  1 +
 http.c  |  1 +
 mru.h   |  1 +
 object-store.h  | 33 ++-
 pack-bitmap.c   |  4 +++-
 pack-check.c|  1 +
 pack-revindex.c |  1 +
 packfile.c  | 39 +++--
 reachable.c |  1 +
 server-info.c   |  6 --
 sha1_name.c |  5 +++--
 23 files changed, 115 insertions(+), 72 deletions(-)
 create mode 100644 contrib/coccinelle/packed_git.cocci

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 33343818c8..9334648dcc 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -7,6 +7,8 @@
 #include "cache.h"
 #include "config.h"
 #include "dir.h"
+#include "repository.h"
+#include "object-store.h"
 #include "builtin.h"
 #include "parse-options.h"
 #include "quote.h"
@@ -120,9 +122,9 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
-   if (!packed_git)
+   if (!the_repository->objects.packed_git)
prepare_packed_git();
-   for (p = packed_git; p; p = p->next) {
+   for (p = the_repository->objects.packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
if (open_pack_index(p))
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1048255da1..d4d249c2ed 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "repository.h"
 #include "config.h"
+#include "object-store.h"
 #include "commit.h"
 #include "tree.h"
 #include "blob.h"
@@ -707,7 +708,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
prepare_packed_git();
 
if (show_progress) {
-   for (p = packed_git; p; p = p->next) {
+   for (p = the_repository->objects.packed_git; p;
+p = p->next) {
if (open_pack_index(p))
continue;
total += p->num_objects;
@@ -715,7 +717,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
 
progress = start_progress(_("Checking 
objects"), total);
}
-   for (p = packed_git; p; p = p->next) {
+   for (p = the_repository->objects.packed_git; p;
+p = p->next) {
/* verify gives error messages itself */
if (verify_pack(p, fsck_obj_buffer,
progress, count))
diff --git a/builtin/gc.c b/builtin/gc.c
index 3c5eae0edf..6970f325e5 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -11,6 +11,7 @@
  */
 
 #include "builtin.h"
+#include "repository.h"
 #include "config.h"
 #include "tempfile.h"
 #include "lockfile.h"
@@ -18,6 +19,7 @@
 #include "run-command.h"
 #include "sigchain.h"
 #include "argv-array.h"
+#include "object-store.h"
 #include "commit.h"
 #include "packfile.h"
 
@@ -173,7 +175,7 @@ static int 

[PATCH 001/194] repository: introduce object store field

[Stefan Beller]

The object store field will contain any objects needed for access to
objects in a given repository.

This patch introduces the object store but for now it is empty.  C99
forbids empty structs, but common C compilers cope well with them and
this struct will gain members very soon (starting with the next
patch).

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
---
 object-store.h | 8 
 repository.c   | 4 +++-
 repository.h   | 7 +++
 3 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 object-store.h

diff --git a/object-store.h b/object-store.h
new file mode 100644
index 00..05722cdde0
--- /dev/null
+++ b/object-store.h
@@ -0,0 +1,8 @@
+#ifndef OBJECT_STORE_H
+#define OBJECT_STORE_H
+
+struct object_store {
+};
+#define OBJECT_STORE_INIT {}
+
+#endif /* OBJECT_STORE_H */
diff --git a/repository.c b/repository.c
index 998413b8bb..0ec9648f53 100644
--- a/repository.c
+++ b/repository.c
@@ -1,11 +1,13 @@
 #include "cache.h"
 #include "repository.h"
+#include "object-store.h"
 #include "config.h"
 #include "submodule-config.h"
 
 /* The main repository */
 static struct repository the_repo = {
-   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
0, 0
+   NULL, NULL, NULL, OBJECT_STORE_INIT,
+   NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 0, 0
 };
 struct repository *the_repository = _repo;
 
diff --git a/repository.h b/repository.h
index 0329e40c7f..ba7b3b7cb9 100644
--- a/repository.h
+++ b/repository.h
@@ -1,6 +1,8 @@
 #ifndef REPOSITORY_H
 #define REPOSITORY_H
 
+#include "object-store.h"
+
 struct config_set;
 struct index_state;
 struct submodule_cache;
@@ -26,6 +28,11 @@ struct repository {
 */
char *objectdir;
 
+   /*
+* Holds any information related to the object store.
+*/
+   struct object_store objects;
+
/*
 * Path to the repository's graft file.
 * Cannot be NULL after initialization.
-- 
2.15.1.433.g936d1b9894.dirty

[RFH/PATCH] blame: tighten command line parser

[Junio C Hamano]

The command line parser of "git blame" is prepared to take an
ancient odd argument order "blame  " in addition to the
usual "blame [] ".  It has at least two negative
ramifications:

 - In order to tell these two apart, it checks if the last command
   line argument names a path in the working tree, using
   file_exists().  However, "blame  " is a request to
   explalin each and every line in the contents of  stored in
   revision  and does not need to have a working tree version
   of the file.  A check with file_exists() is simply wrong.

 - To coerce that mistaken file_exists() check to work, the code
   calls setup_work_tree() before doing so, because the path it has
   is relative to the top-level of the project tree.  However,
   "blame  " MUST be usable even in a bare repository,
   and there is no reason for letting setup_work_tree() to complain
   and die with "This operation must be run in a work tree".

To correct the former, switch to check if the last token is a
revision (and if so, parse arguments using "blame  "
rule).  Correct the latter by getting rid of setup_work_tree() and
file_exists() check--the only case the call to this function matters
is when we are running "blame " (i.e. no starting revision and
asking to blame the working tree file at , digging through the
HEAD revision), but there is a call in setup_scoreboard() just
before it calls fake_working_tree_commit().

Also attempt to give a bit more sensible error message when "blame
XYZ" is given and XYZ cannot be a path.

   side note: I am not happy with the "only one arg, which is a rev,
   given in a bare repository" condition to give the new error
   message, but this should be a good starting point.

Signed-off-by: Junio C Hamano 
---
 * Somebody noticed that this transcript looked funny.

   $ cd .git
   $ git blame HEAD
   fatal: This operation must be run in a work tree
   
   If it were a request for blaming Makefile instead, this error
   message may make some sense, though.

   By the way, I am not happy with is_a_rev() either.  There should
   be a handy helper function (or two) we already have that I am
   forgetting.

 builtin/blame.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 005f55aaa2..9dcb367b90 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -649,6 +649,15 @@ static int blame_move_callback(const struct option 
*option, const char *arg, int
return 0;
 }
 
+static int is_a_rev(const char *name)
+{
+   struct object_id oid;
+
+   if (get_oid(name, ))
+   return 0;
+   return OBJ_NONE < sha1_object_info(oid.hash, NULL);
+}
+
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
struct rev_info revs;
@@ -845,16 +854,15 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
} else {
if (argc < 2)
usage_with_options(blame_opt_usage, options);
-   path = add_prefix(prefix, argv[argc - 1]);
-   if (argc == 3 && !file_exists(path)) { /* (2b) */
+   if (argc == 3 && is_a_rev(argv[argc - 1])) { /* (2b) */
path = add_prefix(prefix, argv[1]);
argv[1] = argv[2];
+   } else {/* (2a) */
+   if (argc == 2 && is_a_rev(argv[1]) && 
!get_git_work_tree())
+   die("missing  to blame");
+   path = add_prefix(prefix, argv[argc - 1]);
}
argv[argc - 1] = "--";
-
-   setup_work_tree();
-   if (!file_exists(path))
-   die_errno("cannot stat path '%s'", path);
}
 
revs.disable_stdin = 1;
-- 
2.16.1-72-g5be1f00a9a

Re: [PATCH v7 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases

[Elijah Newren]

On Mon, Feb 5, 2018 at 1:52 PM, Stefan Beller  wrote:
> On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:

>> +   /*
>> +* Stupid double negatives in remove_file; it somehow manages
>> +* to repeatedly mess me up.  So, just for myself:
>> +*1) update_wd iff !ren_src_was_dirty.
>> +*2) no_wd iff !update_wd
>> +*3) so, no_wd == !!ren_src_was_dirty == 
>> ren_src_was_dirty
>> +*/
>
> Not sure iff this comment is at the right place and is a good addition to
> the code base.

Fair enough, and I should apologize for letting my frustration come
through there.  However, what if I replaced the first two lines of the
comment with:

"Because the double negatives somehow keep confusing me..."

so that it reads:
/*
 * Because the double negatives somehow keep confusing me...
 *1) update_wd iff !ren_src_was_dirty.
 *2) no_wd iff !update_wd
 *3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
 */

Even if my wording was suboptimal, the rest of the comment did seem
pretty important because I messed up the line after the comment
multiple times.  (You'd think that the odds of getting it right should
be 50/50 and that a simple inversion would fix it, so one could only
mess the line up once, but I'm apparently special).  And then I came
back to look at it later and was still confused.  For some reason, I
seem to need the longer explanation.

> However it hints at the underlying issue of a bad API that is provided
> by remove_file ?

I'd definitely agree with that.

Re: cherry-pick '-m' curiosity

[Stefan Beller]

On Mon, Feb 5, 2018 at 3:46 AM, Sergey Organov  wrote:
> Hello,
>
> $ git help cherry-pick
>
> -m parent-number, --mainline parent-number
>Usually you cannot cherry-pick a merge because you do not
>know which side of the merge should be considered the
>mainline.
>
> Isn't it always the case that "mainline" is the first parent, as that's
> how "git merge" happens to work?
>
> Is, say, "-m 2" ever useful?

Say you want to backport everything except that topic using cherry-picks.
Then -m2 would be useful?

Re: [PATCH v7 31/31] merge-recursive: ensure we write updates for directory-renamed file

[Stefan Beller]

On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:
> When a file is present in HEAD before the merge and the other side of the
> merge does not modify that file, we try to avoid re-writing the file and
> making it stat-dirty.  However, when a file is present in HEAD before the
> merge and was in a directory that was renamed by the other side of the
> merge, we have to move the file to a new location and re-write it.
> Update the code that checks whether we can skip the update to also work in
> the presence of directory renames.
>
> Signed-off-by: Elijah Newren 
> ---

Now I did not just review the tests, but also the code of this series.
(though I think I was more detail oriented on Friday)
All patches that had no comments are fine with me,
I left some comments on others.

Thanks,
Stefan

Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache

[Brandon Williams]

On 02/05, Ben Peart wrote:
> The untracked cache saves its current state in the UNTR index extension.
> Currently, _any_ change to that state causes the index to be flagged as dirty
> and written out to disk.  Unfortunately, the cost to write out the index can
> exceed the savings gained by using the untracked cache.  Since it is a cache
> that can be updated from the current state of the working directory, there is
> no functional requirement that the index be written out for every change to 
> the
> untracked cache.
> 
> Update the untracked cache logic so that it no longer forces the index to be
> written to disk except in the case where the extension is being turned on or
> off.  When some other git command requires the index to be written to disk, 
> the
> untracked cache will take advantage of that to save it's updated state as 
> well.
> This results in a performance win when looked at over common sequences of git
> commands (ie such as a status followed by add, commit, etc).
> 
> After this patch, all the logic to track statistics for the untracked cache
> could be removed as it is only used by debug tracing used to debug the 
> untracked
> cache.

So we don't need to update it every time because its just a cache
and if its inaccurate between status calls that's ok?  So only
operations like add and commit will actually write out the untracked
cache (as a part of writing out the index).  Sounds ok.

What benefit is there to using the untracked cache then?  Sounds like
you should just turn it off instead?
(I'm sure this is a naive question :D )

> 
> Signed-off-by: Ben Peart 
> ---
> 
> Notes:
> Base Ref: master
> Web-Diff: https://github.com/benpeart/git/commit/20c2e8d787
> Checkout: git fetch https://github.com/benpeart/git untracked-cache-v1 && 
> git checkout 20c2e8d787
> 
>  dir.c | 3 ++-
>  t/t7063-status-untracked-cache.sh | 3 +++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/dir.c b/dir.c
> index 7c4b45e30e..da93374f0c 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2297,7 +2297,8 @@ int read_directory(struct dir_struct *dir, struct 
> index_state *istate,
>dir->untracked->gitignore_invalidated,
>dir->untracked->dir_invalidated,
>dir->untracked->dir_opened);
> - if (dir->untracked == istate->untracked &&
> + if (getenv("GIT_TEST_UNTRACKED_CACHE") &&
> + dir->untracked == istate->untracked &&
>   (dir->untracked->dir_opened ||
>dir->untracked->gitignore_invalidated ||
>dir->untracked->dir_invalidated))
> diff --git a/t/t7063-status-untracked-cache.sh 
> b/t/t7063-status-untracked-cache.sh
> index e5fb892f95..6ef520e823 100755
> --- a/t/t7063-status-untracked-cache.sh
> +++ b/t/t7063-status-untracked-cache.sh
> @@ -14,6 +14,9 @@ test_description='test untracked cache'
>  # See <20160803174522.5571-1-pclo...@gmail.com> if you want to know
>  # more.
>  
> +GIT_TEST_UNTRACKED_CACHE=true
> +export GIT_TEST_UNTRACKED_CACHE
> +
>  sync_mtime () {
>   find . -type d -ls >/dev/null
>  }
> 
> base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
> -- 
> 2.15.0.windows.1
> 

-- 
Brandon Williams

Re: [PATCH v7 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases

[Stefan Beller]

On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:
> Signed-off-by: Elijah Newren 
> ---
>  merge-recursive.c   | 26 +++---
>  t/t6043-merge-rename-directories.sh |  8 
>  2 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index fba1a0d207..62e4266d21 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1320,11 +1320,23 @@ static int handle_file(struct merge_options *o,
>
> add = filespec_from_entry(, dst_entry, stage ^ 1);
> if (add) {
> +   int ren_src_was_dirty = was_dirty(o, rename->path);
> char *add_name = unique_path(o, rename->path, other_branch);
> if (update_file(o, 0, >oid, add->mode, add_name))
> return -1;
>
> -   remove_file(o, 0, rename->path, 0);
> +   if (ren_src_was_dirty) {
> +   output(o, 1, _("Refusing to lose dirty file at %s"),
> +  rename->path);
> +   }
> +   /*
> +* Stupid double negatives in remove_file; it somehow manages
> +* to repeatedly mess me up.  So, just for myself:
> +*1) update_wd iff !ren_src_was_dirty.
> +*2) no_wd iff !update_wd
> +*3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
> +*/

Not sure iff this comment is at the right place and is a good addition to
the code base.

However it hints at the underlying issue of a bad API that is provided
by remove_file ?

Re: [GSoC][PATCH] commit: add a commit.signOff config variable

[Stefan Beller]

On Sat, Feb 3, 2018 at 6:03 PM, Chen Jingpiao  wrote:
> Add the commit.signOff configuration variable to use the -s or --signoff
> option of git commit by default.
>
> Signed-off-by: Chen Jingpiao 
> ---

Welcome to the Git community!

>
> Though we can configure signoff using format.signOff variable. Someone like to
> add Signed-off-by line by the committer.

There is more discussion about this at
https://public-inbox.org/git/1482946838-28779-1-git-send-email-ehabk...@redhat.com/
specifically
https://public-inbox.org/git/xmqqtw9m5s5m@gitster.mtv.corp.google.com/

Not sure if there was any other reasons and discussions brought up
since then, but that discussion seems to not favor patches that
add .signoff options.

Thanks,
Stefan

>
>  Documentation/config.txt |  4 +++
>  Documentation/git-commit.txt |  2 ++
>  builtin/commit.c |  4 +++
>  t/t7501-commit.sh| 69 
> 
>  4 files changed, 79 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 0e25b2c92..5dec3f0cb 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1303,6 +1303,10 @@ commit.gpgSign::
> convenient to use an agent to avoid typing your GPG passphrase
> several times.
>
> +commit.signOff::
> +   A boolean value which lets you enable the `-s/--signoff` option of
> +   `git commit` by default. See linkgit:git-commit[1].
> +
>  commit.status::
> A boolean to enable/disable inclusion of status information in the
> commit message template when using an editor to prepare the commit
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index f970a4342..7a28ea765 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -166,6 +166,8 @@ The `-m` option is mutually exclusive with `-c`, `-C`, 
> and `-F`.
> the rights to submit this work under the same license and
> agrees to a Developer Certificate of Origin
> (see http://developercertificate.org/ for more information).
> +   See the `commit.signOff` configuration variable in
> +   linkgit:git-config[1].
>
>  -n::
>  --no-verify::
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 4610e3d8e..324213254 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1548,6 +1548,10 @@ static int git_commit_config(const char *k, const char 
> *v, void *cb)
> sign_commit = git_config_bool(k, v) ? "" : NULL;
> return 0;
> }
> +   if (!strcmp(k, "commit.signoff")) {
> +   signoff = git_config_bool(k, v);
> +   return 0;
> +   }
> if (!strcmp(k, "commit.verbose")) {
> int is_bool;
> config_commit_verbose = git_config_bool_or_int(k, v, 
> _bool);
> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
> index fa61b1a4e..46733ed2a 100755
> --- a/t/t7501-commit.sh
> +++ b/t/t7501-commit.sh
> @@ -505,6 +505,75 @@ Myfooter: x" &&
> test_cmp expected actual
>  '
>
> +test_expect_success "commit.signoff=true and --signoff omitted" '
> +   echo 7 >positive &&
> +   git add positive &&
> +   git -c commit.signoff=true commit -m "thank you" &&
> +   git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> +   (
> +   echo thank you
> +   echo
> +   git var GIT_COMMITTER_IDENT |
> +   sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> +   ) >expected &&
> +   test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=true and --signoff" '
> +   echo 8 >positive &&
> +   git add positive &&
> +   git -c commit.signoff=true commit --signoff -m "thank you" &&
> +   git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> +   (
> +   echo thank you
> +   echo
> +   git var GIT_COMMITTER_IDENT |
> +   sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> +   ) >expected &&
> +   test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=true and --no-signoff" '
> +   echo 9 >positive &&
> +   git add positive &&
> +   git -c commit.signoff=true commit --no-signoff -m "thank you" &&
> +   git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> +   echo thank you >expected &&
> +   test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --signoff omitted" '
> +   echo 10 >positive &&
> +   git add positive &&
> +   git -c commit.signoff=false commit -m "thank you" &&
> +   git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> +   echo thank you >expected &&
> +   test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --signoff" '
> +   echo 11 >positive &&
> +   git add positive &&
> +   git -c commit.signoff=false 

Re: [PATCH v7 17/31] merge-recursive: add a new hashmap for storing directory renames

[Elijah Newren]

On Mon, Feb 5, 2018 at 11:44 AM, Stefan Beller  wrote:
>>> Having a stringlist of potentially new dirs sounds like the algorithm is
>>> at least n^2, but how do I know? I'll read on.
>>
>> Yes, I suppose it's technically n^2, but n is expected to be O(1).
>> While one can trivially construct a case making n arbitrarily large,
>> statistically for real world repositories, I expected the mode of n to
>> be 1 and the mean to be less than 2.  My original idea was to use a
>> hash for possible_new_dirs, but since hashes are so painful in C and n
>> should be very small anyway, I didn't bother.  If anyone can find an
>> example of a real world open source repository (linux, webkit, git,
>> etc.) with a merge where n is greater than about 10, I'll be
>> surprised.
>>
>> Does that address your concern, or does it sound like I'm kicking the
>> can down the road?  If it's the latter, we can switch it out.
>
> I think that is fine for now; the real world usage matters more
> than the big O notation. But maybe you want to hint at the possibility of
> speedup (in the commit message or in code?), once someone produces
> a slow case and digs up the code.

Sounds like a good idea; I'll add a comment about this issue to the
commit message.

Re: [PATCH v7 27/31] merge-recursive: fix overwriting dirty files involved in renames

[Elijah Newren]

On Mon, Feb 5, 2018 at 12:52 PM, Stefan Beller  wrote:
> On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:
>> This fixes an issue that existed before my directory rename detection
>> patches that affects both normal renames and renames implied by
>> directory rename detection.  Additional codepaths that only affect
>> overwriting of directy files that are involved in directory rename

Ugh, "dirty" not "directy".  I must have gotten my fingers trained to
type "directory" too much.  I'll fix that up.

>> detection will be added in a subsequent commit.
>>
>> Signed-off-by: Elijah Newren 
>
> So this fixes bugs in the current rename detection with
> overwriting untracked files? Then this is an additional
> selling point of this series, maybe worth covering in the
> cover letter!

Yes, with a nitpick: the existing issue it fixes is with dirty files
(by which I mean uncommitted changes to tracked files) involved in
renames rather than being an issue with untracked files.

I did mention this fix in my original cover letter[1], but it would
have been really easy to miss because it was a really long cover
letter, and the mention came at the very end.  Quoting from it:

"""
These last three deal with untracked and dirty file overwriting
headaches.  The middle patch in particular, isn't just a fix for
directory rename detection but fixes a bug in current versions of git
in overwriting dirty files that are involved in a rename.  That patch
could be backported and submitted independent of this series, but the
final patch depends heavily on it.
"""

[1] https://public-inbox.org/git/20171110190550.27059-1-new...@gmail.com/

Re: [PATCH RFC v2 14/25] ref-filter: add is_cat flag

[Junio C Hamano]

Olga Telezhnaya  writes:

> diff --git a/ref-filter.c b/ref-filter.c
> index 34a54db168265..91290b62450b3 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -101,6 +101,7 @@ static struct used_atom {
>  } *used_atom;
>  static int used_atom_cnt, need_tagged, need_symref;
>  struct expand_data *cat_file_info;
> +static int is_cat = 0;
>   ...
> @@ -739,6 +740,7 @@ int verify_ref_format(struct ref_format *format)
>   const char *cp, *sp;
>  
>   cat_file_info = format->cat_file_data;
> + is_cat = format->is_cat;

Eek.  The global cat_file_info itself may already be bad enough, but
now we have another one?

Hopefully these are all cleaned up in later steps in the series?

>   format->need_color_reset_at_eol = 0;
>   for (cp = format->format; *cp && (sp = find_next(cp)); ) {
>   const char *color, *ep = strchr(sp, ')');
> @@ -748,7 +750,7 @@ int verify_ref_format(struct ref_format *format)
>   return error(_("malformed format string %s"), sp);
>   /* sp points at "%(" and ep points at the closing ")" */
>  
> - if (format->cat_file_data)
> + if (is_cat)
>   at = parse_ref_filter_atom(format, valid_cat_file_atom,
>  
> ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
>   else {
> @@ -1438,7 +1440,7 @@ int populate_value(struct ref_array_item *ref)
>   ref->symref = "";
>   }
>  
> - if (cat_file_info && check_and_fill_for_cat(ref))
> + if (is_cat && check_and_fill_for_cat(ref))
>   return -1;
>  
>   /* Fill in specials first */
> diff --git a/ref-filter.h b/ref-filter.h
> index 5c6e019998716..69271e8c39f40 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -125,6 +125,7 @@ struct ref_format {
>* hopefully would be reduced later.
>*/
>   struct expand_data *cat_file_data;
> + int is_cat;
>  };
>  
>  #define REF_FORMAT_INIT { NULL, 0, -1 }
>
> --
> https://github.com/git/git/pull/452

Re: [PATCH v7 20/31] merge-recursive: check for directory level conflicts

[Elijah Newren]

On Mon, Feb 5, 2018 at 12:00 PM, Stefan Beller  wrote:
> On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:
>> Before trying to apply directory renames to paths within the given
>> directories, we want to make sure that there aren't conflicts at the
>> directory level.  There will be additional checks at the individual
>> file level too, which will be added later.
>
>
>> +static int tree_has_path(struct tree *tree, const char *path)
>> +{
>> +   unsigned char hashy[20];
>
> What is 20? ;)
> I think you want to use GIT_MAX_RAWSZ instead.

Indeed, thanks!

Re: [PATCH v2 07/14] commit-graph: implement git-commit-graph --update-head

[Derrick Stolee]

On 2/1/2018 8:35 PM, SZEDER Gábor wrote:

It is possible to have multiple commit graph files in a pack directory,
but only one is important at a time. Use a 'graph_head' file to point
to the important file.

This implies that all those other files are ignored, right?


Yes. We do not use directory listings to find graph files.




Teach git-commit-graph to write 'graph_head' upon
writing a new commit graph file.

Signed-off-by: Derrick Stolee 
---
  Documentation/git-commit-graph.txt | 34 ++
  builtin/commit-graph.c | 38 +++---
  commit-graph.c | 25 +
  commit-graph.h |  2 ++
  t/t5318-commit-graph.sh| 12 ++--
  5 files changed, 106 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index 09aeaf6c82..99ced16ddc 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -12,15 +12,49 @@ SYNOPSIS
  'git commit-graph' --write  [--pack-dir ]
  'git commit-graph' --read  [--pack-dir ]
  
+OPTIONS

+---

Oh, look, the 'OPTIONS' section I missed in the previous patches! ;)

This should be split up and squashed into the previous patches where
the individual --options are first mentioned.


+--pack-dir::
+   Use given directory for the location of packfiles, graph-head,
+   and graph files.
+
+--read::
+   Read a graph file given by the graph-head file and output basic
+   details about the graph file. (Cannot be combined with --write.)

 From the output of 'git commit-graph --read' it seems that it's not a
generally useful option to the user.  Perhaps it should be mentioned
that it's only intended as a debugging aid?  Or maybe it doesn't
really matter, because eventually this command will become irrelevant,
as other commands (clone, fetch, gc) will invoke it automagically...


I'll add some wording to make this clear.




+--graph-id::
+   When used with --read, consider the graph file graph-.graph.
+
+--write::
+   Write a new graph file to the pack directory. (Cannot be combined
+   with --read.)

I think this should also mention that it prints the generated graph
file's checksum.


+
+--update-head::
+   When used with --write, update the graph-head file to point to
+   the written graph file.

So it should be used with '--write', noted.


  EXAMPLES
  
  
+* Output the hash of the graph file pointed to by /graph-head.

++
+
+$ git commit-graph --pack-dir=
+
+
  * Write a commit graph file for the packed commits in your local .git folder.
  +
  
  $ git commit-graph --write
  
  
+* Write a graph file for the packed commits in your local .git folder,

+* and update graph-head.
++
+
+$ git commit-graph --write --update-head
+
+
  * Read basic information from a graph file.
  +
  
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 218740b1f8..d73cbc907d 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -11,7 +11,7 @@
  static char const * const builtin_commit_graph_usage[] = {
N_("git commit-graph [--pack-dir ]"),
N_("git commit-graph --read [--graph-hash=]"),
-   N_("git commit-graph --write [--pack-dir ]"),
+   N_("git commit-graph --write [--pack-dir ] [--update-head]"),
NULL
  };
  
@@ -20,6 +20,9 @@ static struct opts_commit_graph {

int read;
const char *graph_hash;
int write;
+   int update_head;
+   int has_existing;
+   struct object_id old_graph_hash;
  } opts;
  
  static int graph_read(void)

@@ -30,8 +33,8 @@ static int graph_read(void)
  
  	if (opts.graph_hash && strlen(opts.graph_hash) == GIT_MAX_HEXSZ)

get_oid_hex(opts.graph_hash, _hash);
-   else
-   die("no graph hash specified");
+   else if (!get_graph_head_hash(opts.pack_dir, _hash))
+   die("no graph-head exists");
  
  	graph_file = get_commit_graph_filename_hash(opts.pack_dir, _hash);

graph = load_commit_graph_one(graph_file, opts.pack_dir);
@@ -62,10 +65,33 @@ static int graph_read(void)
return 0;
  }
  
+static void update_head_file(const char *pack_dir, const struct object_id *graph_hash)

+{
+   struct strbuf head_path = STRBUF_INIT;
+   int fd;
+   struct lock_file lk = LOCK_INIT;
+
+   strbuf_addstr(_path, pack_dir);
+   strbuf_addstr(_path, "/");
+   strbuf_addstr(_path, "graph-head");

strbuf_addstr(_path, "/graph-head"); ?


+
+   fd = hold_lock_file_for_update(, head_path.buf, 

Re: [PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache

[Junio C Hamano]

Ben Peart  writes:

> The untracked cache saves its current state in the UNTR index extension.
> Currently, _any_ change to that state causes the index to be flagged as dirty
> and written out to disk.  Unfortunately, the cost to write out the index can
> exceed the savings gained by using the untracked cache.  Since it is a cache
> that can be updated from the current state of the working directory, there is
> no functional requirement that the index be written out for every change to 
> the
> untracked cache.
>
> Update the untracked cache logic so that it no longer forces the index to be
> written to disk except in the case where the extension is being turned on or
> off.  When some other git command requires the index to be written to disk, 
> the
> untracked cache will take advantage of that to save it's updated state as 
> well.
> This results in a performance win when looked at over common sequences of git
> commands (ie such as a status followed by add, commit, etc).
>
> After this patch, all the logic to track statistics for the untracked cache
> could be removed as it is only used by debug tracing used to debug the 
> untracked
> cache.
>
> Signed-off-by: Ben Peart 
> ---
>

OK, so in other words (note: not a suggestion to use different
wording in the log message; just making sure I got the motivation
behind this change correctly), without this new environment
variable, changes to untracked cache alone (due to observed changes
in the filesystem) does not count as "in-core index changed so we
need to write it back to the disk".

That makes sense to me.

Is it envisioned that we want to have similar but different "testing
only" behaviour around this area?  If not, this environment variable
sounds more like "force-flush untracked cache", not "test untracked
cache", to me.

> +GIT_TEST_UNTRACKED_CACHE=true
> +export GIT_TEST_UNTRACKED_CACHE
> +
>  sync_mtime () {
>   find . -type d -ls >/dev/null
>  }
>
> base-commit: 5be1f00a9a701532232f57958efab4be8c959a29

Re: [PATCH v7 27/31] merge-recursive: fix overwriting dirty files involved in renames

[Stefan Beller]

On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:
> This fixes an issue that existed before my directory rename detection
> patches that affects both normal renames and renames implied by
> directory rename detection.  Additional codepaths that only affect
> overwriting of directy files that are involved in directory rename
> detection will be added in a subsequent commit.
>
> Signed-off-by: Elijah Newren 

So this fixes bugs in the current rename detection with
overwriting untracked files? Then this is an additional
selling point of this series, maybe worth covering in the
cover letter!

Re: git: CVE-2018-1000021: client prints server sent ANSI escape codes to the terminal, allowing for unverified messages to potentially execute arbitrary commands

[Jonathan Nieder]

+cc: upstream
Hi,

Salvatore Bonaccorso wrote[1]:

> the following vulnerability was published for git.
>
> CVE-2018-121[0]:
> |client prints server sent ANSI escape codes to the terminal, allowing
> |for unverified messages to potentially execute arbitrary commands
>
> Creating this bug to track the issue in the BTS. Apparently the CVE
> was sssigned without notifying/discussing it with upstream, at least
> according to [1].
>
> If you fix the vulnerability please also make sure to include the
> CVE (Common Vulnerabilities & Exposures) id in your changelog entry.
>
> For further information see:
>
> [0] https://security-tracker.debian.org/tracker/CVE-2018-121
> https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-121
> [1] https://bugzilla.novell.com/show_bug.cgi?id=1079389#c1

Thanks.  Upstream was notified about this and we dropped the ball on
passing it on to a more public forum.  Sorry about that.

I'd be interested in your advice on this.  There are cases where the
user may *want* ANSI escape codes to be passed through without change
and other cases where the user doesn't want that.  Commands like "git
diff" pass their output through a pager by default, which itself may
or may not sanitize the output.

In other words, there are multiple components at play:

 1. A terminal.  IMHO, it is completely inexcusable these days for a
terminal to allow arbitrary code execution by writing output to
it.  If bugs of that kind still exist, I think we should fix them
(and perhaps even make it a requirement in Debian policy to make
the expectations clear for new terminals).

That said, for defense in depth, it can be useful to also guard
against this kind of issue in other components.  In particular:

 2. A pager.  Are there clear guidelines for what it is safe and not
safe for a pager to write to a terminal?

"less -R" tries to only allow ANSI "color" escape sequences
through but I wouldn't be surprised if there are some cases it
misses.

 3. Output formats.  Some git commands are designed for scripting
and do not have a sensible way to sanitize their output without
breaking scripts.  Fortunately, in the case of "git diff", git
has a notion of a "binary patch" where everything is sanitized,
at the cost of the output being unreadable to a human (email-safe
characters but not something that a human can read at a glance).
So if we know what sequences to avoid writing to stdout, then we
can treat files with those sequences as binary.

Pointers welcome.

Thanks,
Jonathan

[1] https://bugs.debian.org/889680

Re: [PATCH v2 3/3] worktree: teach "add" to check out existing branches

[Thomas Gummerer]

On 02/05, Duy Nguyen wrote:
> On Sun, Feb 04, 2018 at 10:13:05PM +, Thomas Gummerer wrote:
> > -   if (opts->new_branch)
> > +   if (opts->checkout_existing_branch)
> > +   fprintf(stderr, _(", checking out existing branch '%s'"),
> > +   refname);
> > +   else if (opts->new_branch)
> > fprintf(stderr, _(", creating new branch '%s'"), 
> > opts->new_branch);
> 
> I wonder if "creating branch" and "checkout out branch" are enough.

I thought printing the branch name might be a good idea just to show
more clearly what the dwim did.  Especially if we ever introduce new
heuristics, for example to cater for the case you mentioned in an
earlier email [1], I thought it might be helpful to also print the
branch name.  Especially because it makes it clear that the dwim the
reporter of the github issue hoped for didn't kick in at this point.

That said, I don't really think the branchname is of much use for my
usage of 'git worktree add', so I may be overestimating the need for
it.  I'm happy to remove it if that's preferred.

[1]: 

Re: [PATCH v2 3/3] worktree: teach "add" to check out existing branches

[Junio C Hamano]

Duy Nguyen  writes:

> On Sun, Feb 04, 2018 at 10:13:05PM +, Thomas Gummerer wrote:
>> -if (opts->new_branch)
>> +if (opts->checkout_existing_branch)
>> +fprintf(stderr, _(", checking out existing branch '%s'"),
>> +refname);
>> +else if (opts->new_branch)
>>  fprintf(stderr, _(", creating new branch '%s'"), 
>> opts->new_branch);
>
> I wonder if "creating branch" and "checkout out branch" are enough.

Yeah, I think new/existing are redundant.  If these were shown at
the same time, then the extra adjectives would help differenciating
them visually, but they are never shown at the same time, so...

Are concurrent git operations on the same repo safe?

[Ian Norton]

Hi all,

I'm generally used to the idea that if a thing is not marked as
"thread-safe" then it isn't thread safe, but I thought I'd ask anyway
to be sure.

Is it safe for me to do several operations with git concurrently on
the same local repo? Specifically I'm trying to speed up "git
submodule update" by doing several at the same time.  I've noticed
some odd side effects afterwards though when trying to commit changes
to my super project.

Apologies if this is answered elsewhere, my google-foo is weak today.

Many thanks

Ian

Re: [PATCH v2 1/3] worktree: improve message when creating a new worktree

[Junio C Hamano]

Duy Nguyen  writes:

> On Sun, Feb 04, 2018 at 10:13:03PM +, Thomas Gummerer wrote:
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index 7cef5b120b..d1549e441d 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -303,7 +303,7 @@ static int add_worktree(const char *path, const char 
>> *refname,
>>  strbuf_addf(, "%s/commondir", sb_repo.buf);
>>  write_file(sb.buf, "../..");
>>  
>> -fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
>> +fprintf(stderr, _("Preparing %s (identifier %s)"), path, name);
>>  
>>  argv_array_pushf(_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
>>  argv_array_pushf(_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
>> @@ -320,10 +320,19 @@ static int add_worktree(const char *path, const char 
>> *refname,
>>  if (ret)
>>  goto done;
>>  
>> +fprintf(stderr, _(", setting HEAD to %s"),
>
> As a former translator, I'm not thrilled to see a sentence broken into
> two pieces like this. I'm not a Japanese translator, but I think this
> sentence is translated differently when the translator sees the whole
> line "Preparing ..., setting ...".
>
> Since the code between the first fprintf and this one should not take
> long to execute, perhaps we can just delete the first printf and print
> a whole [*] sentence here?

Yeah, also it is a bit troubling that "Preparing" is an incomplete line,
and when update-ref/symblic-ref call fails, will stay to be incomplete.

> I think the purpose of "Preparing..." in the first place is to show
> something when git is busy checkout out the worktree. As long as we
> print it before git-reset, we should be good.
>
>> +find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
>> +
>> +strbuf_reset();
>> +pp_commit_easy(CMIT_FMT_ONELINE, commit, );
>> +if (sb.len > 0)
>> +fprintf(stderr, " %s", sb.buf);
>
> [*] Yes I know it's not "whole" because of this piece. But this one is
> more or less a separate sentence already so it's probably ok.

Doing all the additional fprintf/fputc at the same place (instead of
across update-ref/symbolic-ref) would make it easier to follow and
also allow it to make "whole" even with that piece.  Prepare the
abbreviated commit->object.oid.hash and FMT_ONELINE in the same
strbuf and say that the HEAD is there.  I think it also makes sense
to split it into two sentences, so from that point of view, just
dropping the change in the "Preparing" hunk and then saying "HEAD is
now at '9876fdea title of the commit'" after update/symbolic-ref may
also be a good change.  That automatically removes the "what hapepns
to the rest of the incomplete line when run_command() fails?" issue.

> Is it a bit too long to print everything in one line though?
> CMIT_FMT_ONELINE could already fill 70 columns easily.
>
>> +fputc('\n', stderr);
>> +
>>  if (opts->checkout) {
>>  cp.argv = NULL;
>>  argv_array_clear();
>> -argv_array_pushl(, "reset", "--hard", NULL);
>> +argv_array_pushl(, "reset", "--hard", "--quiet", NULL);
>>  cp.env = child_env.argv;
>>  ret = run_command();
>>  if (ret)
>> -- 
>> 2.16.1.101.gde0f0111ea
>> 

Re: [PATCH v2 1/3] worktree: improve message when creating a new worktree

[Thomas Gummerer]

On 02/05, Duy Nguyen wrote:
> On Sun, Feb 04, 2018 at 10:13:03PM +, Thomas Gummerer wrote:
> > diff --git a/builtin/worktree.c b/builtin/worktree.c
> > index 7cef5b120b..d1549e441d 100644
> > --- a/builtin/worktree.c
> > +++ b/builtin/worktree.c
> > @@ -303,7 +303,7 @@ static int add_worktree(const char *path, const char 
> > *refname,
> > strbuf_addf(, "%s/commondir", sb_repo.buf);
> > write_file(sb.buf, "../..");
> >  
> > -   fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
> > +   fprintf(stderr, _("Preparing %s (identifier %s)"), path, name);
> >  
> > argv_array_pushf(_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
> > argv_array_pushf(_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
> > @@ -320,10 +320,19 @@ static int add_worktree(const char *path, const char 
> > *refname,
> > if (ret)
> > goto done;
> >  
> > +   fprintf(stderr, _(", setting HEAD to %s"),
> 
> As a former translator, I'm not thrilled to see a sentence broken into
> two pieces like this. I'm not a Japanese translator, but I think this
> sentence is translated differently when the translator sees the whole
> line "Preparing ..., setting ...".

Good point, I didn't think about the sentence structure other
languages could require here. 

> Since the code between the first fprintf and this one should not take
> long to execute, perhaps we can just delete the first printf and print
> a whole [*] sentence here?
> 
> I think the purpose of "Preparing..." in the first place is to show
> something when git is busy checkout out the worktree. As long as we
> print it before git-reset, we should be good.
> 
> > +   find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
> > +
> > +   strbuf_reset();
> > +   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
> > +   if (sb.len > 0)
> > +   fprintf(stderr, " %s", sb.buf);
> 
> [*] Yes I know it's not "whole" because of this piece. But this one is
> more or less a separate sentence already so it's probably ok.
> 
> Is it a bit too long to print everything in one line though?
> CMIT_FMT_ONELINE could already fill 70 columns easily.

Yeah, it does get a bit long.  Maybe just splitting the sentences here
would be the best solution, then we also won't have to worry about the
split not being good for translation.  I'll do that.

> > +   fputc('\n', stderr);
> > +
> > if (opts->checkout) {
> > cp.argv = NULL;
> > argv_array_clear();
> > -   argv_array_pushl(, "reset", "--hard", NULL);
> > +   argv_array_pushl(, "reset", "--hard", "--quiet", NULL);
> > cp.env = child_env.argv;
> > ret = run_command();
> > if (ret)
> > -- 
> > 2.16.1.101.gde0f0111ea
> > 

Re: Missing git options

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 05 2018, Martin Häcker jotted:

> Hi there,
>
> I just recently learned that not all command line switches seem to 
> automatically correlate to options in the git configuration.
>
> This seems something that should be relatively easy to fix.
>
> What I’m most missing is
>
> — snip —
> [log]
>   graph = true
>   patch = true
> — snap —
>
> which would / should correspond to `git log —graph —patch`.
>
> What do you guys think?

FWIW my WIP
https://public-inbox.org/git/20170328131316.32516-1-ava...@gmail.com/
had some discussion around this.

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Ævar Arnfjörð Bjarmason]

On Sun, Feb 04 2018, Lucas Werkmeister jotted:

> On 04.02.2018 19:55, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Sun, Feb 04 2018, Lucas Werkmeister jotted:
>> 
>>>  [--inetd |
>>>   [--listen=] [--port=]
>>>   [--user= [--group=]]]
>>> +[--log-destination=(stderr|syslog|none)]
>> 
>> I micronit, but maybe worthwhile to have a preceeding commit to fix up
>> that indentation of --listen and --user.
>
> I thought the indentation here is intentional, since we’re still inside
> the [] pair (either --inetd or --listen, --port, …).

Yes, sorry. Nevrmind.

>> 
>>> +--log-destination=::
>>> +   Send log messages to the specified destination.
>>> +   Note that this option does not imply --verbose,
>> 
>> Should `` quote --verbose, although I see similar to the WS change I
>> noted above there's plenty of existing stuff in that doc doing it wrong.
>
> I could send a follow-up to consistently ``-quote all options in
> git-daemon.txt… or would that be rejected as unnecessarily cluttering
> the history or the `git blame`?

I don't think anyone would mind. Tiny doc cleanups are neat-o.

>>> +   } else
>>> +   die("unknown log destination '%s'", v);
>> 
>> Should be die(_("unknown..., i.e. use the _() macro.
>> 
>> Anyway, this looks fine to be with our without these proposed
>> bikeshedding changes above. Thanks.
>>

Re: [PATCH v7 21/31] merge-recursive: add a new hashmap for storing file collisions

[Stefan Beller]

On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:
> Directory renames with the ability to merge directories opens up the
> possibility of add/add/add/.../add conflicts, if each of the N
> directories being merged into one target directory all had a file with
> the same name.  We need a way to check for and report on such
> collisions; this hashmap will be used for this purpose.
>
> Signed-off-by: Elijah Newren 
> ---

The same comment on compiling applies here.
Nice to review, but it may not compile due to unused functions
that a strict compiler may not be happy about.

Re: [PATCH v7 17/31] merge-recursive: add a new hashmap for storing directory renames

[Junio C Hamano]

Johannes Sixt  writes:

> Am 03.02.2018 um 22:34 schrieb Elijah Newren:
>>  If anyone can find an
>> example of a real world open source repository (linux, webkit, git,
>> etc.) with a merge where n is greater than about 10, I'll be
>> surprised.
>
> git rev-list --parents --merges master |
>  grep " .* .* .* .* .* .* .* .* .* .* "

Twinkle twinkle little stars? ;-)

rev-list can take --min-parents=10 option for a thing like this.  In
fact, --merges is implemented in terms of that option.

Re: [PATCH v7 20/31] merge-recursive: check for directory level conflicts

[Stefan Beller]

On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren  wrote:
> Before trying to apply directory renames to paths within the given
> directories, we want to make sure that there aren't conflicts at the
> directory level.  There will be additional checks at the individual
> file level too, which will be added later.


> +static int tree_has_path(struct tree *tree, const char *path)
> +{
> +   unsigned char hashy[20];

What is 20? ;)
I think you want to use GIT_MAX_RAWSZ instead.

[PATCH v1] dir.c: don't flag the index as dirty for changes to the untracked cache

[Ben Peart]

The untracked cache saves its current state in the UNTR index extension.
Currently, _any_ change to that state causes the index to be flagged as dirty
and written out to disk.  Unfortunately, the cost to write out the index can
exceed the savings gained by using the untracked cache.  Since it is a cache
that can be updated from the current state of the working directory, there is
no functional requirement that the index be written out for every change to the
untracked cache.

Update the untracked cache logic so that it no longer forces the index to be
written to disk except in the case where the extension is being turned on or
off.  When some other git command requires the index to be written to disk, the
untracked cache will take advantage of that to save it's updated state as well.
This results in a performance win when looked at over common sequences of git
commands (ie such as a status followed by add, commit, etc).

After this patch, all the logic to track statistics for the untracked cache
could be removed as it is only used by debug tracing used to debug the untracked
cache.

Signed-off-by: Ben Peart 
---

Notes:
Base Ref: master
Web-Diff: https://github.com/benpeart/git/commit/20c2e8d787
Checkout: git fetch https://github.com/benpeart/git untracked-cache-v1 && 
git checkout 20c2e8d787

 dir.c | 3 ++-
 t/t7063-status-untracked-cache.sh | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 7c4b45e30e..da93374f0c 100644
--- a/dir.c
+++ b/dir.c
@@ -2297,7 +2297,8 @@ int read_directory(struct dir_struct *dir, struct 
index_state *istate,
 dir->untracked->gitignore_invalidated,
 dir->untracked->dir_invalidated,
 dir->untracked->dir_opened);
-   if (dir->untracked == istate->untracked &&
+   if (getenv("GIT_TEST_UNTRACKED_CACHE") &&
+   dir->untracked == istate->untracked &&
(dir->untracked->dir_opened ||
 dir->untracked->gitignore_invalidated ||
 dir->untracked->dir_invalidated))
diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index e5fb892f95..6ef520e823 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -14,6 +14,9 @@ test_description='test untracked cache'
 # See <20160803174522.5571-1-pclo...@gmail.com> if you want to know
 # more.
 
+GIT_TEST_UNTRACKED_CACHE=true
+export GIT_TEST_UNTRACKED_CACHE
+
 sync_mtime () {
find . -type d -ls >/dev/null
 }

base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
-- 
2.15.0.windows.1

Re: cherry-pick '-m' curiosity

[Junio C Hamano]

Sergey Organov  writes:

> Isn't it always the case that "mainline" is the first parent, as that's
> how "git merge" happens to work?

You may not be merging into the "mainline" in the first place.

Imagine forking two topics at the same commit on the mainline, and
merging these two topics of equal importance together into a single
bigger topic, before asking the mainline to pull the whole.

$ git checkout mainline
$ git tag fork-point
$ git checkout -b frontend-topic fork-point
$ work work work
$ git checkout -b backend-topic fork-point
$ work work work
$ git checkout -b combined
$ git merge frontend-topic
$ git tag merged

The backend-topic may be recorded as the first-parent of the
resulting merge, but logically the two topics are of equal footing,
so merge^1..merge and merge^2..merge are both equally interesting.

Re: [PATCH v7 17/31] merge-recursive: add a new hashmap for storing directory renames

[Stefan Beller]

>> Having a stringlist of potentially new dirs sounds like the algorithm is
>> at least n^2, but how do I know? I'll read on.
>
> Yes, I suppose it's technically n^2, but n is expected to be O(1).
> While one can trivially construct a case making n arbitrarily large,
> statistically for real world repositories, I expected the mode of n to
> be 1 and the mean to be less than 2.  My original idea was to use a
> hash for possible_new_dirs, but since hashes are so painful in C and n
> should be very small anyway, I didn't bother.  If anyone can find an
> example of a real world open source repository (linux, webkit, git,
> etc.) with a merge where n is greater than about 10, I'll be
> surprised.
>
> Does that address your concern, or does it sound like I'm kicking the
> can down the road?  If it's the latter, we can switch it out.

I think that is fine for now; the real world usage matters more
than the big O notation. But maybe you want to hint at the possibility of
speedup (in the commit message or in code?), once someone produces
a slow case and digs up the code.

Re: Missing git options

[Junio C Hamano]

Stefan Beller  writes:

> I had the impression that git-log was a pseudo-plumbing,
> despite it being explicitly marked porcelain
> as there is no good plumbing alternative.

I do not think that is a fair assessment of the situation.  The more
troublesome is that depending on what exactly you want to do, you
need to use a different plumbing command and/or combinations of
them, which means people need to _learn_ Git scripting.  

We recently saw that somebody used "log -1 --pretty=format:%T
$commit" when the code only needed to say "rev-parse --verify
$commit^{tree}" and got broken due to end-user configuration ;-)

Re: [PATCH v7 19/31] merge-recursive: add get_directory_renames()

[Stefan Beller]

> /*
>  * For
>  *"a/b/c/d/e/foo.c" -> "a/b/some/thing/else/e/foo.c"
>  * the "e/foo.c" part is the same, we just want to know that
>  *"a/b/c/d" was renamed to "a/b/some/thing/else"
>  * so, for this example, this function returns "a/b/c/d" in
>  * *old_dir and "a/b/some/thing/else" in *new_dir.
>  *
>  * Also, if the basename of the file changed, we don't care.  We
>  * want to know which portion of the directory, if any, changed.
>  */
>
> Is that better?

yes, that helps me understanding pretty much.

>
>>> +*
>>> +* Also, if the basename of the file changed, we don't care.  We
>>> +* want to know which portion of the directory, if any, changed.
>>> +*/
>>> +   end_of_old = strrchr(old_path, '/');
>>> +   end_of_new = strrchr(new_path, '/');
>>> +
>>> +   if (end_of_old == NULL || end_of_new == NULL)
>>> +   return;
>>
>> return early as the files are in the top level, and apparently we do
>> not support top level renaming?
>>
>> What about a commit like 81b50f3ce4 (Move 'builtin-*' into
>> a 'builtin/' subdirectory, 2010-02-22) ?
>>
>> Well that specific commit left many files outside the new builtin dir,
>> but conceptually we could see a directory rename of
>>
>> /* => /src/*
>
> We had some internal usecases for want to support a "rename" of the
> toplevel directory into a subdirectory of itself.  However, attempting
> to support that opened much too big a can of worms for me.  We'd open
> up some big surprises somewhere.


ok, I was just trying to understand the code.
(As said before, I was not asking for having this implemented.)

> In particular, note that not supporting a "rename" of the toplevel
> directory is a special case of not supporting a "rename" of any
> directory to a subdirectory below itself, which in turn is a very
> special case of excluding partial directory renames.  I addressed this
> in the cover letter of my original submission, as follows:
>
> """
> Further, there's a basic question about when directory rename detection
> should be applied at all.  I have a simple rule:
>
>   3) If a given directory still exists on both sides of a merge, we do
>  not consider it to have been renamed.
>
> Rule 3 may sound obvious at first, but it will probably arise as a
> question for some users -- what if someone "mostly" moved a directory but
> still left some files around, or, equivalently (from the perspective of the
> three-way merge that merge-recursive performs), fully renamed a directory
> in one commmit and then recreated that directory in a later commit adding
> some new files and then tried to merge?  See the big comment in section 4
> of the new t6043 for further discussion of this rule.
> """
>
> Patch 04/31 is the one that adds that big comment with further discussion.
>
> Maybe there's a way to support toplevel renames, but I think it'd make
> this series longer and more complicated...and may cause more edge
> cases that confuse users.

Thanks for reminding!

>
>>> +   while (*--end_of_new == *--end_of_old &&
>>> +  end_of_old != old_path &&
>>> +  end_of_new != new_path)
>>> +   ; /* Do nothing; all in the while loop */
>>
>> We have to compare manually as we'd want to find
>> the first non-equal and there doesn't seem to be a good
>> library function for that.
>>
>> Assuming many repos are UTF8 (including in their paths),
>> how does this work with display characters longer than one char?
>> It should be fine as we cut at the slash?
>
> Oh, UTF-8.  Ugh.
> Can UTF-8 characters, other than '/', have a byte whose value matches
> (unsigned char)('/')?  If so, then I'll need to figure out how to do
> utf-8 character parsing.  Anyone have pointers?

Oh right we are always cutting at '/', which hex 2F, so we cannot split
a codepoint in half accidentally (finding the slash inside a codepoint).

And renaming a directory from one utf8 codepoint to another, which
have the same prefix is not an issue at this layer, too.
(Think of abc -> abd just all of abc/d in one code point, but there
but even for multi code points/ASCII we repeat the prefix when
printing the rename)

>> So the first loop is about counting the number of files in each directory
>> that are renamed and the later while loop is about mapping them?
>
> Close; would adding the following comment at the top of the function help?
>
> /*
>  * Typically, we think of a directory rename as all files from a
>  * certain directory being moved to a target directory.  However,
>  * what if someone first moved two files from the original
>  * directory in one commit, and then renamed the directory
>  * somewhere else in a later commit?  At merge time, we just know
>  * that files from the original directory went to two different
>  * places, and that the bulk of them ended up in the same place.
>  * We want each directory rename to follow the 

[bug report]: error doing_rebase

[Bulat Musin]

Hi.

To reproduce:

git init testrepo

cd testrepo

echo 1 >> file

git add file

git commit -m'1'

echo 2 >> file

git add file

git commit -m'2'

echo 3 >> file

git add file

git commit -m'3'

Now there are 3 sequential commits, I want to squash them into 1:

git rebase -i HEAD~2

In editor I changed all "pick" to "squash", saved file, I got:

error: cannot 'squash' without a previous commit
You can fix this with 'git rebase --edit-todo' and then run 'git rebase 
--continue'.

Or you can abort the rebase with 'git rebase --abort'.

However, 2.14.1 from Ubuntu's repo does the job - squashes 3 commits into 1.

Thanks.




git version 2.16.1.72.g5be1f00a9

Re: [PATCH v2 06/14] commit-graph: implement git-commit-graph --read

[Derrick Stolee]

On 2/1/2018 7:23 PM, Jonathan Tan wrote:

On Tue, 30 Jan 2018 16:39:35 -0500
Derrick Stolee  wrote:


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

One overall question - is the "read" command meant to be a command used
by the end user, or is it here just to test that some aspects of reading
works? If the former, I'm not sure how useful it is. And if the latter,
I think that it is more useful to just implementing something that reads
it, then make the 11/14 change (modifying parse_commit_gently) and
include a perf test to show that your commit graph reading is both
correct and (performance-)effective.


The "read" command is intended for use with the tests to verify that the 
different --write commands write the correct number of commits at a 
time. For example, we can verify that the closure under reachability 
works when using --stdin-commits, that we get every commit in a pack 
when using --stdin-packs, and that we don't get more commits than we should.


It doesn't serve much purpose on the user-facing side, but this is 
intended to be a plumbing command that is called by other porcelain 
commands.

Re: [GSoC][PATCH v2] commit: add a commit.signOff config variable

[Junio C Hamano]

Chen Jingpiao  writes:

> Add the commit.signOff configuration variable to use the -s or --signoff
> option of git commit by default.

This is a rather old topic.  Here is one from 2006:


https://public-inbox.org/git/pine.lnx.4.63.0611281426311.30...@wbgn013.biozentrum.uni-wuerzburg.de/

which was referred to in another discussion in late 2008:

https://public-inbox.org/git/20081227070228.6...@nanako3.lavabit.com/
https://public-inbox.org/git/7vabaijvxl@gitster.siamese.dyndns.org/

I am not sure if the reasons why the last effort was retracted still
apply to this round (the world certainly has changed in the past 10
years); it would be good to explain why this time it is different
;-).

Assuming that the new configuration variable is a desirable thing to
add, the change to the code looks OK.  Documentation updates may
need more thought in the light of past discussions, though.

Thanks.

Re: [PATCH v7 12/31] merge-recursive: move the get_renames() function

[Stefan Beller]

> Move this function so it can re-use some others (without either
> moving all of them or adding an annoying split between function
> declarations and definitions).  Cheat slightly by adding a blank line
> for readability, and in order to silence checkpatch.pl.
>
> Does that sound better?

Yeah, that sounds perfect (no red flags on reading)

Thanks!

Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)

[Junio C Hamano]

Duy Nguyen  writes:

> On Sat, Feb 3, 2018 at 2:59 AM, Junio C Hamano  wrote:
>> Nguyễn Thái Ngọc Duy   writes:
>>
>>> Changes since v2 [1]:
>>>
>>> - goes back to my original version (yay!) where the extra info
>>>   is appended after the path name. More is described in 2/2
>>> - --compact-summary is now renamed --stat-with-summary and implies
>>>   --stat
>>> - 1/2 is just a cleanup patch to make it easier to add 2/2
>>
>> It may be just me and other old timers, but --X-with-Y naming means
>> quite different thing around these commands, and --stat-with-summary
>> would hint, at least to us, that it would behave as if the two
>> options "--stat --summary" are given at the same time.
>>
>> And from that point of view, the new name is a bit confusing one.
>
> I don't have any good alternative name to be honest. It's kinda hard
> to come up with another word that says "extended header information
> such as creations, renames and mode changes", except maybe the vague
> name --stat-extended?

I actually think compact-summary was a good way to phrase it.

Personally, I think it was a UI mistake that --summary can be given
independently with or without --stat (instead, there shouldn't have
been the --summary option, and instead when it was added, --stat
just should have gained an extra kind of output).  A single option
that can give both kinds of info may be a good way forward, so
another possibility may be --summary-in-stat (meaning: the info
given by summary is included in stat output).  I dunno.

Re: Missing git options

[Stefan Beller]

On Mon, Feb 5, 2018 at 1:12 AM, Martin Häcker
 wrote:
> Hi there,
>
> I just recently learned that not all command line switches seem to 
> automatically correlate to options in the git configuration.
>
> This seems something that should be relatively easy to fix.
>
> What I’m most missing is
>
> — snip —
> [log]
> graph = true
> patch = true
> — snap —
>
> which would / should correspond to `git log —graph —patch`.
>
> What do you guys think?

Feel free to send patches adding these options.

I had the impression that git-log was a pseudo-plumbing,
despite it being explicitly marked porcelain
as there is no good plumbing alternative.
But as we already have things like log.date, log.decorate,
that change the output of git-log, I think a patch or graph
option would be ok.

Thanks,
Stefan

Re: [PATCH v2 05/14] commit-graph: implement git-commit-graph --write

[Junio C Hamano]

Jeff King  writes:

> The big advantage of your scheme is that you can update the graph index
> without repacking. The traditional advice has been that you should
> always do a full repack during a gc (since it gives the most delta
> opportunities). So metadata like reachability bitmaps were happy to tied
> to packs, since you're repacking anyway during a gc. But my
> understanding is that this doesn't really fly with the Windows
> repository, where it's simply so big that you never obtain a single
> pack, and just pass around slices of history in pack format.
>
> So I think I'm OK with the direction here of keeping metadata caches
> separate from the pack storage.

OK.  I guess that the topology information surviving repacking is a
reason good enough to keep this separate from pack files, and I
agree with your "If they're not tied to packs,..." below, too.

Thanks.

> If they're not tied to packs, then I think having a separate builtin
> like this is the best approach. It gives you a plumbing command to
> experiment with, and it can easily be called from git-gc.
>
> -Peff

Re: Bug Report: Subtrees and GPG Signed Commits

[Junio C Hamano]

Stephen R Guglielmo  writes:

> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index cc033af73..dec085a23 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -475,7 +475,7 @@ squash_msg () {
>
>  toptree_for_commit () {
> commit="$1"
> -   git log -1 --pretty=format:'%T' "$commit" -- || exit $?
> +   git log --no-show-signature -1 --pretty=format:'%T' "$commit"
> -- || exit $?
>  }

Given that all references to this shell function seem to do

sometree=$(toptree_for_commit $something)

and then $sometree is used as if it were a tree object name, I can
understand why the lack of --no-show-signature in the original
breaks it when the user has show-signature configured.

It probably makes more sense to replace the "git log" with something
more appropirate for the job, like

git rev-parse --verify "$commit^{tree}"

though.

HI

[ELISABETH ARFORED]

Greeting please is me Elisabeth, i send you message few days ago till
now i have not hear from you, please did you receive this one?

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Junio C Hamano]

Eric Sunshine  writes:

> On Sun, Feb 4, 2018 at 1:30 PM, Lucas Werkmeister
>  wrote:
>> This new option can be used to override the implicit --syslog of
>> ...
> Thanks. With the 'log_destination' initialization bug fixed, this
> version looks good; I didn't find anything else worth commenting upon.
> Ævar's micronits[1] could be addressed by a follow-up patch (if
> desirable), but probably needn't hold up this patch.
>
> [1]: https://public-inbox.org/git/871si0mvo0@evledraar.gmail.com/

Nicely done.  Thanks, all.

Will queue.

Re: git: handling HTTPS proxy w/ password inappropriately

[Yangfl]

2018-02-06 1:55 GMT+08:00 Jonathan Nieder :
> Hi,
>
> Yangfl  wrote[1]:
>
>> not affected any more
>
> Can you say a little more about this?  Do you mean that newer versions
> of Git are working better for you or that your proxy setup changed?
>
> This looks similar to
> https://public-inbox.org/git/cahnnmh6qcnhtycbmdljffyoxw4dertzothsprkydhzknxch...@mail.gmail.com/
> to me, which makes me fear it hasn't been fixed.
>
> Thanks,
> Jonathan
>
> [1] http://bugs.debian.org/873424

I just repeated it three times to make sure it works perfectly.
However, I still see 2 connection attempts in my wireshark, the first
one without password (then FINed), and the following one with.

$ git --version
git version 2.15.1

Versions of packages git depends on:
ii  git-man  1:2.15.1-3
ii  libc62.26-4
ii  libcurl3-gnutls  7.58.0-2
ii  liberror-perl0.17025-1
ii  libexpat12.2.5-3
ii  libpcre2-8-0 10.22-5
ii  perl 5.26.1-4
ii  zlib1g   1:1.2.8.dfsg-5

Versions of packages git recommends:
ii  less 487-0.1
ii  openssh-client [ssh-client]  1:7.6p1-3
ii  patch2.7.5-1+b2

Versions of packages git suggests:
ii  gettext-base  0.19.8.1-4
pn  git-cvs   
pn  git-daemon-run | git-daemon-sysvinit  
pn  git-doc   
pn  git-el
pn  git-email 
pn  git-gui   
pn  git-mediawiki 
pn  git-svn   
pn  gitk  
pn  gitweb

Re: [PATCH v2 05/14] commit-graph: implement git-commit-graph --write

[Derrick Stolee]

On 2/1/2018 6:48 PM, SZEDER Gábor wrote:

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

Signed-off-by: Derrick Stolee 
---
  Documentation/git-commit-graph.txt | 18 +++
  builtin/commit-graph.c | 30 
  t/t5318-commit-graph.sh| 96 ++
  3 files changed, 144 insertions(+)
  create mode 100755 t/t5318-commit-graph.sh

diff --git a/Documentation/git-commit-graph.txt 
b/Documentation/git-commit-graph.txt
index c8ea548dfb..3f3790d9a8 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -5,3 +5,21 @@ NAME
  
  git-commit-graph - Write and verify Git commit graphs (.graph files)
  
+

+SYNOPSIS
+
+[verse]
+'git commit-graph' --write  [--pack-dir ]
+

What do these options do and what is the command's output?  IOW, an
'OPTIONS' section would be nice.


+EXAMPLES
+
+
+* Write a commit graph file for the packed commits in your local .git folder.
++
+
+$ git commit-graph --write
+
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
new file mode 100755
index 00..6bcd1cc264
--- /dev/null
+++ b/t/t5318-commit-graph.sh
@@ -0,0 +1,96 @@
+#!/bin/sh
+
+test_description='commit graph'
+. ./test-lib.sh
+
+test_expect_success 'setup full repo' \
+'rm -rf .git &&
+ mkdir full &&
+ cd full &&
+ git init &&
+ git config core.commitgraph true &&
+ git config pack.threads 1 &&

Does this pack.threads=1 make a difference?


+ packdir=".git/objects/pack"'

We tend to put single quotes around tests like this:

   test_expect_success 'setup full repo' '
 do-this &&
 check-that
   '

This is not a mere style nit: those newlines before and after the test
block make the test's output with '--verbose-log' slightly more
readable.

Furthermore, we prefer tabs for indentation.


Oops! My bad for using t5302-pack-index.sh as my model for creating test 
scripts. It's pretty old, but I do see some of the newer tests using 
this newer style.



Finally, 'cd'-ing around such that it affects subsequent tests is
usually frowned upon.  However, in this particular case (going into
one repo, doing a bunch of tests there, then going into another repo,
and doing another bunch of tests) I think it's better than changing
directory in a subshell in every single test.


+
+test_expect_success 'write graph with no packs' \
+'git commit-graph --write --pack-dir .'
+
+test_expect_success 'create commits and repack' \
+'for i in $(test_seq 5)
+ do
+echo $i >$i.txt &&
+git add $i.txt &&
+git commit -m "commit $i" &&
+git branch commits/$i
+ done &&
+ git repack'
+
+test_expect_success 'write graph' \
+'graph1=$(git commit-graph --write) &&
+ test_path_is_file ${packdir}/graph-${graph1}.graph'

Style nit:  those {} around the variable names are unnecessary, but I
see you use them a lot.


+
+t_expect_success 'Add more commits' \

This must be test_expect_success.


+'git reset --hard commits/3 &&
+ for i in $(test_seq 6 10)
+ do
+echo $i >$i.txt &&
+git add $i.txt &&
+git commit -m "commit $i" &&
+git branch commits/$i
+ done &&
+ git reset --hard commits/3 &&
+ for i in $(test_seq 11 15)
+ do
+echo $i >$i.txt &&
+git add $i.txt &&
+git commit -m "commit $i" &&
+git branch commits/$i
+ done &&
+ git reset --hard commits/7 &&
+ git merge commits/11 &&
+ git branch merge/1 &&
+ git reset --hard commits/8 &&
+ git merge commits/12 &&
+ git branch merge/2 &&
+ git reset --hard commits/5 &&
+ git merge commits/10 commits/15 &&
+ git branch merge/3 &&
+ git repack'
+
+# Current graph structure:
+#
+#  M3
+# / |\_
+#/ 10  15
+#   /   |  |
+#  /9 M2   14
+# | |/  \  |
+# | 8 M1 | 13
+# | |/ | \_|
+# 5 7  |   12
+# | |   \__|
+# 4 6  11
+# |/__/
+# 3
+# |
+# 2
+# |
+# 1
+
+test_expect_success 'write graph with merges' \
+'graph2=$(git commit-graph --write) &&
+ test_path_is_file ${packdir}/graph-${graph2}.graph'
+
+test_expect_success 'setup bare repo' \
+'cd .. &&
+ git clone --bare full bare &&
+ cd bare &&
+ git config core.graph true &&
+ git config pack.threads 1 &&
+ baredir="objects/pack"'
+
+test_expect_success 'write graph in bare repo' \
+'graphbare=$(git commit-graph --write) &&
+ test_path_is_file ${baredir}/graph-${graphbare}.graph'
+
+test_done
--
2.16.0.15.g9c3cf44.dirty

Re: git: handling HTTPS proxy w/ password inappropriately

[Jonathan Nieder]

Hi,

Yangfl  wrote[1]:

> not affected any more

Can you say a little more about this?  Do you mean that newer versions
of Git are working better for you or that your proxy setup changed?

This looks similar to
https://public-inbox.org/git/cahnnmh6qcnhtycbmdljffyoxw4dertzothsprkydhzknxch...@mail.gmail.com/
to me, which makes me fear it hasn't been fixed.

Thanks,
Jonathan

[1] http://bugs.debian.org/873424

Re: [PATCH] dir.c: ignore paths containing .git when invalidating untracked cache

[Ben Peart]

On 2/4/2018 4:38 AM, Nguyễn Thái Ngọc Duy wrote:

read_directory() code ignores all paths named ".git" even if it's not
a valid git repository. See treat_path() for details. Since ".git" is
basically invisible to read_directory(), when we are asked to
invalidate a path that contains ".git", we can safely ignore it
because the slow path would not consider it anyway.

This helps when fsmonitor is used and we have a real ".git" repo at
worktree top. Occasionally .git/index will be updated and if the
fsmonitor hook does not filter it, untracked cache is asked to
invalidate the path ".git/index".

Without this patch, we invalidate the root directory unncessarily,
which:

- makes read_directory() fall back to slow path for root directory
   (slower)

- makes the index dirty (because UNTR extension is updated). Depending
   on the index size, writing it down could also be slow.

Noticed-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
  Sorry for the resend, I forgot git@vger.

  dir.c | 13 -
  git-compat-util.h |  2 ++
  wrapper.c | 12 
  3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 7c4b45e30e..f8b4cabba9 100644
--- a/dir.c
+++ b/dir.c
@@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct 
*dir,
if (!de)
return treat_path_fast(dir, untracked, cdir, istate, path,
   baselen, pathspec);
-   if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git"))
+   if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git"))
return path_none;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de->d_name);
@@ -2970,8 +2970,19 @@ static int invalidate_one_component(struct 
untracked_cache *uc,
  void untracked_cache_invalidate_path(struct index_state *istate,
 const char *path)
  {
+   const char *end;
+   int skipped;
+
if (!istate->untracked || !istate->untracked->root)
return;


Thank you for this patch!  It's great to see people helping improve the 
performance of git.


I ran a quick test and this is not sufficient to prevent the index from 
getting flagged as dirty and written out to disk when fsmonitor detects 
changes for files under the .git folder.  What I'm seeing is that when 
".git/index" is returned, the test below doesn't catch them and end 
early as would be expected.


As a result, invalidate_one_component() is called which calls 
invalidate_one_directory() which increments dir_invalidated counter and 
the regular dirty process continues which triggers the index to be 
written to disk unnecessarily.



+   if (!fspathcmp(path, ".git"))
+   return;
+   if (ignore_case)
+   skipped = skip_caseprefix(path, "/.git", );
+   else
+   skipped = skip_prefix(path, "/.git", );
+   if (skipped && (*end == '\0' || *end == '/'))
+   return;


If I replace the above lines with:

if (ignore_case)
skipped = skip_caseprefix(path, ".git", );
else
skipped = skip_prefix(path, ".git", );

Then it correctly skips _all_ files under ".git".  I'm not sure if by 
removing the leading slash, I'm breaking some other case but I was not 
able to find where that was expected or needed. Removing the leading 
slash also allows me to remove the fsmpathcmp() call as it is now redundant.


I wondered what would happen if there was some other directory named 
".git" and how that would behave.  With or without this patch and with 
or without the untracked cache, a file "dir1/.git/foo" is ignored by git 
so no change in behavior there either.



invalidate_one_component(istate->untracked, istate->untracked->root,
 path, strlen(path));
  }
diff --git a/git-compat-util.h b/git-compat-util.h
index 68b2ad531e..27e0b761a3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -484,6 +484,8 @@ static inline int skip_prefix(const char *str, const char 
*prefix,
return 0;
  }
  
+int skip_caseprefix(const char *str, const char *prefix, const char **out);

+
  /*
   * If the string "str" is the same as the string in "prefix", then the "arg"
   * parameter is set to the "def" parameter and 1 is returned.
diff --git a/wrapper.c b/wrapper.c
index d20356a776..bb888d9401 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -690,3 +690,15 @@ int xgethostname(char *buf, size_t len)
buf[len - 1] = 0;
return ret;
  }
+
+int skip_caseprefix(const char *str, const char *prefix, const char **out)
+{
+   do {
+   if (!*prefix) {
+   *out = str;
+   return 1;
+   }
+   } while (tolower(*str++) == tolower(*prefix++));
+
+   return 0;
+}

Hello My Dear Friend,

[Mrs.Zainab Ahmed]

I have a business proposal in the tune of $10.2 Million USD for you to handle 
with me. I have opportunity to transfer this abandon fund to your bank account 
in your country which belongs to our client.

I am inviting you in this transaction where this money can be shared between us 
at ratio of 60/40% and help the needy around us don’t be afraid of anything I 
am with you I will instruct you what you will do to maintain this fund.

Please kindly contact me with your information's if you are interested in this 
transaction for more details.

Your Name:..
Your Bank Name:.
Your Account Number:...
Your Telephone Number:
Your Country And Address:
Your Age And Sex:...

Thanks
Mrs.Zainab Ahmed,

Re: [PATCH v2 04/14] commit-graph: implement construct_commit_graph()

[Derrick Stolee]

On 2/2/2018 10:32 AM, SZEDER Gábor wrote:

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

I'm afraid that scanning all packed objects is a bit of a roundabout
way to approach this.

In my git repo, with 9 pack files at the moment, i.e. not that big a
repo and not that many pack files:

   $ time ./git commit-graph --write --update-head
   4df41a3d1cc408b7ad34bea87b51ec4ccbf4b803

   real0m27.550s
   user0m27.113s
   sys 0m0.376s

In comparison, performing a good old revision walk to gather all the
info that is written into the graph file:

   $ time git log --all --topo-order --format='%H %T %P %cd' |wc -l
   52954

   real0m0.903s
   user0m0.972s
   sys 0m0.058s


Two reasons this is in here:

(1) It's easier to get the write implemented this way and add the 
reachable closure later (which I do).


(2) For GVFS, we want to add all commits that arrived in a "prefetch 
pack" to the graph even if we do not have a ref that points to the 
commit yet. We expect many commits to become reachable soon and having 
them in the graph saves a lot of time in merge-base calculations.


So, (1) is for patch simplicity, and (2) is why I want it to be an 
option in the final version. See the --stdin-packs argument later for a 
way to do this incrementally.


I expect almost all users to use the reachable closure method with 
--stdin-commits (and that's how I will integrate automatic updates with 
'fetch', 'repack', and 'gc' in a later patch).





+char* get_commit_graph_filename_hash(const char *pack_dir,
+struct object_id *hash)
+{
+   size_t len;
+   struct strbuf head_path = STRBUF_INIT;
+   strbuf_addstr(_path, pack_dir);
+   strbuf_addstr(_path, "/graph-");
+   strbuf_addstr(_path, oid_to_hex(hash));
+   strbuf_addstr(_path, ".graph");

Nit: this is assembling the path of a graph file, not that of a
graph-head, so the strbuf should be renamed accordingly.


+
+   return strbuf_detach(_path, );
+}

Re: [PATCH v7 17/31] merge-recursive: add a new hashmap for storing directory renames

[Elijah Newren]

// Re-sending because of bounce

On Sun, Feb 4, 2018 at 12:54 AM, Johannes Sixt  wrote:
> Am 03.02.2018 um 22:34 schrieb Elijah Newren:
>>  If anyone can find an
>> example of a real world open source repository (linux, webkit, git,
>> etc.) with a merge where n is greater than about 10, I'll be
>> surprised.
>
> git rev-list --parents --merges master |
>  grep " .* .* .* .* .* .* .* .* .* .* "
>
> returns quite a few hits in the Linux repository. Most notable is
> fa623d1b0222adbe8f822e53c08003b9679a410c; spoiler: it has 30 parents.
>
> -- Hannes

Sorry, I didn't make it very clear what n represented. This is in the
context of detecting directory renames, and in this discussion n
represents the number of distinct directories that files were renamed
into from a single original directory on a given side of the merge. So
your example of number of parents of a commit isn't directly relevant
to this case (also, even along your tangent, merge-recursive is only
invoked when the number of parents is precisely two). However, I find
your nugget rather interesting, even if unrelated. I had known of
merges with more than 10 parents, but somehow was unaware that the
limit of 16 parents had been lifted. And digging through the history,
it was apparently removed quite a while ago. I love the commit message
that lifted the limit too:

"There is no really good reason to have a merge with more than 16
parents, but we have a history of giving our users rope."

:-)

Re: Bug Report: Subtrees and GPG Signed Commits

[Stephen R Guglielmo]

On Mon, Feb 5, 2018 at 9:30 AM, Stephen R Guglielmo
 wrote:
> On Wed, Jan 31, 2018 at 7:33 AM, Stephen R Guglielmo
>  wrote:
>> On Tue, Jan 30, 2018 at 6:37 PM, Avery Pennarun  wrote:
>>> On Tue, Jan 30, 2018 at 6:24 PM, Junio C Hamano  wrote:
 Stefan Beller  writes:
> There has not been feedback for a while on this thread.
> I think that is because subtrees are not in anyone's hot
> interest area currently.
>
> This is definitely the right place to submit bugs.
> Looking through "git log --format="%ae %s" -S subtree",
> it seems as if Avery (apenw...@gmail.com) was mostly
> interested in developing subtrees, though I think he has
> moved on. Originally it was invented by Junio, who is
> the active maintainer of the project in 68faf68938
> (A new merge stragety 'subtree'., 2007-02-15)

 Thanks for trying to help, but I have *NOTHING* to do with the "git
 subtree" subcommand (and I personally have no interest in it).  What
 I did was a subtree merge strategy (i.e. "git merge -s subtree"),
 which is totally a different thing.

 David Greene offered to take it over in 2015, and then we saw some
 activity by David Aguilar in 2016, but otherwise the subcommand from
 contrib/ has pretty much been dormant these days.
>>>
>>> Strictly speaking, the 'git subtree' command does in fact use 'git
>>> merge -s subtree' under the covers, so Junio is at least partly
>>> responsible for giving me the idea :)
>>>
>>> I actually have never looked into how signed commits work and although
>>> I still use git-subtree occasionally (it hasn't needed any
>>> maintenance, for my simple use cases), I have never used it with
>>> signed commits.
>>>
>>> git-subtree maintains a cache that maps commit ids in the "original
>>> project" with their equivalents in the "merged project."  If there's
>>> something magic about how commit ids work with signed commits, I could
>>> imagine that causing the "no a valid object name" problems.  Or,
>>> git-subtree in --squash mode actually generates new commit objects
>>> using some magic of its own.  If it were to accidentally copy a
>>> signature into a commit that no longer matches the original, I imagine
>>> that new object might get rejected.
>>>
>>> Unfortunately I don't have time to look into it.  The git-subtree code
>>> is pretty straightforward, though, so if Stephen has an hour or two to
>>> look deeper it's probably possible to fix it up.  The tool is not
>>> actually as magical and difficult as it might seem at first glance :)
>>>
>>> Sorry I can't help more.
>>>
>>> Good luck,
>>>
>>> Avery
>>
>> Thanks all for the discussion/replies.
>>
>> We use subtrees extensively in our environment right now. The "sub"
>> repos (90+) are located on GitHub, while the "main/parent" repo is
>> provided by a vendor on website hosting infrastructure.
>>
>> I will take a look at:
>> git/Documentation/CodingGuidelines
>> git/Documentation/SubmittingPatches
>> git/contrib/subtree/
>>
>> Should I follow up in this thread with a patch (it might be a while)?
>>
>> Thanks!
>> Steve
>
> Hi all,
>
> It looks like I've found the cause of the issue. I have
> log.showsignature=true in my gitconfig. The toptree_for_commit()
> function calls `git log` and passes the output to `git commit-tree` in
> new_squash_commit(). Apparently commit-tree doesn't like GPG sigs.
>
> The fix was simple: --no-show-signature. However, I believe this was
> added in git v2.10.0, so it's not fully backwards compatible. I'm open
> to suggestions on a better fix if this is not acceptable.
>
> Thanks!
>
>
> https://github.com/srguglielmo/git/commit/822c8a45d049f86ea5c59c0b434303964e4e6f3d
>
>
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index cc033af73..dec085a23 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -475,7 +475,7 @@ squash_msg () {
>
>  toptree_for_commit () {
> commit="$1"
> -   git log -1 --pretty=format:'%T' "$commit" -- || exit $?
> +   git log --no-show-signature -1 --pretty=format:'%T' "$commit"
> -- || exit $?
>  }
>
>  subtree_for_commit () {

Hey again,

Actually, to follow up on this, I added --no-show-signature to several
other locations. The above patch fixes the fatal, however the GPG sig
info is still included in the commit merge message for `subtree pull`.
This fixes that as well.

https://github.com/srguglielmo/git/commit/ebd2f628ddb960931aac5087c45a54b953976e99


diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index cc033af73..8126132dc 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -297,7 +297,7 @@ find_latest_squash () {
main=
sub=
git log --grep="^git-subtree-dir: $dir/*\$" \
-   --pretty=format:'START %H%n%s%n%n%b%nEND%n' HEAD 

Re: Bug Report: Subtrees and GPG Signed Commits

[Stephen R Guglielmo]

On Wed, Jan 31, 2018 at 7:33 AM, Stephen R Guglielmo
 wrote:
> On Tue, Jan 30, 2018 at 6:37 PM, Avery Pennarun  wrote:
>> On Tue, Jan 30, 2018 at 6:24 PM, Junio C Hamano  wrote:
>>> Stefan Beller  writes:
 There has not been feedback for a while on this thread.
 I think that is because subtrees are not in anyone's hot
 interest area currently.

 This is definitely the right place to submit bugs.
 Looking through "git log --format="%ae %s" -S subtree",
 it seems as if Avery (apenw...@gmail.com) was mostly
 interested in developing subtrees, though I think he has
 moved on. Originally it was invented by Junio, who is
 the active maintainer of the project in 68faf68938
 (A new merge stragety 'subtree'., 2007-02-15)
>>>
>>> Thanks for trying to help, but I have *NOTHING* to do with the "git
>>> subtree" subcommand (and I personally have no interest in it).  What
>>> I did was a subtree merge strategy (i.e. "git merge -s subtree"),
>>> which is totally a different thing.
>>>
>>> David Greene offered to take it over in 2015, and then we saw some
>>> activity by David Aguilar in 2016, but otherwise the subcommand from
>>> contrib/ has pretty much been dormant these days.
>>
>> Strictly speaking, the 'git subtree' command does in fact use 'git
>> merge -s subtree' under the covers, so Junio is at least partly
>> responsible for giving me the idea :)
>>
>> I actually have never looked into how signed commits work and although
>> I still use git-subtree occasionally (it hasn't needed any
>> maintenance, for my simple use cases), I have never used it with
>> signed commits.
>>
>> git-subtree maintains a cache that maps commit ids in the "original
>> project" with their equivalents in the "merged project."  If there's
>> something magic about how commit ids work with signed commits, I could
>> imagine that causing the "no a valid object name" problems.  Or,
>> git-subtree in --squash mode actually generates new commit objects
>> using some magic of its own.  If it were to accidentally copy a
>> signature into a commit that no longer matches the original, I imagine
>> that new object might get rejected.
>>
>> Unfortunately I don't have time to look into it.  The git-subtree code
>> is pretty straightforward, though, so if Stephen has an hour or two to
>> look deeper it's probably possible to fix it up.  The tool is not
>> actually as magical and difficult as it might seem at first glance :)
>>
>> Sorry I can't help more.
>>
>> Good luck,
>>
>> Avery
>
> Thanks all for the discussion/replies.
>
> We use subtrees extensively in our environment right now. The "sub"
> repos (90+) are located on GitHub, while the "main/parent" repo is
> provided by a vendor on website hosting infrastructure.
>
> I will take a look at:
> git/Documentation/CodingGuidelines
> git/Documentation/SubmittingPatches
> git/contrib/subtree/
>
> Should I follow up in this thread with a patch (it might be a while)?
>
> Thanks!
> Steve

Hi all,

It looks like I've found the cause of the issue. I have
log.showsignature=true in my gitconfig. The toptree_for_commit()
function calls `git log` and passes the output to `git commit-tree` in
new_squash_commit(). Apparently commit-tree doesn't like GPG sigs.

The fix was simple: --no-show-signature. However, I believe this was
added in git v2.10.0, so it's not fully backwards compatible. I'm open
to suggestions on a better fix if this is not acceptable.

Thanks!


https://github.com/srguglielmo/git/commit/822c8a45d049f86ea5c59c0b434303964e4e6f3d



diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index cc033af73..dec085a23 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -475,7 +475,7 @@ squash_msg () {

 toptree_for_commit () {
commit="$1"
-   git log -1 --pretty=format:'%T' "$commit" -- || exit $?
+   git log --no-show-signature -1 --pretty=format:'%T' "$commit"
-- || exit $?
 }

 subtree_for_commit () {

Re: [PATCH v2 10/27] protocol: introduce enum protocol_version value protocol_v2

[Derrick Stolee]

On 2/2/2018 5:44 PM, Brandon Williams wrote:

On 01/31, Derrick Stolee wrote:

On 1/25/2018 6:58 PM, Brandon Williams wrote:

Introduce protocol_v2, a new value for 'enum protocol_version'.
Subsequent patches will fill in the implementation of protocol_v2.

Signed-off-by: Brandon Williams 
---
   builtin/fetch-pack.c   | 3 +++
   builtin/receive-pack.c | 6 ++
   builtin/send-pack.c| 3 +++
   builtin/upload-pack.c  | 7 +++
   connect.c  | 3 +++
   protocol.c | 2 ++
   protocol.h | 1 +
   remote-curl.c  | 3 +++
   transport.c| 9 +
   9 files changed, 37 insertions(+)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 85d4faf76..f492e8abd 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -201,6 +201,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
   PACKET_READ_GENTLE_ON_EOF);
switch (discover_version()) {
+   case protocol_v2:
+   die("support for protocol v2 not implemented yet");
+   break;
case protocol_v1:
case protocol_v0:
get_remote_heads(, , 0, NULL, );
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b7ce7c7f5..3656e94fd 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1963,6 +1963,12 @@ int cmd_receive_pack(int argc, const char **argv, const 
char *prefix)
unpack_limit = receive_unpack_limit;
switch (determine_protocol_version_server()) {
+   case protocol_v2:
+   /*
+* push support for protocol v2 has not been implemented yet,
+* so ignore the request to use v2 and fallback to using v0.
+*/
+   break;
case protocol_v1:
/*
 * v1 is just the original protocol with a version string,
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 83cb125a6..b5427f75e 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -263,6 +263,9 @@ int cmd_send_pack(int argc, const char **argv, const char 
*prefix)
   PACKET_READ_GENTLE_ON_EOF);
switch (discover_version()) {
+   case protocol_v2:
+   die("support for protocol v2 not implemented yet");
+   break;
case protocol_v1:
case protocol_v0:
get_remote_heads(, _refs, REF_NORMAL,
diff --git a/builtin/upload-pack.c b/builtin/upload-pack.c
index 2cb5cb35b..8d53e9794 100644
--- a/builtin/upload-pack.c
+++ b/builtin/upload-pack.c
@@ -47,6 +47,13 @@ int cmd_upload_pack(int argc, const char **argv, const char 
*prefix)
die("'%s' does not appear to be a git repository", dir);
switch (determine_protocol_version_server()) {
+   case protocol_v2:
+   /*
+* fetch support for protocol v2 has not been implemented yet,
+* so ignore the request to use v2 and fallback to using v0.
+*/
+   upload_pack();
+   break;
case protocol_v1:
/*
 * v1 is just the original protocol with a version string,
diff --git a/connect.c b/connect.c
index db3c9d24c..f2157a821 100644
--- a/connect.c
+++ b/connect.c
@@ -84,6 +84,9 @@ enum protocol_version discover_version(struct packet_reader 
*reader)
/* Maybe process capabilities here, at least for v2 */
switch (version) {
+   case protocol_v2:
+   die("support for protocol v2 not implemented yet");
+   break;
case protocol_v1:
/* Read the peeked version line */
packet_reader_read(reader);
diff --git a/protocol.c b/protocol.c
index 43012b7eb..5e636785d 100644
--- a/protocol.c
+++ b/protocol.c
@@ -8,6 +8,8 @@ static enum protocol_version parse_protocol_version(const char 
*value)
return protocol_v0;
else if (!strcmp(value, "1"))
return protocol_v1;
+   else if (!strcmp(value, "2"))
+   return protocol_v2;
else
return protocol_unknown_version;
   }
diff --git a/protocol.h b/protocol.h
index 1b2bc94a8..2ad35e433 100644
--- a/protocol.h
+++ b/protocol.h
@@ -5,6 +5,7 @@ enum protocol_version {
protocol_unknown_version = -1,
protocol_v0 = 0,
protocol_v1 = 1,
+   protocol_v2 = 2,
   };
   /*
diff --git a/remote-curl.c b/remote-curl.c
index 9f6d07683..dae8a4a48 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -185,6 +185,9 @@ static struct ref *parse_git_refs(struct discovery *heads, 
int for_push)
   PACKET_READ_GENTLE_ON_EOF);
switch (discover_version()) {
+   case protocol_v2:
+   die("support for protocol v2 not implemented yet");
+   break;
case protocol_v1:
case protocol_v0:
get_remote_heads(, , for_push ? 

Re: Bug? Error during commit

[Duy Nguyen]

On Mon, Feb 5, 2018 at 8:48 PM, Andreas Kalz  wrote:
> Hello,
>
> I am using git frequently and usually it is running great.
>
> I read to write to this eMail address regarding problems and possible bugs.
> I am using git version 2.16.1.windows.2 / 64 Bit and during commit the 
> following error message comes up:
> e:\Internet>git commit -m 2018-01-27
> fatal: unable to generate diffstat for 
> Thunderbird/andreas-kalz.de/Mail/pop.gmx.net/Inbox
> [master f74cf30] 2018-01-27
>
> I also tried this before with an older git version with same problem.
>
> Can you help me with this problem please? Thanks in advance.

I think if you add -q to that "git commit" command, diffstat is not
generated and you can get past that. If that particular commit can be
published in public, it'll help us find out why diffstat could not be
generated.
-- 
Duy

Bug? Error during commit

[Andreas Kalz]

Hello,
 
I am using git frequently and usually it is running great.
 
I read to write to this eMail address regarding problems and possible bugs.
I am using git version 2.16.1.windows.2 / 64 Bit and during commit the 
following error message comes up:
e:\Internet>git commit -m 2018-01-27
fatal: unable to generate diffstat for 
Thunderbird/andreas-kalz.de/Mail/pop.gmx.net/Inbox
[master f74cf30] 2018-01-27
 
I also tried this before with an older git version with same problem.
 
Can you help me with this problem please? Thanks in advance.
 
All the best,
Andreas

Re: [PATCH 3/7] worktree move: new command

[Duy Nguyen]

On Fri, Feb 2, 2018 at 6:23 PM, Eric Sunshine  wrote:
> On Fri, Feb 2, 2018 at 4:15 AM, Eric Sunshine  wrote:
>> On Wed, Jan 24, 2018 at 4:53 AM, Nguyễn Thái Ngọc Duy  
>> wrote:
>>> +static int move_worktree(int ac, const char **av, const char *prefix)
>>> +{
>>> +   [...]
>>> +   worktrees = get_worktrees(0);
>>> +   wt = find_worktree(worktrees, prefix, av[0]);
>>> +   if (!wt)
>>> +   die(_("'%s' is not a working tree"), av[0]);
>>
>> This is still leaking 'worktrees'[1]. You probably want
>> free_worktrees() immediately after the find_worktree() invocation.
>
> Sorry, free_worktrees() after the last use of 'wt' since you still
> need to access its fields, which would be the end of the function.

I learned SANITIZE=leak today! It not only catches this but also "dst".

Jeff is there any ongoing effort to make the test suite pass with
SANITIZE=leak? My t2038 passed, so I went ahead with the full test
suite and saw so many failures. I did see in your original mails that
you focused on t and t0001 only..
-- 
Duy

cherry-pick '-m' curiosity

[Sergey Organov]

Hello,

$ git help cherry-pick

-m parent-number, --mainline parent-number
   Usually you cannot cherry-pick a merge because you do not
   know which side of the merge should be considered the
   mainline.

Isn't it always the case that "mainline" is the first parent, as that's
how "git merge" happens to work?

Is, say, "-m 2" ever useful?

-- 
Sergey

Re: [PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)

[Duy Nguyen]

On Sat, Feb 3, 2018 at 2:59 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> Changes since v2 [1]:
>>
>> - goes back to my original version (yay!) where the extra info
>>   is appended after the path name. More is described in 2/2
>> - --compact-summary is now renamed --stat-with-summary and implies
>>   --stat
>> - 1/2 is just a cleanup patch to make it easier to add 2/2
>
> It may be just me and other old timers, but --X-with-Y naming means
> quite different thing around these commands, and --stat-with-summary
> would hint, at least to us, that it would behave as if the two
> options "--stat --summary" are given at the same time.
>
> And from that point of view, the new name is a bit confusing one.

I don't have any good alternative name to be honest. It's kinda hard
to come up with another word that says "extended header information
such as creations, renames and mode changes", except maybe the vague
name --stat-extended?
-- 
Duy

[PATCH RFC v2 17/25] ref-filter: make cat_file_info independent

[Olga Telezhnaya]

Remove connection between expand_data variable
in cat-file and in ref-filter.
It will help further to get rid of using expand_data in cat-file.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c |  2 +-
 ref-filter.c   | 29 +++--
 ref-filter.h   |  1 -
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 37adf626d0e55..5b9869cdb9096 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -292,6 +292,7 @@ static void batch_object_write(const char *obj_name, struct 
batch_options *opt,
if (populate_value())
return;
 
+   data->type = item.type;
strbuf_expand(, opt->format.format, expand_format, );
strbuf_addch(, '\n');
batch_write(opt, buf.buf, buf.len);
@@ -393,7 +394,6 @@ static int batch_objects(struct batch_options *opt)
 * object.
 */
memset(, 0, sizeof(data));
-   opt->format.cat_file_data = 
opt->format.is_cat = 1;
opt->format.all_objects = opt->all_objects;
verify_ref_format(>format);
diff --git a/ref-filter.c b/ref-filter.c
index 7dcd36cd2cddc..a65a90790fd2c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,7 +100,7 @@ static struct used_atom {
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
-struct expand_data *cat_file_info;
+struct expand_data cat_file_info;
 static int is_cat = 0;
 
 static void color_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *color_value)
@@ -256,9 +256,9 @@ static void objectname_atom_parser(const struct ref_format 
*format, struct used_
 static void objectsize_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
 {
if (!arg)
-   cat_file_info->info.sizep = _file_info->size;
+   cat_file_info.info.sizep = _file_info.size;
else if (!strcmp(arg, "disk"))
-   cat_file_info->info.disk_sizep = _file_info->disk_size;
+   cat_file_info.info.disk_sizep = _file_info.disk_size;
else
die(_("urecognized %%(objectsize) argument: %s"), arg);
 }
@@ -266,7 +266,7 @@ static void objectsize_atom_parser(const struct ref_format 
*format, struct used_
 static void objecttype_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
 {
if (!arg)
-   cat_file_info->info.typep = _file_info->type;
+   cat_file_info.info.typep = _file_info.type;
else
die(_("urecognized %%(objecttype) argument: %s"), arg);
 }
@@ -274,7 +274,7 @@ static void objecttype_atom_parser(const struct ref_format 
*format, struct used_
 static void deltabase_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
 {
if (!arg)
-   cat_file_info->info.delta_base_sha1 = 
cat_file_info->delta_base_oid.hash;
+   cat_file_info.info.delta_base_sha1 = 
cat_file_info.delta_base_oid.hash;
else
die(_("urecognized %%(deltabase) argument: %s"), arg);
 }
@@ -752,7 +752,6 @@ int verify_ref_format(struct ref_format *format)
 {
const char *cp, *sp;
 
-   cat_file_info = format->cat_file_data;
is_cat = format->is_cat;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
@@ -779,8 +778,8 @@ int verify_ref_format(struct ref_format *format)
format->need_color_reset_at_eol = 0;
if (is_cat && format->all_objects) {
struct object_info empty = OBJECT_INFO_INIT;
-   if (!memcmp(_file_info->info, , sizeof(empty)))
-   cat_file_info->skip_object_info = 1;
+   if (!memcmp(_file_info.info, , sizeof(empty)))
+   cat_file_info.skip_object_info = 1;
}
return 0;
 }
@@ -1422,18 +1421,20 @@ static const char *get_refname(struct used_atom *atom, 
struct ref_array_item *re
 
 static int check_and_fill_for_cat(struct ref_array_item *ref)
 {
-   if (!cat_file_info->skip_object_info &&
-   sha1_object_info_extended(ref->oid.hash, _file_info->info,
+   if (!cat_file_info.info.typep)
+   cat_file_info.info.typep = _file_info.type;
+   if (!cat_file_info.skip_object_info &&
+   sha1_object_info_extended(ref->oid.hash, _file_info.info,
  OBJECT_INFO_LOOKUP_REPLACE) < 0) {
const char *e = ref->objectname;
printf("%s missing\n", e ? e : oid_to_hex(>oid));
fflush(stdout);
return -1;
}
-   ref->type = cat_file_info->type;
-   ref->size = cat_file_info->size;
-   ref->disk_size = cat_file_info->disk_size;
-   ref->delta_base_oid = 

[PATCH RFC v2 14/25] ref-filter: add is_cat flag

[Olga Telezhnaya]

Add is_cat flag, further it helps to get rid of cat_file_data field
in ref_format.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 1 +
 ref-filter.c   | 8 +---
 ref-filter.h   | 1 +
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 179c955b86bd5..e8e788f41b890 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -395,6 +395,7 @@ static int batch_objects(struct batch_options *opt)
 */
memset(, 0, sizeof(data));
opt->format.cat_file_data = 
+   opt->format.is_cat = 1;
verify_ref_format(>format);
if (opt->cmdmode)
data.split_on_whitespace = 1;
diff --git a/ref-filter.c b/ref-filter.c
index 34a54db168265..91290b62450b3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -101,6 +101,7 @@ static struct used_atom {
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
 struct expand_data *cat_file_info;
+static int is_cat = 0;
 
 static void color_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *color_value)
 {
@@ -493,7 +494,7 @@ static int parse_ref_filter_atom(const struct ref_format 
*format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
-   if (cat_file_info && !strcmp(valid_atom[i].name, "rest"))
+   if (is_cat && !strcmp(valid_atom[i].name, "rest"))
cat_file_info->split_on_whitespace = 1;
return at;
 }
@@ -739,6 +740,7 @@ int verify_ref_format(struct ref_format *format)
const char *cp, *sp;
 
cat_file_info = format->cat_file_data;
+   is_cat = format->is_cat;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
const char *color, *ep = strchr(sp, ')');
@@ -748,7 +750,7 @@ int verify_ref_format(struct ref_format *format)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
 
-   if (format->cat_file_data)
+   if (is_cat)
at = parse_ref_filter_atom(format, valid_cat_file_atom,
   
ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
else {
@@ -1438,7 +1440,7 @@ int populate_value(struct ref_array_item *ref)
ref->symref = "";
}
 
-   if (cat_file_info && check_and_fill_for_cat(ref))
+   if (is_cat && check_and_fill_for_cat(ref))
return -1;
 
/* Fill in specials first */
diff --git a/ref-filter.h b/ref-filter.h
index 5c6e019998716..69271e8c39f40 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -125,6 +125,7 @@ struct ref_format {
 * hopefully would be reduced later.
 */
struct expand_data *cat_file_data;
+   int is_cat;
 };
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }

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

[PATCH RFC v2 09/25] cat-file: start use ref_array_item struct

[Olga Telezhnaya]

Moving from using expand_data to ref_array_item structure.
That helps us to reuse functions from ref-filter easier.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 32 
 ref-filter.h   |  5 +
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 909412747cbd2..61b7acc79155d 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -183,27 +183,27 @@ static int is_atom(const char *atom, const char *s, int 
slen)
 }
 
 static void expand_atom(struct strbuf *sb, const char *atom, int len,
-struct expand_data *data)
+struct ref_array_item *item)
 {
if (is_atom("objectname", atom, len))
-   strbuf_addstr(sb, oid_to_hex(>oid));
+   strbuf_addstr(sb, oid_to_hex(>objectname));
else if (is_atom("objecttype", atom, len))
-   strbuf_addstr(sb, typename(data->type));
+   strbuf_addstr(sb, typename(item->type));
else if (is_atom("objectsize", atom, len))
-   strbuf_addf(sb, "%lu", data->size);
+   strbuf_addf(sb, "%lu", item->size);
else if (is_atom("objectsize:disk", atom, len))
-   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)item->disk_size);
else if (is_atom("rest", atom, len)) {
-   if (data->rest)
-   strbuf_addstr(sb, data->rest);
+   if (item->rest)
+   strbuf_addstr(sb, item->rest);
} else if (is_atom("deltabase", atom, len))
-   strbuf_addstr(sb, oid_to_hex(>delta_base_oid));
+   strbuf_addstr(sb, oid_to_hex(item->delta_base_oid));
 }
 
-static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
+static size_t expand_format(struct strbuf *sb, const char *start, void *data)
 {
const char *end;
-   struct expand_data *data = vdata;
+   struct ref_array_item *item = data;
 
if (*start != '(')
return 0;
@@ -211,7 +211,7 @@ static size_t expand_format(struct strbuf *sb, const char 
*start, void *vdata)
if (!end)
die("format element '%s' does not end in ')'", start);
 
-   expand_atom(sb, start + 1, end - start - 1, data);
+   expand_atom(sb, start + 1, end - start - 1, item);
return end - start + 1;
 }
 
@@ -283,6 +283,7 @@ static void batch_object_write(const char *obj_name, struct 
batch_options *opt,
   struct expand_data *data)
 {
struct strbuf buf = STRBUF_INIT;
+   struct ref_array_item item = {0};
 
if (!data->skip_object_info &&
sha1_object_info_extended(data->oid.hash, >info,
@@ -293,7 +294,14 @@ static void batch_object_write(const char *obj_name, 
struct batch_options *opt,
return;
}
 
-   strbuf_expand(, opt->format.format, expand_format, data);
+   item.objectname = data->oid;
+   item.type = data->type;
+   item.size = data->size;
+   item.disk_size = data->disk_size;
+   item.rest = data->rest;
+   item.delta_base_oid = >delta_base_oid;
+
+   strbuf_expand(, opt->format.format, expand_format, );
strbuf_addch(, '\n');
batch_write(opt, buf.buf, buf.len);
strbuf_release();
diff --git a/ref-filter.h b/ref-filter.h
index 52e07dbe6864a..781921d4e0978 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -40,6 +40,11 @@ struct ref_array_item {
const char *symref;
struct commit *commit;
struct atom_value *value;
+   enum object_type type;
+   unsigned long size;
+   off_t disk_size;
+   const char *rest;
+   struct object_id *delta_base_oid;
char refname[FLEX_ARRAY];
 };
 

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

[PATCH RFC v2 23/25] for-each-ref: tests for new atoms added

[Olga Telezhnaya]

Add tests for new formatting atoms: rest, deltabase, objectsize:disk.
rest means nothing and we expand it into empty string.
We need this atom for cat-file command.
Have plans to support deltabase and objectsize:disk further
(as it is done in cat-file), now also expand it to empty string.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 t/t6300-for-each-ref.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c128dfc579079..eee656a6abba9 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -316,6 +316,24 @@ test_expect_success 'exercise strftime with odd fields' '
test_cmp expected actual
 '
 
+test_expect_success 'Check format %(objectsize:disk) gives empty output ' '
+   echo >expected &&
+   git for-each-ref --format="%(objectsize:disk)" refs/heads >actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'Check format %(rest) gives empty output ' '
+   echo >expected &&
+   git for-each-ref --format="%(rest)" refs/heads >actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'Check format %(deltabase) gives empty output ' '
+   echo >expected &&
+   git for-each-ref --format="%(deltabase)" refs/heads >actual &&
+   test_cmp expected actual
+'
+
 cat >expected <<\EOF
 refs/heads/master
 refs/remotes/origin/master

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

[PATCH RFC v2 07/25] cat-file: start migrating formatting to ref-filter

[Olga Telezhnaya]

Move mark_atom_in_object_info() from cat-file to ref-filter and
start using it in verify_ref_format().
It also means that we start reusing verify_ref_format() in cat-file.

Start from simple moving of mark_atom_in_object_info(),
it would be removed later by integrating all needed processes into
ref-filter logic.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 30 +++---
 ref-filter.c   | 41 -
 ref-filter.h   | 12 ++--
 3 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index edb04a96d9bd3..909412747cbd2 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,25 +182,6 @@ static int is_atom(const char *atom, const char *s, int 
slen)
return alen == slen && !memcmp(atom, s, alen);
 }
 
-static void mark_atom_in_object_info(const char *atom, int len,
-struct expand_data *data)
-{
-   if (is_atom("objectname", atom, len))
-   ; /* do nothing */
-   else if (is_atom("objecttype", atom, len))
-   data->info.typep = >type;
-   else if (is_atom("objectsize", atom, len))
-   data->info.sizep = >size;
-   else if (is_atom("objectsize:disk", atom, len))
-   data->info.disk_sizep = >disk_size;
-   else if (is_atom("rest", atom, len))
-   data->split_on_whitespace = 1;
-   else if (is_atom("deltabase", atom, len))
-   data->info.delta_base_sha1 = data->delta_base_oid.hash;
-   else
-   die("unknown format element: %.*s", len, atom);
-}
-
 static void expand_atom(struct strbuf *sb, const char *atom, int len,
 struct expand_data *data)
 {
@@ -230,11 +211,7 @@ static size_t expand_format(struct strbuf *sb, const char 
*start, void *vdata)
if (!end)
die("format element '%s' does not end in ')'", start);
 
-   if (data->mark_query)
-   mark_atom_in_object_info(start + 1, end - start - 1, data);
-   else
-   expand_atom(sb, start + 1, end - start - 1, data);
-
+   expand_atom(sb, start + 1, end - start - 1, data);
return end - start + 1;
 }
 
@@ -418,9 +395,8 @@ static int batch_objects(struct batch_options *opt)
 * object.
 */
memset(, 0, sizeof(data));
-   data.mark_query = 1;
-   strbuf_expand(, opt->format.format, expand_format, );
-   data.mark_query = 0;
+   opt->format.cat_file_data = 
+   verify_ref_format(>format);
if (opt->cmdmode)
data.split_on_whitespace = 1;
 
diff --git a/ref-filter.c b/ref-filter.c
index 5e7ed0f338490..ff87e632f463c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -392,6 +392,31 @@ struct atom_value {
struct used_atom *atom;
 };
 
+static int is_atom(const char *atom, const char *s, int slen)
+{
+   int alen = strlen(atom);
+   return alen == slen && !memcmp(atom, s, alen);
+}
+
+static void mark_atom_in_object_info(const char *atom, int len,
+   struct expand_data *data)
+{
+   if (is_atom("objectname", atom, len))
+   ; /* do nothing */
+   else if (is_atom("objecttype", atom, len))
+   data->info.typep = >type;
+   else if (is_atom("objectsize", atom, len))
+   data->info.sizep = >size;
+   else if (is_atom("objectsize:disk", atom, len))
+   data->info.disk_sizep = >disk_size;
+   else if (is_atom("rest", atom, len))
+   data->split_on_whitespace = 1;
+   else if (is_atom("deltabase", atom, len))
+   data->info.delta_base_sha1 = data->delta_base_oid.hash;
+   else
+   die("unknown format element: %.*s", len, atom);
+}
+
 /*
  * Used to parse format string and sort specifiers
  */
@@ -709,12 +734,18 @@ int verify_ref_format(struct ref_format *format)
if (!ep)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
-   at = parse_ref_filter_atom(format, valid_atom,
-  ARRAY_SIZE(valid_atom), sp + 2, ep);
-   cp = ep + 1;
 
-   if (skip_prefix(used_atom[at].name, "color:", ))
-   format->need_color_reset_at_eol = !!strcmp(color, 
"reset");
+   if (format->cat_file_data)
+   mark_atom_in_object_info(sp + 2, ep - sp - 2,
+   format->cat_file_data);
+   else {
+   at = parse_ref_filter_atom(format, valid_atom,
+  ARRAY_SIZE(valid_atom), sp + 
2, ep);
+   if 

[PATCH RFC v2 21/25] ref-filter: unifying formatting of cat-file opts

[Olga Telezhnaya]

cat-file options are now filled by general logic.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 31 ++-
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 70c685851466b..aa15dd0b4723e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -825,8 +825,10 @@ static void grab_common_values(struct atom_value *val, int 
deref, struct object
else if (!strcmp(name, "objectsize")) {
v->value = sz;
v->s = xstrfmt("%lu", sz);
-   }
-   else if (deref)
+   } else if (!strcmp(name, "objectsize:disk")) {
+   v->value = cat_file_info.disk_size;
+   v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
+   } else if (deref)
grab_objectname(name, obj->oid.hash, v, _atom[i]);
}
 }
@@ -1466,21 +1468,7 @@ static int populate_value(struct ref_array_item *ref)
name++;
}
 
-   if (is_cat) {
-   if (starts_with(name, "objectname"))
-   v->s = oid_to_hex(>oid);
-   else if (starts_with(name, "objecttype"))
-   v->s = typename(ref->type);
-   else if (starts_with(name, "objectsize")) {
-   v->s = xstrfmt("%lu", ref->size);
-   } else if (starts_with(name, "objectsize:disk")) {
-   v->s = xstrfmt("%"PRIuMAX, 
(uintmax_t)ref->disk_size);
-   } else if (starts_with(name, "rest"))
-   v->s = ref->rest;
-   else if (starts_with(name, "deltabase"))
-   v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
-   continue;
-   } else if (starts_with(name, "refname"))
+   if (starts_with(name, "refname"))
refname = get_refname(atom, ref);
else if (starts_with(name, "symref"))
refname = get_symref(atom, ref);
@@ -1536,6 +1524,15 @@ static int populate_value(struct ref_array_item *ref)
else
v->s = " ";
continue;
+   } else if (starts_with(name, "rest")) {
+   v->s = ref->rest ? ref->rest : "";
+   continue;
+   } else if (starts_with(name, "deltabase")) {
+   if (ref->delta_base_oid)
+   v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
+   else
+   v->s = "";
+   continue;
} else if (starts_with(name, "align")) {
v->handler = align_atom_handler;
continue;

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

[PATCH RFC v2 20/25] ref-filter: make populate_value() internal again

[Olga Telezhnaya]

Remove populate_value() from header file. We needed that
for interim step, now it could be returned back.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 2 +-
 ref-filter.h | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index e34580e8db508..70c685851466b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1429,7 +1429,7 @@ static int check_and_fill_for_cat(struct ref_array_item 
*ref)
  * Parse the object referred by ref, and grab needed value.
  * Return 0 if everything was successful, -1 otherwise.
  */
-int populate_value(struct ref_array_item *ref)
+static int populate_value(struct ref_array_item *ref)
 {
void *buf;
struct object *obj;
diff --git a/ref-filter.h b/ref-filter.h
index 244a27bfc4e12..c0edb17aa404a 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -177,9 +177,6 @@ void setup_ref_filter_porcelain_msg(void);
 void pretty_print_ref(const char *name, const unsigned char *sha1,
  const struct ref_format *format);
 
-/* Fill the values of request and prepare all data for final string creation */
-int populate_value(struct ref_array_item *ref);
-
 /* Search for atom in given format. */
 int is_atom_used(const struct ref_format *format, const char *atom);
 

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

[PATCH RFC v2 04/25] ref-filter: make valid_atom as function parameter

[Olga Telezhnaya]

Make valid_atom as a function parameter,
there could be another variable further.
Need that for further reusing of formatting logic in cat-file.c.

We do not need to allow users to pass their own valid_atom variable in
global functions like verify_ref_format() because in the end we want to
have same set of valid atoms for all commands. But, as a first step
of migrating, I create further another version of valid_atom
for cat-file.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 9ed5e66066a7a..5e7ed0f338490 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -325,7 +325,7 @@ static void head_atom_parser(const struct ref_format 
*format, struct used_atom *
atom->u.head = resolve_refdup("HEAD", RESOLVE_REF_READING, NULL, NULL);
 }
 
-static struct {
+static struct valid_atom {
const char *name;
cmp_type cmp_type;
void (*parser)(const struct ref_format *format, struct used_atom *atom, 
const char *arg);
@@ -396,6 +396,7 @@ struct atom_value {
  * Used to parse format string and sort specifiers
  */
 static int parse_ref_filter_atom(const struct ref_format *format,
+const struct valid_atom *valid_atom, int 
n_atoms,
 const char *atom, const char *ep)
 {
const char *sp;
@@ -425,13 +426,13 @@ static int parse_ref_filter_atom(const struct ref_format 
*format,
atom_len = (arg ? arg : ep) - sp;
 
/* Is the atom a valid one? */
-   for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
+   for (i = 0; i < n_atoms; i++) {
int len = strlen(valid_atom[i].name);
if (len == atom_len && !memcmp(valid_atom[i].name, sp, len))
break;
}
 
-   if (ARRAY_SIZE(valid_atom) <= i)
+   if (n_atoms <= i)
die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
 
/* Add it in, including the deref prefix */
@@ -708,7 +709,8 @@ int verify_ref_format(struct ref_format *format)
if (!ep)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
-   at = parse_ref_filter_atom(format, sp + 2, ep);
+   at = parse_ref_filter_atom(format, valid_atom,
+  ARRAY_SIZE(valid_atom), sp + 2, ep);
cp = ep + 1;
 
if (skip_prefix(used_atom[at].name, "color:", ))
@@ -2145,7 +2147,9 @@ int format_ref_array_item(struct ref_array_item *info,
if (cp < sp)
append_literal(cp, sp, );
if (get_ref_atom_value(info,
-  parse_ref_filter_atom(format, sp + 2, 
ep),
+  parse_ref_filter_atom(format, valid_atom,
+
ARRAY_SIZE(valid_atom),
+sp + 2, ep),
   ))
return -1;
atomv->handler(atomv, );
@@ -2198,7 +2202,8 @@ static int parse_sorting_atom(const char *atom)
 */
struct ref_format dummy = REF_FORMAT_INIT;
const char *end = atom + strlen(atom);
-   return parse_ref_filter_atom(, atom, end);
+   return parse_ref_filter_atom(, valid_atom,
+ARRAY_SIZE(valid_atom), atom, end);
 }
 
 /*  If no sorting option is given, use refname to sort as default */

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

[PATCH RFC v2 15/25] ref_filter: add is_atom_used function

[Olga Telezhnaya]

Delete all items related to split_on_whitespace from ref-filter
and add new function for handling the logic.
Now cat-file could invoke that function to implementing its logic.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c |  8 +++-
 ref-filter.c   | 17 +++--
 ref-filter.h   | 10 +++---
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index e8e788f41b890..a55138f1fd1d1 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -382,8 +382,7 @@ static int batch_objects(struct batch_options *opt)
 {
struct strbuf buf = STRBUF_INIT;
struct expand_data data;
-   int save_warning;
-   int retval = 0;
+   int save_warning, is_rest, retval = 0;
 
if (!opt->format.format)
opt->format.format = "%(objectname) %(objecttype) 
%(objectsize)";
@@ -397,8 +396,6 @@ static int batch_objects(struct batch_options *opt)
opt->format.cat_file_data = 
opt->format.is_cat = 1;
verify_ref_format(>format);
-   if (opt->cmdmode)
-   data.split_on_whitespace = 1;
 
if (opt->all_objects) {
struct object_info empty = OBJECT_INFO_INIT;
@@ -437,9 +434,10 @@ static int batch_objects(struct batch_options *opt)
 */
save_warning = warn_on_object_refname_ambiguity;
warn_on_object_refname_ambiguity = 0;
+   is_rest = opt->cmdmode || is_atom_used(>format, "rest");
 
while (strbuf_getline(, stdin) != EOF) {
-   if (data.split_on_whitespace) {
+   if (is_rest) {
/*
 * Split at first whitespace, tying off the beginning
 * of the string and saving the remainder (or NULL) in
diff --git a/ref-filter.c b/ref-filter.c
index 91290b62450b3..bbcd507d179a9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -494,8 +494,6 @@ static int parse_ref_filter_atom(const struct ref_format 
*format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
-   if (is_cat && !strcmp(valid_atom[i].name, "rest"))
-   cat_file_info->split_on_whitespace = 1;
return at;
 }
 
@@ -731,6 +729,21 @@ static const char *find_next(const char *cp)
return NULL;
 }
 
+/* Search for atom in given format. */
+int is_atom_used(const struct ref_format *format, const char *atom)
+{
+   const char *cp, *sp;
+   for (cp = format->format; *cp && (sp = find_next(cp)); ) {
+   const char *ep = strchr(sp, ')');
+   int atom_len = ep - sp - 2;
+   sp += 2;
+   if (atom_len == strlen(atom) && !memcmp(sp, atom, atom_len))
+   return 1;
+   cp = ep + 1;
+   }
+   return 0;
+}
+
 /*
  * Make sure the format string is well formed, and parse out
  * the used atoms.
diff --git a/ref-filter.h b/ref-filter.h
index 69271e8c39f40..f590e5d694df4 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -86,13 +86,6 @@ struct expand_data {
const char *rest;
struct object_id delta_base_oid;
 
-   /*
-* Whether to split the input on whitespace before feeding it to
-* get_sha1; this is decided during the mark_query phase based on
-* whether we have a %(rest) token in our format.
-*/
-   int split_on_whitespace;
-
/*
 * After a mark_query run, this object_info is set up to be
 * passed to sha1_object_info_extended. It will point to the data
@@ -187,4 +180,7 @@ void pretty_print_ref(const char *name, const unsigned char 
*sha1,
 /* Fill the values of request and prepare all data for final string creation */
 int populate_value(struct ref_array_item *ref);
 
+/* Search for atom in given format. */
+int is_atom_used(const struct ref_format *format, const char *atom);
+
 #endif /*  REF_FILTER_H  */

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

[PATCH RFC v2 19/25] cat-file: reuse printing logic from ref-filter

[Olga Telezhnaya]

Reuse code from ref-filter to print resulting message.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 51 ---
 ref-filter.c   | 21 +++--
 2 files changed, 23 insertions(+), 49 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5b9869cdb9096..746b02ff150a7 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,45 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
return 0;
 }
 
-static int is_atom(const char *atom, const char *s, int slen)
-{
-   int alen = strlen(atom);
-   return alen == slen && !memcmp(atom, s, alen);
-}
-
-static void expand_atom(struct strbuf *sb, const char *atom, int len,
-struct ref_array_item *item)
-{
-   if (is_atom("objectname", atom, len))
-   strbuf_addstr(sb, oid_to_hex(>oid));
-   else if (is_atom("objecttype", atom, len))
-   strbuf_addstr(sb, typename(item->type));
-   else if (is_atom("objectsize", atom, len))
-   strbuf_addf(sb, "%lu", item->size);
-   else if (is_atom("objectsize:disk", atom, len))
-   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)item->disk_size);
-   else if (is_atom("rest", atom, len)) {
-   if (item->rest)
-   strbuf_addstr(sb, item->rest);
-   } else if (is_atom("deltabase", atom, len))
-   strbuf_addstr(sb, oid_to_hex(item->delta_base_oid));
-}
-
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
-{
-   const char *end;
-   struct ref_array_item *item = data;
-
-   if (*start != '(')
-   return 0;
-   end = strchr(start + 1, ')');
-   if (!end)
-   die("format element '%s' does not end in ')'", start);
-
-   expand_atom(sb, start + 1, end - start - 1, item);
-   return end - start + 1;
-}
-
 static void batch_write(struct batch_options *opt, const void *data, int len)
 {
if (opt->buffer_output) {
@@ -282,23 +243,19 @@ static void print_object_or_die(struct batch_options 
*opt, struct expand_data *d
 static void batch_object_write(const char *obj_name, struct batch_options *opt,
   struct expand_data *data)
 {
-   struct strbuf buf = STRBUF_INIT;
struct ref_array_item item = {0};
 
item.oid = data->oid;
item.rest = data->rest;
item.objectname = obj_name;
 
-   if (populate_value())
+   if (show_ref_array_item(, >format))
return;
-
-   data->type = item.type;
-   strbuf_expand(, opt->format.format, expand_format, );
-   strbuf_addch(, '\n');
-   batch_write(opt, buf.buf, buf.len);
-   strbuf_release();
+   if (!opt->buffer_output)
+   fflush(stdout);
 
if (opt->print_contents) {
+   data->type = item.type;
print_object_or_die(opt, data);
batch_write(opt, "\n", 1);
}
diff --git a/ref-filter.c b/ref-filter.c
index 3f3583ac515a5..e34580e8db508 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1466,7 +1466,21 @@ int populate_value(struct ref_array_item *ref)
name++;
}
 
-   if (starts_with(name, "refname"))
+   if (is_cat) {
+   if (starts_with(name, "objectname"))
+   v->s = oid_to_hex(>oid);
+   else if (starts_with(name, "objecttype"))
+   v->s = typename(ref->type);
+   else if (starts_with(name, "objectsize")) {
+   v->s = xstrfmt("%lu", ref->size);
+   } else if (starts_with(name, "objectsize:disk")) {
+   v->s = xstrfmt("%"PRIuMAX, 
(uintmax_t)ref->disk_size);
+   } else if (starts_with(name, "rest"))
+   v->s = ref->rest;
+   else if (starts_with(name, "deltabase"))
+   v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
+   continue;
+   } else if (starts_with(name, "refname"))
refname = get_refname(atom, ref);
else if (starts_with(name, "symref"))
refname = get_symref(atom, ref);
@@ -2208,6 +,7 @@ int format_ref_array_item(struct ref_array_item *info,
 {
const char *cp, *sp, *ep;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
+   int retval = 0;
 
state.quote_style = format->quote_style;
push_stack_element();
@@ -2224,6 +2239,8 @@ int format_ref_array_item(struct ref_array_item *info,
return -1;
atomv->handler(atomv, );
}
+   

[PATCH RFC v2 18/25] ref-filter: make valid_atom general again

[Olga Telezhnaya]

Stop using valid_cat_file_atom, making the code more general.
Further commits will contain some tests, docs and
support of new features.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 34 +-
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index a65a90790fd2c..3f3583ac515a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -359,8 +359,8 @@ static struct valid_atom {
void (*parser)(const struct ref_format *format, struct used_atom *atom, 
const char *arg);
 } valid_atom[] = {
{ "refname" , FIELD_STR, refname_atom_parser },
-   { "objecttype" },
-   { "objectsize", FIELD_ULONG },
+   { "objecttype", FIELD_STR, objecttype_atom_parser },
+   { "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "objectname", FIELD_STR, objectname_atom_parser },
{ "tree" },
{ "parent" },
@@ -397,12 +397,6 @@ static struct valid_atom {
{ "if", FIELD_STR, if_atom_parser },
{ "then" },
{ "else" },
-};
-
-static struct valid_atom valid_cat_file_atom[] = {
-   { "objectname" },
-   { "objecttype", FIELD_STR, objecttype_atom_parser },
-   { "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "rest" },
{ "deltabase", FIELD_STR, deltabase_atom_parser },
 };
@@ -432,7 +426,6 @@ struct atom_value {
  * Used to parse format string and sort specifiers
  */
 static int parse_ref_filter_atom(const struct ref_format *format,
-const struct valid_atom *valid_atom, int 
n_atoms,
 const char *atom, const char *ep)
 {
const char *sp;
@@ -462,13 +455,13 @@ static int parse_ref_filter_atom(const struct ref_format 
*format,
atom_len = (arg ? arg : ep) - sp;
 
/* Is the atom a valid one? */
-   for (i = 0; i < n_atoms; i++) {
+   for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
int len = strlen(valid_atom[i].name);
if (len == atom_len && !memcmp(valid_atom[i].name, sp, len))
break;
}
 
-   if (n_atoms <= i)
+   if (ARRAY_SIZE(valid_atom) <= i)
die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
 
/* Add it in, including the deref prefix */
@@ -762,15 +755,9 @@ int verify_ref_format(struct ref_format *format)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
 
-   if (is_cat)
-   at = parse_ref_filter_atom(format, valid_cat_file_atom,
-  
ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
-   else {
-   at = parse_ref_filter_atom(format, valid_atom,
-  ARRAY_SIZE(valid_atom), sp + 
2, ep);
-   if (skip_prefix(used_atom[at].name, "color:", ))
-   format->need_color_reset_at_eol = 
!!strcmp(color, "reset");
-   }
+   at = parse_ref_filter_atom(format, sp + 2, ep);
+   if (skip_prefix(used_atom[at].name, "color:", ))
+   format->need_color_reset_at_eol = !!strcmp(color, 
"reset");
 
cp = ep + 1;
}
@@ -2232,9 +2219,7 @@ int format_ref_array_item(struct ref_array_item *info,
if (cp < sp)
append_literal(cp, sp, );
if (get_ref_atom_value(info,
-  parse_ref_filter_atom(format, valid_atom,
-
ARRAY_SIZE(valid_atom),
-sp + 2, ep),
+  parse_ref_filter_atom(format, sp + 2, 
ep),
   ))
return -1;
atomv->handler(atomv, );
@@ -2287,8 +2272,7 @@ static int parse_sorting_atom(const char *atom)
 */
struct ref_format dummy = REF_FORMAT_INIT;
const char *end = atom + strlen(atom);
-   return parse_ref_filter_atom(, valid_atom,
-ARRAY_SIZE(valid_atom), atom, end);
+   return parse_ref_filter_atom(, atom, end);
 }
 
 /*  If no sorting option is given, use refname to sort as default */

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

[PATCH RFC v2 25/25] cat-file: update of docs

[Olga Telezhnaya]

Update the docs for cat-file command. Some new formatting atoms added
because of reusing ref-filter code.
We do not support cat-file atoms in general formatting logic
(there is just the support for cat-file), that is why some of the atoms
are still explained in cat-file docs.
We need to move these explanations when atoms will be supported
by other commands.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 Documentation/git-cat-file.txt | 13 ++---
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index f90f09b03fae5..90639ac21d0e8 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -187,17 +187,8 @@ linkgit:git-rev-parse[1].
 You can specify the information shown for each object by using a custom
 ``. The `` is copied literally to stdout for each
 object, with placeholders of the form `%(atom)` expanded, followed by a
-newline. The available atoms are:
-
-`objectname`::
-   The 40-hex object name of the object.
-
-`objecttype`::
-   The type of the object (the same as `cat-file -t` reports).
-
-`objectsize`::
-   The size, in bytes, of the object (the same as `cat-file -s`
-   reports).
+newline. The available atoms are the same as that of
+linkgit:git-for-each-ref[1], but there are some additional ones:
 
 `objectsize:disk`::
The size, in bytes, that the object takes up on disk. See the

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

[PATCH RFC v2 10/25] ref-filter: make populate_value() global

[Olga Telezhnaya]

Make function global for further using in cat-file.
In the end of patch series this function becomes internal again,
so this is a part of middle step. cat-file would use more general
functions further.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 2 +-
 ref-filter.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index 5c75259b1ab8c..4acd391b5dfac 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1407,7 +1407,7 @@ static const char *get_refname(struct used_atom *atom, 
struct ref_array_item *re
  * Parse the object referred by ref, and grab needed value.
  * Return 0 if everything was successful, -1 otherwise.
  */
-static int populate_value(struct ref_array_item *ref)
+int populate_value(struct ref_array_item *ref)
 {
void *buf;
struct object *obj;
diff --git a/ref-filter.h b/ref-filter.h
index 781921d4e0978..e16ea2a990119 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -182,4 +182,7 @@ void setup_ref_filter_porcelain_msg(void);
 void pretty_print_ref(const char *name, const unsigned char *sha1,
  const struct ref_format *format);
 
+/* Fill the values of request and prepare all data for final string creation */
+int populate_value(struct ref_array_item *ref);
+
 #endif /*  REF_FILTER_H  */

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

[PATCH RFC v2 12/25] cat-file: start reusing populate_value()

[Olga Telezhnaya]

Move logic related to getting object info from cat-file to ref-filter.
It will help to reuse whole formatting logic from ref-filter further.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 17 -
 ref-filter.c   | 20 
 ref-filter.h   |  1 +
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 367b1bd5802dc..179c955b86bd5 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -285,21 +285,12 @@ static void batch_object_write(const char *obj_name, 
struct batch_options *opt,
struct strbuf buf = STRBUF_INIT;
struct ref_array_item item = {0};
 
-   if (!data->skip_object_info &&
-   sha1_object_info_extended(data->oid.hash, >info,
- OBJECT_INFO_LOOKUP_REPLACE) < 0) {
-   printf("%s missing\n",
-  obj_name ? obj_name : oid_to_hex(>oid));
-   fflush(stdout);
-   return;
-   }
-
item.oid = data->oid;
-   item.type = data->type;
-   item.size = data->size;
-   item.disk_size = data->disk_size;
item.rest = data->rest;
-   item.delta_base_oid = >delta_base_oid;
+   item.objectname = obj_name;
+
+   if (populate_value())
+   return;
 
strbuf_expand(, opt->format.format, expand_format, );
strbuf_addch(, '\n');
diff --git a/ref-filter.c b/ref-filter.c
index d09ec1bde6d54..3f92a27d98b6c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1403,6 +1403,23 @@ static const char *get_refname(struct used_atom *atom, 
struct ref_array_item *re
return show_ref(>u.refname, ref->refname);
 }
 
+static int check_and_fill_for_cat(struct ref_array_item *ref)
+{
+   if (!cat_file_info->skip_object_info &&
+   sha1_object_info_extended(ref->oid.hash, _file_info->info,
+ OBJECT_INFO_LOOKUP_REPLACE) < 0) {
+   const char *e = ref->objectname;
+   printf("%s missing\n", e ? e : oid_to_hex(>oid));
+   fflush(stdout);
+   return -1;
+   }
+   ref->type = cat_file_info->type;
+   ref->size = cat_file_info->size;
+   ref->disk_size = cat_file_info->disk_size;
+   ref->delta_base_oid = _file_info->delta_base_oid;
+   return 0;
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  * Return 0 if everything was successful, -1 otherwise.
@@ -1424,6 +1441,9 @@ int populate_value(struct ref_array_item *ref)
ref->symref = "";
}
 
+   if (cat_file_info && check_and_fill_for_cat(ref))
+   return -1;
+
/* Fill in specials first */
for (i = 0; i < used_atom_cnt; i++) {
struct used_atom *atom = _atom[i];
diff --git a/ref-filter.h b/ref-filter.h
index 87b026b8b76d0..5c6e019998716 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -45,6 +45,7 @@ struct ref_array_item {
off_t disk_size;
const char *rest;
struct object_id *delta_base_oid;
+   const char *objectname;
char refname[FLEX_ARRAY];
 };
 

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

[PATCH RFC v2 03/25] cat-file: reuse struct ref_format

[Olga Telezhnaya]

Start using ref_format struct instead of simple char*.
Need that for further reusing of formatting logic from ref-filter.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f5fa4fd75af26..98fc5ec069a49 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -13,15 +13,16 @@
 #include "tree-walk.h"
 #include "sha1-array.h"
 #include "packfile.h"
+#include "ref-filter.h"
 
 struct batch_options {
+   struct ref_format format;
int enabled;
int follow_symlinks;
int print_contents;
int buffer_output;
int all_objects;
int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
-   const char *format;
 };
 
 static const char *force_path;
@@ -348,7 +349,7 @@ static void batch_object_write(const char *obj_name, struct 
batch_options *opt,
return;
}
 
-   strbuf_expand(, opt->format, expand_format, data);
+   strbuf_expand(, opt->format.format, expand_format, data);
strbuf_addch(, '\n');
batch_write(opt, buf.buf, buf.len);
strbuf_release();
@@ -441,8 +442,8 @@ static int batch_objects(struct batch_options *opt)
int save_warning;
int retval = 0;
 
-   if (!opt->format)
-   opt->format = "%(objectname) %(objecttype) %(objectsize)";
+   if (!opt->format.format)
+   opt->format.format = "%(objectname) %(objecttype) 
%(objectsize)";
 
/*
 * Expand once with our special mark_query flag, which will prime the
@@ -451,7 +452,7 @@ static int batch_objects(struct batch_options *opt)
 */
memset(, 0, sizeof(data));
data.mark_query = 1;
-   strbuf_expand(, opt->format, expand_format, );
+   strbuf_expand(, opt->format.format, expand_format, );
data.mark_query = 0;
if (opt->cmdmode)
data.split_on_whitespace = 1;
@@ -543,7 +544,7 @@ static int batch_option_callback(const struct option *opt,
 
bo->enabled = 1;
bo->print_contents = !strcmp(opt->long_name, "batch");
-   bo->format = arg;
+   bo->format.format = arg;
 
return 0;
 }
@@ -552,7 +553,7 @@ int cmd_cat_file(int argc, const char **argv, const char 
*prefix)
 {
int opt = 0;
const char *exp_type = NULL, *obj_name = NULL;
-   struct batch_options batch = {0};
+   struct batch_options batch = { REF_FORMAT_INIT };
int unknown_type = 0;
 
const struct option options[] = {

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

[PATCH RFC v2 02/25] ref-filter: add return value to some functions

[Olga Telezhnaya]

Add return flag to format_ref_array_item(), show_ref_array_item(),
get_ref_array_info() and populate_value() for further using.
Need it to handle situations when item is broken but we can not invoke
die() because we are in batch mode and all items need to be processed.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 37 -
 ref-filter.h | 14 ++
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index d04295e33448e..9ed5e66066a7a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1356,8 +1356,9 @@ static const char *get_refname(struct used_atom *atom, 
struct ref_array_item *re
 
 /*
  * Parse the object referred by ref, and grab needed value.
+ * Return 0 if everything was successful, -1 otherwise.
  */
-static void populate_value(struct ref_array_item *ref)
+static int populate_value(struct ref_array_item *ref)
 {
void *buf;
struct object *obj;
@@ -1482,7 +1483,7 @@ static void populate_value(struct ref_array_item *ref)
}
}
if (used_atom_cnt <= i)
-   return;
+   return 0;
 
buf = get_obj(>objectname, , , );
if (!buf)
@@ -1501,7 +1502,7 @@ static void populate_value(struct ref_array_item *ref)
 * object, we are done.
 */
if (!need_tagged || (obj->type != OBJ_TAG))
-   return;
+   return 0;
 
/*
 * If it is a tag object, see if we use a value that derefs
@@ -1525,19 +1526,24 @@ static void populate_value(struct ref_array_item *ref)
grab_values(ref->value, 1, obj, buf, size);
if (!eaten)
free(buf);
+
+   return 0;
 }
 
 /*
  * Given a ref, return the value for the atom.  This lazily gets value
  * out of the object by calling populate value.
+ * Return 0 if everything was successful, -1 otherwise.
  */
-static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct 
atom_value **v)
+static int get_ref_atom_value(struct ref_array_item *ref, int atom, struct 
atom_value **v)
 {
+   int retval = 0;
if (!ref->value) {
-   populate_value(ref);
+   retval = populate_value(ref);
fill_missing_values(ref->value);
}
*v = >value[atom];
+   return retval;
 }
 
 /*
@@ -2122,7 +2128,7 @@ static void append_literal(const char *cp, const char 
*ep, struct ref_formatting
}
 }
 
-void format_ref_array_item(struct ref_array_item *info,
+int format_ref_array_item(struct ref_array_item *info,
   const struct ref_format *format,
   struct strbuf *final_buf)
 {
@@ -2138,9 +2144,10 @@ void format_ref_array_item(struct ref_array_item *info,
ep = strchr(sp, ')');
if (cp < sp)
append_literal(cp, sp, );
-   get_ref_atom_value(info,
-  parse_ref_filter_atom(format, sp + 2, ep),
-  );
+   if (get_ref_atom_value(info,
+  parse_ref_filter_atom(format, sp + 2, 
ep),
+  ))
+   return -1;
atomv->handler(atomv, );
}
if (*cp) {
@@ -2156,17 +2163,21 @@ void format_ref_array_item(struct ref_array_item *info,
die(_("format: %%(end) atom missing"));
strbuf_addbuf(final_buf, >output);
pop_stack_element();
+   return 0;
 }
 
-void show_ref_array_item(struct ref_array_item *info,
+int show_ref_array_item(struct ref_array_item *info,
 const struct ref_format *format)
 {
struct strbuf final_buf = STRBUF_INIT;
+   int retval = format_ref_array_item(info, format, _buf);
 
-   format_ref_array_item(info, format, _buf);
-   fwrite(final_buf.buf, 1, final_buf.len, stdout);
+   if (!retval) {
+   fwrite(final_buf.buf, 1, final_buf.len, stdout);
+   putchar('\n');
+   }
strbuf_release(_buf);
-   putchar('\n');
+   return retval;
 }
 
 void pretty_print_ref(const char *name, const unsigned char *sha1,
diff --git a/ref-filter.h b/ref-filter.h
index 0d98342b34319..b75c8ac45248e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,12 +109,18 @@ void ref_array_clear(struct ref_array *array);
 int verify_ref_format(struct ref_format *format);
 /*  Sort the given ref_array as per the ref_sorting provided */
 void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
-/*  Based on the given format and quote_style, fill the strbuf */
-void format_ref_array_item(struct ref_array_item *info,
+/*
+ * Based on the given format and quote_style, fill the strbuf.
+ * Return 0 if everything was successful, -1 otherwise (and strbuf remains 

[PATCH RFC v2 24/25] cat-file: tests for new atoms added

[Olga Telezhnaya]

Add some tests for new formatting atoms from ref-filter.
Some of new atoms are supported automatically,
some of them are expanded into empty string
(because they are useless for some types of objects),
some of them could be supported later in other patches.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 t/t1006-cat-file.sh | 48 
 1 file changed, 48 insertions(+)

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index b19f332694620..e72fcaf0e02c5 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -20,6 +20,19 @@ maybe_remove_timestamp () {
 fi
 }
 
+test_atom () {
+name=$1
+sha1=$2
+atoms=$3
+expected=$4
+
+test_expect_success "$name" '
+   echo "$expected" >expect &&
+   echo $sha1 | git cat-file --batch-check="$atoms" >actual &&
+   test_cmp expect actual
+'
+}
+
 run_tests () {
 type=$1
 sha1=$2
@@ -119,6 +132,13 @@ $content"
maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
test_cmp expect actual
 '
+
+for atom in refname parent body trailers upstream push symref flag
+do
+   test_atom "Check %($atom) gives empty output" "$sha1" "%($atom)" ""
+done
+
+test_atom "Check %(HEAD) gives only one space as output" "$sha1" '%(HEAD)' 
' '
 }
 
 hello_content="Hello World"
@@ -140,6 +160,12 @@ test_expect_success '--batch-check without %(rest) 
considers whole line' '
test_cmp expect actual
 '
 
+shortname=`echo $hello_sha1 | sed 's/^.\{0\}\(.\{7\}\).*/\1/'`
+test_atom 'Check format option %(objectname:short) works' "$hello_sha1" 
'%(objectname:short)' "$shortname"
+
+test_atom 'Check format option %(align) is not broken' \
+"$hello_sha1" "%(align:8)%(objecttype)%(end)%(objectname)" "blob
$hello_sha1"
+
 tree_sha1=$(git write-tree)
 tree_size=33
 tree_pretty_content="100644 blob $hello_sha1   hello"
@@ -157,6 +183,17 @@ $commit_message"
 
 run_tests 'commit' $commit_sha1 $commit_size "$commit_content" 
"$commit_content" 1
 
+test_atom "Check format option %(if) is not broken" "$commit_sha1" \
+"%(if)%(author)%(then)%(objectname)%(end)" "$commit_sha1"
+test_atom "Check %(tree) works for commit" "$commit_sha1" "%(tree)" 
"$tree_sha1"
+test_atom "Check %(numparent) works for commit" "$commit_sha1" "%(numparent)" 
"0"
+test_atom "Check %(authorname) works for commit" "$commit_sha1" 
"%(authorname)" "$GIT_AUTHOR_NAME"
+test_atom "Check %(authoremail) works for commit" "$commit_sha1" 
"%(authoremail)" "<$GIT_AUTHOR_EMAIL>"
+test_atom "Check %(committername) works for commit" "$commit_sha1" 
"%(committername)" "$GIT_COMMITTER_NAME"
+test_atom "Check %(committeremail) works for commit" "$commit_sha1" 
"%(committeremail)" "<$GIT_COMMITTER_EMAIL>"
+test_atom "Check %(subject) works for commit" "$commit_sha1" "%(subject)" 
"$commit_message"
+test_atom "Check %(contents) works for commit" "$commit_sha1" "%(contents)" 
"$commit_message"
+
 tag_header_without_timestamp="object $hello_sha1
 type blob
 tag hellotag
@@ -171,6 +208,17 @@ tag_size=$(strlen "$tag_content")
 
 run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_content" 1
 
+test_atom "Check %(object) works for tag" "$tag_sha1" "%(object)" "$hello_sha1"
+test_atom "Check %(type) works for tag" "$tag_sha1" "%(type)" "blob"
+test_atom "Check %(tag) works for tag" "$tag_sha1" "%(tag)" "hellotag"
+test_atom "Check %(taggername) works for tag" "$tag_sha1" "%(taggername)" 
"$GIT_COMMITTER_NAME"
+test_atom "Check %(taggeremail) works for tag" "$tag_sha1" "%(taggeremail)" 
"<$GIT_COMMITTER_EMAIL>"
+test_atom "Check %(subject) works for tag" "$tag_sha1" "%(subject)" 
"$tag_description"
+test_atom "Check %(contents) works for tag" "$tag_sha1" "%(contents)" 
"$tag_description"
+
+test_atom "Check %(color) gives no additional output" "$sha1" \
+"%(objectname) %(color:green) %(objecttype)" "$sha1  $type"
+
 test_expect_success \
 "Reach a blob from a tag pointing to it" \
 "test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\""

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

[PATCH RFC v2 01/25] ref-filter: get rid of goto

[Olga Telezhnaya]

Get rid of goto command in ref-filter for better readability.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index f9e25aea7a97e..d04295e33448e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1477,12 +1477,13 @@ static void populate_value(struct ref_array_item *ref)
 
for (i = 0; i < used_atom_cnt; i++) {
struct atom_value *v = >value[i];
-   if (v->s == NULL)
-   goto need_obj;
+   if (v->s == NULL) {
+   break;
+   }
}
-   return;
+   if (used_atom_cnt <= i)
+   return;
 
- need_obj:
buf = get_obj(>objectname, , , );
if (!buf)
die(_("missing object %s for %s"),

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

[PATCH RFC v2 06/25] cat-file: split expand_atom() into 2 functions

[Olga Telezhnaya]

Split expand_atom() into 2 different functions,
mark_atom_in_object_info() prepares variable for further filling,
(new) expand_atom() creates resulting string.
Need that for further reusing of formatting logic from ref-filter.
Both functions will be step-by-step removed by the end of this patch.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 73 --
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 37d6096d201b5..edb04a96d9bd3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,47 +182,47 @@ static int is_atom(const char *atom, const char *s, int 
slen)
return alen == slen && !memcmp(atom, s, alen);
 }
 
-static void expand_atom(struct strbuf *sb, const char *atom, int len,
-   void *vdata)
+static void mark_atom_in_object_info(const char *atom, int len,
+struct expand_data *data)
 {
-   struct expand_data *data = vdata;
+   if (is_atom("objectname", atom, len))
+   ; /* do nothing */
+   else if (is_atom("objecttype", atom, len))
+   data->info.typep = >type;
+   else if (is_atom("objectsize", atom, len))
+   data->info.sizep = >size;
+   else if (is_atom("objectsize:disk", atom, len))
+   data->info.disk_sizep = >disk_size;
+   else if (is_atom("rest", atom, len))
+   data->split_on_whitespace = 1;
+   else if (is_atom("deltabase", atom, len))
+   data->info.delta_base_sha1 = data->delta_base_oid.hash;
+   else
+   die("unknown format element: %.*s", len, atom);
+}
 
-   if (is_atom("objectname", atom, len)) {
-   if (!data->mark_query)
-   strbuf_addstr(sb, oid_to_hex(>oid));
-   } else if (is_atom("objecttype", atom, len)) {
-   if (data->mark_query)
-   data->info.typep = >type;
-   else
-   strbuf_addstr(sb, typename(data->type));
-   } else if (is_atom("objectsize", atom, len)) {
-   if (data->mark_query)
-   data->info.sizep = >size;
-   else
-   strbuf_addf(sb, "%lu", data->size);
-   } else if (is_atom("objectsize:disk", atom, len)) {
-   if (data->mark_query)
-   data->info.disk_sizep = >disk_size;
-   else
-   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
-   } else if (is_atom("rest", atom, len)) {
-   if (data->mark_query)
-   data->split_on_whitespace = 1;
-   else if (data->rest)
+static void expand_atom(struct strbuf *sb, const char *atom, int len,
+struct expand_data *data)
+{
+   if (is_atom("objectname", atom, len))
+   strbuf_addstr(sb, oid_to_hex(>oid));
+   else if (is_atom("objecttype", atom, len))
+   strbuf_addstr(sb, typename(data->type));
+   else if (is_atom("objectsize", atom, len))
+   strbuf_addf(sb, "%lu", data->size);
+   else if (is_atom("objectsize:disk", atom, len))
+   strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+   else if (is_atom("rest", atom, len)) {
+   if (data->rest)
strbuf_addstr(sb, data->rest);
-   } else if (is_atom("deltabase", atom, len)) {
-   if (data->mark_query)
-   data->info.delta_base_sha1 = data->delta_base_oid.hash;
-   else
-   strbuf_addstr(sb,
- oid_to_hex(>delta_base_oid));
-   } else
-   die("unknown format element: %.*s", len, atom);
+   } else if (is_atom("deltabase", atom, len))
+   strbuf_addstr(sb, oid_to_hex(>delta_base_oid));
 }
 
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
+static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
 {
const char *end;
+   struct expand_data *data = vdata;
 
if (*start != '(')
return 0;
@@ -230,7 +230,10 @@ static size_t expand_format(struct strbuf *sb, const char 
*start, void *data)
if (!end)
die("format element '%s' does not end in ')'", start);
 
-   expand_atom(sb, start + 1, end - start - 1, data);
+   if (data->mark_query)
+   mark_atom_in_object_info(start + 1, end - start - 1, data);
+   else
+   expand_atom(sb, start + 1, end - start - 1, data);
 
return end - start + 1;
 }

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

[PATCH RFC v2 13/25] ref-filter: get rid of mark_atom_in_object_info()

[Olga Telezhnaya]

Remove mark_atom_in_object_info() and create same logic
in terms of ref-filter style.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 45 +
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 3f92a27d98b6c..34a54db168265 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -255,13 +255,29 @@ static void objectname_atom_parser(const struct 
ref_format *format, struct used_
 static void objectsize_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
 {
if (!arg)
-   ; /* default to normal object size */
+   cat_file_info->info.sizep = _file_info->size;
else if (!strcmp(arg, "disk"))
cat_file_info->info.disk_sizep = _file_info->disk_size;
else
die(_("urecognized %%(objectsize) argument: %s"), arg);
 }
 
+static void objecttype_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
+{
+   if (!arg)
+   cat_file_info->info.typep = _file_info->type;
+   else
+   die(_("urecognized %%(objecttype) argument: %s"), arg);
+}
+
+static void deltabase_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
+{
+   if (!arg)
+   cat_file_info->info.delta_base_sha1 = 
cat_file_info->delta_base_oid.hash;
+   else
+   die(_("urecognized %%(deltabase) argument: %s"), arg);
+}
+
 static void refname_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
 {
refname_atom_parser_internal(>u.refname, arg, atom->name);
@@ -384,10 +400,10 @@ static struct valid_atom {
 
 static struct valid_atom valid_cat_file_atom[] = {
{ "objectname" },
-   { "objecttype" },
+   { "objecttype", FIELD_STR, objecttype_atom_parser },
{ "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "rest" },
-   { "deltabase" },
+   { "deltabase", FIELD_STR, deltabase_atom_parser },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -411,25 +427,6 @@ struct atom_value {
struct used_atom *atom;
 };
 
-static int is_atom(const char *atom, const char *s, int slen)
-{
-   int alen = strlen(atom);
-   return alen == slen && !memcmp(atom, s, alen);
-}
-
-static void mark_atom_in_object_info(const char *atom, int len,
-   struct expand_data *data)
-{
-   if (is_atom("objecttype", atom, len))
-   data->info.typep = >type;
-   else if (is_atom("objectsize", atom, len))
-   data->info.sizep = >size;
-   else if (is_atom("rest", atom, len))
-   data->split_on_whitespace = 1;
-   else if (is_atom("deltabase", atom, len))
-   data->info.delta_base_sha1 = data->delta_base_oid.hash;
-}
-
 /*
  * Used to parse format string and sort specifiers
  */
@@ -496,8 +493,8 @@ static int parse_ref_filter_atom(const struct ref_format 
*format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
-   if (cat_file_info)
-   mark_atom_in_object_info(atom, atom_len, cat_file_info);
+   if (cat_file_info && !strcmp(valid_atom[i].name, "rest"))
+   cat_file_info->split_on_whitespace = 1;
return at;
 }
 

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

[PATCH RFC v2 11/25] ref-filter: rename field in ref_array_item stuct

[Olga Telezhnaya]

Rename objectname field to oid in struct ref_array_item.
Next commit will add objectname field that will contain
string representation of object id.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c |  4 ++--
 ref-filter.c   | 10 +-
 ref-filter.h   |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 61b7acc79155d..367b1bd5802dc 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -186,7 +186,7 @@ static void expand_atom(struct strbuf *sb, const char 
*atom, int len,
 struct ref_array_item *item)
 {
if (is_atom("objectname", atom, len))
-   strbuf_addstr(sb, oid_to_hex(>objectname));
+   strbuf_addstr(sb, oid_to_hex(>oid));
else if (is_atom("objecttype", atom, len))
strbuf_addstr(sb, typename(item->type));
else if (is_atom("objectsize", atom, len))
@@ -294,7 +294,7 @@ static void batch_object_write(const char *obj_name, struct 
batch_options *opt,
return;
}
 
-   item.objectname = data->oid;
+   item.oid = data->oid;
item.type = data->type;
item.size = data->size;
item.disk_size = data->disk_size;
diff --git a/ref-filter.c b/ref-filter.c
index 4acd391b5dfac..d09ec1bde6d54 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1489,7 +1489,7 @@ int populate_value(struct ref_array_item *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   } else if (!deref && grab_objectname(name, 
ref->objectname.hash, v, atom)) {
+   } else if (!deref && grab_objectname(name, ref->oid.hash, v, 
atom)) {
continue;
} else if (!strcmp(name, "HEAD")) {
if (atom->u.head && !strcmp(ref->refname, atom->u.head))
@@ -1534,13 +1534,13 @@ int populate_value(struct ref_array_item *ref)
if (used_atom_cnt <= i)
return 0;
 
-   buf = get_obj(>objectname, , , );
+   buf = get_obj(>oid, , , );
if (!buf)
die(_("missing object %s for %s"),
-   oid_to_hex(>objectname), ref->refname);
+   oid_to_hex(>oid), ref->refname);
if (!obj)
die(_("parse_object_buffer failed on %s for %s"),
-   oid_to_hex(>objectname), ref->refname);
+   oid_to_hex(>oid), ref->refname);
 
grab_values(ref->value, 0, obj, buf, size);
if (!eaten)
@@ -1890,7 +1890,7 @@ static struct ref_array_item *new_ref_array_item(const 
char *refname,
 {
struct ref_array_item *ref;
FLEX_ALLOC_STR(ref, refname, refname);
-   hashcpy(ref->objectname.hash, objectname);
+   hashcpy(ref->oid.hash, objectname);
ref->flag = flag;
 
return ref;
diff --git a/ref-filter.h b/ref-filter.h
index e16ea2a990119..87b026b8b76d0 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -34,7 +34,7 @@ struct ref_sorting {
 };
 
 struct ref_array_item {
-   struct object_id objectname;
+   struct object_id oid;
int flag;
unsigned int kind;
const char *symref;

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

[PATCH RFC v2 16/25] cat-file: move skip_object_info into ref-filter

[Olga Telezhnaya]

Move logic related to skip_object_info into ref-filter,
so that cat-file does not use that field at all.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 7 +--
 ref-filter.c   | 5 +
 ref-filter.h   | 1 +
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index a55138f1fd1d1..37adf626d0e55 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -395,14 +395,9 @@ static int batch_objects(struct batch_options *opt)
memset(, 0, sizeof(data));
opt->format.cat_file_data = 
opt->format.is_cat = 1;
+   opt->format.all_objects = opt->all_objects;
verify_ref_format(>format);
 
-   if (opt->all_objects) {
-   struct object_info empty = OBJECT_INFO_INIT;
-   if (!memcmp(, , sizeof(empty)))
-   data.skip_object_info = 1;
-   }
-
/*
 * If we are printing out the object, then always fill in the type,
 * since we will want to decide whether or not to stream.
diff --git a/ref-filter.c b/ref-filter.c
index bbcd507d179a9..7dcd36cd2cddc 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -777,6 +777,11 @@ int verify_ref_format(struct ref_format *format)
}
if (format->need_color_reset_at_eol && !want_color(format->use_color))
format->need_color_reset_at_eol = 0;
+   if (is_cat && format->all_objects) {
+   struct object_info empty = OBJECT_INFO_INIT;
+   if (!memcmp(_file_info->info, , sizeof(empty)))
+   cat_file_info->skip_object_info = 1;
+   }
return 0;
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index f590e5d694df4..e882eb5126118 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -119,6 +119,7 @@ struct ref_format {
 */
struct expand_data *cat_file_data;
int is_cat;
+   int all_objects;
 };
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }

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

[PATCH RFC v2 22/25] ref-filter: work with objectsize:disk

[Olga Telezhnaya]

Make a temporary solution for commands that could use
objectsize:disk atom.
It's better to fill it with value or give an error if there is no value
for this atom, but as a first solution we do dothing.
It means that if objectsize:disk is used, we put an empty string there.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index aa15dd0b4723e..b21358aea476b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -826,8 +826,10 @@ static void grab_common_values(struct atom_value *val, int 
deref, struct object
v->value = sz;
v->s = xstrfmt("%lu", sz);
} else if (!strcmp(name, "objectsize:disk")) {
-   v->value = cat_file_info.disk_size;
-   v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
+   if (is_cat) {
+   v->value = cat_file_info.disk_size;
+   v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
+   }
} else if (deref)
grab_objectname(name, obj->oid.hash, v, _atom[i]);
}

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

[PATCH RFC v2 05/25] cat-file: move struct expand_data into ref-filter

[Olga Telezhnaya]

Need that for further reusing of formatting logic in cat-file.
Have plans to get rid of using expand_data in cat-file at all,
and use it only in ref-filter for collecting, formatting and printing
needed data.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 36 
 ref-filter.h   | 36 
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 98fc5ec069a49..37d6096d201b5 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,42 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
return 0;
 }
 
-struct expand_data {
-   struct object_id oid;
-   enum object_type type;
-   unsigned long size;
-   off_t disk_size;
-   const char *rest;
-   struct object_id delta_base_oid;
-
-   /*
-* If mark_query is true, we do not expand anything, but rather
-* just mark the object_info with items we wish to query.
-*/
-   int mark_query;
-
-   /*
-* Whether to split the input on whitespace before feeding it to
-* get_sha1; this is decided during the mark_query phase based on
-* whether we have a %(rest) token in our format.
-*/
-   int split_on_whitespace;
-
-   /*
-* After a mark_query run, this object_info is set up to be
-* passed to sha1_object_info_extended. It will point to the data
-* elements above, so you can retrieve the response from there.
-*/
-   struct object_info info;
-
-   /*
-* This flag will be true if the requested batch format and options
-* don't require us to call sha1_object_info, which can then be
-* optimized out.
-*/
-   unsigned skip_object_info : 1;
-};
-
 static int is_atom(const char *atom, const char *s, int slen)
 {
int alen = strlen(atom);
diff --git a/ref-filter.h b/ref-filter.h
index b75c8ac45248e..17f2ac24d2739 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -72,6 +72,42 @@ struct ref_filter {
verbose;
 };
 
+struct expand_data {
+   struct object_id oid;
+   enum object_type type;
+   unsigned long size;
+   off_t disk_size;
+   const char *rest;
+   struct object_id delta_base_oid;
+
+   /*
+* If mark_query is true, we do not expand anything, but rather
+* just mark the object_info with items we wish to query.
+*/
+   int mark_query;
+
+   /*
+* Whether to split the input on whitespace before feeding it to
+* get_sha1; this is decided during the mark_query phase based on
+* whether we have a %(rest) token in our format.
+*/
+   int split_on_whitespace;
+
+   /*
+* After a mark_query run, this object_info is set up to be
+* passed to sha1_object_info_extended. It will point to the data
+* elements above, so you can retrieve the response from there.
+*/
+   struct object_info info;
+
+   /*
+* This flag will be true if the requested batch format and options
+* don't require us to call sha1_object_info, which can then be
+* optimized out.
+*/
+   unsigned skip_object_info : 1;
+};
+
 struct ref_format {
/*
 * Set these to define the format; make sure you call

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

[PATCH RFC v2 08/25] ref-filter: reuse parse_ref_filter_atom()

[Olga Telezhnaya]

Continue migrating formatting logic from cat-file to ref-filter.
Reuse parse_ref_filter_atom() for unifying all processes in ref-filter
and further removing of mark_atom_in_object_info().

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 ref-filter.c | 34 +-
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index ff87e632f463c..5c75259b1ab8c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,6 +100,7 @@ static struct used_atom {
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
+struct expand_data *cat_file_info;
 
 static void color_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *color_value)
 {
@@ -251,6 +252,16 @@ static void objectname_atom_parser(const struct ref_format 
*format, struct used_
die(_("unrecognized %%(objectname) argument: %s"), arg);
 }
 
+static void objectsize_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
+{
+   if (!arg)
+   ; /* default to normal object size */
+   else if (!strcmp(arg, "disk"))
+   cat_file_info->info.disk_sizep = _file_info->disk_size;
+   else
+   die(_("urecognized %%(objectsize) argument: %s"), arg);
+}
+
 static void refname_atom_parser(const struct ref_format *format, struct 
used_atom *atom, const char *arg)
 {
refname_atom_parser_internal(>u.refname, arg, atom->name);
@@ -371,6 +382,14 @@ static struct valid_atom {
{ "else" },
 };
 
+static struct valid_atom valid_cat_file_atom[] = {
+   { "objectname" },
+   { "objecttype" },
+   { "objectsize", FIELD_ULONG, objectsize_atom_parser },
+   { "rest" },
+   { "deltabase" },
+};
+
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
 
 struct ref_formatting_stack {
@@ -401,20 +420,14 @@ static int is_atom(const char *atom, const char *s, int 
slen)
 static void mark_atom_in_object_info(const char *atom, int len,
struct expand_data *data)
 {
-   if (is_atom("objectname", atom, len))
-   ; /* do nothing */
-   else if (is_atom("objecttype", atom, len))
+   if (is_atom("objecttype", atom, len))
data->info.typep = >type;
else if (is_atom("objectsize", atom, len))
data->info.sizep = >size;
-   else if (is_atom("objectsize:disk", atom, len))
-   data->info.disk_sizep = >disk_size;
else if (is_atom("rest", atom, len))
data->split_on_whitespace = 1;
else if (is_atom("deltabase", atom, len))
data->info.delta_base_sha1 = data->delta_base_oid.hash;
-   else
-   die("unknown format element: %.*s", len, atom);
 }
 
 /*
@@ -483,6 +496,8 @@ static int parse_ref_filter_atom(const struct ref_format 
*format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
+   if (cat_file_info)
+   mark_atom_in_object_info(atom, atom_len, cat_file_info);
return at;
 }
 
@@ -726,6 +741,7 @@ int verify_ref_format(struct ref_format *format)
 {
const char *cp, *sp;
 
+   cat_file_info = format->cat_file_data;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
const char *color, *ep = strchr(sp, ')');
@@ -736,8 +752,8 @@ int verify_ref_format(struct ref_format *format)
/* sp points at "%(" and ep points at the closing ")" */
 
if (format->cat_file_data)
-   mark_atom_in_object_info(sp + 2, ep - sp - 2,
-   format->cat_file_data);
+   at = parse_ref_filter_atom(format, valid_cat_file_atom,
+  
ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
else {
at = parse_ref_filter_atom(format, valid_atom,
   ARRAY_SIZE(valid_atom), sp + 
2, ep);

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