Re: [PATCH v2 3/3] unpack_trees_options: free messages when done

[Martin Ågren]

On 18 May 2018 at 00:10, Junio C Hamano  wrote:
> Martin Ågren  writes:
>
>> The `opts` string array contains multiple copies of the same pointers.
>> Be careful to only free each pointer once, then zeroize the whole array
>> so that we do not leave any dangling pointers.

> I wonder if an approach that is longer-term a bit more maintainable
> is to add a new string-list instance to opts, save these xstrfmt()'ed
> messages to it when setup_unpack_trees_porcelain() create them, and
> then make clear_unpack_trees_porcelain() pay *no* attention to msg[]
> array and the positions of these allocated messages and duplicates
> but just reclaim the resources held in that string-list, or
> something like that.

Thank you for thoughts and this suggestion. I will try this out,
hopefully later today.

Martin

Re: [PATCH 6/8] diff.c: decouple white space treatment from move detection algorithm

[Simon Ruderich]

On Thu, May 17, 2018 at 12:46:51PM -0700, Stefan Beller wrote:
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index bb9f1b7cd82..7b2527b9a19 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -292,6 +292,19 @@ dimmed_zebra::
>   blocks are considered interesting, the rest is uninteresting.
>  --
>
> +--color-moved-[no-]ignore-space-at-eol::
> + Ignore changes in whitespace at EOL when performing the move
> + detection for --color-moved.
> +--color-moved-[no-]ignore-space-change::
> + Ignore changes in amount of whitespace when performing the move
> + detection for --color-moved.  This ignores whitespace
> + at line end, and considers all other sequences of one or
> + more whitespace characters to be equivalent.
> +--color-moved-[no-]ignore-all-space::
> + Ignore whitespace when comparing lines when performing the move
> + detection for --color-moved.  This ignores differences even if
> + one line has whitespace where the other line has none.
> +
>  --word-diff[=]::
>   Show a word diff, using the  to delimit changed words.
>   By default, words are delimited by whitespace; see

Hello,

I think it would be better to specify the options unabbreviated.
Not being able to search the man page for
"--color-moved-ignore-space-at-eol" or
"--color-moved-no-ignore-space-at-eol" can be a major pain when
looking for documentation. So maybe something like this instead:

> +--color-moved-ignore-space-at-eol::
> +--color-moved-no-ignore-space-at-eol::
> + Ignore changes in whitespace at EOL when performing the move
> + detection for --color-moved.

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

Re: [PATCH v5 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'

[Taylor Blau]

On Sat, May 12, 2018 at 03:07:04PM +0900, Junio C Hamano wrote:
> Taylor Blau  writes:
>
> > I re-read your note and understand more clearly now what your suggestion
> > is. To ensure that we're in agreement, do you mean:
> >
> >   1. '--column -v' will _never_ give a column, but will never die(),
> >   either
>
> No, I don't.
>
> >   2. '--column --[and | or | not]' will never give a column, but will
> >   also never die(), either.
>
> No, I don't.
>
> If a file does not have substring "foo", then
>
>   git grep -v -e foo file
>   git grep --not -e foo file
>
> would hit all lines, just like
>
>   git grep -e '.*' file
>
> does.
>
> I would expect that all of these
>
>   git grep --column/-o -v -e foo file
>   git grep --column/-o --not -e foo file
>   git grep --column/-o -e '.*' file
>
> give the same output, which is what we would get if we consider the
> hit from "choose lines that lack 'foo'" on a line without 'foo' is
> caused by the entire contents on the line.  That is in line with
> "choose lines that has anything (including nothing)" aka ".*" would
> result in the entire line being reported via -o.  The byte offset of
> the first hit on such a line reported by --column is also 1, and
> that is a good and real answer to the question "git grep --column/-o"
> can give.

I agree with your message now and thank you for explaining what you
had written. I spoke with Peff off-list for a while to determine what I
think is essentially the answer to ``what are a set of semantics for
filling out a regmatch_t given an extended expression?''

It's helpful to recognize that the extended expressions are implemented
very much like a tree, so a reasonable semantics will lend itself well
to the way in which match_expr_eval() is implemented. Here's what we
came up with:

  * `git grep -e foo`. This is the case where the extended expression
has a single atomic node in its tree. This falls into the "just call
match_one_pattern()" case and has a simple answer: the starting
offset and ending offset are that of whatever match_one_pattern
gives.

  * `git grep --not -e foo`. This has the set of semantics that you
describe above (the starting offset is 1), with the addition that
the ending offset is the end of the line. This is similar to the
fact that `--not foo` is very similar to `.$`.

  * `git grep --and -e foo -e bar`. This binary operation should recur
on its sub-expressions and take the minimum of the starting offset
and the maximum of the ending offset.

For inputs of the form "foobar" and "foo bar", it will do the right
thing (give the starting and ending offset for "foobar" and give no
match, respectively).

  * `git grep --or -e foo -e bar`. This is the most complicated case, in
my opinion. In going with the min/max idea in the and case above, I
think that `--or` should also min/max its sub-expressions, but in
fact we short-circuit evaluating the second sub-expression when we
find a match for the first.

So, in cases like matching `--or -e foo -e bar` with "foo baz bar",
we'll do the right thing, since `foo` is the first sub-expression
and happens to be the left-most match. In other words, we __adhere
to our answer with the left-most match first__ semantics, but only
because __the first sub-expression is the left-most match__.

In the other case where we try and match the same expression against
"bar baz foo", we'll return the starting offset of "foo", even
though it isn't the left-most match, violating our semantics.

So, I propose we adopt the following: use the trivial answer for "foo",
the whole line for "--not", and min/max the starting/ending offsets for
binary operators, knowing that we will sometimes produce a weird answer
for --or.

I think that the semantics for --or are OK to go forward with, but would
be interested in the thoughts of others to figure out whether this is
sensible to everyone else.

Does this seem like an OK approach? Perhaps Peff can clarify some of
what's shared here, since we did speak elsewhere about it.

Thanks,
Taylor

Re: [PATCH v2 13/14] merge: use commit-slab in merge remote desc instead of commit->util

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> diff --git a/commit.c b/commit.c
> index 57049118a5..8202067cd5 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1574,13 +1574,21 @@ int commit_tree_extended(const char *msg, size_t 
> msg_len,
>   return result;
>  }
>  
> +define_commit_slab(merge_desc_slab, struct merge_remote_desc *);
> +struct merge_desc_slab merge_desc_slab = COMMIT_SLAB_INIT(1, 
> merge_desc_slab);

s/^/static /; otherwise sparse would complain?

Re: [PATCH v2 1/3] merge: setup `opts` later in `checkout_fast_forward()`

[Jacob Keller]

On Thu, May 17, 2018 at 2:48 PM, Junio C Hamano  wrote:
> Martin Ågren  writes:
>
>> After we initialize the various fields in `opts` but before we actually
>> use them, we might return early. Move the initialization further down,
>> to immediately before we use `opts`.
>>
>> This limits the scope of `opts` and will help a later commit fix a
>> memory leak without having to worry about those early returns.
>>
>> This patch is best viewed using something like this (note the tab!):
>> --color-moved --anchored="trees[nr_trees] = parse_tree_indirect"
>
> This side remark is interesting because it totally depends on how
> you look at it.  I think "initialize opts late" and "attempt to
> parse the trees first and fail early" are the sides of the same
> coin, and the diff shown without the anchor matches the latter,
> which is also perfectly acceptable interpretation of what this patch
> does.
>

Yes. I like that we have tools available to show diffs in different
hopefully meaningful ways.

I happen to like when the diff matches my mental map of the change
after reading the commit message, so having the author indicate how
best to view it is useful, but definitely cool to see that we can get
different interpretations.

Thanks,
Jake

[PATCH 05/19] commit: add repository argument to read_graft_file

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow the caller of read_graft_file 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: Jonathan Nieder 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 commit.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/commit.c b/commit.c
index 4e8d3488425..b5c0aec24a0 100644
--- a/commit.c
+++ b/commit.c
@@ -177,7 +177,8 @@ struct commit_graft *read_graft_line(struct strbuf *line)
return NULL;
 }
 
-static int read_graft_file(const char *graft_file)
+#define read_graft_file(r, f) read_graft_file_##r(f)
+static int read_graft_file_the_repository(const char *graft_file)
 {
FILE *fp = fopen_or_warn(graft_file, "r");
struct strbuf buf = STRBUF_INIT;
@@ -204,7 +205,7 @@ static void prepare_commit_graft(void)
if (commit_graft_prepared)
return;
graft_file = get_graft_file();
-   read_graft_file(graft_file);
+   read_graft_file(the_repository, graft_file);
/* make sure shallows are read */
is_repository_shallow();
commit_graft_prepared = 1;
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 15/19] cache: convert get_graft_file to handle arbitrary repositories

[Stefan Beller]

This conversion was done without the #define trick used in the earlier
series refactoring to have better repository access, because this function
is easy to review, as all lines are converted and it has only one caller.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 cache.h   | 2 +-
 commit.c  | 2 +-
 environment.c | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index ab716011b7e..cb1aeb1dcbf 100644
--- a/cache.h
+++ b/cache.h
@@ -476,7 +476,7 @@ extern const char *get_git_dir(void);
 extern const char *get_git_common_dir(void);
 extern char *get_object_directory(void);
 extern char *get_index_file(void);
-extern char *get_graft_file(void);
+extern char *get_graft_file(struct repository *r);
 extern int set_git_dir(const char *path);
 extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
 extern int get_common_dir(struct strbuf *sb, const char *gitdir);
diff --git a/commit.c b/commit.c
index 3fcb2fd66ce..24028fd257a 100644
--- a/commit.c
+++ b/commit.c
@@ -204,7 +204,7 @@ static void prepare_commit_graft_the_repository(void)
 
if (commit_graft_prepared)
return;
-   graft_file = get_graft_file();
+   graft_file = get_graft_file(the_repository);
read_graft_file(the_repository, graft_file);
/* make sure shallows are read */
is_repository_shallow(the_repository);
diff --git a/environment.c b/environment.c
index 87d9e52ffde..ab42346e563 100644
--- a/environment.c
+++ b/environment.c
@@ -316,11 +316,11 @@ char *get_index_file(void)
return the_repository->index_file;
 }
 
-char *get_graft_file(void)
+char *get_graft_file(struct repository *r)
 {
-   if (!the_repository->graft_file)
+   if (!r->graft_file)
BUG("git environment hasn't been setup");
-   return the_repository->graft_file;
+   return r->graft_file;
 }
 
 int set_git_dir(const char *path)
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 09/19] shallow: add repository argument to register_shallow

[Stefan Beller]

Add a repository argument to allow callers of register_shallow
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 
---
 builtin/pack-objects.c | 2 +-
 builtin/receive-pack.c | 2 +-
 commit.h   | 3 ++-
 fetch-pack.c   | 2 +-
 shallow.c  | 4 ++--
 upload-pack.c  | 7 ---
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d65eb4a9478..97a5963efb6 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2875,7 +2875,7 @@ static void get_object_list(int ac, const char **av)
struct object_id oid;
if (get_oid_hex(line + 10, ))
die("not an SHA-1 '%s'", line + 10);
-   register_shallow();
+   register_shallow(the_repository, );
use_bitmap_index = 0;
continue;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 36906fd5e98..c666820b69a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -906,7 +906,7 @@ static int update_shallow_ref(struct command *cmd, struct 
shallow_info *si)
 * not lose these new roots..
 */
for (i = 0; i < extra.nr; i++)
-   register_shallow([i]);
+   register_shallow(the_repository, [i]);
 
si->shallow_ref[cmd->index] = 0;
oid_array_clear();
diff --git a/commit.h b/commit.h
index f88c854e2f6..59346de5512 100644
--- a/commit.h
+++ b/commit.h
@@ -191,7 +191,8 @@ extern struct commit_list 
*get_merge_bases_many_dirty(struct commit *one, int n,
 
 struct oid_array;
 struct ref;
-extern int register_shallow(const struct object_id *oid);
+#define register_shallow(r, o) register_shallow_##r(o);
+extern int register_shallow_the_repository(const struct object_id *oid);
 extern int unregister_shallow(const struct object_id *oid);
 extern int for_each_commit_graft(each_commit_graft_fn, void *);
 extern int is_repository_shallow(void);
diff --git a/fetch-pack.c b/fetch-pack.c
index a1535b37b9b..e3e99e44962 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -428,7 +428,7 @@ static int find_common(struct fetch_pack_args *args,
if (skip_prefix(line, "shallow ", )) {
if (get_oid_hex(arg, ))
die(_("invalid shallow line: %s"), 
line);
-   register_shallow();
+   register_shallow(the_repository, );
continue;
}
if (skip_prefix(line, "unshallow ", )) {
diff --git a/shallow.c b/shallow.c
index 73cb11a9162..0fadd5330d2 100644
--- a/shallow.c
+++ b/shallow.c
@@ -29,7 +29,7 @@ void set_alternate_shallow_file_the_repository(const char 
*path, int override)
alternate_shallow_file = xstrdup_or_null(path);
 }
 
-int register_shallow(const struct object_id *oid)
+int register_shallow_the_repository(const struct object_id *oid)
 {
struct commit_graft *graft =
xmalloc(sizeof(struct commit_graft));
@@ -70,7 +70,7 @@ int is_repository_shallow(void)
struct object_id oid;
if (get_oid_hex(buf, ))
die("bad shallow line: %s", buf);
-   register_shallow();
+   register_shallow(the_repository, );
}
fclose(fp);
return is_shallow;
diff --git a/upload-pack.c b/upload-pack.c
index a11c6d192ce..4e4ac0f0d95 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -663,7 +663,7 @@ static void send_shallow(struct commit_list *result)
if (!(object->flags & (CLIENT_SHALLOW|NOT_SHALLOW))) {
packet_write_fmt(1, "shallow %s",
 oid_to_hex(>oid));
-   register_shallow(>oid);
+   register_shallow(the_repository, >oid);
shallow_nr++;
}
result = result->next;
@@ -700,7 +700,7 @@ static void send_unshallow(const struct object_array 
*shallows)
add_object_array(object, NULL, _edge_obj);
}
/* make sure commit traversal conforms to client */
-   register_shallow(>oid);
+   register_shallow(the_repository, >oid);
}
 }
 
@@ -912,7 +912,8 @@ static void receive_needs(void)
if (shallows.nr > 0) {
int i;
for (i = 0; i < 

Re: What's cooking in git.git (May 2018, #02; Thu, 17)

[Stefan Beller]

>> Please do not replace what already hit 'next'.
>
> Here is what I made these two into a relative fix on top.

Thanks, I was about to prepare the same.

Re: [PATCH] grep: handle corrupt index files early

[Junio C Hamano]

Stefan Beller  writes:

> On Wed, May 16, 2018 at 6:36 PM, Junio C Hamano  wrote:
>> Duy Nguyen  writes:
>>
>>> With a majority of call sites dying like this though, I wonder if we
>>> should just add repo_read_index_or_die() with die() inside. Then the
>>> next person won't likely accidentally forget _()
>>
>> Yuck.
>>
>> That sounds like inviting a major code churn.  I tend to agree that
>> it would be a good clean-up for longer term maintenance, but I am
>> not sure if I can honestly say I'd look forward to such a clean-up
>> at this point in the cycle when there are tons of large-ish topics
>> in flight X-<.
>
> ok, consider the series
> https://public-inbox.org/git/20180516222118.233868-1-sbel...@google.com/
> retracted for this cycle; I will keep it around and resend it at some future
> date, hopefully.

Thanks.  I didn't realize you've _already_ done that.

[PATCH 12/19] commit: convert commit_graft_pos() to handle arbitrary repositories

[Stefan Beller]

From: Brandon Williams 

Signed-off-by: Brandon Williams 
Signed-off-by: Stefan Beller 
---
 commit.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/commit.c b/commit.c
index 684eeaa2ccd..0ec3d22813a 100644
--- a/commit.c
+++ b/commit.c
@@ -104,11 +104,10 @@ static const unsigned char 
*commit_graft_sha1_access(size_t index, void *table)
return commit_graft_table[index]->oid.hash;
 }
 
-#define commit_graft_pos(r, s) commit_graft_pos_##r(s)
-static int commit_graft_pos_the_repository(const unsigned char *sha1)
+static int commit_graft_pos(struct repository *r, const unsigned char *sha1)
 {
-   return sha1_pos(sha1, the_repository->parsed_objects->grafts,
-   the_repository->parsed_objects->grafts_nr,
+   return sha1_pos(sha1, r->parsed_objects->grafts,
+   r->parsed_objects->grafts_nr,
commit_graft_sha1_access);
 }
 
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 14/19] commit: convert read_graft_file to handle arbitrary repositories

[Stefan Beller]

From: Brandon Williams 

Signed-off-by: Brandon Williams 
Signed-off-by: Stefan Beller 
---
 commit.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/commit.c b/commit.c
index 8a2ab53fc67..3fcb2fd66ce 100644
--- a/commit.c
+++ b/commit.c
@@ -177,8 +177,7 @@ struct commit_graft *read_graft_line(struct strbuf *line)
return NULL;
 }
 
-#define read_graft_file(r, f) read_graft_file_##r(f)
-static int read_graft_file_the_repository(const char *graft_file)
+static int read_graft_file(struct repository *r, const char *graft_file)
 {
FILE *fp = fopen_or_warn(graft_file, "r");
struct strbuf buf = STRBUF_INIT;
@@ -189,7 +188,7 @@ static int read_graft_file_the_repository(const char 
*graft_file)
struct commit_graft *graft = read_graft_line();
if (!graft)
continue;
-   if (register_commit_graft(the_repository, graft, 1))
+   if (register_commit_graft(r, graft, 1))
error("duplicate graft data: %s", buf.buf);
}
fclose(fp);
-- 
2.17.0.582.gccdcbd54c44.dirty

Re: [PATCH 0/1] Hot fix for js/empty-config-section-fix

[Junio C Hamano]

Johannes Schindelin  writes:

> In https://public-inbox.org/git/20180508134248.ga25...@sigill.intra.peff.net/
> Jeff King pointed out that an invalid config section is not an indicator
> of a bug, as it is usually provided by the user.
>
> So we should not throw a fit and tell the user about a bug that they
> might even report.
>
> Instead, let's just error out.

Yeah, makes sense.  Thanks for a prompt fix-up.


>
>
> Johannes Schindelin (1):
>   config: a user-provided invalid section is not a BUG
>
>  config.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> base-commit: ccdcbd54c4475c2238b310f7113ab3075b5abc9c
> Published-As: 
> https://github.com/dscho/git/releases/tag/empty-config-section-fix-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git 
> empty-config-section-fix-v1

[PATCH 01/19] object-store: move object access functions to object-store.h

[Stefan Beller]

This should make these functions easier to find and cache.h less
overwhelming to read.

In particular, this moves:
- read_object_file
- oid_object_info
- write_object_file

As a result, most of the codebase needs to #include object-store.h.
In this patch the #include is only added to files that would fail to
compile otherwise.  It would be better to #include wherever
identifiers from the header are used.  That can happen later
when we have better tooling for it.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 apply.c  |   1 +
 archive-tar.c|   1 +
 archive-zip.c|   1 +
 archive.c|   1 +
 blame.c  |   1 +
 builtin/blame.c  |   1 +
 builtin/cat-file.c   |   1 +
 builtin/checkout.c   |   1 +
 builtin/clone.c  |   1 +
 builtin/commit-tree.c|   1 +
 builtin/describe.c   |   1 +
 builtin/difftool.c   |   1 +
 builtin/fast-export.c|   1 +
 builtin/fetch.c  |   1 +
 builtin/fmt-merge-msg.c  |   1 +
 builtin/hash-object.c|   1 +
 builtin/log.c|   1 +
 builtin/ls-tree.c|   1 +
 builtin/merge-tree.c |   1 +
 builtin/mktag.c  |   1 +
 builtin/mktree.c |   1 +
 builtin/notes.c  |   1 +
 builtin/prune.c  |   1 +
 builtin/receive-pack.c   |   1 +
 builtin/reflog.c |   1 +
 builtin/remote.c |   1 +
 builtin/rev-list.c   |   1 +
 builtin/show-ref.c   |   1 +
 builtin/tag.c|   1 +
 builtin/unpack-file.c|   1 +
 builtin/unpack-objects.c |   1 +
 builtin/verify-commit.c  |   1 +
 bulk-checkin.c   |   1 +
 bundle.c |   1 +
 cache-tree.c |   1 +
 cache.h  | 117 ---
 combine-diff.c   |   1 +
 commit.c |   1 +
 config.c |   1 +
 convert.c|   1 +
 diff.c   |   1 +
 diffcore-rename.c|   1 +
 dir.c|   1 +
 entry.c  |   1 +
 fetch-pack.c |   1 +
 fsck.c   |   1 +
 grep.c   |   1 +
 list-objects-filter.c|   1 +
 list-objects.c   |   1 +
 log-tree.c   |   1 +
 mailmap.c|   1 +
 match-trees.c|   1 +
 merge-blobs.c|   1 +
 merge-recursive.c|   1 +
 notes-cache.c|   1 +
 notes-merge.c|   1 +
 notes.c  |   1 +
 object-store.h   | 117 +++
 object.c |   1 +
 pack-bitmap-write.c  |   1 +
 packfile.h   |   5 ++
 read-cache.c |   1 +
 ref-filter.c |   1 +
 refs.c   |   1 +
 remote-testsvn.c |   1 +
 remote.c |   1 +
 rerere.c |   1 +
 revision.c   |   1 +
 send-pack.c  |   1 +
 sequencer.c  |   1 +
 shallow.c|   1 +
 submodule-config.c   |   1 +
 tag.c|   1 +
 tree-walk.c  |   1 +
 tree.c   |   1 +
 unpack-trees.c   |   1 +
 upload-pack.c|   1 +
 walker.c |   1 +
 xdiff-interface.c|   1 +
 79 files changed, 198 insertions(+), 117 deletions(-)

diff --git a/apply.c b/apply.c
index 7e5792c996f..cbc45fa1b0e 100644
--- a/apply.c
+++ b/apply.c
@@ -9,6 +9,7 @@
 
 #include "cache.h"
 #include "config.h"
+#include "object-store.h"
 #include "blob.h"
 #include "delta.h"
 #include "diff.h"
diff --git a/archive-tar.c b/archive-tar.c
index f93409324f9..e38435eb4ef 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -5,6 +5,7 @@
 #include "config.h"
 #include "tar.h"
 #include "archive.h"
+#include "object-store.h"
 #include "streaming.h"
 #include "run-command.h"
 
diff --git a/archive-zip.c b/archive-zip.c
index 74f3fe91034..abc556e5a75 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -6,6 +6,7 @@
 #include "archive.h"
 #include "streaming.h"
 #include "utf8.h"
+#include "object-store.h"
 #include "userdiff.h"
 #include "xdiff-interface.h"
 
diff --git a/archive.c b/archive.c
index 93ab175b0b4..9da1e3664a6 100644
--- a/archive.c
+++ b/archive.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "config.h"
 #include "refs.h"
+#include "object-store.h"
 #include "commit.h"
 #include "tree-walk.h"
 #include "attr.h"
diff --git a/blame.c b/blame.c
index 3a11f1ce52b..f689bde31cd 100644
--- a/blame.c
+++ b/blame.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "refs.h"
+#include "object-store.h"
 #include "cache-tree.h"
 #include "mergesort.h"
 #include "diff.h"
diff --git a/builtin/blame.c b/builtin/blame.c
index bfdf7cc1325..0ffd1d443ea 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -22,6 +22,7 @@
 #include "line-log.h"
 #include "dir.h"
 #include "progress.h"
+#include "object-store.h"
 #include "blame.h"
 
 static char blame_usage[] 

[PATCH 03/19] commit: add repository argument to commit_graft_pos

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow callers of commit_graft_pos 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: Jonathan Nieder 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 commit.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/commit.c b/commit.c
index a0c9eb05c82..2cd5b8a0b96 100644
--- a/commit.c
+++ b/commit.c
@@ -104,7 +104,8 @@ static const unsigned char *commit_graft_sha1_access(size_t 
index, void *table)
return commit_graft_table[index]->oid.hash;
 }
 
-static int commit_graft_pos(const unsigned char *sha1)
+#define commit_graft_pos(r, s) commit_graft_pos_##r(s)
+static int commit_graft_pos_the_repository(const unsigned char *sha1)
 {
return sha1_pos(sha1, the_repository->parsed_objects->grafts,
the_repository->parsed_objects->grafts_nr,
@@ -113,7 +114,7 @@ static int commit_graft_pos(const unsigned char *sha1)
 
 int register_commit_graft(struct commit_graft *graft, int ignore_dups)
 {
-   int pos = commit_graft_pos(graft->oid.hash);
+   int pos = commit_graft_pos(the_repository, graft->oid.hash);
 
if (0 <= pos) {
if (ignore_dups)
@@ -213,7 +214,7 @@ struct commit_graft *lookup_commit_graft(const struct 
object_id *oid)
 {
int pos;
prepare_commit_graft();
-   pos = commit_graft_pos(oid->hash);
+   pos = commit_graft_pos(the_repository, oid->hash);
if (pos < 0)
return NULL;
return the_repository->parsed_objects->grafts[pos];
@@ -229,7 +230,7 @@ int for_each_commit_graft(each_commit_graft_fn fn, void 
*cb_data)
 
 int unregister_shallow(const struct object_id *oid)
 {
-   int pos = commit_graft_pos(oid->hash);
+   int pos = commit_graft_pos(the_repository, oid->hash);
if (pos < 0)
return -1;
if (pos + 1 < the_repository->parsed_objects->grafts_nr)
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 13/19] commit: convert register_commit_graft to handle arbitrary repositories

[Stefan Beller]

From: Brandon Williams 

Signed-off-by: Brandon Williams 
Signed-off-by: Stefan Beller 
---
 commit.c | 29 +++--
 commit.h |  3 +--
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/commit.c b/commit.c
index 0ec3d22813a..8a2ab53fc67 100644
--- a/commit.c
+++ b/commit.c
@@ -111,30 +111,31 @@ static int commit_graft_pos(struct repository *r, const 
unsigned char *sha1)
commit_graft_sha1_access);
 }
 
-int register_commit_graft_the_repository(struct commit_graft *graft, int 
ignore_dups)
+int register_commit_graft(struct repository *r, struct commit_graft *graft,
+ int ignore_dups)
 {
-   int pos = commit_graft_pos(the_repository, graft->oid.hash);
+   int pos = commit_graft_pos(r, graft->oid.hash);
 
if (0 <= pos) {
if (ignore_dups)
free(graft);
else {
-   free(the_repository->parsed_objects->grafts[pos]);
-   the_repository->parsed_objects->grafts[pos] = graft;
+   free(r->parsed_objects->grafts[pos]);
+   r->parsed_objects->grafts[pos] = graft;
}
return 1;
}
pos = -pos - 1;
-   ALLOC_GROW(the_repository->parsed_objects->grafts,
-  the_repository->parsed_objects->grafts_nr + 1,
-  the_repository->parsed_objects->grafts_alloc);
-   the_repository->parsed_objects->grafts_nr++;
-   if (pos < the_repository->parsed_objects->grafts_nr)
-   memmove(the_repository->parsed_objects->grafts + pos + 1,
-   the_repository->parsed_objects->grafts + pos,
-   (the_repository->parsed_objects->grafts_nr - pos - 1) *
-   sizeof(*the_repository->parsed_objects->grafts));
-   the_repository->parsed_objects->grafts[pos] = graft;
+   ALLOC_GROW(r->parsed_objects->grafts,
+  r->parsed_objects->grafts_nr + 1,
+  r->parsed_objects->grafts_alloc);
+   r->parsed_objects->grafts_nr++;
+   if (pos < r->parsed_objects->grafts_nr)
+   memmove(r->parsed_objects->grafts + pos + 1,
+   r->parsed_objects->grafts + pos,
+   (r->parsed_objects->grafts_nr - pos - 1) *
+   sizeof(*r->parsed_objects->grafts));
+   r->parsed_objects->grafts[pos] = graft;
return 0;
 }
 
diff --git a/commit.h b/commit.h
index c7f25d6490a..d04bbed81cf 100644
--- a/commit.h
+++ b/commit.h
@@ -174,8 +174,7 @@ struct commit_graft {
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
 struct commit_graft *read_graft_line(struct strbuf *line);
-#define register_commit_graft(r, g, i) register_commit_graft_##r(g, i)
-int register_commit_graft_the_repository(struct commit_graft *, int);
+int register_commit_graft(struct repository *r, struct commit_graft *, int);
 #define lookup_commit_graft(r, o) lookup_commit_graft_##r(o)
 struct commit_graft *lookup_commit_graft_the_repository(const struct object_id 
*oid);
 
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 10/19] shallow: add repository argument to check_shallow_file_for_update

[Stefan Beller]

Add a repository argument to allow callers of check_shallow_file_for_update
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 
---
 shallow.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/shallow.c b/shallow.c
index 0fadd5330d2..0028e4ea776 100644
--- a/shallow.c
+++ b/shallow.c
@@ -217,7 +217,8 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, 
const char **av,
return result;
 }
 
-static void check_shallow_file_for_update(void)
+#define check_shallow_file_for_update(r) check_shallow_file_for_update_##r()
+static void check_shallow_file_for_update_the_repository(void)
 {
if (is_shallow == -1)
die("BUG: shallow must be initialized by now");
@@ -319,7 +320,7 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
 
fd = hold_lock_file_for_update(shallow_lock, git_path_shallow(),
   LOCK_DIE_ON_ERROR);
-   check_shallow_file_for_update();
+   check_shallow_file_for_update(the_repository);
if (write_shallow_commits(, 0, extra)) {
if (write_in_full(fd, sb.buf, sb.len) < 0)
die_errno("failed to write to %s",
@@ -366,7 +367,7 @@ void prune_shallow(int show_only)
}
fd = hold_lock_file_for_update(_lock, git_path_shallow(),
   LOCK_DIE_ON_ERROR);
-   check_shallow_file_for_update();
+   check_shallow_file_for_update(the_repository);
if (write_shallow_commits_1(, 0, NULL, SEEN_ONLY)) {
if (write_in_full(fd, sb.buf, sb.len) < 0)
die_errno("failed to write to %s",
-- 
2.17.0.582.gccdcbd54c44.dirty

Re: What's cooking in git.git (May 2018, #02; Thu, 17)

[Junio C Hamano]

Junio C Hamano  writes:

> Stefan Beller  writes:
>
>>> * sb/object-store-replace (2018-05-10) 2 commits
>>>   (merged to 'next' on 2018-05-16 at 41bbedcc81)
>>>  + replace-object.c: remove the_repository from prepare_replace_object
>>>  + object.c: free replace map in raw_object_store_clear
>>>
>>>  Hotfix.
>>>
>>>  Will merge to 'master'.
>>
>> Please do not.
>> (Or do, but then be prepared for another hotfix.)
>>
>> The commit sb/object-store-replace^ needs more free'ing and shall be
>> replaced with
>
> Please do not replace what already hit 'next'.

Here is what I made these two into a relative fix on top.

-- >8 --
From: Stefan Beller 
Date: Thu, 17 May 2018 11:29:57 -0700
Subject: [PATCH] object.c: clear replace map before freeing it

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 object.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/object.c b/object.c
index cdf520084d..97245fdea2 100644
--- a/object.c
+++ b/object.c
@@ -480,6 +480,8 @@ void raw_object_store_clear(struct raw_object_store *o)
 {
FREE_AND_NULL(o->objectdir);
FREE_AND_NULL(o->alternate_db);
+
+   oidmap_free(o->replace_map, 1);
FREE_AND_NULL(o->replace_map);
 
free_alt_odbs(o);
-- 
2.17.0-582-gccdcbd54c4

[PATCH 00/19] object store: grafts and shallow.

[Stefan Beller]

v1:

This reroll:
* includes the fixup by Ramsay
  
http://public-inbox.org/git/ae96f1c4-add2-d9d8-f08b-a765fe277...@ramsayjones.plus.com
* fixes commit messages
* changes the macro for the global git_path definitions such that we need fewer
  characters (omits the duplicates "path_" that were the same for every 
function)
* the range diff is below
  (per 
https://public-inbox.org/git/20180508034815.gb7...@sigill.intra.peff.net/)

Thanks,
Stefan

v0/RFC:
This applies on top of sb/object-store-alloc[1], and is the next part of the
object store series. 

I think we're getting close to actually being done in the object store series,
as the next series to build on top of this will convert the
lookup_{object, commit, ...} functions.

However I marked this series as RFC, as I expect heavy conflicts with the
code base. sb/object-store-alloc builds on an older base of the code base
and there have been some series that will conflict with this one.
For example the patch to migrate path functions into a repository world
will collide with one of Dschos series as he added another path helper
function there. I am happy to reroll on top of that, but for now I chose
sb/object-store-alloc to keep the momentum in the object-store series'.

There is another object store series that is not part of the critical path:
There is a mem leak in the pack files stored in the raw object store.
Upon free'ing the repository we do not free its pack files. We cannot
free the packfiles, as otherwise the bitmap code would have dangling
pointers into packfiles that no longer exists, leading to segfaults.
So we'll need to have an object store series covering the bitmap code.
I have something local, which I'll send out shortly.

Thanks,
Stefan

[1] with the latest patch replaced as in
https://public-inbox.org/git/20180515214842.108713-1-sbel...@google.com/

Brandon Williams (3):
  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 (6):
  object: move grafts to object parser
  commit: add repository argument to commit_graft_pos
  commit: add repository argument to register_commit_graft
  commit: add repository argument to read_graft_file
  commit: add repository argument to prepare_commit_graft
  commit: add repository argument to lookup_commit_graft

Stefan Beller (10):
  object-store: move object access functions to object-store.h
  shallow: add repository argument to set_alternate_shallow_file
  shallow: add repository argument to register_shallow
  shallow: add repository argument to check_shallow_file_for_update
  shallow: add repository argument to is_repository_shallow
  cache: convert get_graft_file to handle arbitrary repositories
  path.c: migrate global git_path_* to take a repository argument
  shallow: migrate shallow information into the object parser
  commit: allow prepare_commit_graft to handle arbitrary repositories
  commit: allow lookup_commit_graft to handle arbitrary repositories

 apply.c  |   1 +
 archive-tar.c|   1 +
 archive-zip.c|   1 +
 archive.c|   1 +
 blame.c  |   9 ++-
 branch.c |  14 ++---
 builtin/blame.c  |   4 +-
 builtin/cat-file.c   |   1 +
 builtin/checkout.c   |   1 +
 builtin/clone.c  |   1 +
 builtin/commit-tree.c|   1 +
 builtin/commit.c |  38 ++---
 builtin/describe.c   |   1 +
 builtin/difftool.c   |   1 +
 builtin/fast-export.c|   1 +
 builtin/fetch.c  |   7 ++-
 builtin/fmt-merge-msg.c  |   1 +
 builtin/hash-object.c|   1 +
 builtin/log.c|   1 +
 builtin/ls-tree.c|   1 +
 builtin/merge-tree.c |   1 +
 builtin/merge.c  |  37 ++--
 builtin/mktag.c  |   1 +
 builtin/mktree.c |   1 +
 builtin/notes.c  |   1 +
 builtin/pack-objects.c   |   6 +-
 builtin/prune.c  |   3 +-
 builtin/pull.c   |   4 +-
 builtin/receive-pack.c   |   3 +-
 builtin/reflog.c |   1 +
 builtin/remote.c |   1 +
 builtin/reset.c  |   2 +-
 builtin/rev-list.c   |   1 +
 builtin/rev-parse.c  |   3 +-
 builtin/show-ref.c   |   1 +
 builtin/tag.c|   1 +
 builtin/unpack-file.c|   1 +
 builtin/unpack-objects.c |   1 +
 builtin/verify-commit.c  |   1 +
 bulk-checkin.c   |   1 +
 bundle.c |   1 +
 cache-tree.c |   1 +
 cache.h  | 119 +--
 combine-diff.c   |   1 +
 commit.c |  77 +
 commit.h |  10 ++--
 config.c |   1 +
 convert.c|   1 +
 diff.c   |   1 +
 diffcore-rename.c|   1 +
 dir.c|   1 +
 entry.c  |   1 +
 environment.c|   8 

[PATCH 02/19] object: move grafts to object parser

[Stefan Beller]

From: Jonathan Nieder 

Grafts are only meaningful in the context of a single repository.
Therefore they cannot be global.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano 
---
 commit.c | 42 +++---
 object.h |  4 
 2 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/commit.c b/commit.c
index b053f07f305..a0c9eb05c82 100644
--- a/commit.c
+++ b/commit.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "tag.h"
 #include "commit.h"
+#include "repository.h"
 #include "object-store.h"
 #include "pkt-line.h"
 #include "utf8.h"
@@ -97,9 +98,6 @@ static timestamp_t parse_commit_date(const char *buf, const 
char *tail)
return parse_timestamp(dateptr, NULL, 10);
 }
 
-static struct commit_graft **commit_graft;
-static int commit_graft_alloc, commit_graft_nr;
-
 static const unsigned char *commit_graft_sha1_access(size_t index, void *table)
 {
struct commit_graft **commit_graft_table = table;
@@ -108,7 +106,8 @@ static const unsigned char *commit_graft_sha1_access(size_t 
index, void *table)
 
 static int commit_graft_pos(const unsigned char *sha1)
 {
-   return sha1_pos(sha1, commit_graft, commit_graft_nr,
+   return sha1_pos(sha1, the_repository->parsed_objects->grafts,
+   the_repository->parsed_objects->grafts_nr,
commit_graft_sha1_access);
 }
 
@@ -120,18 +119,22 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
if (ignore_dups)
free(graft);
else {
-   free(commit_graft[pos]);
-   commit_graft[pos] = graft;
+   free(the_repository->parsed_objects->grafts[pos]);
+   the_repository->parsed_objects->grafts[pos] = graft;
}
return 1;
}
pos = -pos - 1;
-   ALLOC_GROW(commit_graft, commit_graft_nr + 1, commit_graft_alloc);
-   commit_graft_nr++;
-   if (pos < commit_graft_nr)
-   MOVE_ARRAY(commit_graft + pos + 1, commit_graft + pos,
-  commit_graft_nr - pos - 1);
-   commit_graft[pos] = graft;
+   ALLOC_GROW(the_repository->parsed_objects->grafts,
+  the_repository->parsed_objects->grafts_nr + 1,
+  the_repository->parsed_objects->grafts_alloc);
+   the_repository->parsed_objects->grafts_nr++;
+   if (pos < the_repository->parsed_objects->grafts_nr)
+   memmove(the_repository->parsed_objects->grafts + pos + 1,
+   the_repository->parsed_objects->grafts + pos,
+   (the_repository->parsed_objects->grafts_nr - pos - 1) *
+   sizeof(*the_repository->parsed_objects->grafts));
+   the_repository->parsed_objects->grafts[pos] = graft;
return 0;
 }
 
@@ -213,14 +216,14 @@ struct commit_graft *lookup_commit_graft(const struct 
object_id *oid)
pos = commit_graft_pos(oid->hash);
if (pos < 0)
return NULL;
-   return commit_graft[pos];
+   return the_repository->parsed_objects->grafts[pos];
 }
 
 int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)
 {
int i, ret;
-   for (i = ret = 0; i < commit_graft_nr && !ret; i++)
-   ret = fn(commit_graft[i], cb_data);
+   for (i = ret = 0; i < the_repository->parsed_objects->grafts_nr && 
!ret; i++)
+   ret = fn(the_repository->parsed_objects->grafts[i], cb_data);
return ret;
 }
 
@@ -229,10 +232,11 @@ int unregister_shallow(const struct object_id *oid)
int pos = commit_graft_pos(oid->hash);
if (pos < 0)
return -1;
-   if (pos + 1 < commit_graft_nr)
-   MOVE_ARRAY(commit_graft + pos, commit_graft + pos + 1,
-  commit_graft_nr - pos - 1);
-   commit_graft_nr--;
+   if (pos + 1 < the_repository->parsed_objects->grafts_nr)
+   MOVE_ARRAY(the_repository->parsed_objects->grafts + pos,
+  the_repository->parsed_objects->grafts + pos + 1,
+  the_repository->parsed_objects->grafts_nr - pos - 1);
+   the_repository->parsed_objects->grafts_nr--;
return 0;
 }
 
diff --git a/object.h b/object.h
index 7916edb4edf..ec908f9bcc1 100644
--- a/object.h
+++ b/object.h
@@ -12,6 +12,10 @@ struct parsed_object_pool {
struct alloc_state *tag_state;
struct alloc_state *object_state;
unsigned commit_count;
+
+   /* parent substitutions from .git/info/grafts and .git/shallow */
+   struct commit_graft **grafts;
+   int grafts_alloc, grafts_nr;
 };
 
 struct parsed_object_pool *parsed_object_pool_new(void);
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 08/19] shallow: add repository argument to set_alternate_shallow_file

[Stefan Beller]

Add a repository argument to allow callers of set_alternate_shallow_file
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 
---
 commit.h  | 3 ++-
 environment.c | 2 +-
 git.c | 2 +-
 shallow.c | 2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/commit.h b/commit.h
index f6746125766..f88c854e2f6 100644
--- a/commit.h
+++ b/commit.h
@@ -199,7 +199,8 @@ extern struct commit_list *get_shallow_commits(struct 
object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
 extern struct commit_list *get_shallow_commits_by_rev_list(
int ac, const char **av, int shallow_flag, int 
not_shallow_flag);
-extern void set_alternate_shallow_file(const char *path, int override);
+#define set_alternate_shallow_file(r, p, o) set_alternate_shallow_file_##r(p, 
o)
+extern void set_alternate_shallow_file_the_repository(const char *path, int 
override);
 extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
 const struct oid_array *extra);
 extern void setup_alternate_shallow(struct lock_file *shallow_lock,
diff --git a/environment.c b/environment.c
index b991fc0a87c..87d9e52ffde 100644
--- a/environment.c
+++ b/environment.c
@@ -189,7 +189,7 @@ void setup_git_env(const char *git_dir)
git_namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
if (shallow_file)
-   set_alternate_shallow_file(shallow_file, 0);
+   set_alternate_shallow_file(the_repository, shallow_file, 0);
 }
 
 int is_bare_repository(void)
diff --git a/git.c b/git.c
index 3a89893712e..5e8903681e6 100644
--- a/git.c
+++ b/git.c
@@ -207,7 +207,7 @@ static int handle_options(const char ***argv, int *argc, 
int *envchanged)
} else if (!strcmp(cmd, "--shallow-file")) {
(*argv)++;
(*argc)--;
-   set_alternate_shallow_file((*argv)[0], 1);
+   set_alternate_shallow_file(the_repository, (*argv)[0], 
1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "-C")) {
diff --git a/shallow.c b/shallow.c
index ca360c700c5..73cb11a9162 100644
--- a/shallow.c
+++ b/shallow.c
@@ -19,7 +19,7 @@ static int is_shallow = -1;
 static struct stat_validity shallow_stat;
 static char *alternate_shallow_file;
 
-void set_alternate_shallow_file(const char *path, int override)
+void set_alternate_shallow_file_the_repository(const char *path, int override)
 {
if (is_shallow != -1)
die("BUG: is_repository_shallow must not be called before 
set_alternate_shallow_file");
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 04/19] commit: add repository argument to register_commit_graft

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow callers of register_commit_graft 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: Jonathan Nieder 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/blame.c | 3 ++-
 commit.c| 4 ++--
 commit.h| 3 ++-
 shallow.c   | 3 ++-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 0ffd1d443ea..7a07bff2423 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -8,6 +8,7 @@
 #include "cache.h"
 #include "config.h"
 #include "builtin.h"
+#include "repository.h"
 #include "commit.h"
 #include "diff.h"
 #include "revision.h"
@@ -491,7 +492,7 @@ static int read_ancestry(const char *graft_file)
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line();
if (graft)
-   register_commit_graft(graft, 0);
+   register_commit_graft(the_repository, graft, 0);
}
fclose(fp);
strbuf_release();
diff --git a/commit.c b/commit.c
index 2cd5b8a0b96..4e8d3488425 100644
--- a/commit.c
+++ b/commit.c
@@ -112,7 +112,7 @@ static int commit_graft_pos_the_repository(const unsigned 
char *sha1)
commit_graft_sha1_access);
 }
 
-int register_commit_graft(struct commit_graft *graft, int ignore_dups)
+int register_commit_graft_the_repository(struct commit_graft *graft, int 
ignore_dups)
 {
int pos = commit_graft_pos(the_repository, graft->oid.hash);
 
@@ -188,7 +188,7 @@ static int read_graft_file(const char *graft_file)
struct commit_graft *graft = read_graft_line();
if (!graft)
continue;
-   if (register_commit_graft(graft, 1))
+   if (register_commit_graft(the_repository, graft, 1))
error("duplicate graft data: %s", buf.buf);
}
fclose(fp);
diff --git a/commit.h b/commit.h
index 2d764ab7d8e..40a5b5e2585 100644
--- a/commit.h
+++ b/commit.h
@@ -174,7 +174,8 @@ struct commit_graft {
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
 struct commit_graft *read_graft_line(struct strbuf *line);
-int register_commit_graft(struct commit_graft *, int);
+#define register_commit_graft(r, g, i) register_commit_graft_##r(g, i)
+int register_commit_graft_the_repository(struct commit_graft *, int);
 struct commit_graft *lookup_commit_graft(const struct object_id *oid);
 
 extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit 
*rev2);
diff --git a/shallow.c b/shallow.c
index c2f81a5a5a8..ef802deed41 100644
--- a/shallow.c
+++ b/shallow.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "repository.h"
 #include "tempfile.h"
 #include "lockfile.h"
 #include "object-store.h"
@@ -38,7 +39,7 @@ int register_shallow(const struct object_id *oid)
graft->nr_parent = -1;
if (commit && commit->object.parsed)
commit->parents = NULL;
-   return register_commit_graft(graft, 0);
+   return register_commit_graft(the_repository, graft, 0);
 }
 
 int is_repository_shallow(void)
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 07/19] commit: add repository argument to lookup_commit_graft

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow callers of lookup_commit_graft 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: Jonathan Nieder 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 commit.c  | 4 ++--
 commit.h  | 3 ++-
 fsck.c| 2 +-
 shallow.c | 5 +++--
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/commit.c b/commit.c
index a0400b93a1e..c832133f055 100644
--- a/commit.c
+++ b/commit.c
@@ -212,7 +212,7 @@ static void prepare_commit_graft_the_repository(void)
commit_graft_prepared = 1;
 }
 
-struct commit_graft *lookup_commit_graft(const struct object_id *oid)
+struct commit_graft *lookup_commit_graft_the_repository(const struct object_id 
*oid)
 {
int pos;
prepare_commit_graft(the_repository);
@@ -359,7 +359,7 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
bufptr += tree_entry_len + 1; /* "tree " + "hex sha1" + "\n" */
pptr = >parents;
 
-   graft = lookup_commit_graft(>object.oid);
+   graft = lookup_commit_graft(the_repository, >object.oid);
while (bufptr + parent_entry_len < tail && !memcmp(bufptr, "parent ", 
7)) {
struct commit *new_parent;
 
diff --git a/commit.h b/commit.h
index 40a5b5e2585..f6746125766 100644
--- a/commit.h
+++ b/commit.h
@@ -176,7 +176,8 @@ typedef int (*each_commit_graft_fn)(const struct 
commit_graft *, void *);
 struct commit_graft *read_graft_line(struct strbuf *line);
 #define register_commit_graft(r, g, i) register_commit_graft_##r(g, i)
 int register_commit_graft_the_repository(struct commit_graft *, int);
-struct commit_graft *lookup_commit_graft(const struct object_id *oid);
+#define lookup_commit_graft(r, o) lookup_commit_graft_##r(o)
+struct commit_graft *lookup_commit_graft_the_repository(const struct object_id 
*oid);
 
 extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit 
*rev2);
 extern struct commit_list *get_merge_bases_many(struct commit *one, int n, 
struct commit **twos);
diff --git a/fsck.c b/fsck.c
index 59b0c7d640e..104c3c0a434 100644
--- a/fsck.c
+++ b/fsck.c
@@ -738,7 +738,7 @@ static int fsck_commit_buffer(struct commit *commit, const 
char *buffer,
buffer += 41;
parent_line_count++;
}
-   graft = lookup_commit_graft(>object.oid);
+   graft = lookup_commit_graft(the_repository, >object.oid);
parent_count = commit_list_count(commit->parents);
if (graft) {
if (graft->nr_parent == -1 && !parent_count)
diff --git a/shallow.c b/shallow.c
index ef802deed41..ca360c700c5 100644
--- a/shallow.c
+++ b/shallow.c
@@ -109,7 +109,7 @@ struct commit_list *get_shallow_commits(struct object_array 
*heads, int depth,
cur_depth++;
if ((depth != INFINITE_DEPTH && cur_depth >= depth) ||
(is_repository_shallow() && !commit->parents &&
-(graft = lookup_commit_graft(>object.oid)) != NULL 
&&
+(graft = lookup_commit_graft(the_repository, 
>object.oid)) != NULL &&
 graft->nr_parent < 0)) {
commit_list_insert(commit, );
commit->object.flags |= shallow_flag;
@@ -398,7 +398,8 @@ void prepare_shallow_info(struct shallow_info *info, struct 
oid_array *sa)
for (i = 0; i < sa->nr; i++) {
if (has_object_file(sa->oid + i)) {
struct commit_graft *graft;
-   graft = lookup_commit_graft(>oid[i]);
+   graft = lookup_commit_graft(the_repository,
+   >oid[i]);
if (graft && graft->nr_parent < 0)
continue;
info->ours[info->nr_ours++] = i;
-- 
2.17.0.582.gccdcbd54c44.dirty

Re: [PATCH 0/8] Reroll of sb/diff-color-move-more

[Jonathan Tan]

On Thu, 17 May 2018 12:46:45 -0700
Stefan Beller  wrote:

> Stefan Beller (8):
>   xdiff/xdiff.h: remove unused flags
>   xdiff/xdiffi.c: remove unneeded function declarations
>   diff.c: do not pass diff options as keydata to hashmap
>   diff.c: adjust hash function signature to match hashmap expectation
>   diff.c: add a blocks mode for moved code detection
>   diff.c: decouple white space treatment from move detection algorithm
>   diff.c: add --color-moved-ignore-space-delta option
>   diff: color-moved white space handling options imply color-moved

The test in patch 7 is indeed fixed, and patch 8 looks good to me.

There are still some review comments of mine outstanding [1] [2] but if
we decide that this series is good enough for now, that's OK.

[1] 
https://public-inbox.org/git/20180424153513.dc2404cd111c44ac78bd8...@google.com/
[2] 
https://public-inbox.org/git/20180424171123.7092788b94498908c25ec...@google.com/

[PATCH 06/19] commit: add repository argument to prepare_commit_graft

[Stefan Beller]

From: Jonathan Nieder 

Add a repository argument to allow the caller of prepare_commit_graft
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: Jonathan Nieder 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 commit.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/commit.c b/commit.c
index b5c0aec24a0..a0400b93a1e 100644
--- a/commit.c
+++ b/commit.c
@@ -197,7 +197,8 @@ static int read_graft_file_the_repository(const char 
*graft_file)
return 0;
 }
 
-static void prepare_commit_graft(void)
+#define prepare_commit_graft(r) prepare_commit_graft_##r()
+static void prepare_commit_graft_the_repository(void)
 {
static int commit_graft_prepared;
char *graft_file;
@@ -214,7 +215,7 @@ static void prepare_commit_graft(void)
 struct commit_graft *lookup_commit_graft(const struct object_id *oid)
 {
int pos;
-   prepare_commit_graft();
+   prepare_commit_graft(the_repository);
pos = commit_graft_pos(the_repository, oid->hash);
if (pos < 0)
return NULL;
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 18/19] commit: allow prepare_commit_graft to handle arbitrary repositories

[Stefan Beller]

Move the global variable 'commit_graft_prepared' into the object
pool and convert the function prepare_commit_graft to work
an arbitrary repositories.

Signed-off-by: Stefan Beller 
---
 commit.c | 14 ++
 object.h |  2 ++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/commit.c b/commit.c
index 24028fd257a..eef1675d692 100644
--- a/commit.c
+++ b/commit.c
@@ -196,19 +196,17 @@ static int read_graft_file(struct repository *r, const 
char *graft_file)
return 0;
 }
 
-#define prepare_commit_graft(r) prepare_commit_graft_##r()
-static void prepare_commit_graft_the_repository(void)
+static void prepare_commit_graft(struct repository *r)
 {
-   static int commit_graft_prepared;
char *graft_file;
 
-   if (commit_graft_prepared)
+   if (r->parsed_objects->commit_graft_prepared)
return;
-   graft_file = get_graft_file(the_repository);
-   read_graft_file(the_repository, graft_file);
+   graft_file = get_graft_file(r);
+   read_graft_file(r, graft_file);
/* make sure shallows are read */
-   is_repository_shallow(the_repository);
-   commit_graft_prepared = 1;
+   is_repository_shallow(r);
+   r->parsed_objects->commit_graft_prepared = 1;
 }
 
 struct commit_graft *lookup_commit_graft_the_repository(const struct object_id 
*oid)
diff --git a/object.h b/object.h
index a314331acaf..4af499ab03e 100644
--- a/object.h
+++ b/object.h
@@ -20,6 +20,8 @@ struct parsed_object_pool {
int is_shallow;
struct stat_validity *shallow_stat;
char *alternate_shallow_file;
+
+   int commit_graft_prepared;
 };
 
 struct parsed_object_pool *parsed_object_pool_new(void);
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 11/19] shallow: add repository argument to is_repository_shallow

[Stefan Beller]

Add a repository argument to allow callers of is_repository_shallow
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 
---
 builtin/fetch.c| 2 +-
 builtin/pack-objects.c | 4 ++--
 builtin/prune.c| 2 +-
 builtin/rev-parse.c| 3 ++-
 commit.c   | 2 +-
 commit.h   | 3 ++-
 fetch-pack.c   | 4 ++--
 send-pack.c| 6 +++---
 shallow.c  | 8 
 upload-pack.c  | 2 +-
 10 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index c1f2df97965..55140671ef3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1445,7 +1445,7 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
if (unshallow) {
if (depth)
die(_("--depth and --unshallow cannot be used 
together"));
-   else if (!is_repository_shallow())
+   else if (!is_repository_shallow(the_repository))
die(_("--unshallow on a complete repository does not 
make sense"));
else
depth = xstrfmt("%d", INFINITE_DEPTH);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 97a5963efb6..0f1eec2eecd 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2857,7 +2857,7 @@ static void get_object_list(int ac, const char **av)
setup_revisions(ac, av, , NULL);
 
/* make sure shallows are read */
-   is_repository_shallow();
+   is_repository_shallow(the_repository);
 
while (fgets(line, sizeof(line), stdin) != NULL) {
int len = strlen(line);
@@ -3142,7 +3142,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
use_bitmap_index = use_bitmap_index_default;
 
/* "hard" reasons not to use bitmaps; these just won't work at all */
-   if (!use_internal_rev_list || (!pack_to_stdout && write_bitmap_index) 
|| is_repository_shallow())
+   if (!use_internal_rev_list || (!pack_to_stdout && write_bitmap_index) 
|| is_repository_shallow(the_repository))
use_bitmap_index = 0;
 
if (pack_to_stdout || !rev_list_all)
diff --git a/builtin/prune.c b/builtin/prune.c
index 8cc8659612f..70ec35aa058 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -160,7 +160,7 @@ int cmd_prune(int argc, const char **argv, const char 
*prefix)
remove_temporary_files(s);
free(s);
 
-   if (is_repository_shallow())
+   if (is_repository_shallow(the_repository))
prune_shallow(show_only);
 
return 0;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 36b20877828..a8a9b506ff6 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -879,7 +879,8 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, "--is-shallow-repository")) {
-   printf("%s\n", is_repository_shallow() ? "true"
+   printf("%s\n",
+   
is_repository_shallow(the_repository) ? "true"
: "false");
continue;
}
diff --git a/commit.c b/commit.c
index c832133f055..684eeaa2ccd 100644
--- a/commit.c
+++ b/commit.c
@@ -208,7 +208,7 @@ static void prepare_commit_graft_the_repository(void)
graft_file = get_graft_file();
read_graft_file(the_repository, graft_file);
/* make sure shallows are read */
-   is_repository_shallow();
+   is_repository_shallow(the_repository);
commit_graft_prepared = 1;
 }
 
diff --git a/commit.h b/commit.h
index 59346de5512..c7f25d6490a 100644
--- a/commit.h
+++ b/commit.h
@@ -195,7 +195,8 @@ struct ref;
 extern int register_shallow_the_repository(const struct object_id *oid);
 extern int unregister_shallow(const struct object_id *oid);
 extern int for_each_commit_graft(each_commit_graft_fn, void *);
-extern int is_repository_shallow(void);
+#define is_repository_shallow(r) is_repository_shallow_##r()
+extern int is_repository_shallow_the_repository(void);
 extern struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
 extern struct commit_list *get_shallow_commits_by_rev_list(
diff --git a/fetch-pack.c b/fetch-pack.c
index e3e99e44962..90befd370fe 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -397,7 +397,7 @@ static int find_common(struct fetch_pack_args *args,
return 1;
}
 
-   

[PATCH 17/19] shallow: migrate shallow information into the object parser

[Stefan Beller]

We need to convert the shallow functions all at the same time
as we move the data structures they operate on into the repository.

Signed-off-by: Stefan Beller 
---
 commit.h  |  9 +++--
 object.c  |  3 +++
 object.h  |  4 
 shallow.c | 50 +++---
 4 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/commit.h b/commit.h
index d04bbed81cf..45114a95b25 100644
--- a/commit.h
+++ b/commit.h
@@ -190,18 +190,15 @@ extern struct commit_list 
*get_merge_bases_many_dirty(struct commit *one, int n,
 
 struct oid_array;
 struct ref;
-#define register_shallow(r, o) register_shallow_##r(o);
-extern int register_shallow_the_repository(const struct object_id *oid);
+extern int register_shallow(struct repository *r, const struct object_id *oid);
 extern int unregister_shallow(const struct object_id *oid);
 extern int for_each_commit_graft(each_commit_graft_fn, void *);
-#define is_repository_shallow(r) is_repository_shallow_##r()
-extern int is_repository_shallow_the_repository(void);
+extern int is_repository_shallow(struct repository *r);
 extern struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
 extern struct commit_list *get_shallow_commits_by_rev_list(
int ac, const char **av, int shallow_flag, int 
not_shallow_flag);
-#define set_alternate_shallow_file(r, p, o) set_alternate_shallow_file_##r(p, 
o)
-extern void set_alternate_shallow_file_the_repository(const char *path, int 
override);
+extern void set_alternate_shallow_file(struct repository *r, const char *path, 
int override);
 extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
 const struct oid_array *extra);
 extern void setup_alternate_shallow(struct lock_file *shallow_lock,
diff --git a/object.c b/object.c
index 0116ed6529a..30b8a721cf6 100644
--- a/object.c
+++ b/object.c
@@ -464,6 +464,9 @@ struct parsed_object_pool *parsed_object_pool_new(void)
o->tag_state = allocate_alloc_state();
o->object_state = allocate_alloc_state();
 
+   o->is_shallow = -1;
+   o->shallow_stat = xcalloc(1, sizeof(*o->shallow_stat));
+
return o;
 }
 
diff --git a/object.h b/object.h
index ec908f9bcc1..a314331acaf 100644
--- a/object.h
+++ b/object.h
@@ -16,6 +16,10 @@ struct parsed_object_pool {
/* parent substitutions from .git/info/grafts and .git/shallow */
struct commit_graft **grafts;
int grafts_alloc, grafts_nr;
+
+   int is_shallow;
+   struct stat_validity *shallow_stat;
+   char *alternate_shallow_file;
 };
 
 struct parsed_object_pool *parsed_object_pool_new(void);
diff --git a/shallow.c b/shallow.c
index a0e338459f9..9f6ee351319 100644
--- a/shallow.c
+++ b/shallow.c
@@ -14,22 +14,19 @@
 #include "commit-slab.h"
 #include "revision.h"
 #include "list-objects.h"
+#include "repository.h"
 
-static int is_shallow = -1;
-static struct stat_validity shallow_stat;
-static char *alternate_shallow_file;
-
-void set_alternate_shallow_file_the_repository(const char *path, int override)
+void set_alternate_shallow_file(struct repository *r, const char *path, int 
override)
 {
-   if (is_shallow != -1)
+   if (r->parsed_objects->is_shallow != -1)
die("BUG: is_repository_shallow must not be called before 
set_alternate_shallow_file");
-   if (alternate_shallow_file && !override)
+   if (r->parsed_objects->alternate_shallow_file && !override)
return;
-   free(alternate_shallow_file);
-   alternate_shallow_file = xstrdup_or_null(path);
+   free(r->parsed_objects->alternate_shallow_file);
+   r->parsed_objects->alternate_shallow_file = xstrdup_or_null(path);
 }
 
-int register_shallow_the_repository(const struct object_id *oid)
+int register_shallow(struct repository *r, const struct object_id *oid)
 {
struct commit_graft *graft =
xmalloc(sizeof(struct commit_graft));
@@ -39,41 +36,41 @@ int register_shallow_the_repository(const struct object_id 
*oid)
graft->nr_parent = -1;
if (commit && commit->object.parsed)
commit->parents = NULL;
-   return register_commit_graft(the_repository, graft, 0);
+   return register_commit_graft(r, graft, 0);
 }
 
-int is_repository_shallow_the_repository(void)
+int is_repository_shallow(struct repository *r)
 {
FILE *fp;
char buf[1024];
-   const char *path = alternate_shallow_file;
+   const char *path = r->parsed_objects->alternate_shallow_file;
 
-   if (is_shallow >= 0)
-   return is_shallow;
+   if (r->parsed_objects->is_shallow >= 0)
+   return r->parsed_objects->is_shallow;
 
if (!path)
-   path = git_path_shallow(the_repository);
+   path = git_path_shallow(r);
/*
 * fetch-pack sets '--shallow-file ""' as an 

[PATCH 16/19] path.c: migrate global git_path_* to take a repository argument

[Stefan Beller]

Migrate all git_path_* functions that are defined in path.c to take a
repository argument. Unlike other patches in this series, do not use the
 #define trick, as we rewrite the whole function, which is rather small.

This doesn't migrate all the functions, as other builtins have their own
local path functions defined using GIT_PATH_FUNC. So keep that macro
around to serve the other locations.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 blame.c  |  8 +---
 branch.c | 14 +++---
 builtin/commit.c | 38 +++---
 builtin/fetch.c  |  4 ++--
 builtin/merge.c  | 37 +++--
 builtin/pull.c   |  4 ++--
 builtin/reset.c  |  2 +-
 fetch-pack.c |  2 +-
 path.c   | 18 +-
 path.h   | 40 +++-
 repository.h |  5 +
 rerere.c |  7 ---
 sequencer.c  | 37 +++--
 shallow.c| 12 +++-
 wt-status.c  |  8 
 15 files changed, 135 insertions(+), 101 deletions(-)

diff --git a/blame.c b/blame.c
index f689bde31cd..c22184c2dad 100644
--- a/blame.c
+++ b/blame.c
@@ -112,17 +112,19 @@ static void append_merge_parents(struct commit_list 
**tail)
int merge_head;
struct strbuf line = STRBUF_INIT;
 
-   merge_head = open(git_path_merge_head(), O_RDONLY);
+   merge_head = open(git_path_merge_head(the_repository), O_RDONLY);
if (merge_head < 0) {
if (errno == ENOENT)
return;
-   die("cannot open '%s' for reading", git_path_merge_head());
+   die("cannot open '%s' for reading",
+   git_path_merge_head(the_repository));
}
 
while (!strbuf_getwholeline_fd(, merge_head, '\n')) {
struct object_id oid;
if (line.len < GIT_SHA1_HEXSZ || get_oid_hex(line.buf, ))
-   die("unknown line in '%s': %s", git_path_merge_head(), 
line.buf);
+   die("unknown line in '%s': %s",
+   git_path_merge_head(the_repository), line.buf);
tail = append_parent(tail, );
}
close(merge_head);
diff --git a/branch.c b/branch.c
index 2672054f0b5..9b2742de32a 100644
--- a/branch.c
+++ b/branch.c
@@ -339,13 +339,13 @@ void create_branch(const char *name, const char 
*start_name,
 
 void remove_branch_state(void)
 {
-   unlink(git_path_cherry_pick_head());
-   unlink(git_path_revert_head());
-   unlink(git_path_merge_head());
-   unlink(git_path_merge_rr());
-   unlink(git_path_merge_msg());
-   unlink(git_path_merge_mode());
-   unlink(git_path_squash_msg());
+   unlink(git_path_cherry_pick_head(the_repository));
+   unlink(git_path_revert_head(the_repository));
+   unlink(git_path_merge_head(the_repository));
+   unlink(git_path_merge_rr(the_repository));
+   unlink(git_path_merge_msg(the_repository));
+   unlink(git_path_merge_mode(the_repository));
+   unlink(git_path_squash_msg(the_repository));
 }
 
 void die_if_checked_out(const char *branch, int ignore_current_worktree)
diff --git a/builtin/commit.c b/builtin/commit.c
index 37fcb55ab0a..7c22879777d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -145,9 +145,9 @@ static int opt_parse_m(const struct option *opt, const char 
*arg, int unset)
 
 static void determine_whence(struct wt_status *s)
 {
-   if (file_exists(git_path_merge_head()))
+   if (file_exists(git_path_merge_head(the_repository)))
whence = FROM_MERGE;
-   else if (file_exists(git_path_cherry_pick_head())) {
+   else if (file_exists(git_path_cherry_pick_head(the_repository))) {
whence = FROM_CHERRY_PICK;
if (file_exists(git_path_seq_dir()))
sequencer_in_use = 1;
@@ -696,21 +696,21 @@ static int prepare_to_commit(const char *index_file, 
const char *prefix,
if (have_option_m)
strbuf_addbuf(, );
hook_arg1 = "message";
-   } else if (!stat(git_path_merge_msg(), )) {
+   } else if (!stat(git_path_merge_msg(the_repository), )) {
/*
 * prepend SQUASH_MSG here if it exists and a
 * "merge --squash" was originally performed
 */
-   if (!stat(git_path_squash_msg(), )) {
-   if (strbuf_read_file(, git_path_squash_msg(), 0) < 0)
+   if (!stat(git_path_squash_msg(the_repository), )) {
+   if (strbuf_read_file(, 
git_path_squash_msg(the_repository), 0) < 0)
die_errno(_("could not read SQUASH_MSG"));
hook_arg1 = "squash";
} else
hook_arg1 = "merge";
-   if (strbuf_read_file(, 

[PATCH 19/19] commit: allow lookup_commit_graft to handle arbitrary repositories

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 commit.c | 8 
 commit.h | 3 +--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/commit.c b/commit.c
index eef1675d692..b01cc0c3e0c 100644
--- a/commit.c
+++ b/commit.c
@@ -209,14 +209,14 @@ static void prepare_commit_graft(struct repository *r)
r->parsed_objects->commit_graft_prepared = 1;
 }
 
-struct commit_graft *lookup_commit_graft_the_repository(const struct object_id 
*oid)
+struct commit_graft *lookup_commit_graft(struct repository *r, const struct 
object_id *oid)
 {
int pos;
-   prepare_commit_graft(the_repository);
-   pos = commit_graft_pos(the_repository, oid->hash);
+   prepare_commit_graft(r);
+   pos = commit_graft_pos(r, oid->hash);
if (pos < 0)
return NULL;
-   return the_repository->parsed_objects->grafts[pos];
+   return r->parsed_objects->grafts[pos];
 }
 
 int for_each_commit_graft(each_commit_graft_fn fn, void *cb_data)
diff --git a/commit.h b/commit.h
index 45114a95b25..6de6f10cd04 100644
--- a/commit.h
+++ b/commit.h
@@ -175,8 +175,7 @@ typedef int (*each_commit_graft_fn)(const struct 
commit_graft *, void *);
 
 struct commit_graft *read_graft_line(struct strbuf *line);
 int register_commit_graft(struct repository *r, struct commit_graft *, int);
-#define lookup_commit_graft(r, o) lookup_commit_graft_##r(o)
-struct commit_graft *lookup_commit_graft_the_repository(const struct object_id 
*oid);
+struct commit_graft *lookup_commit_graft(struct repository *r, const struct 
object_id *oid);
 
 extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit 
*rev2);
 extern struct commit_list *get_merge_bases_many(struct commit *one, int n, 
struct commit **twos);
-- 
2.17.0.582.gccdcbd54c44.dirty

Re: What's cooking in git.git (May 2018, #02; Thu, 17)

[Stefan Beller]

On Thu, May 17, 2018 at 3:36 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>> * sb/object-store-replace (2018-05-10) 2 commits
>>>   (merged to 'next' on 2018-05-16 at 41bbedcc81)
>>>  + replace-object.c: remove the_repository from prepare_replace_object
>>>  + object.c: free replace map in raw_object_store_clear
>>>
>>>  Hotfix.
>>>
>>>  Will merge to 'master'.
>>
>> Please do not.
>> (Or do, but then be prepared for another hotfix.)
>>
>> The commit sb/object-store-replace^ needs more free'ing and shall be
>> replaced with
>
> Please do not replace what already hit 'next'.

I missed that it hit next. Sorry about that. I'll roll a patch on top then.

Re: What's cooking in git.git (May 2018, #02; Thu, 17)

[Junio C Hamano]

Stefan Beller  writes:

>> * sb/object-store-replace (2018-05-10) 2 commits
>>   (merged to 'next' on 2018-05-16 at 41bbedcc81)
>>  + replace-object.c: remove the_repository from prepare_replace_object
>>  + object.c: free replace map in raw_object_store_clear
>>
>>  Hotfix.
>>
>>  Will merge to 'master'.
>
> Please do not.
> (Or do, but then be prepared for another hotfix.)
>
> The commit sb/object-store-replace^ needs more free'ing and shall be
> replaced with

Please do not replace what already hit 'next'.

> https://public-inbox.org/git/20180510195849.28023-4-sbel...@google.com/
> I'll resend shortly.
>
>>
>> * sb/submodule-merge-in-merge-recursive (2018-05-16) 3 commits
>>  - merge-recursive: give notice when submodule commit gets fast-forwarded
>>  - merge-recursive: i18n submodule merge output and respect verbosity
>>  - submodule.c: move submodule merging to merge-recursive.c
>>
>>  By code restructuring of submodule merge in merge-recursive,
>>  informational messages from the codepath are now given using the
>>  same mechanism as other output, and honor the merge.verbosity
>>  configuration.  The code also learned to give a few new messages
>>  when a submodule three-way merge resolves cleanly when one side
>>  records a descendant of the commit chosen by the other side.
>>
>>  Will merge to 'next'.
>
> Merging would be ok, but I would rather not.
> A resend will be only for cosmetic effect, as I messed up the last commit
>
> So, please hold in pu.
>
>> * sb/diff-color-move-more (2018-04-25) 7 commits
>>  - diff.c: add --color-moved-ignore-space-delta option
>>  - diff.c: decouple white space treatment from move detection algorithm
>>  - diff.c: add a blocks mode for moved code detection
>>  - diff.c: adjust hash function signature to match hashmap expectation
>>  - diff.c: do not pass diff options as keydata to hashmap
>>  - xdiff/xdiffi.c: remove unneeded function declarations
>>  - xdiff/xdiff.h: remove unused flags
>>
>>  "git diff --color-moved" feature has further been tweaked.
>>
>>  Will merge to 'next'.
>
> I did not get around to fix it up, there are still review
> comments outstanding. (The test is broken in the last commit.)
>
> Please hold in pu;
>
> Thanks,
> Stefan

Re: Troubles with picking an editor during Git update

[Johannes Schindelin]

Hi Bartosz,

On Thu, 17 May 2018, Bartosz Konikiewicz wrote:

> I had an issue with Git installer for Windows while trying to update
> my instance of the software. My previous version was "git version
> 2.15.1.windows.2", while my operating system prompted me to upgrade to
> "2.17.0". The installer asked me to "choose the default editor for
> Git". One of these options was Notepad++ - my editor of choice. Vim
> was selected by default and I've picked Notepad++ from a drop-down
> list. As soon as I did it, a "next" button greyed out. When I moved
> back to the previous step and then forward to the editor choice, the
> "Notepad++" option was still highlighted, and the "next" button wasn't
> greyed out anymore - it was active and I was able to press it and
> continue installation.
> 
> Steps to reproduce:
> 
> 1. Have Notepad++ 6.6.9 installed on Windows 10 64-bit 10.0.17134 Build 17134.
> 2. Use an installer for version 2.17.0 to upgrade from version 2.15.1.
> 3. On an editor selection screen, choose Notepad++ instead of Vim. You
> should be unable to continue installation because of the "next" button
> being disabled.
> 4. Press "prev".
> 5. Press "next". Notepad++ should be still highlighted, and the "next"
> button should be active, allowing to continue installation.
> 
> I find it to be a crafty trick to make me use Vim. I have considered
> it for a good moment.

The code to generate that wizard page is here:
https://github.com/git-for-windows/build-extra/blob/5c6f2997acd694f5375722d8d494fb1337abc1fa/installer/install.iss#L-L1196

Can you spot anything obvious that could be responsible for the issue you
reported? (I can't.)

Ciao,
Johannes

Re: [PATCH v2 3/3] unpack_trees_options: free messages when done

[Junio C Hamano]

Martin Ågren  writes:

> The strings allocated in `setup_unpack_trees_porcelain()` are never
> freed. Provide a function `clear_unpack_trees_porcelain()` to do so and
> call it where we use `setup_unpack_trees_porcelain()`. The only
> non-trivial user is `unpack_trees_start()`, where we should place the
> new call in `unpack_trees_finish()`.
>
> The `opts` string array contains multiple copies of the same pointers.
> Be careful to only free each pointer once, then zeroize the whole array
> so that we do not leave any dangling pointers.

The verb to make it zero or fill it with zero is "to zero", I would
think.

To be honest I am not sure if I like the way this change is done.
The clear_unpack_trees_porcelain() function has too intimate
knowledge of what happens inside the setup_unpack_trees_porcelain()
function; it not just knows which fields are always allocated but
which are duplicates, which must be double checked for updates
whenever the latter gets modified, yet there is no large warning
sign painted in red in the latter, so it is easy to change the
latter and invalidate the assumption the former makes by mistake,
leading to new leaks and/or double freeing.

I wonder if an approach that is longer-term a bit more maintainable
is to add a new string-list instance to opts, save these xstrfmt()'ed
messages to it when setup_unpack_trees_porcelain() create them, and
then make clear_unpack_trees_porcelain() pay *no* attention to msg[]
array and the positions of these allocated messages and duplicates
but just reclaim the resources held in that string-list, or
something like that.

Re: [PATCH v2 1/3] merge: setup `opts` later in `checkout_fast_forward()`

[Junio C Hamano]

Martin Ågren  writes:

> After we initialize the various fields in `opts` but before we actually
> use them, we might return early. Move the initialization further down,
> to immediately before we use `opts`.
>
> This limits the scope of `opts` and will help a later commit fix a
> memory leak without having to worry about those early returns.
>
> This patch is best viewed using something like this (note the tab!):
> --color-moved --anchored="trees[nr_trees] = parse_tree_indirect"

This side remark is interesting because it totally depends on how
you look at it.  I think "initialize opts late" and "attempt to
parse the trees first and fail early" are the sides of the same
coin, and the diff shown without the anchor matches the latter,
which is also perfectly acceptable interpretation of what this patch
does.

[PATCH 0/1] Hot fix for js/empty-config-section-fix

[Johannes Schindelin]

In https://public-inbox.org/git/20180508134248.ga25...@sigill.intra.peff.net/
Jeff King pointed out that an invalid config section is not an indicator
of a bug, as it is usually provided by the user.

So we should not throw a fit and tell the user about a bug that they
might even report.

Instead, let's just error out.


Johannes Schindelin (1):
  config: a user-provided invalid section is not a BUG

 config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: ccdcbd54c4475c2238b310f7113ab3075b5abc9c
Published-As: 
https://github.com/dscho/git/releases/tag/empty-config-section-fix-v1
Fetch-It-Via: git fetch https://github.com/dscho/git empty-config-section-fix-v1
-- 
2.17.0.windows.1.42.gaba71d8cd65

[PATCH 1/1] config: a user-provided invalid section is not a BUG

[Johannes Schindelin]

This was pointed out by Jeff King while the empty-config-section-fix
patch series was cooking, and was not addressed in time for that patch
series to advance to `master`.

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

diff --git a/config.c b/config.c
index 6f8f1d8c113..2392a8076b7 100644
--- a/config.c
+++ b/config.c
@@ -2359,7 +2359,7 @@ static int store_aux_event(enum config_event_t type,
 
if (type == CONFIG_EVENT_SECTION) {
if (cf->var.len < 2 || cf->var.buf[cf->var.len - 1] != '.')
-   BUG("Invalid section name '%s'", cf->var.buf);
+   return error("invalid section name '%s'", cf->var.buf);
 
/* Is this the section we were looking for? */
store->is_keys_section =
-- 
2.17.0.windows.1.42.gaba71d8cd65

Re: [PATCH 0/2] generating ref-prefixes for configured refspecs

[Junio C Hamano]

Brandon Williams  writes:

> Here's my short follow on series to the refspec refactoring.
>
> When v2 was introduced ref-prefixes were only generated for user
> provided refspecs (given via the command line).  This means that you can
> only benefit from server-side ref filtering if you explicitly provide a
> refspec, so this short series extends this to generate the ref-prefixes
> even for the refspecs which are configured in 'remote..fetch'.
>
> This series is based on the v2 of the refspec refactoring series.

Makes sense.

>
> Brandon Williams (2):
>   refspec: consolidate ref-prefix generation logic
>   fetch: generate ref-prefixes when using a configured refspec
>
>  builtin/fetch.c| 19 ---
>  refspec.c  | 29 +
>  refspec.h  |  4 
>  t/t5702-protocol-v2.sh | 14 ++
>  transport.c| 21 +
>  5 files changed, 56 insertions(+), 31 deletions(-)

Re: Troubles with picking an editor during Git update

[Philip Oakley, CEng MIET]

Hi Bartosz,

From: "Bartosz Konikiewicz" 

Hi there!

I had an issue with Git installer for Windows while trying to update


The Git for Windows package is managed, via https://gitforwindows.org/, as a 
separate application, based on Git.



my instance of the software. My previous version was "git version
2.15.1.windows.2", while my operating system prompted me to upgrade to
"2.17.0". The installer asked me to "choose the default editor for
Git". One of these options was Notepad++ - my editor of choice. Vim
was selected by default and I've picked Notepad++ from a drop-down
list. As soon as I did it, a "next" button greyed out. When I moved
back to the previous step and then forward to the editor choice, the
"Notepad++" option was still highlighted, and the "next" button wasn't
greyed out anymore - it was active and I was able to press it and
continue installation.

Steps to reproduce:

1. Have Notepad++ 6.6.9 installed on Windows 10 64-bit 10.0.17134 Build 
17134.

2. Use an installer for version 2.17.0 to upgrade from version 2.15.1.
3. On an editor selection screen, choose Notepad++ instead of Vim. You
should be unable to continue installation because of the "next" button
being disabled.
4. Press "prev".
5. Press "next". Notepad++ should be still highlighted, and the "next"
button should be active, allowing to continue installation.

I find it to be a crafty trick to make me use Vim. I have considered
it for a good moment.

The best place to report the issue, and perhaps contribure is via the 'GfW' 
Issue tracker https://github.com/git-for-windows/git/issues.


Building Git for Windows via the SDK has become even easier with recent 
updates, so it should be relativley easy to spot the offending line in the 
installer and perhaps even propose a PR (Pull Request) to fix the issue.


regards
Philip

[PATCH 8/8] diff: color-moved white space handling options imply color-moved

[Stefan Beller]

When giving options how move coloring should treat white spaces, the user
expects that move coloring is actually active. If it is not active, we
can just take the default mode.

Signed-off-by: Stefan Beller 
---
 diff.c | 18 +-
 t/t4015-diff-whitespace.sh |  9 +++--
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index 1227a4d2a83..a10184e576e 100644
--- a/diff.c
+++ b/diff.c
@@ -4713,15 +4713,23 @@ int diff_opt_parse(struct diff_options *options,
options->color_moved_ws_handling &= 
~XDF_IGNORE_WHITESPACE_AT_EOL;
else if (!strcmp(arg, "--color-moved-no-ignore-space-prefix-delta"))
options->color_moved_ws_handling &= 
~COLOR_MOVED_DELTA_WHITESPACES;
-   else if (!strcmp(arg, "--color-moved-ignore-all-space"))
+   else if (!strcmp(arg, "--color-moved-ignore-all-space")) {
+   if (options->color_moved == COLOR_MOVED_NO)
+   options->color_moved = COLOR_MOVED_DEFAULT;
options->color_moved_ws_handling |= XDF_IGNORE_WHITESPACE;
-   else if (!strcmp(arg, "--color-moved-ignore-space-change"))
+   } else if (!strcmp(arg, "--color-moved-ignore-space-change")) {
options->color_moved_ws_handling |= 
XDF_IGNORE_WHITESPACE_CHANGE;
-   else if (!strcmp(arg, "--color-moved-ignore-space-at-eol"))
+   if (options->color_moved == COLOR_MOVED_NO)
+   options->color_moved = COLOR_MOVED_DEFAULT;
+   } else if (!strcmp(arg, "--color-moved-ignore-space-at-eol")) {
options->color_moved_ws_handling |= 
XDF_IGNORE_WHITESPACE_AT_EOL;
-   else if (!strcmp(arg, "--color-moved-ignore-space-prefix-delta"))
+   if (options->color_moved == COLOR_MOVED_NO)
+   options->color_moved = COLOR_MOVED_DEFAULT;
+   } else if (!strcmp(arg, "--color-moved-ignore-space-prefix-delta")) {
options->color_moved_ws_handling |= 
COLOR_MOVED_DELTA_WHITESPACES;
-   else if (!strcmp(arg, "--indent-heuristic"))
+   if (options->color_moved == COLOR_MOVED_NO)
+   options->color_moved = COLOR_MOVED_DEFAULT;
+   } else if (!strcmp(arg, "--indent-heuristic"))
DIFF_XDL_SET(options, INDENT_HEURISTIC);
else if (!strcmp(arg, "--no-indent-heuristic"))
DIFF_XDL_CLR(options, INDENT_HEURISTIC);
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 37ff528822f..e1ab08d7bb4 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1875,7 +1875,11 @@ test_expect_success 'compare whitespace delta across 
moved blocks' '
 
git diff --color --color-moved --color-moved-ignore-space-prefix-delta |
grep -v "index" |
-   test_decode_color >actual &&
+   test_decode_color >actual1 &&
+
+   git diff --color --color-moved-ignore-space-prefix-delta |
+   grep -v "index" |
+   test_decode_color >actual2 &&
 
q_to_tab <<-\EOF >expected &&
diff --git a/text.txt b/text.txt
@@ -1898,7 +1902,8 @@ test_expect_success 'compare whitespace delta across 
moved blocks' '
+not adjust
EOF
 
-   test_cmp expected actual
+   test_cmp expected actual1 &&
+   test_cmp expected actual2
 '
 
 test_done
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 6/8] diff.c: decouple white space treatment from move detection algorithm

[Stefan Beller]

In the original implementation of the move detection logic the choice for
ignoring white space changes is the same for the move detection as it is
for the regular diff.  Some cases came up where different treatment would
have been nice.

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

As we change the default, we'll adjust the tests.

For now we do not infer any options to treat whitespaces in the move
detection from the generic white space options given to diff.
This can be tuned later to reasonable default.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 Documentation/diff-options.txt | 13 +
 diff.c | 19 ++-
 diff.h |  1 +
 t/t4015-diff-whitespace.sh | 90 +++---
 4 files changed, 114 insertions(+), 9 deletions(-)

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

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

[Stefan Beller]

When we initialize the hashmap, we give it a pointer to the
diff_options, which it then passes along to each call of the
hashmap_cmp_fn function. There's no need to pass it a second time as
the "keydata" parameter, and our comparison functions never look at
keydata.

This was a mistake left over from an earlier round of 2e2d5ac184
(diff.c: color moved lines differently, 2017-06-30), before hashmap
learned to pass the data pointer for us.

Explanation-by: Jeff King 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 1289df4b1f9..ce7bedc1b92 100644
--- a/diff.c
+++ b/diff.c
@@ -842,13 +842,13 @@ static void mark_color_as_moved(struct diff_options *o,
case DIFF_SYMBOL_PLUS:
hm = del_lines;
key = prepare_entry(o, n);
-   match = hashmap_get(hm, key, o);
+   match = hashmap_get(hm, key, NULL);
free(key);
break;
case DIFF_SYMBOL_MINUS:
hm = add_lines;
key = prepare_entry(o, n);
-   match = hashmap_get(hm, key, o);
+   match = hashmap_get(hm, key, NULL);
free(key);
break;
default:
-- 
2.17.0.582.gccdcbd54c44.dirty

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

[Stefan Beller]

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

Signed-off-by: Stefan Beller 
---
 Documentation/diff-options.txt |  4 ++
 diff.c | 83 +++---
 diff.h |  2 +
 t/t4015-diff-whitespace.sh | 54 ++
 4 files changed, 137 insertions(+), 6 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 7b2527b9a19..facdbc8f95f 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -304,6 +304,10 @@ dimmed_zebra::
Ignore whitespace when comparing lines when performing the move
detection for --color-moved.  This ignores differences even if
one line has whitespace where the other line has none.
+--color-moved-[no-]ignore-space-prefix-delta::
+   Ignores whitespace when comparing lines when performing the move
+   detection for --color-moved. This ignores uniform differences
+   of white space at the beginning lines in moved blocks.
 
 --word-diff[=]::
Show a word diff, using the  to delimit changed words.
diff --git a/diff.c b/diff.c
index b5819dd538f..1227a4d2a83 100644
--- a/diff.c
+++ b/diff.c
@@ -709,6 +709,31 @@ struct moved_entry {
struct moved_entry *next_line;
 };
 
+struct ws_delta {
+   char *string; /* The prefix delta, which is the same in the block */
+   int direction; /* adding or removing the line? */
+   int missmatch; /* in the remainder */
+};
+#define WS_DELTA_INIT { NULL, 0, 0 }
+
+static void compute_ws_delta(const struct emitted_diff_symbol *a,
+const struct emitted_diff_symbol *b,
+struct ws_delta *out)
+{
+   const struct emitted_diff_symbol *longer =  a->len > b->len ? a : b;
+   const struct emitted_diff_symbol *shorter = a->len > b->len ? b : a;
+   int d = longer->len - shorter->len;
+
+   out->missmatch = !memcmp(longer->line + d, shorter->line, shorter->len);
+   out->string = xmemdupz(longer->line, d);
+   out->direction = (a == longer);
+}
+
+static int compare_ws_delta(const struct ws_delta *a, const struct ws_delta *b)
+{
+   return a->direction == b->direction && !strcmp(a->string, b->string);
+}
+
 static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
   const void *entry,
   const void *entry_or_key,
@@ -720,6 +745,15 @@ static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
unsigned flags = diffopt->color_moved_ws_handling
 & XDF_WHITESPACE_FLAGS;
 
+   if (diffopt->color_moved_ws_handling & COLOR_MOVED_DELTA_WHITESPACES)
+   /*
+* As there is not specific white space config given,
+* we'd need to check for a new block, so ignore all
+* white space. The setup of the white space
+* configuration for the next block is done else where
+*/
+   flags |= XDF_IGNORE_WHITESPACE;
+
return !xdiff_compare_lines(a->es->line, a->es->len,
b->es->line, b->es->len,
flags);
@@ -772,7 +806,8 @@ static void add_lines_to_move_detection(struct diff_options 
*o,
 }
 
 static int shrink_potential_moved_blocks(struct moved_entry **pmb,
-int pmb_nr)
+int pmb_nr,
+struct ws_delta **wsd)
 {
int lp, rp;
 
@@ -788,6 +823,10 @@ static int shrink_potential_moved_blocks(struct 
moved_entry **pmb,
 
if (lp < pmb_nr && rp > -1 && lp < rp) {
pmb[lp] = pmb[rp];
+   if (*wsd) {
+   free((*wsd)[lp].string);
+   (*wsd)[lp] = (*wsd)[rp];
+   }
pmb[rp] = NULL;
rp--;
lp++;
@@ -837,8 +876,11 @@ static void mark_color_as_moved(struct diff_options *o,
 {
struct moved_entry **pmb = NULL; /* potentially moved blocks */
int pmb_nr = 0, pmb_alloc = 0;
-   int n, flipped_block = 1, block_length = 0;
 
+   struct ws_delta *wsd = NULL; /* white space deltas between pmb */
+   int wsd_alloc = 0;
+
+   int n, flipped_block = 1, block_length = 0;
 
for (n = 0; n < o->emitted_symbols->nr; n++) {
struct hashmap *hm = NULL;
@@ -881,14 +923,31 @@ static void mark_color_as_moved(struct diff_options *o,
struct moved_entry *p = pmb[i];
struct moved_entry *pnext = (p && p->next_line) ?
p->next_line : NULL;
-   if (pnext && !hm->cmpfn(o, pnext, match, NULL)) {
-   pmb[i] = p->next_line;

[PATCH 0/8] Reroll of sb/diff-color-move-more

[Stefan Beller]

>> * sb/diff-color-move-more (2018-04-25) 7 commits
>...
>>
>>  Will merge to 'next'.
>
>I did not get around to fix it up, there are still review
>comments outstanding. (The test is broken in the last commit.)

This is a reroll of sb/diff-color-move-more, with the test fixed as well
as another extra patch, that would have caught the bad test. 

The range diff is below.

Thanks,
Stefan

Stefan Beller (8):
  xdiff/xdiff.h: remove unused flags
  xdiff/xdiffi.c: remove unneeded function declarations
  diff.c: do not pass diff options as keydata to hashmap
  diff.c: adjust hash function signature to match hashmap expectation
  diff.c: add a blocks mode for moved code detection
  diff.c: decouple white space treatment from move detection algorithm
  diff.c: add --color-moved-ignore-space-delta option
  diff: color-moved white space handling options imply color-moved

 Documentation/diff-options.txt |  25 -
 diff.c | 138 +++
 diff.h |   8 +-
 t/t4015-diff-whitespace.sh | 197 +++--
 xdiff/xdiff.h  |   8 --
 xdiff/xdiffi.c |  17 ---
 6 files changed, 336 insertions(+), 57 deletions(-)

-- 
2.17.0.582.gccdcbd54c44.dirty

1:  a7a7af6b76b = 1:  a7a7af6b76b xdiff/xdiff.h: remove unused flags
2:  a7b6aaf7bc0 = 2:  a7b6aaf7bc0 xdiff/xdiffi.c: remove unneeded function 
declarations
3:  d9e57cc6b05 = 3:  d9e57cc6b05 diff.c: do not pass diff options as keydata 
to hashmap
4:  87111ba726d = 4:  87111ba726d diff.c: adjust hash function signature to 
match hashmap expectation
5:  9559b8cb456 = 5:  9559b8cb456 diff.c: add a blocks mode for moved code 
detection
6:  41a70464209 = 6:  41a70464209 diff.c: decouple white space treatment from 
move detection algorithm
7:  c0114b2ce56 ! 7:  72bb8213cab diff.c: add --color-moved-ignore-space-delta 
option
@@ -6,7 +6,6 @@
 changes uniformly.
 
 Signed-off-by: Stefan Beller 
-Signed-off-by: Junio C Hamano 
 
 diff --git a/Documentation/diff-options.txt 
b/Documentation/diff-options.txt
 --- a/Documentation/diff-options.txt
@@ -237,7 +236,7 @@
 +  not adjust
 +  EOF
 +
-+  git diff --color --color-moved-ignore-space-prefix-delta |
++  git diff --color --color-moved --color-moved-ignore-space-prefix-delta |
 +  grep -v "index" |
 +  test_decode_color >actual &&
 +
@@ -246,20 +245,20 @@
 +  --- a/text.txt
 +  +++ b/text.txt
 +  @@ -1,7 +1,7 @@
-+  -QIndented
-+  -QText across
-+  -Qthree lines
++  -QIndented
++  -QText across
++  -Qthree lines
 +  -QBut! <- this stands out
-+  -Qthis one
-+  -QQline did
-+  -Qnot adjust
-+  +QQIndented
-+  +QQText across
-+  +QQthree lines
++  -Qthis one
++  -QQline did
++  -Qnot adjust
++  +QQIndented
++  +QQText across
++  +QQthree lines
 +  +QQQBut! <- this stands out
-+  +this one
-+  +Qline did
-+  +not adjust
++  +this one
++  +Qline did
++  +not adjust
 +  EOF
 +
 +  test_cmp expected actual
-:  --- > 8:  9e0508c8381 diff: color-moved white space handling 
options imply color-moved

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

[Stefan Beller]

This makes the follow up patch easier.

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

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

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

[Stefan Beller]

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

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

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

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

[Stefan Beller]

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

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

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

[PATCH 5/8] diff.c: add a blocks mode for moved code detection

[Stefan Beller]

The new "blocks" mode provides a middle ground between plain and zebra.
It is as intuitive (few colors) as plain, but still has the requirement
for a minimum of lines/characters to count a block as moved.

Suggested-by: Ævar Arnfjörð Bjarmason 
 (https://public-inbox.org/git/87o9j0uljo@evledraar.gmail.com/)
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 Documentation/diff-options.txt |  8 --
 diff.c |  6 +++--
 diff.h |  5 ++--
 t/t4015-diff-whitespace.sh | 48 +-
 4 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index e3a44f03cdc..bb9f1b7cd82 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -276,10 +276,14 @@ plain::
that are added somewhere else in the diff. This mode picks up any
moved line, but it is not very useful in a review to determine
if a block of code was moved without permutation.
-zebra::
+blocks:
Blocks of moved text of at least 20 alphanumeric characters
are detected greedily. The detected blocks are
-   painted using either the 'color.diff.{old,new}Moved' color or
+   painted using either the 'color.diff.{old,new}Moved' color.
+   Adjacent blocks cannot be told apart.
+zebra::
+   Blocks of moved text are detected as in 'blocks' mode. The blocks
+   are painted using either the 'color.diff.{old,new}Moved' color or
'color.diff.{old,new}MovedAlternative'. The change between
the two colors indicates that a new block was detected.
 dimmed_zebra::
diff --git a/diff.c b/diff.c
index d1bae900cdc..95c51c0b7df 100644
--- a/diff.c
+++ b/diff.c
@@ -271,6 +271,8 @@ static int parse_color_moved(const char *arg)
return COLOR_MOVED_NO;
else if (!strcmp(arg, "plain"))
return COLOR_MOVED_PLAIN;
+   else if (!strcmp(arg, "blocks"))
+   return COLOR_MOVED_BLOCKS;
else if (!strcmp(arg, "zebra"))
return COLOR_MOVED_ZEBRA;
else if (!strcmp(arg, "default"))
@@ -278,7 +280,7 @@ static int parse_color_moved(const char *arg)
else if (!strcmp(arg, "dimmed_zebra"))
return COLOR_MOVED_ZEBRA_DIM;
else
-   return error(_("color moved setting must be one of 'no', 
'default', 'zebra', 'dimmed_zebra', 'plain'"));
+   return error(_("color moved setting must be one of 'no', 
'default', 'blocks', 'zebra', 'dimmed_zebra', 'plain'"));
 }
 
 int git_diff_ui_config(const char *var, const char *value, void *cb)
@@ -903,7 +905,7 @@ static void mark_color_as_moved(struct diff_options *o,
 
block_length++;
 
-   if (flipped_block)
+   if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS)
l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
}
adjust_last_block(o, n, block_length);
diff --git a/diff.h b/diff.h
index d29560f822c..7bd4f182c33 100644
--- a/diff.h
+++ b/diff.h
@@ -208,8 +208,9 @@ struct diff_options {
enum {
COLOR_MOVED_NO = 0,
COLOR_MOVED_PLAIN = 1,
-   COLOR_MOVED_ZEBRA = 2,
-   COLOR_MOVED_ZEBRA_DIM = 3,
+   COLOR_MOVED_BLOCKS = 2,
+   COLOR_MOVED_ZEBRA = 3,
+   COLOR_MOVED_ZEBRA_DIM = 4,
} color_moved;
#define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA
#define COLOR_MOVED_MIN_ALNUM_COUNT 20
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 17df491a3ab..45091abb192 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1223,7 +1223,7 @@ test_expect_success 'plain moved code, inside file' '
test_cmp expected actual
 '
 
-test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '
+test_expect_success 'detect blocks of moved code' '
git reset --hard &&
cat <<-\EOF >lines.txt &&
long line 1
@@ -1271,6 +1271,52 @@ test_expect_success 'detect permutations inside moved 
code -- dimmed_zebra' '
test_config color.diff.newMovedDimmed "normal cyan" &&
test_config color.diff.oldMovedAlternativeDimmed "normal blue" &&
test_config color.diff.newMovedAlternativeDimmed "normal yellow" &&
+
+   git diff HEAD --no-renames --color-moved=blocks --color |
+   grep -v "index" |
+   test_decode_color >actual &&
+   cat <<-\EOF >expected &&
+   diff --git a/lines.txt b/lines.txt
+   --- a/lines.txt
+   +++ b/lines.txt
+   @@ -1,16 +1,16 @@
+   -long line 1
+   -long line 2
+   -long line 3
+line 4
+line 5
+line 6
+line 7
+line 8
+line 9
+   +long line 1
+   +long line 2
+   +long line 3
+   +long line 14
+   

[PATCH] merge-recursive: give notice when submodule commit gets fast-forwarded

[Stefan Beller]

From: Leif Middelschulte 

Inform the user about an automatically fast-forwarded submodule. The
silent merge behavior was introduced by commit 68d03e4a6e44 ("Implement
automatic fast-forward merge for submodules", 2010-07-07)).

Signed-off-by: Leif Middelschulte 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 merge-recursive.c | 16 
 1 file changed, 16 insertions(+)

>> * sb/submodule-merge-in-merge-recursive (2018-05-16) 3 commits
>>  - merge-recursive: give notice when submodule commit gets fast-forwarded
>>  - merge-recursive: i18n submodule merge output and respect verbosity
>>  - submodule.c: move submodule merging to merge-recursive.c
>>
>>  By code restructuring of submodule merge in merge-recursive,
>>  informational messages from the codepath are now given using the
>>  same mechanism as other output, and honor the merge.verbosity
>>  configuration.  The code also learned to give a few new messages
>>  when a submodule three-way merge resolves cleanly when one side
>>  records a descendant of the commit chosen by the other side.
>>
>>  Will merge to 'next'.
>
>Merging would be ok, but I would rather not.
>A resend will be only for cosmetic effect, as I messed up the last commit
>
>So, please hold in pu.

This is the resend, with an interdiff as follows.

Junio wrote in 
http://public-inbox.org/git/xmqqk1s474vx@gitster-ct.c.googlers.com:

> Perhaps Leif can elaborate why this change is a good idea in the
> first place?

which is also outstanding.

Leif can you pick this patch and resend it with a proper commit message?


# diff --git c/merge-recursive.c w/merge-recursive.c
# index 29a430c418a..a9aecccb8c3 100644
# --- c/merge-recursive.c
# +++ w/merge-recursive.c
# @@ -1094,7 +1094,7 @@ static int merge_submodule(struct merge_options *o,
#   if (in_merge_bases(commit_a, commit_b)) {
#   oidcpy(result, b);
#   if (show(o, 3)) {
# - output(o, 1, _("Fast-forwarding submodule %s to the 
following commit:"), path);
# + output(o, 3, _("Fast-forwarding submodule %s to the 
following commit:"), path);
#   output_commit_title(o, commit_b);
#   } else if (show(o, 2))
#   output(o, 2, _("Fast-forwarding submodule %s to %s"), 
path, oid_to_hex(b));
# @@ -1106,7 +1106,7 @@ static int merge_submodule(struct merge_options *o,
#   if (in_merge_bases(commit_b, commit_a)) {
#   oidcpy(result, a);
#   if (show(o, 3)) {
# - output(o, 1, _("Fast-forwarding submodule %s to the 
following commit:"), path);
# + output(o, 3, _("Fast-forwarding submodule %s to the 
following commit:"), path);
#   output_commit_title(o, commit_a);
#   } else if (show(o, 2))
#   output(o, 2, _("Fast-forwarding submodule %s to %s"), 
path, oid_to_hex(a));


diff --git a/merge-recursive.c b/merge-recursive.c
index 0571919ee0a..a9aecccb8c3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1093,10 +1093,26 @@ static int merge_submodule(struct merge_options *o,
/* Case #1: a is contained in b or vice versa */
if (in_merge_bases(commit_a, commit_b)) {
oidcpy(result, b);
+   if (show(o, 3)) {
+   output(o, 3, _("Fast-forwarding submodule %s to the 
following commit:"), path);
+   output_commit_title(o, commit_b);
+   } else if (show(o, 2))
+   output(o, 2, _("Fast-forwarding submodule %s to %s"), 
path, oid_to_hex(b));
+   else
+   ; /* no output */
+
return 1;
}
if (in_merge_bases(commit_b, commit_a)) {
oidcpy(result, a);
+   if (show(o, 3)) {
+   output(o, 3, _("Fast-forwarding submodule %s to the 
following commit:"), path);
+   output_commit_title(o, commit_a);
+   } else if (show(o, 2))
+   output(o, 2, _("Fast-forwarding submodule %s to %s"), 
path, oid_to_hex(a));
+   else
+   ; /* no output */
+
return 1;
}
 
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 2/2] replace-object.c: remove the_repository from prepare_replace_object

[Stefan Beller]

This was missed in 5982da9d2ce (replace-object: allow
prepare_replace_object to handle arbitrary repositories, 2018-04-11)

Technically the code works correctly as the replace_map is the same
size in different repositories, however it is hard to read. So convert
the code to the familiar pattern of dereferencing the pointer that we
assign in the sizeof itself.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 replace_object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/replace_object.c b/replace_object.c
index 246b98cd4f1..801b5c16789 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -37,7 +37,7 @@ static void prepare_replace_object(struct repository *r)
return;
 
r->objects->replace_map =
-   xmalloc(sizeof(*the_repository->objects->replace_map));
+   xmalloc(sizeof(*r->objects->replace_map));
oidmap_init(r->objects->replace_map, 0);
 
for_each_replace_ref(r, register_replace_ref, NULL);
-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 0/2] Reroll 2 last commits of sb/object-store-replace

[Stefan Beller]

>> * sb/object-store-replace (2018-05-10) 2 commits
>>   (merged to 'next' on 2018-05-16 at 41bbedcc81)
>>  + replace-object.c: remove the_repository from prepare_replace_object
>>  + object.c: free replace map in raw_object_store_clear
>>
>>  Hotfix.
>>
>>  Will merge to 'master'.
>
>Please do not.

Here comes a reroll with another fix, diff below, as the branch-diff did
not give a good output[1].

Thanks,
Stefan 

$ git diff origin/sb/object-store-replace
diff --git c/object.c w/object.c
index cdf520084df..97245fdea25 100644
--- c/object.c
+++ w/object.c
@@ -480,6 +480,8 @@ void raw_object_store_clear(struct raw_object_store *o)
 {
FREE_AND_NULL(o->objectdir);
FREE_AND_NULL(o->alternate_db);
+
+   oidmap_free(o->replace_map, 1);
FREE_AND_NULL(o->replace_map);
 
free_alt_odbs(o);

[1]
$ ./git-branch-diff 
origin/sb/object-store-replace^^..origin/sb/object-store-replace 
sb/object-store-replace^^..sb/object-store-replace
1:  d5873072054 < -:  --- object.c: free replace map in 
raw_object_store_clear
-:  --- > 1:  88d456c5d24 object.c: free replace map in 
raw_object_store_clear
2:  74fd0705bb3 = 2:  6da07fe2f9c replace-object.c: remove the_repository from 
prepare_replace_object

Stefan Beller (2):
  object.c: free replace map in raw_object_store_clear
  replace-object.c: remove the_repository from prepare_replace_object

 object.c | 3 +++
 replace_object.c | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.17.0.582.gccdcbd54c44.dirty

[PATCH 1/2] object.c: free replace map in raw_object_store_clear

[Stefan Beller]

The replace map for objects was missed to free in the object store in
the conversion of c1274495 ("replace-object: eliminate replace objects
prepared flag", 2018-04-11). We also missed to free the replaced objects
that are put into the replace map in that whole series.

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

diff --git a/object.c b/object.c
index 66cffaf6e51..97245fdea25 100644
--- a/object.c
+++ b/object.c
@@ -481,6 +481,9 @@ void raw_object_store_clear(struct raw_object_store *o)
FREE_AND_NULL(o->objectdir);
FREE_AND_NULL(o->alternate_db);
 
+   oidmap_free(o->replace_map, 1);
+   FREE_AND_NULL(o->replace_map);
+
free_alt_odbs(o);
o->alt_odb_tail = NULL;
 
-- 
2.17.0.582.gccdcbd54c44.dirty

Re: [PATCH v2 11/12] gc: automatically write commit-graph files

[Martin Ågren]

On 11 May 2018 at 23:15, Derrick Stolee  wrote:
> The commit-graph file is a very helpful feature for speeding up git
> operations. In order to make it more useful, write the commit-graph file
> by default during standard garbage collection operations.

So does it really write by default...

> Add a 'gc.commitGraph' config setting that triggers writing a
> commit-graph file after any non-trivial 'git gc' command. Defaults to
> false while the commit-graph feature matures. We specifically do not

or not...? I guess the first paragraph has simply been there since
before you changed your mind about the default?

> want to turn this on by default until the commit-graph feature is fully
> integrated with history-modifying features like shallow clones.

So if someone would turn this on with a shallow clone, ... Do we want
some note (warning?) around that in the user documentation?

Martin

Re: What's cooking in git.git (May 2018, #02; Thu, 17)

[Stefan Beller]

> * sb/object-store-replace (2018-05-10) 2 commits
>   (merged to 'next' on 2018-05-16 at 41bbedcc81)
>  + replace-object.c: remove the_repository from prepare_replace_object
>  + object.c: free replace map in raw_object_store_clear
>
>  Hotfix.
>
>  Will merge to 'master'.

Please do not.
(Or do, but then be prepared for another hotfix.)

The commit sb/object-store-replace^ needs more free'ing and shall be
replaced with
https://public-inbox.org/git/20180510195849.28023-4-sbel...@google.com/
I'll resend shortly.

>
> * sb/submodule-merge-in-merge-recursive (2018-05-16) 3 commits
>  - merge-recursive: give notice when submodule commit gets fast-forwarded
>  - merge-recursive: i18n submodule merge output and respect verbosity
>  - submodule.c: move submodule merging to merge-recursive.c
>
>  By code restructuring of submodule merge in merge-recursive,
>  informational messages from the codepath are now given using the
>  same mechanism as other output, and honor the merge.verbosity
>  configuration.  The code also learned to give a few new messages
>  when a submodule three-way merge resolves cleanly when one side
>  records a descendant of the commit chosen by the other side.
>
>  Will merge to 'next'.

Merging would be ok, but I would rather not.
A resend will be only for cosmetic effect, as I messed up the last commit

So, please hold in pu.

> * sb/diff-color-move-more (2018-04-25) 7 commits
>  - diff.c: add --color-moved-ignore-space-delta option
>  - diff.c: decouple white space treatment from move detection algorithm
>  - diff.c: add a blocks mode for moved code detection
>  - diff.c: adjust hash function signature to match hashmap expectation
>  - diff.c: do not pass diff options as keydata to hashmap
>  - xdiff/xdiffi.c: remove unneeded function declarations
>  - xdiff/xdiff.h: remove unused flags
>
>  "git diff --color-moved" feature has further been tweaked.
>
>  Will merge to 'next'.

I did not get around to fix it up, there are still review
comments outstanding. (The test is broken in the last commit.)

Please hold in pu;

Thanks,
Stefan

Re: [PATCH v2 10/12] commit-graph: add '--reachable' option

[Martin Ågren]

On 11 May 2018 at 23:15, Derrick Stolee  wrote:
> When writing commit-graph files, it can be convenient to ask for all
> reachable commits (starting at the ref set) in the resulting file. This
> is particularly helpful when writing to stdin is complicated, such as a
> future integration with 'git gc' which will call
> 'git commit-graph write --reachable' after performing cleanup of the
> object database.
>
> Signed-off-by: Derrick Stolee 
> ---
>  Documentation/git-commit-graph.txt |  8 ++--
>  builtin/commit-graph.c | 41 
> ++
>  2 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-commit-graph.txt 
> b/Documentation/git-commit-graph.txt
> index a222cfab08..cc1715a823 100644
> --- a/Documentation/git-commit-graph.txt
> +++ b/Documentation/git-commit-graph.txt
> @@ -38,12 +38,16 @@ Write a commit graph file based on the commits found in 
> packfiles.
>  +
>  With the `--stdin-packs` option, generate the new commit graph by
>  walking objects only in the specified pack-indexes. (Cannot be combined
> -with --stdin-commits.)
> +with --stdin-commits or --reachable.)

You could enclose --reachable in `...` for nicer rendering and fix
--stdin-commits as well while you're here.

>  With the `--stdin-commits` option, generate the new commit graph by
>  walking commits starting at the commits specified in stdin as a list
>  of OIDs in hex, one OID per line. (Cannot be combined with
> ---stdin-packs.)
> +--stdin-packs or --reachable.)

Ditto.

> +With the `--reachable` option, generate the new commit graph by walking
> +commits starting at all refs. (Cannot be combined with --stdin-commits
> +or --stind-packs.)

Ditto. Also, s/stind/stdin/.

Martin

Re: [PATCH v2 09/12] fsck: verify commit-graph

[Martin Ågren]

On 11 May 2018 at 23:15, Derrick Stolee  wrote:
> If core.commitGraph is true, verify the contents of the commit-graph
> during 'git fsck' using the 'git commit-graph verify' subcommand. Run
> this check on all alternates, as well.

> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index 5ab268a024..91c8406d97 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -205,6 +205,16 @@ test_expect_success 'build graph from commits with 
> append' '
>  graph_git_behavior 'append graph, commit 8 vs merge 1' full commits/8 merge/1
>  graph_git_behavior 'append graph, commit 8 vs merge 2' full commits/8 merge/2
>
> +test_expect_success 'build graph using --reachable' '
> +   cd "$TRASH_DIRECTORY/full" &&
> +   git commit-graph write --reachable &&
> +   test_path_is_file $objdir/info/commit-graph &&
> +   graph_read_expect "11" "large_edges"
> +'

This should be in the next patch.

> +graph_git_behavior 'append graph, commit 8 vs merge 1' full commits/8 merge/1
> +graph_git_behavior 'append graph, commit 8 vs merge 2' full commits/8 merge/2

(Possibly the same here.)

>  test_expect_success 'setup bare repo' '
> cd "$TRASH_DIRECTORY" &&
> git clone --bare --no-local full bare &&
> @@ -335,7 +345,7 @@ test_expect_success 'detect OID not in object database' '
> cd "$TRASH_DIRECTORY/full" &&
> cp $objdir/info/commit-graph commit-graph-backup &&
> test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
> -   corrupt_data $objdir/info/commit-graph 1134 "\01" &&
> +   corrupt_data $objdir/info/commit-graph 1134 "\00" &&

This and two similar ones as well, I guess.

Actually, I can drop them altogether and the tests still pass. Rebase
mishap?

> +test_expect_success 'git fsck (checks commit-graph)' '
> +   cd "$TRASH_DIRECTORY/full" &&
> +   git fsck
> +'

Maybe inject an error and verify that `git fsck` does indeed catch it,
i.e., it does call out to check the commit-graph.

Maybe also a run with `-c core.commitGraph=no` where the error should
not be found because the commit-graph should not be checked?

Martin

Re: [PATCH] grep: handle corrupt index files early

[Stefan Beller]

On Wed, May 16, 2018 at 6:36 PM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> With a majority of call sites dying like this though, I wonder if we
>> should just add repo_read_index_or_die() with die() inside. Then the
>> next person won't likely accidentally forget _()
>
> Yuck.
>
> That sounds like inviting a major code churn.  I tend to agree that
> it would be a good clean-up for longer term maintenance, but I am
> not sure if I can honestly say I'd look forward to such a clean-up
> at this point in the cycle when there are tons of large-ish topics
> in flight X-<.

ok, consider the series
https://public-inbox.org/git/20180516222118.233868-1-sbel...@google.com/
retracted for this cycle; I will keep it around and resend it at some future
date, hopefully.

Feel free to comment on it, though.

Stefan

GREETINGS BELOVED

[barcerue...@ono.com]

GREETINGS BELOVED

I AM BORTE ,I WAS  DIAGNOSE WITH OVARIAN CANCER,WHICH DOCTORS HAVE 
CONFIRMED THAT I HAVE ONLY FEW WEEKS TO LIVE, SO I HAVE DECIDED TO 
DONATE EVERYTHING I HAVE TO THE ORPHANAGE AND THE POOR WIDOWS THROUGH 
YOU AND YOUR HELP .PLEASE KINDLY REPLY THROUGH MY PRIVATE BOX 
(misssborteogo...@gmail.com) HERE  AS SOON AS POSIBLE TO ENABLE ME GIVE 
YOU MORE INFORMATION ABOUT MYSELF AND HOW TO GO ABOUT IT .

THANKS 

MISS BORTE

Troubles with picking an editor during Git update

[Bartosz Konikiewicz]

Hi there!

I had an issue with Git installer for Windows while trying to update
my instance of the software. My previous version was "git version
2.15.1.windows.2", while my operating system prompted me to upgrade to
"2.17.0". The installer asked me to "choose the default editor for
Git". One of these options was Notepad++ - my editor of choice. Vim
was selected by default and I've picked Notepad++ from a drop-down
list. As soon as I did it, a "next" button greyed out. When I moved
back to the previous step and then forward to the editor choice, the
"Notepad++" option was still highlighted, and the "next" button wasn't
greyed out anymore - it was active and I was able to press it and
continue installation.

Steps to reproduce:

1. Have Notepad++ 6.6.9 installed on Windows 10 64-bit 10.0.17134 Build 17134.
2. Use an installer for version 2.17.0 to upgrade from version 2.15.1.
3. On an editor selection screen, choose Notepad++ instead of Vim. You
should be unable to continue installation because of the "next" button
being disabled.
4. Press "prev".
5. Press "next". Notepad++ should be still highlighted, and the "next"
button should be active, allowing to continue installation.

I find it to be a crafty trick to make me use Vim. I have considered
it for a good moment.

Re: Generate more revenue from Git

[Konstantin Ryabitsev]

Michal:

This is strictly a development list. If you would like to discuss any
and all monetization features, please feel free to reach out to me via
email.

Regards,
-K

On Thu, May 17, 2018 at 04:45:18PM +0300, Michal Sapozhnikov wrote:

Hi,

I would like to schedule a quick call this week.

What's the best way to schedule a 15 minute call?

Thanks,
--
Michal Sapozhnikov | Business Manager, Luminati SDK | +972-50-2826778 | Skype:
live:michals_43
http://luminati.io/sdk

On 10-May-18 14:04, 7d (by eremind) wrote:


Hi,

I am writing with the hope of talking to the appropriate person who handles
the
app's monetization.
If it makes sense to have a call, let me know how your schedule looks.

Best Regards,

Re: Generate more revenue from Git

[Michal Sapozhnikov]

Hi,

I would like to schedule a quick call this week.

What's the best way to schedule a 15 minute call?

Thanks,
-- 
Michal Sapozhnikov | Business Manager, Luminati SDK | +972-50-2826778 | Skype:
live:michals_43
http://luminati.io/sdk

On 10-May-18 14:04, 7d (by eremind) wrote:
> 
> Hi,
> 
> I am writing with the hope of talking to the appropriate person who handles 
> the
> app's monetization.
> If it makes sense to have a call, let me know how your schedule looks.
> 
> Best Regards,
> 

Re: jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Jeff King]

On Thu, May 17, 2018 at 11:48:33AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > If there are ample branches, the warning message might be
> > hidden out of screen but we shouldn't rely on that, I
> > suppose.
> 
> Also if we have lots of branches, depending on your pager settings you
> won't see this at all, I have:

We redirect stderr to the pager, as well, exactly to catch this sort of
case. But because git-branch does not kick in the pager until later
(because it only wants to do it for list-mode), that happens _after_
we've emitted the message.

So one fix would be to teach deprecated_reflog_option_cb() to just set a
flag rather than printing the warning, and then issue the warning.
Something like this:

diff --git a/builtin/branch.c b/builtin/branch.c
index 452742fecf..f51b89e962 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -34,6 +34,7 @@ static const char * const builtin_branch_usage[] = {
NULL
 };
 
+static int used_deprecated_reflog_option;
 static const char *head;
 static struct object_id head_oid;
 
@@ -576,8 +577,7 @@ static int edit_branch_description(const char *branch_name)
 static int deprecated_reflog_option_cb(const struct option *opt,
   const char *arg, int unset)
 {
-   warning("the '-l' alias for '--create-reflog' is deprecated;");
-   warning("it will be removed in a future version of Git");
+   used_deprecated_reflog_option = 1;
*(int *)opt->value = !unset;
return 0;
 }
@@ -703,6 +703,11 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if (list)
setup_auto_pager("branch", 1);
 
+   if (used_deprecated_reflog_option) {
+   warning("the '-l' alias for '--create-reflog' is deprecated;");
+   warning("it will be removed in a future version of Git");
+   }
+
if (delete) {
if (!argc)
die(_("branch name required"));

On the other hand, I'm not sure this is that big a deal. The point of
the deprecation warning is to catch people who are actually trying to
use "-l" as "--create-reflog", and that case does not page. The people
doing "git branch -l" are actually getting what they want eventually,
which is to turn it into "--list". In the interim step where it becomes
an unknown option, they'll get a hard error.

-Peff

Re: What's cooking in git.git (May 2018, #02; Thu, 17)

[Derrick Stolee]

On 5/17/2018 2:01 AM, Junio C Hamano wrote:

* ds/generation-numbers (2018-05-02) 11 commits
  - commit-graph.txt: update design document
  - merge: check config before loading commits
  - commit: use generation number in remove_redundant()
  - commit: add short-circuit to paint_down_to_common()
  - commit: use generation numbers for in_merge_bases()
  - ref-filter: use generation number for --contains
  - commit-graph: always load commit-graph information
  - commit: use generations in paint_down_to_common()
  - commit-graph: compute generation numbers
  - commit: add generation number to struct commmit
  - ref-filter: fix outdated comment on in_commit_list
  (this branch is used by ds/commit-graph-lockfile-fix; uses 
ds/lazy-load-trees.)

  A recently added "commit-graph" datafile has learned to store
  pre-computed generation numbers to speed up the decisions to stop
  history traversal.

  Is this ready for 'next' with ds/commit-graph-lockfile-fix?
  A commit with triple 'm' needs its title amended, though.


With the lockfile fix, it should be ready. I've been giving this 
significant testing on my machine and a few other developers here. The 
next version of GVFS is shipping with this code and with GVFS 
controlling the maintenance of the commit-graph file. That code has been 
cooking with our CI builds for a while, with full functional tests 
against the Windows repository. The only bugs we've found are the fix in 
"merge: check config before loading commits" and in 
ds/commit-graph-lockfile-fix.


Sorry for the triple-m.

Thanks,
-Stolee

RE: Add option to git to ignore binary files unless force added

[Randall S. Becker]

On May 16, 2018 11:18 PM, Jacob Keller
> On Wed, May 16, 2018 at 5:45 PM, Anmol Sethi  wrote:
> > I think it’d be great to have an option to have git ignore binary files. My
> repositories are always source only, committing a binary is always a mistake.
> At the moment, I have to configure the .gitignore to ignore every binary file
> and that gets tedious. Having git ignore all binary files would be great.
> >
> > This could be achieved via an option in .gitconfig or maybe a special line 
> > in
> .gitignore.
> >
> > I just want to never accidentally commit a binary again.
> 
> I believe you can do a couple things. There should be a hook which you can
> modify to validate that there are no binary files on pre-commit[1], or pre-
> push[2] to verify that you never push commits with binaries in them.
> 
> You could also implement the update hook on the server if you control it, to
> allow it to block pushes which contain binary files.

What about configuring ${HOME}/.config/git/ignore instead (described at 
https://git-scm.com/docs/gitignore). Inside, put:

*.o
*.exe
*.bin
*.dat
Etc

Cheers,
Randall

Good day, how are you today and your family,

[wilson peter]

Re: [PATCH v2 0/3] unpack_trees_options: free messages when done

[Ben Peart]

On 5/16/2018 5:54 PM, Elijah Newren wrote:

Hi Martin,

On Wed, May 16, 2018 at 9:30 AM, Martin Ågren  wrote:

On 16 May 2018 at 16:32, Elijah Newren  wrote:

On Sat, Apr 28, 2018 at 4:32 AM, Martin Ågren  wrote:

As you noted elsewhere [1], Ben is also working in this area. I'd be
perfectly happy to sit on these patches until both of your contributions
come through to master.

[1] 
https://public-inbox.org/git/CABPp-BFh=gl6rnbst2bgtynkij1z5tmgar1via5_vytef5e...@mail.gmail.com/


Instead of waiting for these to come through to master, could you just
submit based on the top of bp/merge-rename-config?


Sure, here goes. This is based on bp/merge-rename-config, gets rid of
all leaks of memory allocated in `setup_unpack_trees_porcelain()` and
cuts the number of leaks in the test-suite (i.e., the subset of the
tests that I run) by around 10%.


Awesome, thanks.  I've looked over patches 2 & 3; they look good to me.



I like the symmetry of the naming and locality of the functions.  Should 
help people remember to keep the xstrfmt() and associated free() in 
sync.  Patches look good to me as well.

Re: jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Ævar Arnfjörð Bjarmason]

On Thu, May 17 2018, Kaartic Sivaraam wrote:

> Hi Ævar,
>
> On Thursday 17 May 2018 03:18 PM, Ævar Arnfjörð Bjarmason wrote:
>> I've ended up with that $LESS setting via hackery over the years, so
>> maybe I'm doing something retarded, minimal test case:
>>
>> PAGER=less  LESS="--no-init --quit-if-one-screen" git branch -l
>>
>> ...
>>
>> So I think this is probably OK for most users, if the have very few
>> branches they'll see it, and then if they use default pager settings
>> they'll see the stderr output once they quit the pager. I don't know how
>> common my (mis)configuration is.
>>
> I'm not sure this is ok, because I still see the stderr output with your
> minimal test case even when I have enough branches to not fit in one
> screen. The stderr output is of course above the pager output (after I
> quit the pager) and gets hidden out-of display as I stated before. I
> also get weird 'ESC[m' characters with you minimal test case. I'm not
> sure what I'm missing.

I don't get it anywhere, above or below.

> What version of 'less' do you use? Is any other configuration that you
> didn't mention affecting what your observation?

Both "less 481 (GNU regular expressions)" and version 487. I upgraded
and tried both. This is on Debian stable (and newer version from
testing).

It may be something in my system config, but I tried sudo-ing to a user
that only has stock Debian config & none of my custom .bashrc etc, and I
get the same thing.

Re: jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Kaartic Sivaraam]

Hi Ævar,

On Thursday 17 May 2018 03:18 PM, Ævar Arnfjörð Bjarmason wrote:
> I've ended up with that $LESS setting via hackery over the years, so
> maybe I'm doing something retarded, minimal test case:
> 
> PAGER=less  LESS="--no-init --quit-if-one-screen" git branch -l
>
> ...
> 
> So I think this is probably OK for most users, if the have very few
> branches they'll see it, and then if they use default pager settings
> they'll see the stderr output once they quit the pager. I don't know how
> common my (mis)configuration is.
>
I'm not sure this is ok, because I still see the stderr output with your
minimal test case even when I have enough branches to not fit in one
screen. The stderr output is of course above the pager output (after I
quit the pager) and gets hidden out-of display as I stated before. I
also get weird 'ESC[m' characters with you minimal test case. I'm not
sure what I'm missing.

What version of 'less' do you use? Is any other configuration that you
didn't mention affecting what your observation?


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

On Thursday 17 May 2018 02:39 PM, Johannes Schindelin wrote:
> I have great empathy for the desire to see these bugs fixed. The
> conversion must come first, though, and in the interest of making it
> easier on me and other reviewers, I must insist on keeping the conversion
> free of any changes, much in the way as we try to avoid evil merges (i.e.
> merge commits that introduce changes that were not present in any of their
> parents).
> 

Of course, the conversion should be separate from the bug fixes :-)

When I mentioned "while porting it to C", I actually meant the "thought
process of creating a foundation for `git-stash` in C". I thought
hinting at some of the existing and unsolved `git-stash` bugs would
allow the person who would be doing the port of `git-stash` to C to
consider how to avoid this while implementing the basic foundation. I
should have been more explicit about this in my previous mails.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Ævar Arnfjörð Bjarmason]

On Thu, May 17 2018, Kaartic Sivaraam wrote:

> On Thursday 17 May 2018 11:31 AM, Junio C Hamano wrote:
>> * jk/branch-l-0-deprecation (2018-03-26) 3 commits
>>
>> ...
>>
>>  The "-l" option in "git branch -l" is an unfortunate short-hand for
>>  "--create-reflog", but many users, both old and new, somehow expect
>>  it to be something else, perhaps "--list".  This step deprecates
>>  the short-hand and warns about the future removal of the it when it
>>  is used.
>>
>>  Will cook in 'next'.
>>  Perhaps merge to 'master' immediately after 2.18 release?
>
> I still have a slight feeling that we shouldn't list the branches for
> "git branch -l" during the deprecation period. If feel this because
>
>   i) It would avoid confusions for the users during the
>  deprecation period
>
>   ii) The warning message seems to add to the confusion:
>
>   $ git branch -l
>   warning: the '-l' alias for '--create-reflog' is deprecated;
>   warning: it will be removed in a future version of Git
>   * master
>   ...
>
>
>   If there are ample branches, the warning message might be
>   hidden out of screen but we shouldn't rely on that, I
>   suppose.

Also if we have lots of branches, depending on your pager settings you
won't see this at all, I have:

PAGER=less LESS="--IGNORE-CASE --LONG-PROMPT --QUIET --chop-long-lines 
--RAW-CONTROL-CHARS --no-init --quit-if-one-screen"

I've ended up with that $LESS setting via hackery over the years, so
maybe I'm doing something retarded, minimal test case:

PAGER=less  LESS="--no-init --quit-if-one-screen" git branch -l

So with those two options I don't get the stderr output at all, but if I
remove either one of those options once I quit the pager I get the
output at the end.

So I think this is probably OK for most users, if the have very few
branches they'll see it, and then if they use default pager settings
they'll see the stderr output once they quit the pager. I don't know how
common my (mis)configuration is.

Re: [GSoC] A blog about 'git stash' project

[Johannes Schindelin]

Hi Kaartic,

> On Thursday 17 May 2018 12:28 PM, Kaartic Sivaraam wrote:
> 
> > I thought of pointing you to one of the issues with the current
> > implementation of 'git stash' which you could probably fix while
> > porting it to C.
> >
> > ...
> > 
> 
> Forgot to mention about another issue, which I consider to be a bug. I
> have elaborated about it in the following mailing list email.
> 
>   
> https://public-inbox.org/git/aa43f1ff-9095-fb4d-43bc-bf8283b7d...@gmail.com/
> 
> Unfortunately, it didn't receive any replies. See, if you could do
> something about it.

I have great empathy for the desire to see these bugs fixed. The
conversion must come first, though, and in the interest of making it
easier on me and other reviewers, I must insist on keeping the conversion
free of any changes, much in the way as we try to avoid evil merges (i.e.
merge commits that introduce changes that were not present in any of their
parents).

Depending how the conversion goes, I could imagine that there might be
plenty of time to do fun stuff on top, such as fixes for the bugs you
mentioned.

Ciao,
Johannes

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

On Thursday 17 May 2018 12:28 PM, Kaartic Sivaraam wrote:
> Hi Sebi,
> 
> I thought of pointing you to one of the issues with the current
> implementation of 'git stash' which you could probably fix while porting
> it to C.
>
> ...
> 

Forgot to mention about another issue, which I consider to be a bug. I
have elaborated about it in the following mailing list email.


https://public-inbox.org/git/aa43f1ff-9095-fb4d-43bc-bf8283b7d...@gmail.com/

Unfortunately, it didn't receive any replies. See, if you could do
something about it.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

Re: [GSoC] A blog about 'git stash' project

[Kaartic Sivaraam]

Hi Sebi,

I thought of pointing you to one of the issues with the current
implementation of 'git stash' which you could probably fix while porting
it to C. It's about stashing untracked files.

You could find more information about it in the following mailing list
thread:
https://public-inbox.org/git/1505626069.9625.6.ca...@gmail.com/


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17))

[Kaartic Sivaraam]

On Thursday 17 May 2018 11:31 AM, Junio C Hamano wrote:
> * jk/branch-l-0-deprecation (2018-03-26) 3 commits
> 
> ...
>
>  The "-l" option in "git branch -l" is an unfortunate short-hand for
>  "--create-reflog", but many users, both old and new, somehow expect
>  it to be something else, perhaps "--list".  This step deprecates
>  the short-hand and warns about the future removal of the it when it
>  is used.
> 
>  Will cook in 'next'.
>  Perhaps merge to 'master' immediately after 2.18 release?

I still have a slight feeling that we shouldn't list the branches for
"git branch -l" during the deprecation period. If feel this because

i) It would avoid confusions for the users during the
   deprecation period

ii) The warning message seems to add to the confusion:

$ git branch -l
warning: the '-l' alias for '--create-reflog' is deprecated;
warning: it will be removed in a future version of Git
* master
...


If there are ample branches, the warning message might be
hidden out of screen but we shouldn't rely on that, I
suppose.


-- 
Sivaraam

QUOTE:

“The most valuable person on any team is the person who makes everyone
else on the team more valuable, not the person who knows the most.”

  - John Sonmez


Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!



signature.asc
Description: OpenPGP digital signature

wir bieten 2% Kredite

[Ronald Bernstein]

Sehr geehrte Damen und Herren,

Sie brauchen Geld? Sie sind auf der suche nach einem Darlehen? Seriös und
unkompliziert?
Dann sind Sie hier bei uns genau richtig.
Durch unsere jahrelange Erfahrung und kompetente Beratung sind wir
Europaweit tätig.

Wir bieten jedem ein GÜNSTIGES Darlehen zu TOP Konditionen an.
Darlehnen zwischen 5000 CHF/Euro bis zu 20 Millionen CHF/Euro möglich.
Wir erheben dazu 2% Zinssatz.

Lassen Sie sich von unserem kompetenten Team beraten.

Zögern Sie nicht und kontaktieren Sie mich unter für weitere Infos &
Anfragen unter der eingeblendeten Email Adresse.


Ich freue mich von Ihnen zu hören.

What's cooking in git.git (May 2018, #02; Thu, 17)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* hn/sort-ls-remote (2018-04-09) 1 commit
  (merged to 'next' on 2018-04-30 at 244ca5d30a)
 + ls-remote: create '--sort' option

 "git ls-remote" learned an option to allow sorting its output based
 on the refnames being shown.


* sb/object-store-replace (2018-04-12) 15 commits
  (merged to 'next' on 2018-04-25 at 9a213fb505)
 + replace-object: allow lookup_replace_object to handle arbitrary repositories
 + replace-object: allow do_lookup_replace_object to handle arbitrary 
repositories
 + replace-object: allow prepare_replace_object to handle arbitrary repositories
 + refs: allow for_each_replace_ref to handle arbitrary repositories
 + refs: store the main ref store inside the repository struct
 + replace-object: add repository argument to lookup_replace_object
 + replace-object: add repository argument to do_lookup_replace_object
 + replace-object: add repository argument to prepare_replace_object
 + refs: add repository argument to for_each_replace_ref
 + refs: add repository argument to get_main_ref_store
 + replace-object: check_replace_refs is safe in multi repo environment
 + replace-object: eliminate replace objects prepared flag
 + object-store: move lookup_replace_object to replace-object.h
 + replace-object: move replace_map to object store
 + replace_object: use oidmap
 (this branch is used by sb/object-store-alloc and sb/oid-object-info.)

 The effort to pass the repository in-core structure throughout the
 API continues.  This round deals with the code that implements the
 refs/replace/ mechanism.


* ab/git-svn-get-record-typofix (2018-04-09) 1 commit
  (merged to 'next' on 2018-04-30 at 23f875f6b9)
 + git-svn: avoid warning on undef readline()

 "git svn" had a minor thinko/typo which has been fixed.


* ab/nuke-emacs-contrib (2018-04-16) 1 commit
  (merged to 'next' on 2018-04-25 at 9b133d8a65)
 + git{,-blame}.el: remove old bitrotting Emacs code

 The scripts in contrib/emacs/ have outlived their usefulness and
 have been replaced with a stub that errors out and tells the user
 there are replacements.


* ab/simplify-perl-makefile (2018-04-19) 2 commits
  (merged to 'next' on 2018-04-25 at 906cf21682)
 + Makefile: mark perllibdir as a .PHONY target
  (merged to 'next' on 2018-04-17 at 4448756934)
 + perl: fix installing modules from contrib

 Recent simplification of build procedure forgot a bit of tweak to
 the build procedure of contrib/mw-to-git/


* bt/gpg-interface (2018-04-16) 7 commits
  (merged to 'next' on 2018-04-30 at 50c507b7d8)
 + gpg-interface: find the last gpg signature line
 + gpg-interface: extract gpg line matching helper
 + gpg-interface: fix const-correctness of "eol" pointer
 + gpg-interface: use size_t for signature buffer size
 + gpg-interface: modernize function declarations
 + gpg-interface: handle bool user.signingkey
 + t7004: fix mistaken tag name

 What is queued here is only the obviously correct and
 uncontroversial code clean-up part, which is an earlier 7 patches,
 of a larger series.

 The remainder that is not queued introduces a few configuration
 variables to deal with e-signature backends with different
 signature format.


* bw/protocol-v2 (2018-03-15) 35 commits
  (merged to 'next' on 2018-04-11 at 23ee234a2c)
 + remote-curl: don't request v2 when pushing
 + remote-curl: implement stateless-connect command
 + http: eliminate "# service" line when using protocol v2
 + http: don't always add Git-Protocol header
 + http: allow providing extra headers for http requests
 + remote-curl: store the protocol version the server responded with
 + remote-curl: create copy of the service name
 + pkt-line: add packet_buf_write_len function
 + transport-helper: introduce stateless-connect
 + transport-helper: refactor process_connect_service
 + transport-helper: remove name parameter
 + connect: don't request v2 when pushing
 + connect: refactor git_connect to only get the protocol version once
 + fetch-pack: support shallow requests
 + fetch-pack: perform a fetch using v2
 + upload-pack: introduce fetch server command
 + push: pass ref prefixes when pushing
 + fetch: pass ref prefixes when fetching
 + ls-remote: pass ref prefixes when requesting a remote's refs
 + transport: convert transport_get_remote_refs to take a list of ref prefixes
 + transport: convert get_refs_list to take a list of ref prefixes
 + connect: request remote refs using v2
 + ls-refs: introduce ls-refs server command
 + serve: introduce git-serve
 + test-pkt-line: introduce a packet-line test