Re: [PATCH v2] Allow use of TLS 1.3

[Loganaden Velvindron]

On Sat, Mar 24, 2018 at 1:47 AM, Daniel Stenberg  wrote:
> On Fri, 23 Mar 2018, Loganaden Velvindron wrote:
>
>> +#ifdef CURL_SSLVERSION_TLSv1_3
>> +   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
>> +#endif
>
>
> Unfortunately, CURL_SSLVERSION_TLSv1_3 is an enum so this construct won't
> work.
>
> Also, let me just point out that 7.52.0 is 0x073400 in hex and not the one
> used for the first version of this patch.
>

Here's the error i get when I use a recent libcurl, but the OpenSSL
wasn't built with tls 1.3:

 using : GIT_SSL_VERSION=tlsv1.3

Error:
OpenSSL was built without TLS 1.3 support


> --
>
>  / daniel.haxx.se

Re: [RFC PATCH v5 0/8] rebase-interactive

[Wink Saville]

>> I queued everything (with all patch 3-8/8 retitled to share a
>> common prefix, so that "git shortlog" output would stay sane)
>> and I think I resolved the conflicts with Dscho's recreate-merges
>> topic correctly.  Please double check what will appear on 'pu' later
>> today.
>>
>> Thanks.
>>
>
> OK, thank you!

I looked at 'pu' and it LGTM, so I pushed 'pu' as a branch to my
github account to test with Travis-CI. All the linux builds are
green, but the 2 OSX builds are red[1] and the logs show compile
errors:

CC ident.o
CC json-writer.o

json-writer.c:123:38:  error:  format specifies type 'uintmax_t' (aka
'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned
long long') [-Werror,-Wformat]

strbuf_addf(>json, ":%"PRIuMAX, value);
 ~~ ^
json-writer.c:228:37:  error:  format specifies type 'uintmax_t' (aka
'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned
long long') [-Werror,-Wformat] [0m

strbuf_addf(>json, "%"PRIuMAX, value);
 ~~ ^
2 errors generated.
make: *** [json-writer.o] Error 1
make: *** Waiting for unfinished jobs

[RFC Patch 1/1] changes the PRIuMax to PRIu64 to correct the compile error
and now Travis-CI is green [2].

[1]: https://travis-ci.org/winksaville/git/builds/357660624
[2]: https://travis-ci.org/winksaville/git/builds/357681929

-- Wink

[RFC PATCH 1/1] json-writer: incorrect format specifier

[Wink Saville]

In routines jw_object_uint64 and jw_object_double strbuf_addf is
invoked with strbuf_addf(>json, ":%"PRIuMAX, value) where value
is a uint64_t. This causes a compile error on OSX.

The correct format specifier is PRIu64 instead of PRIuMax.

Signed-off-by: Wink Saville 
---
 json-writer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/json-writer.c b/json-writer.c
index 89a6abb57..04045448a 100644
--- a/json-writer.c
+++ b/json-writer.c
@@ -120,7 +120,7 @@ void jw_object_uint64(struct json_writer *jw, const char 
*key, uint64_t value)
maybe_add_comma(jw);
 
append_quoted_string(>json, key);
-   strbuf_addf(>json, ":%"PRIuMAX, value);
+   strbuf_addf(>json, ":%"PRIu64, value);
 }
 
 void jw_object_double(struct json_writer *jw, const char *fmt,
@@ -225,7 +225,7 @@ void jw_array_uint64(struct json_writer *jw, uint64_t value)
assert_in_array(jw);
maybe_add_comma(jw);
 
-   strbuf_addf(>json, "%"PRIuMAX, value);
+   strbuf_addf(>json, "%"PRIu64, value);
 }
 
 void jw_array_double(struct json_writer *jw, const char *fmt, double value)
-- 
2.16.2

[RFC PATCH 0/1] json-writer: incorrect format specifier

[Wink Saville]

Building the pu branch at commit 8b49f5c076c using Travis-CI all
of the linux builds are green but the two OSX builds are red[1] and
the logs show compile errors:

CC ident.o
CC json-writer.o

json-writer.c:123:38:  error:  format specifies type 'uintmax_t' (aka 'unsigned 
long') but the argument has type 'uint64_t' (aka 'unsigned long long') 
[-Werror,-Wformat]

strbuf_addf(>json, ":%"PRIuMAX, value);
 ~~ ^
json-writer.c:228:37:  error:  format specifies type 'uintmax_t' (aka 'unsigned 
long') but the argument has type 'uint64_t' (aka 'unsigned long long') 
[-Werror,-Wformat] [0m

strbuf_addf(>json, "%"PRIuMAX, value);
~~ ^
2 errors generated.
make: *** [json-writer.o] Error 1
make: *** Waiting for unfinished jobs

[RFC Patch 1/1] changes the PRIuMax to PRIu64 to correct the compile error
and now Travis-CI is green [2].

[1]: https://travis-ci.org/winksaville/git/builds/357660624
[2]: https://travis-ci.org/winksaville/git/builds/357681929

Wink Saville (1):
  json-writer: incorrect format specifier

 json-writer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.16.2

Re: [PATCH 00/27] sb/object-store updates

[Duy Nguyen]

On Fri, Mar 23, 2018 at 06:07:33PM -0400, Eric Sunshine wrote:
> On Fri, Mar 23, 2018 at 1:20 PM, Nguyễn Thái Ngọc Duy  
> wrote:
> > Interdiff is big due to the "objects." to "objects->" conversion
> > (blame Brandon for his suggestion, don't blame me :D)
> >
> > diff --git a/packfile.c b/packfile.c
> > @@ -1938,7 +1939,7 @@ static int add_promisor_object(const struct object_id 
> > *oid,
> > /*
> >  * If this is a tree, commit, or tag, the objects it refers
> > -* to are also promisor objects. (Blobs refer to no objects.)
> > +* to are also promisor objects-> (Blobs refer to no objects->)
> >  */
> 
> Meh.

I got too excited when searching and replacing. Here's the fixup
patch.

-- 8< --
Subject: [PATCH] fixup! object-store: move packed_git and packed_git_mru to
 object store

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

diff --git a/packfile.c b/packfile.c
index 63c89ee31a..0906b8f741 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1937,7 +1937,7 @@ static int add_promisor_object(const struct object_id 
*oid,
 
/*
 * If this is a tree, commit, or tag, the objects it refers
-* to are also promisor objects-> (Blobs refer to no objects->)
+* to are also promisor objects. (Blobs refer to no objects.)
 */
if (obj->type == OBJ_TREE) {
struct tree *tree = (struct tree *)obj;
-- 
2.17.0.rc0.348.gd5a49e0b6f

-- 8< --

[GSoC] Info: git log --oneline improvements

[Pratik Karki]

Hi Christian and Johannes,

I will like to send another proposal on git log --oneline improvements.
My first proposal[1] was on "Convert scripts to builtins". Can
you provide me information about "git log --online improvements"
and point me to resources where I should focus on my proposal.



[1]: 
https://public-inbox.org/git/20180321061605.27814-1-predatoram...@gmail.com/

Cheers,
Pratik Karki

Re: [PATCH v2] Allow use of TLS 1.3

[Loganaden Velvindron]

On Sat, Mar 24, 2018 at 1:47 AM, Daniel Stenberg  wrote:
> On Fri, 23 Mar 2018, Loganaden Velvindron wrote:
>
>> +#ifdef CURL_SSLVERSION_TLSv1_3
>> +   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
>> +#endif
>
>
> Unfortunately, CURL_SSLVERSION_TLSv1_3 is an enum so this construct won't
> work.
>
> Also, let me just point out that 7.52.0 is 0x073400 in hex and not the one
> used for the first version of this patch.
>
Yeah, v1 patch is broken. I'm sending a v3 patch which is properly
tested with OpenSSL preview alpha.



> --
>
>  / daniel.haxx.se

Re: [PATCH v2] Allow use of TLS 1.3

[Loganaden Velvindron]

On Sat, Mar 24, 2018 at 1:55 AM, Junio C Hamano  wrote:
> Loganaden Velvindron  writes:
>
>> Subject: Re: [PATCH v2] Allow use of TLS 1.3
>
> Let's retitle it to something like
>
> Subject: [PATCH v2] http: allow use of TLS 1.3
>
>> Add a tlsv1.3 option to http.sslVersion in addition to the existing
>> tlsv1.[012] options. libcurl has supported this since 7.52.0.
>
> Good.
>
>>
>> Done during IETF 101 Hackathon
>
> I am on the fence wrt the value of this line, especially because I
> would strongly suspect that this version is not what you wrote and
> tested during your Hackathon.  Even if it were, would it give value
> to future "git log" readers by supplying extra context?
>

Will remove this line.

>> Signed-off-by: Loganaden Velvindron 
>> ---
>>  Documentation/config.txt | 2 +-
>>  http.c   | 3 +++
>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index ce9102cea..b18cb9104 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -1957,7 +1957,7 @@ http.sslVersion::
>>   - tlsv1.0
>>   - tlsv1.1
>>   - tlsv1.2
>> -
>> + - tlsv1.3
>>  +
>
> Before this change, the block that shows the list of versions had
> one blank line before and after it.  Now we lost the blank line
> after the block.  Is it intended?  Possibilities that come to my
> mind as a reviewer are:
>
>  A. There is no difference in the rendered output if we have zero
> blank line (i.e. with the patch), or one blank line (i.e. before
> the patch applied).
>
> A.1) the submitter made this change on purpose, because it will
> make the source shorter without affecting the output, as a
> "clean-up while at it" change.
>
> A.2) this was an accidental change, which did not break the
> output merely because the submitter was lucky.
>
>  B. The rendered output changes due to the lack of the blank line.
>
> B.1) And it changes in a good way.  The submitter made this
> change on purpose.
>
> B.2) And it changes in a bad way, but the submitter did not
> notice it.
>
> Please do not make reviewers wonder.  Either avoid making
> unnecessary changes (e.g. you could have just added a new line with
> tlsv1.3 on it without touching the blank line), or make the change
> and explain why you made that change that is not essential for the
> purpose of adding tls1.3 which is the main focus of this patch.

Alright.

>
>>  Can be overridden by the `GIT_SSL_VERSION` environment variable.
>>  To force git to use libcurl's default ssl version and ignore any
>> diff --git a/http.c b/http.c
>> index a5bd5d62c..25eb84c11 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -62,6 +62,9 @@ static struct {
>>   { "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
>>   { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
>>  #endif
>> +#ifdef CURL_SSLVERSION_TLSv1_3
>> + { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
>> +#endif
>>  };
>
> It seems to me that
>
> https://github.com/curl/curl/blob/master/include/curl/curl.h#L1956
>
> tells me that this #ifdef would not work.  Did you test it with the
> "test not version but feature" change you made at the last minute?

I compiled it.

>
> I know it is not your fault but is Ævar's, but you're responsible
> for double-checking what you are told on the internet ;-)

Yes, my fault, not Ævar Arnfjörð Bjarmason .


>
> Thanks.

Re: [PATCH v2] Allow use of TLS 1.3

[Loganaden Velvindron]

On Sat, Mar 24, 2018 at 1:47 AM, Daniel Stenberg  wrote:
> On Fri, 23 Mar 2018, Loganaden Velvindron wrote:
>
>> +#ifdef CURL_SSLVERSION_TLSv1_3
>> +   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
>> +#endif
>
>
> Unfortunately, CURL_SSLVERSION_TLSv1_3 is an enum so this construct won't
> work.
>
> Also, let me just point out that 7.52.0 is 0x073400 in hex and not the one
> used for the first version of this patch.
>

Thanks, will fix it.

> --
>
>  / daniel.haxx.se

Re: [ANNOUNCE] Git v2.17.0-rc1

[Bryan Turner]

On Fri, Mar 23, 2018 at 10:47 AM, Johannes Schindelin
 wrote:
> Hi team,
>
> On Wed, 21 Mar 2018, Junio C Hamano wrote:
>
>> A release candidate Git v2.17.0-rc1 is now available for testing
>> at the usual places.  It is comprised of 493 non-merge commits
>> since v2.16.0, contributed by 62 people, 19 of which are new faces.
>>
>> The tarballs are found at:
>>
>> 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.kernel.org_pub_software_scm_git_testing_=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=yXNBIWf9n-gxAIgQyCzXfuKaFkHQaMmwUdtiNBNE8XI=E_Z2M418iwz-HyJg5D0VyTCvyMMd4kGIvYccgJkyTwA=
>
> And Git for Windows v2.17.0-rc1 can be found here:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_git-2Dfor-2Dwindows_git_releases_tag_v2.17.0-2Drc1.windows.1=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=yXNBIWf9n-gxAIgQyCzXfuKaFkHQaMmwUdtiNBNE8XI=7ePu15Fwlwuxo8JGcqj-pBNh1wSZYAfYmboqBvJOyA0=
>
> Please test so that we can hammer out a robust v2.17.0!

I've added 2.16.3 and 2.17.0-rc1, for both Linux and Windows, to the
test matrix for Bitbucket Server. All ~1500 tests have passed for all
4 versions.

Bryan

[PATCH v2 3/4] config.c: introduce 'git_config_color' to parse ANSI colors

[Taylor Blau]

In preparation for adding `--color` to the `git-config(1)` builtin,
let's introduce a color parsing utility, `git_config_color` in a similar
fashion to `git_config_`.

Signed-off-by: Taylor Blau 
---
 config.c | 10 ++
 config.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/config.c b/config.c
index b0c20e6cb..33366b52c 100644
--- a/config.c
+++ b/config.c
@@ -16,6 +16,7 @@
 #include "string-list.h"
 #include "utf8.h"
 #include "dir.h"
+#include "color.h"
 
 struct config_source {
struct config_source *prev;
@@ -1000,6 +1001,15 @@ int git_config_expiry_date(timestamp_t *timestamp, const 
char *var, const char *
return 0;
 }
 
+int git_config_color(char *dest, const char *var, const char *value)
+{
+   if (!value)
+   return config_error_nonbool(var);
+   if (color_parse(value, dest) < 0)
+   return -1;
+   return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
/* This needs a better name */
diff --git a/config.h b/config.h
index ef70a9cac..0e060779d 100644
--- a/config.h
+++ b/config.h
@@ -59,6 +59,7 @@ extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
 extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_expiry_date(timestamp_t *, const char *, const char *);
+extern int git_config_color(char *, const char *, const char *);
 extern int git_config_set_in_file_gently(const char *, const char *, const 
char *);
 extern void git_config_set_in_file(const char *, const char *, const char *);
 extern int git_config_set_gently(const char *, const char *);
-- 
2.16.2.440.gc6284da4f

[PATCH v2 1/4] builtin/config: introduce `--default`

[Taylor Blau]

For some use cases, callers of the `git-config(1)` builtin would like to
fallback to default values when the slot asked for does not exist. In
addition, users would like to use existing type specifiers to ensure
that values are parsed correctly when they do exist in the
configuration.

For example, to fetch a value without a type specifier and fallback to
`$fallback`, the following is required:

  $ git config core.foo || echo "$fallback"

This is fine for most values, but can be tricky for difficult-to-express
`$fallback`'s, like ANSI color codes.

This motivates `--get-color`, which is a one-off exception to the normal
type specifier rules wherein a user specifies both the configuration
slot and an optional fallback. Both are formatted according to their
type specifier, which eases the burden on the user to ensure that values
are correctly formatted.

This commit (and those following it in this series) aim to eventually
replace `--get-color` with a consistent alternative. By introducing
`--default`, we allow the `--get-color` action to be promoted to a
`--color` type specifier, retaining the "fallback" behavior via the
`--default` flag introduced in this commit.

For example, we aim to replace:

  $ git config --get-color slot [default] [...]

with:

  $ git config --default default --color slot [...]

Values filled by `--default` behave exactly as if they were present in
the affected configuration file; they will be parsed by type specifiers
without the knowledge that they are not themselves present in the
configuraion.

Specifically, this means that the following will work:

  $ git config --int --default 1M does.not.exist
  1048576

In subsequent commits, we will offer `--color`, which (in conjunction
with `--default`) will be sufficient to replace `--get-color`.

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt |   6 +-
 builtin/config.c |  17 +
 t/t1310-config-default.sh| 125 +++
 3 files changed, 146 insertions(+), 2 deletions(-)
 create mode 100755 t/t1310-config-default.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e09ed5d7d..d9e389a33 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -233,8 +233,10 @@ See also <>.
using `--file`, `--global`, etc) and `on` when searching all
config files.
 
-CONFIGURATION
--
+--default value::
+  When using `--get`, behave as if value were the value assigned to the given
+  slot.
+
 `pager.config` is only respected when listing configuration, i.e., when
 using `--list` or any of the `--get-*` which may return multiple results.
 The default is to use a pager.
diff --git a/builtin/config.c b/builtin/config.c
index 01169dd62..1410be850 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -26,6 +26,7 @@ static char term = '\n';
 static int use_global_config, use_system_config, use_local_config;
 static struct git_config_source given_config_source;
 static int actions, types;
+static char *default_value;
 static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
@@ -94,6 +95,7 @@ static struct option builtin_config_options[] = {
OPT_BOOL(0, "name-only", _values, N_("show variable names only")),
OPT_BOOL(0, "includes", _includes_opt, N_("respect include 
directives on lookup")),
OPT_BOOL(0, "show-origin", _origin, N_("show origin of config 
(file, standard input, blob, command line)")),
+   OPT_STRING(0, "default", _value, N_("value"), N_("with --get, 
use default value when missing entry")),
OPT_END(),
 };
 
@@ -258,6 +260,16 @@ static int get_value(const char *key_, const char *regex_)
config_with_options(collect_config, ,
_config_source, _options);
 
+   if (!values.nr && default_value) {
+   struct strbuf *item;
+   ALLOC_GROW(values.items, values.nr + 1, values.alloc);
+   item = [values.nr++];
+   strbuf_init(item, 0);
+   if (format_config(item, key_, default_value) < 0) {
+   values.nr = 0;
+   }
+   }
+
ret = !values.nr;
 
for (i = 0; i < values.nr; i++) {
@@ -601,6 +613,11 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
usage_with_options(builtin_config_usage, 
builtin_config_options);
}
 
+   if (default_value && !(actions & ACTION_GET)) {
+   error("--default is only applicable to --get");
+   usage_with_options(builtin_config_usage, 
builtin_config_options);
+   }
+
if (actions & PAGING_ACTIONS)
setup_auto_pager("config", 1);
 
diff --git a/t/t1310-config-default.sh b/t/t1310-config-default.sh
new file mode 100755
index 0..0e464c206
--- /dev/null
+++ b/t/t1310-config-default.sh
@@ -0,0 +1,125 @@
+#!/bin/sh
+
+test_description='Test git 

[PATCH v2 4/4] builtin/config: introduce `--color` type specifier

[Taylor Blau]

As of this commit, the canonical way to retreive an ANSI-compatible
color escape sequence from a configuration file is with the
`--get-color` action.

This is to allow Git to "fall back" on a default value for the color
should the given section not exist in the specified configuration(s).

With the addition of `--default`, this is no longer needed since:

  $ git config --default red --color core.section

will be have exactly as:

  $ git config --get-color core.section red

For consistency, let's introduce `--color` and encourage `--color`,
`--default` together over `--get-color` alone.

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt |  7 +++
 builtin/config.c | 13 +
 t/t1300-repo-config.sh   | 24 
 t/t1310-config-default.sh|  6 ++
 4 files changed, 50 insertions(+)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9d45aca3..e70c0a855 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -56,6 +56,9 @@ conversion to canonical form.  Currently supported type 
specifiers are:
 `expiry-date`::
 The value is taken as a timestamp.
 
+`color`::
+The value is taken as an ANSI color escape sequence.
+
 For more information about the above type specifiers, see <> below.
 
 When reading, the values are read from the system, global and
@@ -198,6 +201,10 @@ See also <>.
a fixed or relative date-string to a timestamp. This option
has no effect when setting the value.
 
+--color::
+  `git config` will ensure that the output is converted to an
+  ANSI color escape sequence.
+
 -z::
 --null::
For all options that output values and/or keys, always
diff --git a/builtin/config.c b/builtin/config.c
index 1410be850..e8661bc12 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -61,6 +61,7 @@ static int show_origin;
 #define TYPE_BOOL_OR_INT (1<<2)
 #define TYPE_PATH (1<<3)
 #define TYPE_EXPIRY_DATE (1<<4)
+#define TYPE_COLOR (1<<5)
 
 static struct option builtin_config_options[] = {
OPT_GROUP(N_("Config file location")),
@@ -90,6 +91,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "bool-or-int", , N_("value is --bool or --int"), 
TYPE_BOOL_OR_INT),
OPT_BIT(0, "path", , N_("value is a path (file or directory 
name)"), TYPE_PATH),
OPT_BIT(0, "expiry-date", , N_("value is an expiry date"), 
TYPE_EXPIRY_DATE),
+   OPT_BIT(0, "color", , N_("value is a color"), TYPE_COLOR),
OPT_GROUP(N_("Other")),
OPT_BOOL('z', "null", _null, N_("terminate values with NUL byte")),
OPT_BOOL(0, "name-only", _values, N_("show variable names only")),
@@ -175,6 +177,11 @@ static int format_config(struct strbuf *buf, const char 
*key_, const char *value
if (git_config_expiry_date(, key_, value_) < 0)
return -1;
strbuf_addf(buf, "%"PRItime, t);
+   } else if (types == TYPE_COLOR) {
+   char v[COLOR_MAXLEN];
+   if (git_config_color(v, key_, value_) < 0)
+   return -1;
+   strbuf_addstr(buf, v);
} else if (value_) {
strbuf_addstr(buf, value_);
} else {
@@ -320,6 +327,12 @@ static char *normalize_value(const char *key, const char 
*value)
else
return xstrdup(v ? "true" : "false");
}
+   if (types == TYPE_COLOR) {
+   char v[COLOR_MAXLEN];
+   if (!git_config_color(v, key, value))
+   return xstrdup(value);
+   die("cannot parse color '%s'", value);
+   }
 
die("BUG: cannot normalize type %d", types);
 }
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 4f8e6f5fd..dab94d0c4 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -931,6 +931,30 @@ test_expect_success 'get --expiry-date' '
test_must_fail git config --expiry-date date.invalid1
 '
 
+test_expect_success 'get --color' '
+   rm .git/config &&
+   git config foo.color "red" &&
+   git config --get --color foo.color | test_decode_color >actual &&
+   echo "" >expect &&
+   test_cmp expect actual
+'
+
+cat >expect << EOF
+[foo]
+   color = red
+EOF
+
+test_expect_success 'set --color' '
+   rm .git/config &&
+   git config --color foo.color "red" &&
+   test_cmp expect .git/config
+'
+
+test_expect_success 'get --color barfs on non-color' '
+   echo "[foo]bar=not-a-color" >.git/config &&
+   test_must_fail git config --get --color foo.bar
+'
+
 cat > expect << EOF
 [quote]
leading = " test"
diff --git a/t/t1310-config-default.sh b/t/t1310-config-default.sh
index 0e464c206..0ebece7d2 100755
--- a/t/t1310-config-default.sh
+++ b/t/t1310-config-default.sh
@@ -62,6 +62,12 @@ test_expect_success 

[PATCH v2 0/4] Teach `--default` to `git-config(1)`

[Taylor Blau]

Hi,

Attached is 'v2' of my patch series to add a `--default` option to `git config`.

I have addressed the following concerns since the first iteration:

  1. Better motivation in 'builtin/config: introduce `--default`'. (cc: Peff,
  Eric)

  2. Fixed a typo in the message of 'builtin/config: introduce `--default`'.
  (cc: Eric).

  3. Changed the first mention of type specifiers in `git config`'s
  documentation from a paragraph to a brief list (cc: Peff, Junio).

  4. Changed the signatures of some new functions, particularly in 'config.c:
  introduce 'git_config_color' to parse ANSI colors'.

I have thus-far avoided mentioning any specific deprecation of `--get-color` and
`--get-colorbool`, but would not be opposed to changing that in this series
before queuing.

Thank you again for all of your feedback, and my apologies that it has taken me
so long to respond. I was out of the office last week, and have been quite busy
since catching up on Git LFS-related issues.


Thanks,
Taylor

Taylor Blau (4):
  builtin/config: introduce `--default`
  Documentation: list all type specifiers in config prose
  config.c: introduce 'git_config_color' to parse ANSI colors
  builtin/config: introduce `--color` type specifier

 Documentation/git-config.txt |  38 +++---
 builtin/config.c |  30 
 config.c |  10 +++
 config.h |   1 +
 t/t1300-repo-config.sh   |  24 +++
 t/t1310-config-default.sh| 131 +++
 6 files changed, 226 insertions(+), 8 deletions(-)
 create mode 100755 t/t1310-config-default.sh

--
2.16.2.440.gc6284da4f

[PATCH v2 2/4] Documentation: list all type specifiers in config prose

[Taylor Blau]

The documentation for the `git-config(1)` builtin has not been recently
updated to include new types, such as `--bool-or-int`, and
`--expiry-date`. To ensure completeness when adding a new type
specifier, let's update the existing documentation to include the new
types.

Since this paragraph is growing in length, let's also convert this to a
list so that it can be read with greater ease. We provide a minimal
description of all valid type specifiers, and reference the <>
section where each specifier is described in full detail.

This commit also prepares for the new type specifier `--color`, so that
this section will not lag behind when yet more new specifiers are added.

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index d9e389a33..d9d45aca3 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -38,12 +38,25 @@ existing values that match the regexp are updated or unset. 
 If
 you want to handle the lines that do *not* match the regex, just
 prepend a single exclamation mark in front (see also <>).
 
-The type specifier can be either `--int` or `--bool`, to make
-'git config' ensure that the variable(s) are of the given type and
-convert the value to the canonical form (simple decimal number for int,
-a "true" or "false" string for bool), or `--path`, which does some
-path expansion (see `--path` below).  If no type specifier is passed, no
-checks or transformations are performed on the value.
+A type specifier option can be used to force interpretation of values and
+conversion to canonical form.  Currently supported type specifiers are:
+
+`bool`::
+The value is taken as a boolean.
+
+`int`::
+The value is taken as an integer.
+
+`bool-or-int`::
+The value is taken as a boolean or integer, as above.
+
+`path`::
+The value is taken as a filepath.
+
+`expiry-date`::
+The value is taken as a timestamp.
+
+For more information about the above type specifiers, see <> below.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-- 
2.16.2.440.gc6284da4f

Re: [RFC PATCH v5 3/8] Indent function git_rebase__interactive

[Wink Saville]

> I actually do not care if line-wrapping is done; it is perfectly
> fine to leave it for future clean-up and leave it outside the scope
> of this series.  If you are going to do as a part of the series,
> yes, I do prefer you limit yourself to those lines that are involved
> in the series in some other way.

Then lets leave the long lines alone for this series.

Re: [PATCH] Allow use of TLS 1.3

[Johannes Schindelin]

Hi,

On Fri, 23 Mar 2018, Loganaden Velvindron wrote:

> On Fri, Mar 23, 2018 at 07:37:08PM +0100, Ævar Arnfjörð Bjarmason wrote:
> > 
> > On Fri, Mar 23 2018, Loganaden Velvindron wrote:
> > 
> > > Done during IETF 101 hackathon
> > 
> > Hi. Thanks. Let's add a meaningful commit message to this though,
> > something like:
> > 
> > Add a tlsv1.3 option to http.sslVersion in addition to the existing
> > tlsv1.[012] options. libcurl has supported this since 7.52.0.

Can we please also add that OpenSSL 1.1.* is required (or that cURL is
built with NSS or BoringSSL as the TLS backend)?

Thanks,
Johannes

Re: [RFC PATCH v5 3/8] Indent function git_rebase__interactive

[Junio C Hamano]

Wink Saville  writes:

> Also, I assume you want me to only change lines in
> git_rebase__interactive.

I actually do not care if line-wrapping is done; it is perfectly
fine to leave it for future clean-up and leave it outside the scope
of this series.  If you are going to do as a part of the series,
yes, I do prefer you limit yourself to those lines that are involved
in the series in some other way.

Thanks.

Re: [RFC PATCH v5 0/8] rebase-interactive

[Wink Saville]

On Fri, Mar 23, 2018 at 3:39 PM, Junio C Hamano  wrote:
> Wink Saville  writes:
>
>> On Fri, Mar 23, 2018 at 2:25 PM, Wink Saville  wrote:
>>> Reworked patch 1 so that all of the backend scriptlets
>>> used by git-rebase use a normal function style invocation.
>>>
>>> Merged the previous patch 2 and 3 have been squashed which
>>> provides reviewers a little easier time to detect any changes
>>> during extraction of the functions.
>>>
>>> Wink Saville (8):
>>>   rebase-interactive: simplify pick_on_preserving_merges
>>>   rebase: update invocation of rebase dot-sourced scripts
>>>   Indent function git_rebase__interactive
>>>   Extract functions out of git_rebase__interactive
>>>   Add and use git_rebase__interactive__preserve_merges
>>>   Remove unused code paths from git_rebase__interactive
>>>   Remove unused code paths from git_rebase__interactive__preserve_merges
>>>   Remove merges_option and a blank line
>>>
>>>  git-rebase--am.sh  |  11 --
>>>  git-rebase--interactive.sh | 407 
>>> -
>>>  git-rebase--merge.sh   |  11 --
>>>  git-rebase.sh  |   1 +
>>>  4 files changed, 216 insertions(+), 214 deletions(-)
>>>
>>> --
>>> 2.16.2
>>>
>>
>> Argh, I misspelled Junio's email address, so when you reply-all try
>> to remember to remove "gis...@pobox.com" from the cc: list.
>
> Heh, too late ;-)
>
> I queued everything (with all patch 3-8/8 retitled to share a
> common prefix, so that "git shortlog" output would stay sane)
> and I think I resolved the conflicts with Dscho's recreate-merges
> topic correctly.  Please double check what will appear on 'pu' later
> today.
>
> Thanks.
>

OK, thank you!

Re: [RFC PATCH v5 3/8] Indent function git_rebase__interactive

[Wink Saville]

On Fri, Mar 23, 2018 at 3:12 PM, Junio C Hamano  wrote:
> Wink Saville  writes:
>
>> Signed-off-by: Wink Saville 
>> ---
>>  git-rebase--interactive.sh | 432 
>> ++---
>>  1 file changed, 215 insertions(+), 217 deletions(-)
>
> Thanks for separating this step out.  "git show -w --stat -p" tells
> us that this is a pure re-indent patch pretty easily ;-).
>
> Overlong lines might want to get rewrapped at some point, and it is
> OK to do that either in this step or in a separate step.
>

The longest line in the file before this change was line 532 which is 108
characters, now there are three lines longer because of the indentation.
Line 762 is 112, line 957 is 110 and 985 110.

My initial reaction is to leave these long lines as is, but if you want them
shorter what is the maximum line length?  At 80 characters per line
I count about 25 lines will need to be shortened.

Also, I assume you want me to only change lines in
git_rebase__interactive.

-- Wink

[PATCH v2] branch: implement shortcut to delete last branch

[Aaron Greenberg]

I updated the commit message to include my first email's cover letter
and cleaned up the test.

Copying Junio, since he also had good comments in the conversation you
linked.

I can appreciate Matthieu's points on the use of "-" in destructive
commands. As of this writing, git-merge supports the "-" shorthand,
which while not destructive, is at least _mutative_. Also,
"git branch -d" is not destructive in the same way that "rm -rf" is
destructive since you can recover the branch using the reflog.

One thing to consider is that approval of this patch extends the
implementation of the "-" shorthand in a piecemeal, rather than
consistent, way (implementing it in a consistent way was the goal of
the patch set you mentioned in your previous email.) Is that okay? Or
is it better to pick up the consistent approach where it was left?

[PATCH] branch: implement shortcut to delete last branch

[Aaron Greenberg]

This patch gives git-branch the ability to delete the previous
checked-out branch using the "-" shortcut. This shortcut already exists
for git-checkout, git-merge, and git-revert. A common workflow is

1. Do some work on a local topic-branch and push it to a remote.
2. 'remote/topic-branch' gets merged in to 'remote/master'.
3. Switch back to local master and fetch 'remote/master'.
4. Delete previously checked-out local topic-branch.

$ git checkout -b topic-a
$ # Do some work...
$ git commit -am "Implement feature A"
$ git push origin topic-a

$ git checkout master
$ git branch -d topic-a
$ # With this patch, a user could simply type
$ git branch -d -

"-" is a useful shortcut for cleaning up a just-merged branch
(or a just switched-from branch.)

Signed-off-by: Aaron Greenberg 
---
 builtin/branch.c  | 3 +++
 t/t3200-branch.sh | 8 
 2 files changed, 11 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6d0cea9..9e37078 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -221,6 +221,9 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
char *target = NULL;
int flags = 0;
 
+   if (!strcmp(argv[i], "-"))
+   argv[i] = "@{-1}";
+
strbuf_branchname(, argv[i], allowed_interpret);
free(name);
name = mkpathdup(fmt, bname.buf);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6c0b7ea..78c25aa 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -776,6 +776,14 @@ test_expect_success 'deleting currently checked out branch 
fails' '
test_must_fail git branch -d my7
 '
 
+test_expect_success 'test deleting last branch' '
+   git checkout -b my7.1 &&
+   git checkout  - &&
+   test_path_is_file .git/refs/heads/my7.1 &&
+   git branch -d - &&
+   test_path_is_missing .git/refs/heads/my7.1
+'
+
 test_expect_success 'test --track without .fetch entries' '
git branch --track my8 &&
test "$(git config branch.my8.remote)" &&
-- 
2.7.4

Re: [RFC PATCH v5 0/8] rebase-interactive

[Junio C Hamano]

Wink Saville  writes:

> On Fri, Mar 23, 2018 at 2:25 PM, Wink Saville  wrote:
>> Reworked patch 1 so that all of the backend scriptlets
>> used by git-rebase use a normal function style invocation.
>>
>> Merged the previous patch 2 and 3 have been squashed which
>> provides reviewers a little easier time to detect any changes
>> during extraction of the functions.
>>
>> Wink Saville (8):
>>   rebase-interactive: simplify pick_on_preserving_merges
>>   rebase: update invocation of rebase dot-sourced scripts
>>   Indent function git_rebase__interactive
>>   Extract functions out of git_rebase__interactive
>>   Add and use git_rebase__interactive__preserve_merges
>>   Remove unused code paths from git_rebase__interactive
>>   Remove unused code paths from git_rebase__interactive__preserve_merges
>>   Remove merges_option and a blank line
>>
>>  git-rebase--am.sh  |  11 --
>>  git-rebase--interactive.sh | 407 
>> -
>>  git-rebase--merge.sh   |  11 --
>>  git-rebase.sh  |   1 +
>>  4 files changed, 216 insertions(+), 214 deletions(-)
>>
>> --
>> 2.16.2
>>
>
> Argh, I misspelled Junio's email address, so when you reply-all try
> to remember to remove "gis...@pobox.com" from the cc: list.

Heh, too late ;-)

I queued everything (with all patch 3-8/8 retitled to share a
common prefix, so that "git shortlog" output would stay sane)
and I think I resolved the conflicts with Dscho's recreate-merges
topic correctly.  Please double check what will appear on 'pu' later
today.

Thanks.

Re: [PATCH v2] Allow use of TLS 1.3

[Ævar Arnfjörð Bjarmason]

On Fri, Mar 23 2018, Junio C. Hamano wrote:

>> @@ -62,6 +62,9 @@ static struct {
>>  { "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
>>  { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
>>  #endif
>> +#ifdef CURL_SSLVERSION_TLSv1_3
>> +{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
>> +#endif
>>  };
>
> It seems to me that
>
> https://github.com/curl/curl/blob/master/include/curl/curl.h#L1956
>
> tells me that this #ifdef would not work.  Did you test it with the
> "test not version but feature" change you made at the last minute?
>
> I know it is not your fault but is Ævar's, but you're responsible
> for double-checking what you are told on the internet ;-)

Yeah I should add some "I haven't actually tried this, but what do you
think about this?" disclaimer.

But it's not a good sign that we have a v2 with an ifdef that'll never
be true, indicating that it wasn't tested against TLSv1.3. Is there some
way we could check for this in our test suite?

Re: [RFC PATCH v5 0/8] rebase-interactive

[Junio C Hamano]

Wink Saville  writes:

> Wink Saville (8):
>   rebase-interactive: simplify pick_on_preserving_merges
>   rebase: update invocation of rebase dot-sourced scripts
>   Indent function git_rebase__interactive
>   Extract functions out of git_rebase__interactive
>   Add and use git_rebase__interactive__preserve_merges
>   Remove unused code paths from git_rebase__interactive
>   Remove unused code paths from git_rebase__interactive__preserve_merges
>   Remove merges_option and a blank line

I felt that the structure of steps 5-7 that adds an identical copy
first and then removes irrelevant parts from both copies was a bit
unusual, but I do not think of a better structure I would use if I
were doing this series myself, and more importantly, the entire
series was a pleasant and straight-forward read.

Will queue and wait for input from others.

Thanks.

Re: [RFC PATCH v5 4/8] Extract functions out of git_rebase__interactive

[Junio C Hamano]

Wink Saville  writes:

> The extracted functions are:
>   - initiate_action
>   - setup_reflog_action
>   - init_basic_state
>   - init_revisions_and_shortrevisions
>   - complete_action
>
> Used by git_rebase__interactive
>
> Signed-off-by: Wink Saville 
> Helped-by: Junio C Hamano 
> Helped-by: Johannes Schindelin 
> ---

I checked the correspondence of lines between the verison before and
after the patch, and did not spot anything suspicious.  The fact
that we do not use "local" and stick to POSIX shell helps a bit, as
we do not have to worry about "does this $variable in the split-out
function refer to the same data as the original?" ;-)

Will queue.

Re: [RFC PATCH v5 3/8] Indent function git_rebase__interactive

[Junio C Hamano]

Wink Saville  writes:

> Signed-off-by: Wink Saville 
> ---
>  git-rebase--interactive.sh | 432 
> ++---
>  1 file changed, 215 insertions(+), 217 deletions(-)

Thanks for separating this step out.  "git show -w --stat -p" tells
us that this is a pure re-indent patch pretty easily ;-).

Overlong lines might want to get rewrapped at some point, and it is
OK to do that either in this step or in a separate step.

Re: [PATCH 00/27] sb/object-store updates

[Eric Sunshine]

On Fri, Mar 23, 2018 at 1:20 PM, Nguyễn Thái Ngọc Duy  wrote:
> Interdiff is big due to the "objects." to "objects->" conversion
> (blame Brandon for his suggestion, don't blame me :D)
>
> diff --git a/packfile.c b/packfile.c
> @@ -1938,7 +1939,7 @@ static int add_promisor_object(const struct object_id 
> *oid,
> /*
>  * If this is a tree, commit, or tag, the objects it refers
> -* to are also promisor objects. (Blobs refer to no objects.)
> +* to are also promisor objects-> (Blobs refer to no objects->)
>  */

Meh.

Re: [PATCH v2] Allow use of TLS 1.3

[Junio C Hamano]

Daniel Stenberg  writes:

> On Fri, 23 Mar 2018, Loganaden Velvindron wrote:
>
>> +#ifdef CURL_SSLVERSION_TLSv1_3
>> +{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
>> +#endif
>
> Unfortunately, CURL_SSLVERSION_TLSv1_3 is an enum so this construct
> won't work.
>
> Also, let me just point out that 7.52.0 is 0x073400 in hex and not the
> one used for the first version of this patch.

Thanks!

Re: [PATCH v2] Allow use of TLS 1.3

[Junio C Hamano]

Loganaden Velvindron  writes:

> Subject: Re: [PATCH v2] Allow use of TLS 1.3

Let's retitle it to something like

Subject: [PATCH v2] http: allow use of TLS 1.3

> Add a tlsv1.3 option to http.sslVersion in addition to the existing 
> tlsv1.[012] options. libcurl has supported this since 7.52.0.

Good.

>
> Done during IETF 101 Hackathon

I am on the fence wrt the value of this line, especially because I
would strongly suspect that this version is not what you wrote and
tested during your Hackathon.  Even if it were, would it give value
to future "git log" readers by supplying extra context?

> Signed-off-by: Loganaden Velvindron 
> ---
>  Documentation/config.txt | 2 +-
>  http.c   | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ce9102cea..b18cb9104 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1957,7 +1957,7 @@ http.sslVersion::
>   - tlsv1.0
>   - tlsv1.1
>   - tlsv1.2
> -
> + - tlsv1.3
>  +

Before this change, the block that shows the list of versions had
one blank line before and after it.  Now we lost the blank line
after the block.  Is it intended?  Possibilities that come to my
mind as a reviewer are:

 A. There is no difference in the rendered output if we have zero
blank line (i.e. with the patch), or one blank line (i.e. before
the patch applied).

A.1) the submitter made this change on purpose, because it will
make the source shorter without affecting the output, as a
"clean-up while at it" change.

A.2) this was an accidental change, which did not break the
output merely because the submitter was lucky.

 B. The rendered output changes due to the lack of the blank line.

B.1) And it changes in a good way.  The submitter made this
change on purpose.

B.2) And it changes in a bad way, but the submitter did not
notice it.

Please do not make reviewers wonder.  Either avoid making
unnecessary changes (e.g. you could have just added a new line with
tlsv1.3 on it without touching the blank line), or make the change
and explain why you made that change that is not essential for the
purpose of adding tls1.3 which is the main focus of this patch.

>  Can be overridden by the `GIT_SSL_VERSION` environment variable.
>  To force git to use libcurl's default ssl version and ignore any
> diff --git a/http.c b/http.c
> index a5bd5d62c..25eb84c11 100644
> --- a/http.c
> +++ b/http.c
> @@ -62,6 +62,9 @@ static struct {
>   { "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
>   { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
>  #endif
> +#ifdef CURL_SSLVERSION_TLSv1_3
> + { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
> +#endif
>  };

It seems to me that

https://github.com/curl/curl/blob/master/include/curl/curl.h#L1956

tells me that this #ifdef would not work.  Did you test it with the
"test not version but feature" change you made at the last minute?

I know it is not your fault but is Ævar's, but you're responsible
for double-checking what you are told on the internet ;-)

Thanks.

Re: [PATCH v2] Allow use of TLS 1.3

[Daniel Stenberg]

On Fri, 23 Mar 2018, Loganaden Velvindron wrote:


+#ifdef CURL_SSLVERSION_TLSv1_3
+   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
+#endif


Unfortunately, CURL_SSLVERSION_TLSv1_3 is an enum so this construct won't 
work.


Also, let me just point out that 7.52.0 is 0x073400 in hex and not the one 
used for the first version of this patch.


--

 / daniel.haxx.se

Re: [RFC PATCH v5 0/8] rebase-interactive

[Wink Saville]

On Fri, Mar 23, 2018 at 2:25 PM, Wink Saville  wrote:
> Reworked patch 1 so that all of the backend scriptlets
> used by git-rebase use a normal function style invocation.
>
> Merged the previous patch 2 and 3 have been squashed which
> provides reviewers a little easier time to detect any changes
> during extraction of the functions.
>
> Wink Saville (8):
>   rebase-interactive: simplify pick_on_preserving_merges
>   rebase: update invocation of rebase dot-sourced scripts
>   Indent function git_rebase__interactive
>   Extract functions out of git_rebase__interactive
>   Add and use git_rebase__interactive__preserve_merges
>   Remove unused code paths from git_rebase__interactive
>   Remove unused code paths from git_rebase__interactive__preserve_merges
>   Remove merges_option and a blank line
>
>  git-rebase--am.sh  |  11 --
>  git-rebase--interactive.sh | 407 
> -
>  git-rebase--merge.sh   |  11 --
>  git-rebase.sh  |   1 +
>  4 files changed, 216 insertions(+), 214 deletions(-)
>
> --
> 2.16.2
>

Argh, I misspelled Junio's email address, so when you reply-all try
to remember to remove "gis...@pobox.com" from the cc: list.

Sorry,
Wink

Re: [PATCH 00/27] sb/object-store updates

[Junio C Hamano]

Brandon Williams  writes:

>> > Interdiff is big due to the "objects." to "objects->" conversion
>> > (blame Brandon for his suggestion, don't blame me :D)
>> 
>> Haha, I'm guessing you prefer having a pointer too then? :P 
>> 
>> The interdiff looks good, though I'll set some time aside today to go
>> through the series as a whole again.
>
> Had some more time than I thought; this series looks good!  Thanks for
> take the time to keep this rolling.  Hopefully we can see this merged
> soon :)

Yeah, I definitely hope that this one is already 'next'-worthy; the
micro-update to ignore-env we saw recently looked good, too.

Thanks.

[RFC PATCH v5 5/8] Add and use git_rebase__interactive__preserve_merges

[Wink Saville]

At the moment it's an exact copy of git_rebase__interactive except
the name has changed.

Signed-off-by: Wink Saville 
---
 git-rebase--interactive.sh | 108 +
 git-rebase.sh  |   2 +-
 2 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2c10a7f1a..ab5513d80 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1058,3 +1058,111 @@ git_rebase__interactive () {
 
complete_action
 }
+
+git_rebase__interactive__preserve_merges () {
+   initiate_action "$action"
+   ret=$?
+   if test $ret = 0; then
+   return 0
+   fi
+
+   setup_reflog_action
+   init_basic_state
+
+   if test t = "$preserve_merges"
+   then
+   if test -z "$rebase_root"
+   then
+   mkdir "$rewritten" &&
+   for c in $(git merge-base --all $orig_head $upstream)
+   do
+   echo $onto > "$rewritten"/$c ||
+   die "$(gettext "Could not init 
rewritten commits")"
+   done
+   else
+   mkdir "$rewritten" &&
+   echo $onto > "$rewritten"/root ||
+   die "$(gettext "Could not init rewritten 
commits")"
+   fi
+   # No cherry-pick because our first pass is to determine
+   # parents to rewrite and skipping dropped commits would
+   # prematurely end our probe
+   merges_option=
+   else
+   merges_option="--no-merges --cherry-pick"
+   fi
+
+   init_revisions_and_shortrevisions
+
+   if test t != "$preserve_merges"
+   then
+   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
+   $revisions ${restrict_revision+^$restrict_revision} 
>"$todo" ||
+   die "$(gettext "Could not generate todo list")"
+   else
+   format=$(git config --get rebase.instructionFormat)
+   # the 'rev-list .. | sed' requires %m to parse; the instruction 
requires %H to parse
+   git rev-list $merges_option --format="%m%H ${format:-%s}" \
+   --reverse --left-right --topo-order \
+   $revisions ${restrict_revision+^$restrict_revision} | \
+   sed -n "s/^>//p" |
+   while read -r sha1 rest
+   do
+
+   if test -z "$keep_empty" && is_empty_commit $sha1 && ! 
is_merge_commit $sha1
+   then
+   comment_out="$comment_char "
+   else
+   comment_out=
+   fi
+
+   if test -z "$rebase_root"
+   then
+   preserve=t
+   for p in $(git rev-list --parents -1 $sha1 | 
cut -d' ' -s -f2-)
+   do
+   if test -f "$rewritten"/$p
+   then
+   preserve=f
+   fi
+   done
+   else
+   preserve=f
+   fi
+   if test f = "$preserve"
+   then
+   touch "$rewritten"/$sha1
+   printf '%s\n' "${comment_out}pick $sha1 $rest" 
>>"$todo"
+   fi
+   done
+   fi
+
+   # Watch for commits that been dropped by --cherry-pick
+   if test t = "$preserve_merges"
+   then
+   mkdir "$dropped"
+   # Save all non-cherry-picked changes
+   git rev-list $revisions --left-right --cherry-pick | \
+   sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
+   # Now all commits and note which ones are missing in
+   # not-cherry-picks and hence being dropped
+   git rev-list $revisions |
+   while read rev
+   do
+   if test -f "$rewritten"/$rev &&
+  ! sane_grep "$rev" "$state_dir"/not-cherry-picks 
>/dev/null
+   then
+   # Use -f2 because if rev-list is telling us 
this commit is
+   # not worthwhile, we don't want to track its 
multiple heads,
+   # just the history of its first-parent for 
others that will
+   # be rebasing on top of it
+   git rev-list --parents -1 $rev | cut -d' ' -s 
-f2 > "$dropped"/$rev
+   

[RFC PATCH v5 1/8] rebase-interactive: simplify pick_on_preserving_merges

[Wink Saville]

Use compound if statement instead of nested if statements to
simplify pick_on_preserving_merges.

Signed-off-by: Wink Saville 
---
 git-rebase--interactive.sh | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 331c8dfea..561e2660e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -307,17 +307,14 @@ pick_one_preserving_merges () {
esac
sha1=$(git rev-parse $sha1)
 
-   if test -f "$state_dir"/current-commit
+   if test -f "$state_dir"/current-commit && test "$fast_forward" = t
then
-   if test "$fast_forward" = t
-   then
-   while read current_commit
-   do
-   git rev-parse HEAD > 
"$rewritten"/$current_commit
-   done <"$state_dir"/current-commit
-   rm "$state_dir"/current-commit ||
-   die "$(gettext "Cannot write current commit's 
replacement sha1")"
-   fi
+   while read current_commit
+   do
+   git rev-parse HEAD > "$rewritten"/$current_commit
+   done <"$state_dir"/current-commit
+   rm "$state_dir"/current-commit ||
+   die "$(gettext "Cannot write current commit's 
replacement sha1")"
fi
 
echo $sha1 >> "$state_dir"/current-commit
-- 
2.16.2

[RFC PATCH v5 6/8] Remove unused code paths from git_rebase__interactive

[Wink Saville]

Since git_rebase__interactive is now never called with
$preserve_merges = t we can remove those code paths.

Signed-off-by: Wink Saville 
---
 git-rebase--interactive.sh | 95 ++
 1 file changed, 4 insertions(+), 91 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ab5513d80..346da0f67 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -961,100 +961,13 @@ git_rebase__interactive () {
setup_reflog_action
init_basic_state
 
-   if test t = "$preserve_merges"
-   then
-   if test -z "$rebase_root"
-   then
-   mkdir "$rewritten" &&
-   for c in $(git merge-base --all $orig_head $upstream)
-   do
-   echo $onto > "$rewritten"/$c ||
-   die "$(gettext "Could not init 
rewritten commits")"
-   done
-   else
-   mkdir "$rewritten" &&
-   echo $onto > "$rewritten"/root ||
-   die "$(gettext "Could not init rewritten 
commits")"
-   fi
-   # No cherry-pick because our first pass is to determine
-   # parents to rewrite and skipping dropped commits would
-   # prematurely end our probe
-   merges_option=
-   else
-   merges_option="--no-merges --cherry-pick"
-   fi
+   merges_option="--no-merges --cherry-pick"
 
init_revisions_and_shortrevisions
 
-   if test t != "$preserve_merges"
-   then
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   $revisions ${restrict_revision+^$restrict_revision} 
>"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-   else
-   format=$(git config --get rebase.instructionFormat)
-   # the 'rev-list .. | sed' requires %m to parse; the instruction 
requires %H to parse
-   git rev-list $merges_option --format="%m%H ${format:-%s}" \
-   --reverse --left-right --topo-order \
-   $revisions ${restrict_revision+^$restrict_revision} | \
-   sed -n "s/^>//p" |
-   while read -r sha1 rest
-   do
-
-   if test -z "$keep_empty" && is_empty_commit $sha1 && ! 
is_merge_commit $sha1
-   then
-   comment_out="$comment_char "
-   else
-   comment_out=
-   fi
-
-   if test -z "$rebase_root"
-   then
-   preserve=t
-   for p in $(git rev-list --parents -1 $sha1 | 
cut -d' ' -s -f2-)
-   do
-   if test -f "$rewritten"/$p
-   then
-   preserve=f
-   fi
-   done
-   else
-   preserve=f
-   fi
-   if test f = "$preserve"
-   then
-   touch "$rewritten"/$sha1
-   printf '%s\n' "${comment_out}pick $sha1 $rest" 
>>"$todo"
-   fi
-   done
-   fi
-
-   # Watch for commits that been dropped by --cherry-pick
-   if test t = "$preserve_merges"
-   then
-   mkdir "$dropped"
-   # Save all non-cherry-picked changes
-   git rev-list $revisions --left-right --cherry-pick | \
-   sed -n "s/^>//p" > "$state_dir"/not-cherry-picks
-   # Now all commits and note which ones are missing in
-   # not-cherry-picks and hence being dropped
-   git rev-list $revisions |
-   while read rev
-   do
-   if test -f "$rewritten"/$rev &&
-  ! sane_grep "$rev" "$state_dir"/not-cherry-picks 
>/dev/null
-   then
-   # Use -f2 because if rev-list is telling us 
this commit is
-   # not worthwhile, we don't want to track its 
multiple heads,
-   # just the history of its first-parent for 
others that will
-   # be rebasing on top of it
-   git rev-list --parents -1 $rev | cut -d' ' -s 
-f2 > "$dropped"/$rev
-   sha1=$(git rev-list -1 $rev)
-   sane_grep -v "^[a-z][a-z]* $sha1" <"$todo" > 
"${todo}2" ; mv "${todo}2" "$todo"
-

[RFC PATCH v5 0/8] rebase-interactive

[Wink Saville]

Reworked patch 1 so that all of the backend scriptlets
used by git-rebase use a normal function style invocation.

Merged the previous patch 2 and 3 have been squashed which
provides reviewers a little easier time to detect any changes
during extraction of the functions.

Wink Saville (8):
  rebase-interactive: simplify pick_on_preserving_merges
  rebase: update invocation of rebase dot-sourced scripts
  Indent function git_rebase__interactive
  Extract functions out of git_rebase__interactive
  Add and use git_rebase__interactive__preserve_merges
  Remove unused code paths from git_rebase__interactive
  Remove unused code paths from git_rebase__interactive__preserve_merges
  Remove merges_option and a blank line

 git-rebase--am.sh  |  11 --
 git-rebase--interactive.sh | 407 -
 git-rebase--merge.sh   |  11 --
 git-rebase.sh  |   1 +
 4 files changed, 216 insertions(+), 214 deletions(-)

-- 
2.16.2

[RFC PATCH v5 8/8] Remove merges_option and a blank line

[Wink Saville]

merges_option is unused in git_rebase__interactive and always empty in
git_rebase__interactive__preserve_merges so it can be removed.

Signed-off-by: Wink Saville 
---
 git-rebase--interactive.sh | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ddbd126f2..50323fc27 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -961,8 +961,6 @@ git_rebase__interactive () {
setup_reflog_action
init_basic_state
 
-   merges_option="--no-merges --cherry-pick"
-
init_revisions_and_shortrevisions
 
git rebase--helper --make-script ${keep_empty:+--keep-empty} \
@@ -996,22 +994,16 @@ git_rebase__interactive__preserve_merges () {
die "$(gettext "Could not init rewritten commits")"
fi
 
-   # No cherry-pick because our first pass is to determine
-   # parents to rewrite and skipping dropped commits would
-   # prematurely end our probe
-   merges_option=
-
init_revisions_and_shortrevisions
 
format=$(git config --get rebase.instructionFormat)
# the 'rev-list .. | sed' requires %m to parse; the instruction 
requires %H to parse
-   git rev-list $merges_option --format="%m%H ${format:-%s}" \
+   git rev-list --format="%m%H ${format:-%s}" \
--reverse --left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n "s/^>//p" |
while read -r sha1 rest
do
-
if test -z "$keep_empty" && is_empty_commit $sha1 && ! 
is_merge_commit $sha1
then
comment_out="$comment_char "
-- 
2.16.2

[RFC PATCH v5 4/8] Extract functions out of git_rebase__interactive

[Wink Saville]

The extracted functions are:
  - initiate_action
  - setup_reflog_action
  - init_basic_state
  - init_revisions_and_shortrevisions
  - complete_action

Used by git_rebase__interactive

Signed-off-by: Wink Saville 
Helped-by: Junio C Hamano 
Helped-by: Johannes Schindelin 
---
 git-rebase--interactive.sh | 182 +++--
 1 file changed, 111 insertions(+), 71 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a79330f45..2c10a7f1a 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -740,8 +740,20 @@ get_missing_commit_check_level () {
printf '%s' "$check_level" | tr 'A-Z' 'a-z'
 }
 
-git_rebase__interactive () {
-   case "$action" in
+# Initiate an action. If the cannot be any
+# further action it  may exec a command
+# or exit and not return.
+#
+# TODO: Consider a cleaner return model so it
+# never exits and always return 0 if process
+# is complete.
+#
+# Parameter 1 is the action to initiate.
+#
+# Returns 0 if the action was able to complete
+# and if 1 if further processing is required.
+initiate_action () {
+   case "$1" in
continue)
if test ! -d "$rewritten"
then
@@ -836,8 +848,13 @@ To continue rebase after editing, run:
show-current-patch)
exec git show REBASE_HEAD --
;;
+   *)
+   return 1 # continue
+   ;;
esac
+}
 
+setup_reflog_action () {
comment_for_reflog start
 
if test ! -z "$switch_to"
@@ -848,13 +865,102 @@ To continue rebase after editing, run:
 
comment_for_reflog start
fi
+}
 
+init_basic_state () {
orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
mkdir -p "$state_dir" || die "$(eval_gettext "Could not create 
temporary \$state_dir")"
rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
: > "$state_dir"/interactive || die "$(gettext "Could not mark as 
interactive")"
write_basic_state
+}
+
+init_revisions_and_shortrevisions () {
+   shorthead=$(git rev-parse --short $orig_head)
+   shortonto=$(git rev-parse --short $onto)
+   if test -z "$rebase_root"
+   # this is now equivalent to ! -z "$upstream"
+   then
+   shortupstream=$(git rev-parse --short $upstream)
+   revisions=$upstream...$orig_head
+   shortrevisions=$shortupstream..$shorthead
+   else
+   revisions=$onto...$orig_head
+   shortrevisions=$shorthead
+   fi
+}
+
+complete_action() {
+   test -s "$todo" || echo noop >> "$todo"
+   test -z "$autosquash" || git rebase--helper --rearrange-squash || exit
+   test -n "$cmd" && git rebase--helper --add-exec-commands "$cmd"
+
+   todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
+   todocount=${todocount##* }
+
+cat >>"$todo" <>"$todo"
+
+   if test -z "$keep_empty"
+   then
+   printf '%s\n' "$comment_char $(gettext "Note that empty commits 
are commented out")" >>"$todo"
+   fi
+
+
+   has_action "$todo" ||
+   return 2
+
+   cp "$todo" "$todo".backup
+   collapse_todo_ids
+   git_sequence_editor "$todo" ||
+   die_abort "$(gettext "Could not execute editor")"
+
+   has_action "$todo" ||
+   return 2
+
+   git rebase--helper --check-todo-list || {
+   ret=$?
+   checkout_onto
+   exit $ret
+   }
+
+   expand_todo_ids
+
+   test -d "$rewritten" || test -n "$force_rebase" ||
+   onto="$(git rebase--helper --skip-unnecessary-picks)" ||
+   die "Could not skip unnecessary pick commands"
+
+   checkout_onto
+   if test -z "$rebase_root" && test ! -d "$rewritten"
+   then
+   require_clean_work_tree "rebase"
+   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
+   --continue
+   fi
+   do_rest
+}
+
+git_rebase__interactive () {
+   initiate_action "$action"
+   ret=$?
+   if test $ret = 0; then
+   return 0
+   fi
+
+   setup_reflog_action
+   init_basic_state
+
if test t = "$preserve_merges"
then
if test -z "$rebase_root"
@@ -878,18 +984,8 @@ To continue rebase after editing, run:
merges_option="--no-merges --cherry-pick"
fi
 
-   shorthead=$(git rev-parse --short $orig_head)
-   shortonto=$(git rev-parse --short $onto)
-   if test -z "$rebase_root"
-   # this is now equivalent to ! -z "$upstream"
-   then
-   shortupstream=$(git rev-parse --short $upstream)
-   revisions=$upstream...$orig_head
-   shortrevisions=$shortupstream..$shorthead
-   else
-   

[RFC PATCH v5 2/8] rebase: update invocation of rebase dot-sourced scripts

[Wink Saville]

Due to historical reasons, the backend scriptlets for "git rebase"
are structured a bit unusually. As originally designed,
dot-sourcing them from "git rebase" was sufficient to invoke the
specific backend.

However, it was later discovered that some shell implementations
(e.g. FreeBSD 9.x) misbehaved by continuing to execute statements
following a top-level "return" rather than returning control to
the next statement in "git rebase" after dot-sourcing the
scriptlet. To work around this shortcoming, the whole body of
git-rebase--$backend.sh was made into a shell function
git_rebase__$backend, and then the very last line of the scriptlet
called that function.

A more normal architecture is for a dot-sourced scriptlet merely
to define functions (thus acting as a function library), and for
those functions to be called by the script doing the dot-sourcing.
Migrate to this arrangement by moving the git_rebase__$backend
call from the end of a scriptlet into "git rebase" itself.

While at it, remove the large comment block from each scriptlet
explaining this historic anomaly since it serves no purpose under
the new normalized architecture in which a scriptlet is merely a
function library.

Signed-off-by: Wink Saville 
Helped-by: Junio C Hamano 
Helped-by: Eric Sunshine 
---
 git-rebase--am.sh  | 11 ---
 git-rebase--interactive.sh | 11 ---
 git-rebase--merge.sh   | 11 ---
 git-rebase.sh  |  1 +
 4 files changed, 1 insertion(+), 33 deletions(-)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index be3f06892..e5fd6101d 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -4,15 +4,6 @@
 # Copyright (c) 2010 Junio C Hamano.
 #
 
-# The whole contents of this file is run by dot-sourcing it from
-# inside a shell function.  It used to be that "return"s we see
-# below were not inside any function, and expected to return
-# to the function that dot-sourced us.
-#
-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
-# construct and continue to run the statements that follow such a "return".
-# As a work-around, we introduce an extra layer of a function
-# here, and immediately call it after defining it.
 git_rebase__am () {
 
 case "$action" in
@@ -105,5 +96,3 @@ fi
 move_to_original_branch
 
 }
-# ... and then we call the whole thing.
-git_rebase__am
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 561e2660e..213d75f43 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -740,15 +740,6 @@ get_missing_commit_check_level () {
printf '%s' "$check_level" | tr 'A-Z' 'a-z'
 }
 
-# The whole contents of this file is run by dot-sourcing it from
-# inside a shell function.  It used to be that "return"s we see
-# below were not inside any function, and expected to return
-# to the function that dot-sourced us.
-#
-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
-# construct and continue to run the statements that follow such a "return".
-# As a work-around, we introduce an extra layer of a function
-# here, and immediately call it after defining it.
 git_rebase__interactive () {
 
 case "$action" in
@@ -1029,5 +1020,3 @@ fi
 do_rest
 
 }
-# ... and then we call the whole thing.
-git_rebase__interactive
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index ceb715453..685f48ca4 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -104,15 +104,6 @@ finish_rb_merge () {
say All done.
 }
 
-# The whole contents of this file is run by dot-sourcing it from
-# inside a shell function.  It used to be that "return"s we see
-# below were not inside any function, and expected to return
-# to the function that dot-sourced us.
-#
-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a
-# construct and continue to run the statements that follow such a "return".
-# As a work-around, we introduce an extra layer of a function
-# here, and immediately call it after defining it.
 git_rebase__merge () {
 
 case "$action" in
@@ -171,5 +162,3 @@ done
 finish_rb_merge
 
 }
-# ... and then we call the whole thing.
-git_rebase__merge
diff --git a/git-rebase.sh b/git-rebase.sh
index a1f6e5de6..6edf8c5b1 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -197,6 +197,7 @@ run_specific_rebase () {
autosquash=
fi
. git-rebase--$type
+   git_rebase__$type
ret=$?
if test $ret -eq 0
then
-- 
2.16.2

[RFC PATCH v5 3/8] Indent function git_rebase__interactive

[Wink Saville]

Signed-off-by: Wink Saville 
---
 git-rebase--interactive.sh | 432 ++---
 1 file changed, 215 insertions(+), 217 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 213d75f43..a79330f45 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -741,27 +741,26 @@ get_missing_commit_check_level () {
 }
 
 git_rebase__interactive () {
-
-case "$action" in
-continue)
-   if test ! -d "$rewritten"
-   then
-   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
-   --continue
-   fi
-   # do we have anything to commit?
-   if git diff-index --cached --quiet HEAD --
-   then
-   # Nothing to commit -- skip this commit
-
-   test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD ||
-   rm "$GIT_DIR"/CHERRY_PICK_HEAD ||
-   die "$(gettext "Could not remove CHERRY_PICK_HEAD")"
-   else
-   if ! test -f "$author_script"
+   case "$action" in
+   continue)
+   if test ! -d "$rewritten"
then
-   gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse 
--sq-quote "$gpg_sign_opt")}
-   die "$(eval_gettext "\
+   exec git rebase--helper ${force_rebase:+--no-ff} 
$allow_empty_message \
+   --continue
+   fi
+   # do we have anything to commit?
+   if git diff-index --cached --quiet HEAD --
+   then
+   # Nothing to commit -- skip this commit
+
+   test ! -f "$GIT_DIR"/CHERRY_PICK_HEAD ||
+   rm "$GIT_DIR"/CHERRY_PICK_HEAD ||
+   die "$(gettext "Could not remove CHERRY_PICK_HEAD")"
+   else
+   if ! test -f "$author_script"
+   then
+   gpg_sign_opt_quoted=${gpg_sign_opt:+$(git 
rev-parse --sq-quote "$gpg_sign_opt")}
+   die "$(eval_gettext "\
 You have staged changes in your working tree.
 If these changes are meant to be
 squashed into the previous commit, run:
@@ -776,197 +775,197 @@ In both cases, once you're done, continue with:
 
   git rebase --continue
 ")"
-   fi
-   . "$author_script" ||
-   die "$(gettext "Error trying to find the author 
identity to amend commit")"
-   if test -f "$amend"
-   then
-   current_head=$(git rev-parse --verify HEAD)
-   test "$current_head" = $(cat "$amend") ||
-   die "$(gettext "\
+   fi
+   . "$author_script" ||
+   die "$(gettext "Error trying to find the author 
identity to amend commit")"
+   if test -f "$amend"
+   then
+   current_head=$(git rev-parse --verify HEAD)
+   test "$current_head" = $(cat "$amend") ||
+   die "$(gettext "\
 You have uncommitted changes in your working tree. Please commit them
 first and then run 'git rebase --continue' again.")"
-   do_with_author git commit --amend --no-verify -F "$msg" 
-e \
-   ${gpg_sign_opt:+"$gpg_sign_opt"} 
$allow_empty_message ||
-   die "$(gettext "Could not commit staged 
changes.")"
-   else
-   do_with_author git commit --no-verify -F "$msg" -e \
-   ${gpg_sign_opt:+"$gpg_sign_opt"} 
$allow_empty_message ||
-   die "$(gettext "Could not commit staged 
changes.")"
+   do_with_author git commit --amend --no-verify 
-F "$msg" -e \
+   ${gpg_sign_opt:+"$gpg_sign_opt"} 
$allow_empty_message ||
+   die "$(gettext "Could not commit staged 
changes.")"
+   else
+   do_with_author git commit --no-verify -F "$msg" 
-e \
+   ${gpg_sign_opt:+"$gpg_sign_opt"} 
$allow_empty_message ||
+   die "$(gettext "Could not commit staged 
changes.")"
+   fi
fi
-   fi
 
-   if test -r "$state_dir"/stopped-sha
-   then
-   record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
-   fi
+   if test -r "$state_dir"/stopped-sha
+   then
+   record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
+   fi
 
-   require_clean_work_tree "rebase"
-   do_rest
-   return 0
-   ;;
-skip)
-   git rerere clear
+   require_clean_work_tree 

[RFC PATCH v5 7/8] Remove unused code paths from git_rebase__interactive__preserve_merges

[Wink Saville]

Since git_rebase__interactive__preserve_merges is now always called with
$preserve_merges = t we can remove the unused code paths.

Signed-off-by: Wink Saville 
---
 git-rebase--interactive.sh | 152 -
 1 file changed, 69 insertions(+), 83 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 346da0f67..ddbd126f2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -982,100 +982,86 @@ git_rebase__interactive__preserve_merges () {
setup_reflog_action
init_basic_state
 
-   if test t = "$preserve_merges"
+   if test -z "$rebase_root"
then
-   if test -z "$rebase_root"
-   then
-   mkdir "$rewritten" &&
-   for c in $(git merge-base --all $orig_head $upstream)
-   do
-   echo $onto > "$rewritten"/$c ||
-   die "$(gettext "Could not init 
rewritten commits")"
-   done
-   else
-   mkdir "$rewritten" &&
-   echo $onto > "$rewritten"/root ||
+   mkdir "$rewritten" &&
+   for c in $(git merge-base --all $orig_head $upstream)
+   do
+   echo $onto > "$rewritten"/$c ||
die "$(gettext "Could not init rewritten 
commits")"
-   fi
-   # No cherry-pick because our first pass is to determine
-   # parents to rewrite and skipping dropped commits would
-   # prematurely end our probe
-   merges_option=
+   done
else
-   merges_option="--no-merges --cherry-pick"
+   mkdir "$rewritten" &&
+   echo $onto > "$rewritten"/root ||
+   die "$(gettext "Could not init rewritten commits")"
fi
 
+   # No cherry-pick because our first pass is to determine
+   # parents to rewrite and skipping dropped commits would
+   # prematurely end our probe
+   merges_option=
+
init_revisions_and_shortrevisions
 
-   if test t != "$preserve_merges"
-   then
-   git rebase--helper --make-script ${keep_empty:+--keep-empty} \
-   $revisions ${restrict_revision+^$restrict_revision} 
>"$todo" ||
-   die "$(gettext "Could not generate todo list")"
-   else
-   format=$(git config --get rebase.instructionFormat)
-   # the 'rev-list .. | sed' requires %m to parse; the instruction 
requires %H to parse
-   git rev-list $merges_option --format="%m%H ${format:-%s}" \
-   --reverse --left-right --topo-order \
-   $revisions ${restrict_revision+^$restrict_revision} | \
-   sed -n "s/^>//p" |
-   while read -r sha1 rest
-   do
+   format=$(git config --get rebase.instructionFormat)
+   # the 'rev-list .. | sed' requires %m to parse; the instruction 
requires %H to parse
+   git rev-list $merges_option --format="%m%H ${format:-%s}" \
+   --reverse --left-right --topo-order \
+   $revisions ${restrict_revision+^$restrict_revision} | \
+   sed -n "s/^>//p" |
+   while read -r sha1 rest
+   do
 
-   if test -z "$keep_empty" && is_empty_commit $sha1 && ! 
is_merge_commit $sha1
-   then
-   comment_out="$comment_char "
-   else
-   comment_out=
-   fi
+   if test -z "$keep_empty" && is_empty_commit $sha1 && ! 
is_merge_commit $sha1
+   then
+   comment_out="$comment_char "
+   else
+   comment_out=
+   fi
 
-   if test -z "$rebase_root"
-   then
-   preserve=t
-   for p in $(git rev-list --parents -1 $sha1 | 
cut -d' ' -s -f2-)
-   do
-   if test -f "$rewritten"/$p
-   then
-   preserve=f
-   fi
-   done
-   else
-   preserve=f
-   fi
-   if test f = "$preserve"
-   then
-   touch "$rewritten"/$sha1
-   printf '%s\n' "${comment_out}pick $sha1 $rest" 
>>"$todo"
-   fi
-   done
-   fi
+   if test -z "$rebase_root"
+   then
+   preserve=t
+   for p 

Re:

[VIC J]

-- 
I need your partnership to transfer $17.5 million to you and you will
profit 40% of fund, details will be disclosed once i receive your
positive reply.

Re: [RFC PATCH v4] rebase: Update invocation of rebase dot-sourced scripts

[Eric Sunshine]

On Fri, Mar 23, 2018 at 5:01 PM, Junio C Hamano  wrote:
> Eric Sunshine  writes:
>> (despite the run-on sentence in the first paragraph and the apparent
>> incorrect explanation of top-level "return" misbehavior -- the in-code
>> comment says top-level "return" was essentially a no-op in broken
>> shells, whereas he said it exited the shell).
>
> My bad, almost entirely.  Sorry.

No apology necessary. That minor error aside, your proposed commit
message gave just the right amount of detail for a person (me) not at
all familiar with the topic to be able to understand it fully and
intuit the patch content before even reading the patch proper. That's
a good commit message.

Re: [PATCH v1 0/2] perf/aggregate: sort result by regression

[Junio C Hamano]

Christian Couder  writes:

> This small patch series makes it easy to spot big performance
> regressions, so that they can later be investigated.
>
> For example:
>
> $ ./aggregate.perl --sortbyregression --subsection "without libpcre" v2.14.3 
> v2.15.1 v2.16.2 p4220-log-grep-engines.sh 

Are we comfortable with the idea that other kinds of sorting, when
invented in the future, would have to say

$ ./aggregate.perl --sortbysomethingelse --subsection "without libpcre" \
A B C p4220-log-grep-engines.sh

or will we regret that and wish if we could write it as

$ ./aggregate.perl --sort-by=somethingelse --subsection "without libpcre" \
A B C p4220-log-grep-engines.sh

If the latter, perhaps we should use --soft-by=regression from day one.

Do we expect that "taking a lot more more rtime than the previous"
will stay to be the only kind of "regression" we care about in the
context of t/perf?  If so, there is no need for further suggestion,
but if not, perhaps we should plan if/how we could also parameterize
the "rtime" part from the command line.  E.g.

$ ./aggregate.perl --sort-by=regression:stime

might be a way to say "we only care about the stime part" in the
future, even though --sort-by=regression may be a short-hand to say
"we care about rtime regression" i.e. "--sort-by=regression:rtime".

Re: [RFC PATCH v4] rebase: Update invocation of rebase dot-sourced scripts

[Wink Saville]

On Fri, Mar 23, 2018 at 1:51 PM, Junio C Hamano  wrote:
> Wink Saville  writes:
>
>> Here is one possibility:
>>
>> git format-patch --cover-letter --rfc --thread -v 5
>> --to=git@vger.kernel.org --cc=sunsh...@sunshineco.com
>> --cc=johannes.schinde...@gmx.de -o patches/v5 master..v5-2
>
> Sounds sensible.
>
>> If this was the first version then the above would seem to be a
>> reasonable choice.
>
> My personal preference (both as a reviewer and an occasional
> multi-patch series submitter) is to use a cover letter for a larger
> series (e.g. more than 3-5 patches), regardless of the iteration.
> In fact, a submitter tends to have _more_ things to say in the cover
> letter for v2 and subsequent iteration than the original iteration.
>
> The motivation behind the series may not change so greatly but will
> be refined as iterations go on, and you want help those who missed
> the earlier iteration understand what you are doing with the updated
> cover letter.  Also cover letter is the ideal place to outline where
> to find older iterations and their discussion and summarize what
> changed since these earlier attempts in this round.
>
>> But this is version 5 and maybe I don't need --cover-letter which, I
>> think means I
>> don't want to use --thread. If that's the case should I add --in-reply-to? 
>> But
>> that leads to the question. from which message should I get the Message-Id?
>
> The most typical practice I've seen around here is that v5's cover
> is made in-reply-to v4's cover.
>

Make sense

Re: [RFC PATCH v4] rebase: Update invocation of rebase dot-sourced scripts

[Junio C Hamano]

Eric Sunshine  writes:

>> When it was discovered that some shell implementations
> ...
> ECANTPARSE: This paragraph is grammatically corrupt.
>
> ECANTPARSE: Grammatically corrupt.
> ...
> (despite the run-on sentence in the first paragraph and the apparent
> incorrect explanation of top-level "return" misbehavior -- the in-code
> comment says top-level "return" was essentially a no-op in broken
> shells, whereas he said it exited the shell).

My bad, almost entirely.  Sorry.

Re: [RFC PATCH v4] rebase: Update invocation of rebase dot-sourced scripts

[Junio C Hamano]

Wink Saville  writes:

> Here is one possibility:
>
> git format-patch --cover-letter --rfc --thread -v 5
> --to=git@vger.kernel.org --cc=sunsh...@sunshineco.com
> --cc=johannes.schinde...@gmx.de -o patches/v5 master..v5-2

Sounds sensible.

> If this was the first version then the above would seem to be a
> reasonable choice.

My personal preference (both as a reviewer and an occasional
multi-patch series submitter) is to use a cover letter for a larger
series (e.g. more than 3-5 patches), regardless of the iteration.
In fact, a submitter tends to have _more_ things to say in the cover
letter for v2 and subsequent iteration than the original iteration.

The motivation behind the series may not change so greatly but will
be refined as iterations go on, and you want help those who missed
the earlier iteration understand what you are doing with the updated
cover letter.  Also cover letter is the ideal place to outline where
to find older iterations and their discussion and summarize what
changed since these earlier attempts in this round.

> But this is version 5 and maybe I don't need --cover-letter which, I
> think means I
> don't want to use --thread. If that's the case should I add --in-reply-to? But
> that leads to the question. from which message should I get the Message-Id?

The most typical practice I've seen around here is that v5's cover
is made in-reply-to v4's cover.

Re: [PATCH v2 5/5] Expand implementation of mem-pool type

[Junio C Hamano]

Jameson Miller  writes:

> +void mem_pool_discard(struct mem_pool *mem_pool)
> +{
> + struct mp_block *block, *block_to_free;
> + for (block = mem_pool->mp_block; block;)
> + {
> + block_to_free = block;
> + block = block->next_block;
> + free(block_to_free);
> + }
> +
> + free(mem_pool);
> +}

This can be used as a (half-)valid justification to the unadvertised
behaviour change made in [2/5] where a large allocation is still
given its own block and connected to a pool (as opposed to bypassing
the pool subsystem altogether).  If an instance of pool does not
keep track of all the pieces of memory mem_pool_alloc() hands out,
mem_pool_discard() cannot be used to reclaim all of the resources.

It however still does not justify how pool_alloc_block_with_size()
appends the new block at the end of the "next" list, which is
scanned for unused spaces when a new request comes in.  "We need to
keep track of them so that disard() can release" would justify a
pointer in the pool struct that chains together allocation blocks,
each of which hosts the memory for a single large allocation request
without extra room to carve out memory for subsequent small
requests.  That pointer does not have to be the usual "mp_block"
pointer.

> diff --git a/mem-pool.h b/mem-pool.h
> index 829ad58ecf..d9e7f21541 100644
> --- a/mem-pool.h
> +++ b/mem-pool.h
> @@ -21,6 +21,17 @@ struct mem_pool {
>   size_t pool_alloc;
>  };
>  
> +/*
> + * Initialize mem_pool with specified parameters for initial size and
> + * how much to grow when a larger memory block is required.
> + */
> +void mem_pool_init(struct mem_pool **mem_pool, size_t alloc_growth_size, 
> size_t initial_size);
> +
> +/*
> + * Discard a memory pool and free all the memory it is responsible for.
> + */
> +void mem_pool_discard(struct mem_pool *mem_pool);
> +
>  /*
>   * Alloc memory from the mem_pool.
>   */
> @@ -31,4 +42,17 @@ void *mem_pool_alloc(struct mem_pool *pool, size_t len);
>   */
>  void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);
>  
> +/*
> + * Move the memory associated with the 'src' pool to the 'dst' pool. The 
> 'src'
> + * pool will be empty and not contain any memory. It still needs to be free'd
> + * with a call to `mem_pool_discard`.
> + */
> +void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src);
> +
> +/*
> + * Check if a memory pointed at by 'mem' is part of the range of
> + * memory managed by the specified mem_pool.
> + */
> +int mem_pool_contains(struct mem_pool *mem_pool, void *mem);
> +
>  #endif

When adding a new "API", we prefer to see its usage demonstrated
with its first user, either in the same patch or in later steps in
the same series.  That would make it easier to evaluate its worth
fairly.

Thanks.

Re: [PATCH v2] json_writer: new routines to create data in JSON format

[Jeff Hostetler]

On 3/23/2018 4:11 PM, René Scharfe wrote:

Am 23.03.2018 um 20:55 schrieb Jeff Hostetler:

+struct json_writer_level
+{
+    unsigned level_is_array : 1;
+    unsigned level_is_empty : 1;
+};
+
+struct json_writer
+{
+    struct json_writer_level *levels;
+    int nr, alloc;
+    struct strbuf json;
+};


A simpler and probably more compact representation of is_array would
be a strbuf with one char per level, e.g. '[' for an array and '{'
for an object (or ']' and '}').

I don't understand the need to track emptiness per level.  Only the
top level array/object can ever be empty, can it?


My expectation was that any sub-object or sub-array could be empty.
That is, this should be valid (and the JSON parser in Python allows):

      {"a":{}, "b":[], "c":[[]], "d":[{}]}


Sure, but the emptiness of finished arrays and objects doesn't matter
for the purposes of error checking, comma setting or closing.  At most
one of them is empty *and* unclosed while writing the overall JSON
object -- the last one opened:

{
{"a":{
{"a":{}, "b":[
{"a":{}, "b":[], "c":[
{"a":{}, "b":[], "c":[[
{"a":{}, "b":[], "c":[[]], "d":[
{"a":{}, "b":[], "c":[[]], "d":[{

Any of the earlier written arrays/objects are either closed or contain
at least a half-done sub-array/object, which makes them non-empty.

René



good point.  i'll revisit.  thanks.
Jeff

Re: [PATCH v2 4/5] Move the reusable parts of memory pool into its own file

[Junio C Hamano]

Jameson Miller  writes:

> This moves the reusable parts of the memory pool logic used by
> fast-import.c into its own file for use by other components.
>
> Signed-off-by: Jameson Miller 
> ---
>  Makefile  |   1 +
>  fast-import.c | 118 
> +-
>  mem-pool.c| 103 ++
>  mem-pool.h|  34 +
>  4 files changed, 139 insertions(+), 117 deletions(-)
>  create mode 100644 mem-pool.c
>  create mode 100644 mem-pool.h

This step looks totally uncontroversial, provided that the result
after [1-3/5] is in a reasonable shape ;-)

Hint for other reviewers.  This

$ git blame -s -b -C HEAD^..HEAD mem-pool.c

can be used to see that most of the lines in this new file are
imported verbatim from fast-import.c at the receiving end.

Re: [PATCH v2] filter-branch: fix errors caused by refs that cannot be used with ^0

[Junio C Hamano]

Yuki Kokubun  writes:

> "git filter-branch -- --all" print unwanted error messages when refs that
> cannot be used with ^0 exist.

It is not incorrect per-se, but if I were writing this, I'd say
"... when refs that point at objects that are not committish" or
something like that, as that is much closer to people (both end
users and Git developers) than the "^0" implementation detail.

> Such refs can be created by "git replace" with
> trees or blobs. Also, "git tag" with trees or blobs can create such refs.

Taking two paragraphs above together, you wrote an excellent
description of the problem to be solved (i.e. what would be seen by
users and under what condition the symptom would trigger).  If there
is a single obvious solution that would trivially follow from the
problem description, it is perfectly fine to stop here.  Otherwise,
it would help to describe how it is solved next.  Something as brief
like

Filter these problematic refs out early, and warn that these
refs are left unwritten while doing so.

would suffice in this case, I think.  We _could_ insert

before they are seen by the logic to see which refs have
been modified and which have been left intact (which is
where the unwanted error messages come from),

between "early," and "and warn", if we wanted to.

> ---
>  git-filter-branch.sh | 14 --
>  t/t7003-filter-branch.sh | 14 ++
>  2 files changed, 26 insertions(+), 2 deletions(-)

The diff looks good.  

Please sign-off your patch (cf. Documentation/SubmittingPatches).

Thanks.

Re: [PATCH 00/12] sb/packfiles-in-repository updates

[Brandon Williams]

On 03/23, Nguyễn Thái Ngọc Duy wrote:
> This is the rebased version on the updated sb/object-store I just sent
> out plus the fix for get_object_directory(). The interdiff (after
> rebased) looks small and nice

Nice! Thanks for fixing that.  This series looks good to me :)

> 
> diff --git a/packfile.c b/packfile.c
> index e02136bebb..63c89ee31a 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -890,7 +890,7 @@ static void prepare_packed_git(struct repository *r)
>  
>   if (r->objects->packed_git_initialized)
>   return;
> - prepare_packed_git_one(r, get_object_directory(), 1);
> + prepare_packed_git_one(r, r->objects->objectdir, 1);
>   prepare_alt_odb(r);
>   for (alt = r->objects->alt_odb_list; alt; alt = alt->next)
>   prepare_packed_git_one(r, alt->path, 0);
> 
> I notice there's still one get_object_directory() left in packfile.c
> but that should not cause problems with converted functions. That
> could be done in "phase 2".
> 
> Nguyễn Thái Ngọc Duy (1):
>   packfile: keep prepare_packed_git() private
> 
> Stefan Beller (11):
>   packfile: allow prepare_packed_git_mru to handle arbitrary
> repositories
>   packfile: allow rearrange_packed_git to handle arbitrary repositories
>   packfile: allow install_packed_git to handle arbitrary repositories
>   packfile: add repository argument to prepare_packed_git_one
>   packfile: add repository argument to prepare_packed_git
>   packfile: add repository argument to reprepare_packed_git
>   packfile: allow prepare_packed_git_one to handle arbitrary
> repositories
>   packfile: allow prepare_packed_git to handle arbitrary repositories
>   packfile: allow reprepare_packed_git to handle arbitrary repositories
>   packfile: add repository argument to find_pack_entry
>   packfile: allow find_pack_entry to handle arbitrary repositories
> 
>  builtin/count-objects.c  |  3 +-
>  builtin/fsck.c   |  2 --
>  builtin/gc.c |  3 +-
>  builtin/pack-objects.c   |  1 -
>  builtin/pack-redundant.c |  2 --
>  builtin/receive-pack.c   |  3 +-
>  bulk-checkin.c   |  3 +-
>  fast-import.c|  3 +-
>  fetch-pack.c |  3 +-
>  http-backend.c   |  1 -
>  http.c   |  2 +-
>  pack-bitmap.c|  1 -
>  packfile.c   | 76 +++-
>  packfile.h   | 11 +++---
>  server-info.c|  1 -
>  sha1_file.c  |  8 ++---
>  sha1_name.c  |  2 --
>  17 files changed, 58 insertions(+), 67 deletions(-)
> 
> -- 
> 2.17.0.rc0.348.gd5a49e0b6f
> 

-- 
Brandon Williams

Re: [PATCH v2] json_writer: new routines to create data in JSON format

[René Scharfe]

Am 23.03.2018 um 20:55 schrieb Jeff Hostetler:
>>> +struct json_writer_level
>>> +{
>>> +    unsigned level_is_array : 1;
>>> +    unsigned level_is_empty : 1;
>>> +};
>>> +
>>> +struct json_writer
>>> +{
>>> +    struct json_writer_level *levels;
>>> +    int nr, alloc;
>>> +    struct strbuf json;
>>> +};
>>
>> A simpler and probably more compact representation of is_array would
>> be a strbuf with one char per level, e.g. '[' for an array and '{'
>> for an object (or ']' and '}').
>>
>> I don't understand the need to track emptiness per level.  Only the
>> top level array/object can ever be empty, can it?
> 
> My expectation was that any sub-object or sub-array could be empty.
> That is, this should be valid (and the JSON parser in Python allows):
> 
>      {"a":{}, "b":[], "c":[[]], "d":[{}]}

Sure, but the emptiness of finished arrays and objects doesn't matter
for the purposes of error checking, comma setting or closing.  At most
one of them is empty *and* unclosed while writing the overall JSON
object -- the last one opened:

{
{"a":{
{"a":{}, "b":[
{"a":{}, "b":[], "c":[
{"a":{}, "b":[], "c":[[
{"a":{}, "b":[], "c":[[]], "d":[
{"a":{}, "b":[], "c":[[]], "d":[{

Any of the earlier written arrays/objects are either closed or contain
at least a half-done sub-array/object, which makes them non-empty.

René

Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive

[Wink Saville]

On Fri, Mar 23, 2018 at 11:24 AM, Junio C Hamano  wrote:
> Johannes Schindelin  writes:
>
>> If you fold this into the previous patch, I am sure that after your 3/9
>> indenting the function properly, the splitting into functions will look
>> more or less like this:
>>
>> -git_rebase__interactive () {
>> +initiate_action () {
>> + action="$1"
>>
>>   [... unchanged code ...]
>> +}
>> +
>> + () {
>> + [... setting up variables ...]
>>
>>   [.. unchanged code ...]
>> +}
>> [... more of the same ...]
>> +
>> +git_rebase__interactive () {
>> + initiate_action "$action" &&
>> +   &&
>> + ...
>> +}
>>
>> In other words, it would be easier to review and to verify that the
>> previous code is left unchanged (although that would have to be verified
>> manually by applying 3/9 and then looking at the diff with the -w option,
>> anyway).
>
> We are on the same page on this.  A series structured to have a step
> that looks exactly like the above would be very pleasant to review.
>
> Thanks for a great suggestion.

SG and I'll rebase on top of my v5 for the first two commits which
I rebased on master.

Re: [PATCH v2] json_writer: new routines to create data in JSON format

[Jeff Hostetler]

On 3/23/2018 2:01 PM, René Scharfe wrote:

Am 21.03.2018 um 20:28 schrieb g...@jeffhostetler.com:

From: Jeff Hostetler 

Add basic routines to generate data in JSON format.

Signed-off-by: Jeff Hostetler 
---
   Makefile|   2 +
   json-writer.c   | 321 +
   json-writer.h   |  86 +
   t/helper/test-json-writer.c | 420 

   t/t0019-json-writer.sh  | 102 +++
   5 files changed, 931 insertions(+)
   create mode 100644 json-writer.c
   create mode 100644 json-writer.h
   create mode 100644 t/helper/test-json-writer.c
   create mode 100755 t/t0019-json-writer.sh

diff --git a/Makefile b/Makefile
index 1a9b23b..57f58e6 100644
--- a/Makefile
+++ b/Makefile
@@ -662,6 +662,7 @@ TEST_PROGRAMS_NEED_X += test-fake-ssh
   TEST_PROGRAMS_NEED_X += test-genrandom
   TEST_PROGRAMS_NEED_X += test-hashmap
   TEST_PROGRAMS_NEED_X += test-index-version
+TEST_PROGRAMS_NEED_X += test-json-writer
   TEST_PROGRAMS_NEED_X += test-lazy-init-name-hash
   TEST_PROGRAMS_NEED_X += test-line-buffer
   TEST_PROGRAMS_NEED_X += test-match-trees
@@ -815,6 +816,7 @@ LIB_OBJS += hashmap.o
   LIB_OBJS += help.o
   LIB_OBJS += hex.o
   LIB_OBJS += ident.o
+LIB_OBJS += json-writer.o
   LIB_OBJS += kwset.o
   LIB_OBJS += levenshtein.o
   LIB_OBJS += line-log.o
diff --git a/json-writer.c b/json-writer.c
new file mode 100644
index 000..89a6abb
--- /dev/null
+++ b/json-writer.c
@@ -0,0 +1,321 @@
+#include "cache.h"
+#include "json-writer.h"
+
+static char g_ch_open[2]  = { '{', '[' };
+static char g_ch_close[2] = { '}', ']' };
+
+/*
+ * Append JSON-quoted version of the given string to 'out'.
+ */
+static void append_quoted_string(struct strbuf *out, const char *in)
+{
+   strbuf_addch(out, '"');
+   for (/**/; *in; in++) {
+   unsigned char c = (unsigned char)*in;
+   if (c == '"')
+   strbuf_add(out, "\\\"", 2);
+   else if (c == '\\')
+   strbuf_add(out, "", 2);
+   else if (c == '\n')
+   strbuf_add(out, "\\n", 2);
+   else if (c == '\r')
+   strbuf_add(out, "\\r", 2);
+   else if (c == '\t')
+   strbuf_add(out, "\\t", 2);
+   else if (c == '\f')
+   strbuf_add(out, "\\f", 2);
+   else if (c == '\b')
+   strbuf_add(out, "\\b", 2);


Using strbuf_addstr() here would result in the same object code (its
strlen() call is inlined for constants) and avoid having to specify
the redundant length 2.


sure. thanks.



+   else if (c < 0x20)
+   strbuf_addf(out, "\\u%04x", c);
+   else
+   strbuf_addch(out, c);
+   }
+   strbuf_addch(out, '"');
+}
+

[...]

+
+void jw_object_double(struct json_writer *jw, const char *fmt,
+ const char *key, double value)
+{
+   assert_in_object(jw, key);
+   maybe_add_comma(jw);
+
+   if (!fmt || !*fmt)
+   fmt = "%f";
+
+   append_quoted_string(>json, key);
+   strbuf_addch(>json, ':');
+   strbuf_addf(>json, fmt, value);


Hmm.  Can compilers check such a caller-supplied format matches the
value's type?  (I don't know how to specify a format attribute for
GCC and Clang for this function.)


I'll look into this.  thanks.



+}

[...]

+
+void jw_object_sub(struct json_writer *jw, const char *key,
+  const struct json_writer *value)
+{
+   assert_is_terminated(value);
+
+   assert_in_object(jw, key);
+   maybe_add_comma(jw);
+
+   append_quoted_string(>json, key);
+   strbuf_addch(>json, ':');
+   strbuf_addstr(>json, value->json.buf);


strbuf_addbuf() would be a better fit here -- it avoids a strlen() call
and NUL handling issues.


sure. thanks. i always forget about _addbuf().



+}
+
+void jw_object_inline_begin_array(struct json_writer *jw, const char *key)
+{
+   assert_in_object(jw, key);
+   maybe_add_comma(jw);
+
+   append_quoted_string(>json, key);
+   strbuf_addch(>json, ':');
+
+   jw_array_begin(jw);
+}


Those duplicate calls in the last ten functions feel mind-numbing.  A
helper function for adding comma, key and colon might be a good idea.


I'll see if I can add another macro to do the dirty work here.

[...]


diff --git a/json-writer.h b/json-writer.h
new file mode 100644
index 000..ad38c95
--- /dev/null
+++ b/json-writer.h
@@ -0,0 +1,86 @@
+#ifndef JSON_WRITER_H
+#define JSON_WRITER_H
+
+/*
+ * JSON data structures are defined at:
+ *  http://json.org/
+ *  http://www.ietf.org/rfc/rfc7159.txt
+ *
+ * The JSON-writer API allows one to build JSON data structures using a
+ * simple wrapper around a "struct strbuf" buffer.  It is intended as a
+ * simple API to build output 

Re: [PATCH v3] routines to generate JSON data

[Jeff Hostetler]

On 3/23/2018 2:14 PM, Ramsay Jones wrote:



On 23/03/18 16:29, g...@jeffhostetler.com wrote:

From: Jeff Hostetler 

This is version 3 of my JSON data format routines.


I have not looked at v3 yet - the patch below is against
the version in 'pu' @3284f940c (presumably that would be v2).



I built my v3 on Linux and didn't see any warnings.  I'll add
your extra CFLAGS and fix anything that I find and send up a v4
shortly.

Thanks.
Jeff


The version in 'pu' broke my build on Linux, but not on
cygwin - which was a bit of a surprise. That is, until I
saw the warnings and remembered that I have this in my
config.mak on Linux, but not on cygwin:

   ifneq ($(CC),clang)
   CFLAGS += -Wold-style-declaration
   CFLAGS += -Wno-pointer-to-int-cast
   CFLAGS += -Wsystem-headers
   endif

... and the warnings were:

   $ diff nout pout
   1c1
   < GIT_VERSION = 2.17.0.rc1.317.g4a561d2cc
   ---
   > GIT_VERSION = 2.17.0.rc1.445.g3284f940c
   29a30
   > CC commit-graph.o
   73a75,87
   > CC json-writer.o
   > json-writer.c:53:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
   >  static void inline assert_in_object(const struct json_writer *jw, const 
char *key)
   >  ^
   > json-writer.c:64:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
   >  static void inline assert_in_array(const struct json_writer *jw)
   >  ^
   > json-writer.c:75:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
   >  static void inline maybe_add_comma(struct json_writer *jw)
   >  ^
   > json-writer.c:87:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
   >  static void inline assert_is_terminated(const struct json_writer *jw)
   >  ^
   83a98
   > CC ls-refs.o

   ...
   $

The '-Wold-style-declaration' gcc warning flag is not a standard
project flag, and I can't quite remember why I have it set, so I
guess you could just ignore it. However, all other 'static inline'
functions in the project have the inline keyword before the return
type, so ... ;-)

Also, sparse spewed 40 warnings for t/helper/test-json-writer.c,
which were mainly about file-local symbols, but had a couple of
'constant is so large ...', like so:

   $ grep warning psp-out | head -8
   t/helper/test-json-writer.c:45:46: warning: constant 0x is 
so big it is unsigned long
   t/helper/test-json-writer.c:108:40: warning: constant 0x is 
so big it is unsigned long
   t/helper/test-json-writer.c:4:12: warning: symbol 'expect_obj1' was not 
declared. Should it be static?
   t/helper/test-json-writer.c:5:12: warning: symbol 'expect_obj2' was not 
declared. Should it be static?
   t/helper/test-json-writer.c:6:12: warning: symbol 'expect_obj3' was not 
declared. Should it be static?
   t/helper/test-json-writer.c:7:12: warning: symbol 'expect_obj4' was not 
declared. Should it be static?
   t/helper/test-json-writer.c:8:12: warning: symbol 'expect_obj5' was not 
declared. Should it be static?
   t/helper/test-json-writer.c:10:20: warning: symbol 'obj1' was not declared. 
Should it be static?
   $

I decided to use the UINT64_C(v) macro from , which is
a C99 feature, and will (hopefully) not be a problem.

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] json-writer: fix up gcc and sparse warnings

Signed-off-by: Ramsay Jones 
---
  json-writer.c   |  8 ++---
  t/helper/test-json-writer.c | 80 ++---
  2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/json-writer.c b/json-writer.c
index 89a6abb57..ba0365d20 100644
--- a/json-writer.c
+++ b/json-writer.c
@@ -50,7 +50,7 @@ static inline void begin(struct json_writer *jw, int is_array)
  /*
   * Assert that we have an open object at this level.
   */
-static void inline assert_in_object(const struct json_writer *jw, const char 
*key)
+static inline void assert_in_object(const struct json_writer *jw, const char 
*key)
  {
if (!jw->nr)
BUG("object: missing jw_object_begin(): '%s'", key);
@@ -61,7 +61,7 @@ static void inline assert_in_object(const struct json_writer 
*jw, const char *ke
  /*
   * Assert that we have an open array at this level.
   */
-static void inline assert_in_array(const struct json_writer *jw)
+static inline void assert_in_array(const struct json_writer *jw)
  {
if (!jw->nr)
BUG("array: missing jw_begin()");
@@ -72,7 +72,7 @@ static void inline assert_in_array(const struct json_writer 
*jw)
  /*
   * Add comma if we have already seen a member at this level.
   */
-static void inline maybe_add_comma(struct json_writer *jw)
+static inline void maybe_add_comma(struct json_writer *jw)
  {
if (jw->levels[jw->nr - 1].level_is_empty)
jw->levels[jw->nr - 1].level_is_empty = 0;
@@ -84,7 +84,7 @@ static void inline maybe_add_comma(struct json_writer *jw)
   * 

[PATCH v2] Allow use of TLS 1.3

[Loganaden Velvindron]

Add a tlsv1.3 option to http.sslVersion in addition to the existing 
tlsv1.[012] options. libcurl has supported this since 7.52.0.

Done during IETF 101 Hackathon

Signed-off-by: Loganaden Velvindron 
---
 Documentation/config.txt | 2 +-
 http.c   | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ce9102cea..b18cb9104 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1957,7 +1957,7 @@ http.sslVersion::
- tlsv1.0
- tlsv1.1
- tlsv1.2
-
+   - tlsv1.3
 +
 Can be overridden by the `GIT_SSL_VERSION` environment variable.
 To force git to use libcurl's default ssl version and ignore any
diff --git a/http.c b/http.c
index a5bd5d62c..25eb84c11 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,9 @@ static struct {
{ "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
{ "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
 #endif
+#ifdef CURL_SSLVERSION_TLSv1_3
+   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
+#endif
 };
 #if LIBCURL_VERSION_NUM >= 0x070903
 static const char *ssl_key;
-- 
2.16.2

Re: user-manual: patch proposals and questions

[kalle]

thank you for your answer and hints.
kalle

Am 19.03.2018 um 01:27 schrieb Eric Sunshine:
> On Sun, Mar 18, 2018 at 7:49 PM, kalle  wrote:
>> 1.I wonder, why the "user-manual" is so hidden on the (official?) site
>> git-scm.com [it is accessible at git-scm.com/docs/user-manual ,but is
>> not viewable in /docs ]
> 
> The git-scm.com website is maintained as a distinct project[1] at
> Github; it is not directly related to the Git project itself. A good
> way to voice your concern about the website is either to open an issue
> ticket[2] or submit a pull request[3] if you have a specific change in
> mind.
> 
> [1]: https://github.com/git/git-scm.com
> [2]: https://github.com/git/git-scm.com/issues
> [3]: https://github.com/git/git-scm.com/pulls
> 
>> 2.I did not receive an answer to my mail. Maybe it could have to do with
>> a possible stopped maintainment of the 'user-manual'
> 
> More likely it was because your email was not composed in a way for
> people to digest and respond to it easily. There are some things you
> can do to help make it easier for people to respond:
> 
> * Send patches inline rather than attachments since it is much easier
> for people to respond to them directly when inline. When they are
> attachments, people are forced to open the attachment, then copy/paste
> parts of the patch back into the email for response, and most people
> won't bother with such effort. Also, make each patch a separate email
> with a meaningful commit message ("git format-patch" and "git
> send-email" can help with this).
> 
> * For your questions about documentation, quote the section of
> documentation you want to talk about directly in your email, then ask
> a question about it. This saves people the effort of manually having
> to open the file and locate the line or paragraph in question. Also,
> line numbers change as changes are made to files, so the line number
> you cite might not match the line number in a version of the file
> someone else is looking at.
> 
>> 3.it would be for non-graphics-users to have the Git-Pro-book in
>> text-format.
> 
> Like the website, the Pro Git book is a distinct project[4], not
> directly related to the Git project. To suggest making the book
> available as plain text, you can open an issue ticket[5] or submit a
> pull request[6] if you implement it yourself.
> 
> [4]: https://github.com/progit/progit2
> [5]: https://github.com/progit/progit2/issues
> [6]: https://github.com/progit/progit2/pulls
> 
> 
>>  Weitergeleitete Nachricht 
>> Betreff: user-manual: patch proposals and questions
>> Datum: Tue, 6 Mar 2018 00:08:55 +0100
>> Von: kalle 
>> An: git@vger.kernel.org
>>
>> The patches are attached.
>> Further some questions:
>>
>> -see the explanations of the branch command, ca. line 280: wouldn't it
>> be better to use other words than 'references'?
>> -sentence "it shows all commits reachable from the parent commit": it
>> seems wrong to me. The last commit is also shown.
>> - chapter "Browsing revisions": it seems counterintuitive to me to have
>> two different logics for the meaning of "branch1..branch2" and
>> "branch1...branch2", according to whether it's the argument of `git log'
>> or `git diff'
>> -section "Check whether two branches point at the same history": 'git
>> diff origin..master' -> shouldn't it be rather 'git diff
>> branch1..branch2'? … or rewrite the example with branch1=origin and
>> branch2=master.
>>
>> greetings,
>> kalle

Re: [PATCH v3] json_writer: new routines to create data in JSON format

[Jeff Hostetler]

On 3/23/2018 1:18 PM, Jonathan Nieder wrote:

g...@jeffhostetler.com wrote:


From: Jeff Hostetler 

Add basic routines to generate data in JSON format.

Signed-off-by: Jeff Hostetler 


If I understand the cover letter correctly, this is a JSON-like
format, not actual JSON.  Am I understanding correctly?

What are the requirements for consuming this output?  Will a typical
JSON library be able to handle it without trouble?  If not, is there
some subset of cases where a typical JSON library is able to handle it
without trouble?


I'll add text to the commit message and the .h file explaining
the Unicode limitation in the next reroll.



Can you say a little about the motivation here?  (I know you already
put some of that in the cover letter, but since that doesn't become
part of permanent history, it doesn't help the audience that matters.)


I'll add a note about that to the commit message.
Thanks.



This would also be easier to review if there were an application of it
in the same series.  It's fine to send an RFC like this without such
an application, but I think we should hold off on merging it until we
have one.  Having an application makes review much easier since we can
see how the API works in practice, how well the approach fits what
users need, etc.


That's fine.  I'm planning to push up another patch series next week
that builds upon this, so hopefully that will help.



Thanks and hope that helps,
Jonathan


Thanks,
Jeff

Re: [RFC PATCH v4] rebase: Update invocation of rebase dot-sourced scripts

[Wink Saville]

On Fri, Mar 23, 2018 at 10:12 AM, Johannes Schindelin
 wrote:
> Hi Wink,
>
> On Thu, 22 Mar 2018, Wink Saville wrote:
>
>> The backend scriptlets for "git rebase" were structured in a
>> bit unusual way for historical reasons.  Originally, it was
>> designed in such a way that dot-sourcing them from "git
>> rebase" would be sufficient to invoke the specific backend.
>>
>> When it was discovered that some shell implementations
>> (e.g. FreeBSD 9.x) misbehaved when exiting with a "return"
>> is executed at the top level of a dot-sourced script (the
>> original was expecting that the control returns to the next
>> command in "git rebase" after dot-sourcing the scriptlet).
>>
>> To fix this issue the whole body of git-rebase--$backend.sh
>> was made into a shell function git_rebase__$backend and then
>> the last statement of the scriptlet would invoke the function.
>>
>> Here the call is moved to "git rebase" side, instead of at the
>> end of each scriptlet.  This give us a more normal arrangement
>> where the scriptlet function library and allows multiple functions
>> to be implemented in a scriptlet.
>>
>> Signed-off-by: Wink Saville 
>> Reviewed-by: Junio C Hamano 
>> Reviewed-by: Eric Sunsine 
>> ---
>>  git-rebase--am.sh  | 11 ---
>>  git-rebase--interactive.sh | 11 ---
>>  git-rebase--merge.sh   | 11 ---
>>  git-rebase.sh  |  2 ++
>
> The patch makes sense to me.
>
> Thanks,
> Johannes

Junio, Eric and Johannes, thanks for the help!!!

I've created v5 with the two patches, what is the suggested
format-patch/send-email command(s)?

Here is one possibility:

git format-patch --cover-letter --rfc --thread -v 5
--to=git@vger.kernel.org --cc=sunsh...@sunshineco.com
--cc=johannes.schinde...@gmx.de -o patches/v5 master..v5-2

If this was the first version then the above would seem to be a
reasonable choice.
But this is version 5 and maybe I don't need --cover-letter which, I
think means I
don't want to use --thread. If that's the case should I add --in-reply-to? But
that leads to the question. from which message should I get the Message-Id?

More likely I'm totally wrong and should do something completely different,
advice appreciated.

-- Wink

Re: [PATCH 00/27] sb/object-store updates

[Duy Nguyen]

On Fri, Mar 23, 2018 at 7:35 PM, Brandon Williams  wrote:
> On 03/23, Nguyễn Thái Ngọc Duy wrote:
>> I think I have addressed all comments I've received so far. What I
>> decided not to do, I have responded individually. One comment I did
>> not respond nor do is the approximate thing, which could be done
>> later.
>>
>> Interdiff is big due to the "objects." to "objects->" conversion
>> (blame Brandon for his suggestion, don't blame me :D)
>
> Haha, I'm guessing you prefer having a pointer too then? :P

That goes without saying or I would be arguing against that instead of
spending time fixing a bunch of compile errors ;-)
-- 
Duy

Re: [PATCH 00/27] sb/object-store updates

[Brandon Williams]

On 03/23, Brandon Williams wrote:
> On 03/23, Nguyễn Thái Ngọc Duy wrote:
> > I think I have addressed all comments I've received so far. What I
> > decided not to do, I have responded individually. One comment I did
> > not respond nor do is the approximate thing, which could be done
> > later.
> > 
> > Interdiff is big due to the "objects." to "objects->" conversion
> > (blame Brandon for his suggestion, don't blame me :D)
> 
> Haha, I'm guessing you prefer having a pointer too then? :P 
> 
> The interdiff looks good, though I'll set some time aside today to go
> through the series as a whole again.

Had some more time than I thought; this series looks good!  Thanks for
take the time to keep this rolling.  Hopefully we can see this merged
soon :)

-- 
Brandon Williams

Re: [PATCH 04/27] object-store: free alt_odb_list

[Brandon Williams]

On 03/23, Nguyễn Thái Ngọc Duy wrote:
> From: Stefan Beller 
> 
> Free the memory and reset alt_odb_{list, tail} to NULL.
> 
> Signed-off-by: Stefan Beller 
> Signed-off-by: Nguyễn Thái Ngọc Duy 

Thanks for fixing the memory leak.  Looks good now.

> ---
>  object.c | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/object.c b/object.c
> index 6ddd61242c..581347b535 100644
> --- a/object.c
> +++ b/object.c
> @@ -454,8 +454,30 @@ struct raw_object_store *raw_object_store_new(void)
>   memset(o, 0, sizeof(*o));
>   return o;
>  }
> +
> +static void free_alt_odb(struct alternate_object_database *alt)
> +{
> + strbuf_release(>scratch);
> + oid_array_clear(>loose_objects_cache);
> + free(alt);
> +}
> +
> +static void free_alt_odbs(struct raw_object_store *o)
> +{
> + while (o->alt_odb_list) {
> + struct alternate_object_database *next;
> +
> + next = o->alt_odb_list->next;
> + free_alt_odb(o->alt_odb_list);
> + o->alt_odb_list = next;
> + }
> +}
> +
>  void raw_object_store_clear(struct raw_object_store *o)
>  {
>   FREE_AND_NULL(o->objectdir);
>   FREE_AND_NULL(o->alternate_db);
> +
> + free_alt_odbs(o);
> + o->alt_odb_tail = NULL;
>  }
> -- 
> 2.17.0.rc0.348.gd5a49e0b6f
> 

-- 
Brandon Williams

Re: [PATCH] Allow use of TLS 1.3

[Loganaden Velvindron]

On Fri, Mar 23, 2018 at 07:37:08PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Mar 23 2018, Loganaden Velvindron wrote:
> 
> > Done during IETF 101 hackathon
> 
> Hi. Thanks. Let's add a meaningful commit message to this though,
> something like:
> 
> Add a tlsv1.3 option to http.sslVersion in addition to the existing
> tlsv1.[012] options. libcurl has supported this since 7.52.0.

Looks good to me.

> 
> > --- a/http.c
> > +++ b/http.c
> > @@ -61,6 +61,9 @@ static struct {
> > { "tlsv1.0", CURL_SSLVERSION_TLSv1_0 },
> > { "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
> > { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
> > +#if LIBCURL_VERSION_NUM >= 0x075200
> > +   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
> > +#endif
> 
> I wonder if this wouldn't be better as:
> 
> +#ifdef CURL_SSLVERSION_TLSv1_3
> + { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
> +#endif
> 
> We've been bitten before by doing version checks on libcurl code, only
> to find that some distros are actively backporting features, so checking
> the specific macros is usually better.

This looks good to me as well. I will send Patch v2, with the suggestions.

> 
> >  #endif
> >  };
> >  #if LIBCURL_VERSION_NUM >= 0x070903

Re: [PATCH] Allow use of TLS 1.3

[Ævar Arnfjörð Bjarmason]

On Fri, Mar 23 2018, Loganaden Velvindron wrote:

> Done during IETF 101 hackathon

Hi. Thanks. Let's add a meaningful commit message to this though,
something like:

Add a tlsv1.3 option to http.sslVersion in addition to the existing
tlsv1.[012] options. libcurl has supported this since 7.52.0.

> --- a/http.c
> +++ b/http.c
> @@ -61,6 +61,9 @@ static struct {
>   { "tlsv1.0", CURL_SSLVERSION_TLSv1_0 },
>   { "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
>   { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
> +#if LIBCURL_VERSION_NUM >= 0x075200
> + { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
> +#endif

I wonder if this wouldn't be better as:

+#ifdef CURL_SSLVERSION_TLSv1_3
+   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
+#endif

We've been bitten before by doing version checks on libcurl code, only
to find that some distros are actively backporting features, so checking
the specific macros is usually better.

>  #endif
>  };
>  #if LIBCURL_VERSION_NUM >= 0x070903

Re: [PATCH 00/27] sb/object-store updates

[Brandon Williams]

On 03/23, Nguyễn Thái Ngọc Duy wrote:
> I think I have addressed all comments I've received so far. What I
> decided not to do, I have responded individually. One comment I did
> not respond nor do is the approximate thing, which could be done
> later.
> 
> Interdiff is big due to the "objects." to "objects->" conversion
> (blame Brandon for his suggestion, don't blame me :D)

Haha, I'm guessing you prefer having a pointer too then? :P 

The interdiff looks good, though I'll set some time aside today to go
through the series as a whole again.

-- 
Brandon Williams

Re: [PATCH 10/44] object-store: move packed_git and packed_git_mru to object store

[Brandon Williams]

On 03/23, Duy Nguyen wrote:
> On Fri, Mar 23, 2018 at 6:03 PM, Duy Nguyen  wrote:
> > On Wed, Mar 21, 2018 at 11:18 PM, Brandon Williams  
> > wrote:
> >> You're marking packed_git
> >> as "private"...well C has no notion of private vs public fields in a
> >> struct so it might be difficult to keep that convention, it also took me
> >> a second to realize that it was only in the scope of packfile.c where it
> >> was ok to reference it directly.  Maybe it'll be ok?  If we really
> >> wanted something to be private we'd need it to be an opaque type
> >> instead, which may be out of the scope of this code refactor.
> >
> > It's true C syntax does not support private/public scoping, but it
> > does not mean we must avoid that. Python has "private convention" with
> > the underscore prefix, no special support from the language either.
> > Yes having compiler support to enforce scoping is nice, but I think we
> > can still live without it (Go devs live fine without "const"
> > arguments, e.g.)
> >
> > So I'm going to make it clearer that these fields are not supposed to
> > be accessed outside packfile.c
> 
> I'm not counting out the making these fields completely opaque of
> course. And with your suggestion of not embedding raw_object_store to
> repository, that's actually possible to do. But I'm still not doing it
> now :) The series is getting long and extending its scope will drag it
> even longer (in terms of both time and number of patches)

Oh of course. I'm a big proponent of taking small steps, so definitely
avoid scope creep and hopefully this series can land quicker so we have
a better base to work with to make some of those other improvements if
we still want to down the road.

-- 
Brandon Williams

[PATCH] Allow use of TLS 1.3

[Loganaden Velvindron]

Done during IETF 101 hackathon

Signed-off-by: Loganaden Velvindron 
---
 Documentation/config.txt | 1 +
 http.c   | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ce9102cea..f31d62772 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1957,6 +1957,7 @@ http.sslVersion::
- tlsv1.0
- tlsv1.1
- tlsv1.2
+   - tlsv1.3
 
 +
 Can be overridden by the `GIT_SSL_VERSION` environment variable.
diff --git a/http.c b/http.c
index 8c11156ae..666fe31f3 100644
--- a/http.c
+++ b/http.c
@@ -61,6 +61,9 @@ static struct {
{ "tlsv1.0", CURL_SSLVERSION_TLSv1_0 },
{ "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
{ "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
+#if LIBCURL_VERSION_NUM >= 0x075200
+   { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
+#endif
 #endif
 };
 #if LIBCURL_VERSION_NUM >= 0x070903
-- 
2.16.2

Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive

[Junio C Hamano]

Johannes Schindelin  writes:

> If you fold this into the previous patch, I am sure that after your 3/9
> indenting the function properly, the splitting into functions will look
> more or less like this:
>
> -git_rebase__interactive () {
> +initiate_action () {
> + action="$1"
>  
>   [... unchanged code ...]
> +}
> +
> + () {
> + [... setting up variables ...]
>  
>   [.. unchanged code ...]
> +}
> [... more of the same ...]
> +
> +git_rebase__interactive () {
> + initiate_action "$action" &&
> +   &&
> + ...
> +}
>
> In other words, it would be easier to review and to verify that the
> previous code is left unchanged (although that would have to be verified
> manually by applying 3/9 and then looking at the diff with the -w option,
> anyway).

We are on the same page on this.  A series structured to have a step
that looks exactly like the above would be very pleasant to review.

Thanks for a great suggestion.

Re: [PATCH v3] routines to generate JSON data

[Ramsay Jones]

On 23/03/18 16:29, g...@jeffhostetler.com wrote:
> From: Jeff Hostetler 
> 
> This is version 3 of my JSON data format routines.

I have not looked at v3 yet - the patch below is against
the version in 'pu' @3284f940c (presumably that would be v2).

The version in 'pu' broke my build on Linux, but not on
cygwin - which was a bit of a surprise. That is, until I
saw the warnings and remembered that I have this in my
config.mak on Linux, but not on cygwin:

  ifneq ($(CC),clang)
  CFLAGS += -Wold-style-declaration
  CFLAGS += -Wno-pointer-to-int-cast
  CFLAGS += -Wsystem-headers
  endif

... and the warnings were:

  $ diff nout pout
  1c1
  < GIT_VERSION = 2.17.0.rc1.317.g4a561d2cc
  ---
  > GIT_VERSION = 2.17.0.rc1.445.g3284f940c
  29a30
  > CC commit-graph.o
  73a75,87
  > CC json-writer.o
  > json-writer.c:53:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
  >  static void inline assert_in_object(const struct json_writer *jw, const 
char *key)
  >  ^
  > json-writer.c:64:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
  >  static void inline assert_in_array(const struct json_writer *jw)
  >  ^
  > json-writer.c:75:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
  >  static void inline maybe_add_comma(struct json_writer *jw)
  >  ^
  > json-writer.c:87:1: warning: ‘inline’ is not at beginning of declaration 
[-Wold-style-declaration]
  >  static void inline assert_is_terminated(const struct json_writer *jw)
  >  ^
  83a98
  > CC ls-refs.o

  ...
  $ 

The '-Wold-style-declaration' gcc warning flag is not a standard
project flag, and I can't quite remember why I have it set, so I
guess you could just ignore it. However, all other 'static inline'
functions in the project have the inline keyword before the return
type, so ... ;-)

Also, sparse spewed 40 warnings for t/helper/test-json-writer.c,
which were mainly about file-local symbols, but had a couple of
'constant is so large ...', like so:

  $ grep warning psp-out | head -8
  t/helper/test-json-writer.c:45:46: warning: constant 0x is so 
big it is unsigned long
  t/helper/test-json-writer.c:108:40: warning: constant 0x is 
so big it is unsigned long
  t/helper/test-json-writer.c:4:12: warning: symbol 'expect_obj1' was not 
declared. Should it be static?
  t/helper/test-json-writer.c:5:12: warning: symbol 'expect_obj2' was not 
declared. Should it be static?
  t/helper/test-json-writer.c:6:12: warning: symbol 'expect_obj3' was not 
declared. Should it be static?
  t/helper/test-json-writer.c:7:12: warning: symbol 'expect_obj4' was not 
declared. Should it be static?
  t/helper/test-json-writer.c:8:12: warning: symbol 'expect_obj5' was not 
declared. Should it be static?
  t/helper/test-json-writer.c:10:20: warning: symbol 'obj1' was not declared. 
Should it be static?
  $ 

I decided to use the UINT64_C(v) macro from , which is
a C99 feature, and will (hopefully) not be a problem.

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] json-writer: fix up gcc and sparse warnings

Signed-off-by: Ramsay Jones 
---
 json-writer.c   |  8 ++---
 t/helper/test-json-writer.c | 80 ++---
 2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/json-writer.c b/json-writer.c
index 89a6abb57..ba0365d20 100644
--- a/json-writer.c
+++ b/json-writer.c
@@ -50,7 +50,7 @@ static inline void begin(struct json_writer *jw, int is_array)
 /*
  * Assert that we have an open object at this level.
  */
-static void inline assert_in_object(const struct json_writer *jw, const char 
*key)
+static inline void assert_in_object(const struct json_writer *jw, const char 
*key)
 {
if (!jw->nr)
BUG("object: missing jw_object_begin(): '%s'", key);
@@ -61,7 +61,7 @@ static void inline assert_in_object(const struct json_writer 
*jw, const char *ke
 /*
  * Assert that we have an open array at this level.
  */
-static void inline assert_in_array(const struct json_writer *jw)
+static inline void assert_in_array(const struct json_writer *jw)
 {
if (!jw->nr)
BUG("array: missing jw_begin()");
@@ -72,7 +72,7 @@ static void inline assert_in_array(const struct json_writer 
*jw)
 /*
  * Add comma if we have already seen a member at this level.
  */
-static void inline maybe_add_comma(struct json_writer *jw)
+static inline void maybe_add_comma(struct json_writer *jw)
 {
if (jw->levels[jw->nr - 1].level_is_empty)
jw->levels[jw->nr - 1].level_is_empty = 0;
@@ -84,7 +84,7 @@ static void inline maybe_add_comma(struct json_writer *jw)
  * Assert that the given JSON object or JSON array has been properly
  * terminated.  (Has closing bracket.)
  */
-static void inline assert_is_terminated(const struct json_writer *jw)
+static inline void assert_is_terminated(const struct json_writer *jw)

Re: [PATCH v2] json_writer: new routines to create data in JSON format

[René Scharfe]

Am 21.03.2018 um 20:28 schrieb g...@jeffhostetler.com:
> From: Jeff Hostetler 
> 
> Add basic routines to generate data in JSON format.
> 
> Signed-off-by: Jeff Hostetler 
> ---
>   Makefile|   2 +
>   json-writer.c   | 321 +
>   json-writer.h   |  86 +
>   t/helper/test-json-writer.c | 420 
> 
>   t/t0019-json-writer.sh  | 102 +++
>   5 files changed, 931 insertions(+)
>   create mode 100644 json-writer.c
>   create mode 100644 json-writer.h
>   create mode 100644 t/helper/test-json-writer.c
>   create mode 100755 t/t0019-json-writer.sh
> 
> diff --git a/Makefile b/Makefile
> index 1a9b23b..57f58e6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -662,6 +662,7 @@ TEST_PROGRAMS_NEED_X += test-fake-ssh
>   TEST_PROGRAMS_NEED_X += test-genrandom
>   TEST_PROGRAMS_NEED_X += test-hashmap
>   TEST_PROGRAMS_NEED_X += test-index-version
> +TEST_PROGRAMS_NEED_X += test-json-writer
>   TEST_PROGRAMS_NEED_X += test-lazy-init-name-hash
>   TEST_PROGRAMS_NEED_X += test-line-buffer
>   TEST_PROGRAMS_NEED_X += test-match-trees
> @@ -815,6 +816,7 @@ LIB_OBJS += hashmap.o
>   LIB_OBJS += help.o
>   LIB_OBJS += hex.o
>   LIB_OBJS += ident.o
> +LIB_OBJS += json-writer.o
>   LIB_OBJS += kwset.o
>   LIB_OBJS += levenshtein.o
>   LIB_OBJS += line-log.o
> diff --git a/json-writer.c b/json-writer.c
> new file mode 100644
> index 000..89a6abb
> --- /dev/null
> +++ b/json-writer.c
> @@ -0,0 +1,321 @@
> +#include "cache.h"
> +#include "json-writer.h"
> +
> +static char g_ch_open[2]  = { '{', '[' };
> +static char g_ch_close[2] = { '}', ']' };
> +
> +/*
> + * Append JSON-quoted version of the given string to 'out'.
> + */
> +static void append_quoted_string(struct strbuf *out, const char *in)
> +{
> + strbuf_addch(out, '"');
> + for (/**/; *in; in++) {
> + unsigned char c = (unsigned char)*in;
> + if (c == '"')
> + strbuf_add(out, "\\\"", 2);
> + else if (c == '\\')
> + strbuf_add(out, "", 2);
> + else if (c == '\n')
> + strbuf_add(out, "\\n", 2);
> + else if (c == '\r')
> + strbuf_add(out, "\\r", 2);
> + else if (c == '\t')
> + strbuf_add(out, "\\t", 2);
> + else if (c == '\f')
> + strbuf_add(out, "\\f", 2);
> + else if (c == '\b')
> + strbuf_add(out, "\\b", 2);

Using strbuf_addstr() here would result in the same object code (its
strlen() call is inlined for constants) and avoid having to specify
the redundant length 2.

> + else if (c < 0x20)
> + strbuf_addf(out, "\\u%04x", c);
> + else
> + strbuf_addch(out, c);
> + }
> + strbuf_addch(out, '"');
> +}
> +
> +
> +static inline void begin(struct json_writer *jw, int is_array)
> +{
> + ALLOC_GROW(jw->levels, jw->nr + 1, jw->alloc);
> +
> + jw->levels[jw->nr].level_is_array = !!is_array;
> + jw->levels[jw->nr].level_is_empty = 1;
> +
> + strbuf_addch(>json, g_ch_open[!!is_array]);
> +
> + jw->nr++;
> +}
> +
> +/*
> + * Assert that we have an open object at this level.
> + */
> +static void inline assert_in_object(const struct json_writer *jw, const char 
> *key)
> +{
> + if (!jw->nr)
> + BUG("object: missing jw_object_begin(): '%s'", key);
> + if (jw->levels[jw->nr - 1].level_is_array)
> + BUG("object: not in object: '%s'", key);
> +}
> +
> +/*
> + * Assert that we have an open array at this level.
> + */
> +static void inline assert_in_array(const struct json_writer *jw)
> +{
> + if (!jw->nr)
> + BUG("array: missing jw_begin()");
> + if (!jw->levels[jw->nr - 1].level_is_array)
> + BUG("array: not in array");
> +}
> +
> +/*
> + * Add comma if we have already seen a member at this level.
> + */
> +static void inline maybe_add_comma(struct json_writer *jw)
> +{
> + if (jw->levels[jw->nr - 1].level_is_empty)
> + jw->levels[jw->nr - 1].level_is_empty = 0;
> + else
> + strbuf_addch(>json, ',');
> +}
> +
> +/*
> + * Assert that the given JSON object or JSON array has been properly
> + * terminated.  (Has closing bracket.)
> + */
> +static void inline assert_is_terminated(const struct json_writer *jw)
> +{
> + if (jw->nr)
> + BUG("object: missing jw_end(): '%s'", jw->json.buf);
> +}
> +
> +void jw_object_begin(struct json_writer *jw)
> +{
> + begin(jw, 0);
> +}
> +
> +void jw_object_string(struct json_writer *jw, const char *key, const char 
> *value)
> +{
> + assert_in_object(jw, key);
> + maybe_add_comma(jw);
> +
> + append_quoted_string(>json, key);
> + strbuf_addch(>json, ':');
> + append_quoted_string(>json, value);
> +}
> +
> +void 

Re: [ANNOUNCE] Git v2.17.0-rc1

[Ævar Arnfjörð Bjarmason]

On Wed, Mar 21 2018, Junio C. Hamano wrote:

> A release candidate Git v2.17.0-rc1 is now available for testing
> at the usual places.  It is comprised of 493 non-merge commits
> since v2.16.0, contributed by 62 people, 19 of which are new faces.

I have this deployed on some tens of K machines who all use git in one
way or another (from automated pulls, to users interactively), and rc0
before that, with a few patches on top from me + Takato + Duy + Derrick
since rc0 was released (and since today based on top of rc1). No issues
so far.

The specific in-house version I have is at:
https://github.com/git/git/compare/v2.17.0-rc1...bookingcom:booking-git-v2018-03-23-1

>  * Some bugs around "untracked cache" feature have been fixed.  This
>will notice corrupt data in the untracked cache left by old and
>buggy code and issue a warning---the index can be fixed by clearing
>the untracked cache from it.
>(merge 0cacebf099 nd/fix-untracked-cache-invalidation later to maint).
>(merge 7bf0be7501 ab/untracked-cache-invalidation-docs later to maint).

FYI over some >10k machines I tested on around 1% had (very) verbose
warnings on "status", which was fixed as a one-off with 'git -c
core.untrackedCache=false status' as the
ab/untracked-cache-invalidation-docs suggest, and it hasn't happened
again since.

Re: [ANNOUNCE] Git v2.17.0-rc1

[Johannes Schindelin]

Hi team,

On Wed, 21 Mar 2018, Junio C Hamano wrote:

> A release candidate Git v2.17.0-rc1 is now available for testing
> at the usual places.  It is comprised of 493 non-merge commits
> since v2.16.0, contributed by 62 people, 19 of which are new faces.
> 
> The tarballs are found at:
> 
> https://www.kernel.org/pub/software/scm/git/testing/

And Git for Windows v2.17.0-rc1 can be found here:

https://github.com/git-for-windows/git/releases/tag/v2.17.0-rc1.windows.1

Please test so that we can hammer out a robust v2.17.0!

Ciao,
Johannes

[PATCH 06/12] packfile: add repository argument to reprepare_packed_git

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

From: Stefan Beller 

See previous patch for explanation.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 builtin/gc.c   | 2 +-
 builtin/receive-pack.c | 3 ++-
 bulk-checkin.c | 3 ++-
 fetch-pack.c   | 3 ++-
 packfile.c | 2 +-
 packfile.h | 3 ++-
 sha1_file.c| 2 +-
 7 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 4c7409946e..a78dad51aa 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -478,7 +478,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
return error(FAILED_RUN, rerere.argv[0]);
 
report_garbage = report_pack_garbage;
-   reprepare_packed_git();
+   reprepare_packed_git(the_repository);
if (pack_garbage.nr > 0)
clean_pack_garbage();
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1a298a6711..469b916707 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1,4 +1,5 @@
 #include "builtin.h"
+#include "repository.h"
 #include "config.h"
 #include "lockfile.h"
 #include "pack.h"
@@ -1777,7 +1778,7 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
status = finish_command();
if (status)
return "index-pack abnormal exit";
-   reprepare_packed_git();
+   reprepare_packed_git(the_repository);
}
return NULL;
 }
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 3310fd210a..eadc2d5172 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -3,6 +3,7 @@
  */
 #include "cache.h"
 #include "bulk-checkin.h"
+#include "repository.h"
 #include "csum-file.h"
 #include "pack.h"
 #include "strbuf.h"
@@ -57,7 +58,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state 
*state)
 
strbuf_release();
/* Make objects we just wrote available to ourselves */
-   reprepare_packed_git();
+   reprepare_packed_git(the_repository);
 }
 
 static int already_written(struct bulk_checkin_state *state, unsigned char 
sha1[])
diff --git a/fetch-pack.c b/fetch-pack.c
index 8253d746e0..eac5928a27 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "repository.h"
 #include "config.h"
 #include "lockfile.h"
 #include "refs.h"
@@ -1192,7 +1193,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
prepare_shallow_info(, shallow);
ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
, pack_lockfile);
-   reprepare_packed_git();
+   reprepare_packed_git(the_repository);
update_shallow(args, sought, nr_sought, );
clear_shallow_info();
return ref_cpy;
diff --git a/packfile.c b/packfile.c
index bb090f8a29..1b4296277a 100644
--- a/packfile.c
+++ b/packfile.c
@@ -899,7 +899,7 @@ void prepare_packed_git_the_repository(void)
the_repository->objects->packed_git_initialized = 1;
 }
 
-void reprepare_packed_git(void)
+void reprepare_packed_git_the_repository(void)
 {
the_repository->objects->approximate_object_count_valid = 0;
the_repository->objects->packed_git_initialized = 0;
diff --git a/packfile.h b/packfile.h
index 3f59456e7e..ab5046938c 100644
--- a/packfile.h
+++ b/packfile.h
@@ -36,7 +36,8 @@ extern void (*report_garbage)(unsigned seen_bits, const char 
*path);
 
 #define prepare_packed_git(r) prepare_packed_git_##r()
 extern void prepare_packed_git_the_repository(void);
-extern void reprepare_packed_git(void);
+#define reprepare_packed_git(r) reprepare_packed_git_##r()
+extern void reprepare_packed_git_the_repository(void);
 extern void install_packed_git(struct repository *r, struct packed_git *pack);
 
 struct packed_git *get_packed_git(struct repository *r);
diff --git a/sha1_file.c b/sha1_file.c
index 0989bbd948..9c024cd957 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1274,7 +1274,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
return 0;
 
/* Not a loose object; someone else may have just packed it. */
-   reprepare_packed_git();
+   reprepare_packed_git(the_repository);
if (find_pack_entry(real, ))
break;
 
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 05/12] packfile: add repository argument to prepare_packed_git

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

From: Stefan Beller 

Add a repository argument to allow prepare_packed_git callers to be
more specific about which repository to handle. See commit "sha1_file:
add repository argument to link_alt_odb_entry" for an explanation of
the #define trick.

Signed-off-by: Stefan Beller 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/count-objects.c  |  2 +-
 builtin/fsck.c   |  2 +-
 builtin/gc.c |  2 +-
 builtin/pack-objects.c   |  2 +-
 builtin/pack-redundant.c |  2 +-
 fast-import.c|  2 +-
 http-backend.c   |  2 +-
 pack-bitmap.c|  2 +-
 packfile.c   | 10 +-
 packfile.h   |  3 ++-
 server-info.c|  2 +-
 sha1_name.c  |  4 ++--
 12 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index b28ff00be2..ea8bd9e2e2 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -123,7 +123,7 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
if (!get_packed_git(the_repository))
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
for (p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 3ef25fab97..d40a82b702 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -729,7 +729,7 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
uint32_t total = 0, count = 0;
struct progress *progress = NULL;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
 
if (show_progress) {
for (p = get_packed_git(the_repository); p;
diff --git a/builtin/gc.c b/builtin/gc.c
index b00238cd5d..4c7409946e 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -174,7 +174,7 @@ static int too_many_packs(void)
if (gc_auto_pack_limit <= 0)
return 0;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
for (cnt = 0, p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 223f2d9fc0..491ce433da 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3152,7 +3152,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
if (progress && all_progress_implied)
progress = 2;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
if (ignore_packed_keep) {
struct packed_git *p;
for (p = get_packed_git(the_repository); p; p = p->next)
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index b5b007e706..da6637f7fc 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -631,7 +631,7 @@ int cmd_pack_redundant(int argc, const char **argv, const 
char *prefix)
break;
}
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
 
if (load_all_packs)
load_all();
diff --git a/fast-import.c b/fast-import.c
index ae4ced3ae1..47981e6db7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3473,7 +3473,7 @@ int cmd_main(int argc, const char **argv)
rc_free[i].next = _free[i + 1];
rc_free[cmd_save - 1].next = NULL;
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
start_packfile();
set_die_routine(die_nicely);
set_checkpoint_signal();
diff --git a/http-backend.c b/http-backend.c
index 64dde78c1b..c1b1c2d557 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -519,7 +519,7 @@ static void get_info_packs(struct strbuf *hdr, char *arg)
size_t cnt = 0;
 
select_getanyfile(hdr);
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
for (p = get_packed_git(the_repository); p; p = p->next) {
if (p->pack_local)
cnt++;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 22cd425788..57fec38f3f 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -336,7 +336,7 @@ static int open_pack_bitmap(void)
 
assert(!bitmap_git.map && !bitmap_git.loaded);
 
-   prepare_packed_git();
+   prepare_packed_git(the_repository);
for (p = get_packed_git(the_repository); p; p = p->next) {
if (open_pack_bitmap_1(p) == 0)
ret = 0;
diff --git a/packfile.c b/packfile.c
index 0e5fa67526..bb090f8a29 100644
--- a/packfile.c
+++ b/packfile.c
@@ -817,7 

[PATCH 02/12] packfile: allow rearrange_packed_git to handle arbitrary repositories

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

From: Stefan Beller 

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 packfile.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/packfile.c b/packfile.c
index 8c335bdcd1..bff22a8c81 100644
--- a/packfile.c
+++ b/packfile.c
@@ -866,10 +866,10 @@ static int sort_pack(const void *a_, const void *b_)
return -1;
 }
 
-static void rearrange_packed_git(void)
+static void rearrange_packed_git(struct repository *r)
 {
-   the_repository->objects->packed_git = llist_mergesort(
-   the_repository->objects->packed_git, get_next_packed_git,
+   r->objects->packed_git = llist_mergesort(
+   r->objects->packed_git, get_next_packed_git,
set_next_packed_git, sort_pack);
 }
 
@@ -893,7 +893,7 @@ void prepare_packed_git(void)
prepare_alt_odb(the_repository);
for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
-   rearrange_packed_git();
+   rearrange_packed_git(the_repository);
prepare_packed_git_mru(the_repository);
the_repository->objects->packed_git_initialized = 1;
 }
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 03/12] packfile: allow install_packed_git to handle arbitrary repositories

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

From: 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 it only has one caller and all lines but the first
two are converted.

We must not convert 'pack_open_fds' to be a repository specific variable,
as it is used to monitor resource usage of the machine that Git executes
on.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 fast-import.c | 2 +-
 http.c| 2 +-
 packfile.c| 8 
 packfile.h| 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index b3492fce5c..ae4ced3ae1 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1038,7 +1038,7 @@ static void end_packfile(void)
if (!new_p)
die("core git rejected index %s", idx_name);
all_packs[pack_id] = new_p;
-   install_packed_git(new_p);
+   install_packed_git(the_repository, new_p);
free(idx_name);
 
/* Print the boundary */
diff --git a/http.c b/http.c
index 4d613d5f6b..111e3c12c8 100644
--- a/http.c
+++ b/http.c
@@ -2134,7 +2134,7 @@ int finish_http_pack_request(struct http_pack_request 
*preq)
return -1;
}
 
-   install_packed_git(p);
+   install_packed_git(the_repository, p);
free(tmp_idx);
return 0;
 }
diff --git a/packfile.c b/packfile.c
index bff22a8c81..ff302142c7 100644
--- a/packfile.c
+++ b/packfile.c
@@ -680,13 +680,13 @@ struct packed_git *add_packed_git(const char *path, 
size_t path_len, int local)
return p;
 }
 
-void install_packed_git(struct packed_git *pack)
+void install_packed_git(struct repository *r, struct packed_git *pack)
 {
if (pack->pack_fd != -1)
pack_open_fds++;
 
-   pack->next = the_repository->objects->packed_git;
-   the_repository->objects->packed_git = pack;
+   pack->next = r->objects->packed_git;
+   r->objects->packed_git = pack;
 }
 
 void (*report_garbage)(unsigned seen_bits, const char *path);
@@ -782,7 +782,7 @@ static void prepare_packed_git_one(char *objdir, int local)
 * corresponding .pack file that we can map.
 */
(p = add_packed_git(path.buf, path.len, local)) != 
NULL)
-   install_packed_git(p);
+   install_packed_git(the_repository, p);
}
 
if (!report_garbage)
diff --git a/packfile.h b/packfile.h
index 5b1ce00f84..77442172f0 100644
--- a/packfile.h
+++ b/packfile.h
@@ -36,7 +36,7 @@ extern void (*report_garbage)(unsigned seen_bits, const char 
*path);
 
 extern void prepare_packed_git(void);
 extern void reprepare_packed_git(void);
-extern void install_packed_git(struct packed_git *pack);
+extern void install_packed_git(struct repository *r, struct packed_git *pack);
 
 struct packed_git *get_packed_git(struct repository *r);
 struct list_head *get_packed_git_mru(struct repository *r);
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 08/12] packfile: allow prepare_packed_git to handle arbitrary repositories

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

From: Stefan Beller 

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 packfile.c | 18 +-
 packfile.h |  3 +--
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/packfile.c b/packfile.c
index f49902539b..ad90c61422 100644
--- a/packfile.c
+++ b/packfile.c
@@ -883,19 +883,19 @@ static void prepare_packed_git_mru(struct repository *r)
list_add_tail(>mru, >objects->packed_git_mru);
 }
 
-void prepare_packed_git_the_repository(void)
+void prepare_packed_git(struct repository *r)
 {
struct alternate_object_database *alt;
 
-   if (the_repository->objects->packed_git_initialized)
+   if (r->objects->packed_git_initialized)
return;
-   prepare_packed_git_one(the_repository, get_object_directory(), 1);
-   prepare_alt_odb(the_repository);
-   for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next)
-   prepare_packed_git_one(the_repository, alt->path, 0);
-   rearrange_packed_git(the_repository);
-   prepare_packed_git_mru(the_repository);
-   the_repository->objects->packed_git_initialized = 1;
+   prepare_packed_git_one(r, r->objects->objectdir, 1);
+   prepare_alt_odb(r);
+   for (alt = r->objects->alt_odb_list; alt; alt = alt->next)
+   prepare_packed_git_one(r, alt->path, 0);
+   rearrange_packed_git(r);
+   prepare_packed_git_mru(r);
+   r->objects->packed_git_initialized = 1;
 }
 
 void reprepare_packed_git_the_repository(void)
diff --git a/packfile.h b/packfile.h
index ab5046938c..3fd9092472 100644
--- a/packfile.h
+++ b/packfile.h
@@ -34,8 +34,7 @@ extern struct packed_git *parse_pack_index(unsigned char 
*sha1, const char *idx_
 #define PACKDIR_FILE_GARBAGE 4
 extern void (*report_garbage)(unsigned seen_bits, const char *path);
 
-#define prepare_packed_git(r) prepare_packed_git_##r()
-extern void prepare_packed_git_the_repository(void);
+extern void prepare_packed_git(struct repository *r);
 #define reprepare_packed_git(r) reprepare_packed_git_##r()
 extern void reprepare_packed_git_the_repository(void);
 extern void install_packed_git(struct repository *r, struct packed_git *pack);
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 04/12] packfile: add repository argument to prepare_packed_git_one

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

From: Stefan Beller 

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 packfile.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/packfile.c b/packfile.c
index ff302142c7..0e5fa67526 100644
--- a/packfile.c
+++ b/packfile.c
@@ -735,7 +735,8 @@ static void report_pack_garbage(struct string_list *list)
report_helper(list, seen_bits, first, list->nr);
 }
 
-static void prepare_packed_git_one(char *objdir, int local)
+#define prepare_packed_git_one(r, o, l) prepare_packed_git_one_##r(o, l)
+static void prepare_packed_git_one_the_repository(char *objdir, int local)
 {
struct strbuf path = STRBUF_INIT;
size_t dirnamelen;
@@ -889,10 +890,10 @@ void prepare_packed_git(void)
 
if (the_repository->objects->packed_git_initialized)
return;
-   prepare_packed_git_one(get_object_directory(), 1);
+   prepare_packed_git_one(the_repository, get_object_directory(), 1);
prepare_alt_odb(the_repository);
for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next)
-   prepare_packed_git_one(alt->path, 0);
+   prepare_packed_git_one(the_repository, alt->path, 0);
rearrange_packed_git(the_repository);
prepare_packed_git_mru(the_repository);
the_repository->objects->packed_git_initialized = 1;
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 00/12] sb/packfiles-in-repository updates

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

This is the rebased version on the updated sb/object-store I just sent
out plus the fix for get_object_directory(). The interdiff (after
rebased) looks small and nice

diff --git a/packfile.c b/packfile.c
index e02136bebb..63c89ee31a 100644
--- a/packfile.c
+++ b/packfile.c
@@ -890,7 +890,7 @@ static void prepare_packed_git(struct repository *r)
 
if (r->objects->packed_git_initialized)
return;
-   prepare_packed_git_one(r, get_object_directory(), 1);
+   prepare_packed_git_one(r, r->objects->objectdir, 1);
prepare_alt_odb(r);
for (alt = r->objects->alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(r, alt->path, 0);

I notice there's still one get_object_directory() left in packfile.c
but that should not cause problems with converted functions. That
could be done in "phase 2".

Nguyễn Thái Ngọc Duy (1):
  packfile: keep prepare_packed_git() private

Stefan Beller (11):
  packfile: allow prepare_packed_git_mru to handle arbitrary
repositories
  packfile: allow rearrange_packed_git to handle arbitrary repositories
  packfile: allow install_packed_git to handle arbitrary repositories
  packfile: add repository argument to prepare_packed_git_one
  packfile: add repository argument to prepare_packed_git
  packfile: add repository argument to reprepare_packed_git
  packfile: allow prepare_packed_git_one to handle arbitrary
repositories
  packfile: allow prepare_packed_git to handle arbitrary repositories
  packfile: allow reprepare_packed_git to handle arbitrary repositories
  packfile: add repository argument to find_pack_entry
  packfile: allow find_pack_entry to handle arbitrary repositories

 builtin/count-objects.c  |  3 +-
 builtin/fsck.c   |  2 --
 builtin/gc.c |  3 +-
 builtin/pack-objects.c   |  1 -
 builtin/pack-redundant.c |  2 --
 builtin/receive-pack.c   |  3 +-
 bulk-checkin.c   |  3 +-
 fast-import.c|  3 +-
 fetch-pack.c |  3 +-
 http-backend.c   |  1 -
 http.c   |  2 +-
 pack-bitmap.c|  1 -
 packfile.c   | 76 +++-
 packfile.h   | 11 +++---
 server-info.c|  1 -
 sha1_file.c  |  8 ++---
 sha1_name.c  |  2 --
 17 files changed, 58 insertions(+), 67 deletions(-)

-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 12/12] packfile: keep prepare_packed_git() private

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

The reason callers have to call this is to make sure either packed_git
or packed_git_mru pointers are initialized since we don't do that by
default. Sometimes it's hard to see this connection between where the
function is called and where packed_git pointer is used (sometimes in
separate functions).

Keep this dependency internal because now all access to packed_git and
packed_git_mru must go through get_xxx() wrappers.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 builtin/count-objects.c  | 3 +--
 builtin/fsck.c   | 2 --
 builtin/gc.c | 1 -
 builtin/pack-objects.c   | 1 -
 builtin/pack-redundant.c | 2 --
 fast-import.c| 1 -
 http-backend.c   | 1 -
 pack-bitmap.c| 1 -
 packfile.c   | 5 -
 packfile.h   | 1 -
 server-info.c| 1 -
 sha1_name.c  | 2 --
 12 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index ea8bd9e2e2..b054713e1a 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -122,8 +122,7 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
-   if (!get_packed_git(the_repository))
-   prepare_packed_git(the_repository);
+
for (p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index d40a82b702..f9632353d9 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -729,8 +729,6 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
uint32_t total = 0, count = 0;
struct progress *progress = NULL;
 
-   prepare_packed_git(the_repository);
-
if (show_progress) {
for (p = get_packed_git(the_repository); p;
 p = p->next) {
diff --git a/builtin/gc.c b/builtin/gc.c
index a78dad51aa..0a667972ab 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -174,7 +174,6 @@ static int too_many_packs(void)
if (gc_auto_pack_limit <= 0)
return 0;
 
-   prepare_packed_git(the_repository);
for (cnt = 0, p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 491ce433da..2f49b03cb1 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3152,7 +3152,6 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
if (progress && all_progress_implied)
progress = 2;
 
-   prepare_packed_git(the_repository);
if (ignore_packed_keep) {
struct packed_git *p;
for (p = get_packed_git(the_repository); p; p = p->next)
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index da6637f7fc..710cd0fb69 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -631,8 +631,6 @@ int cmd_pack_redundant(int argc, const char **argv, const 
char *prefix)
break;
}
 
-   prepare_packed_git(the_repository);
-
if (load_all_packs)
load_all();
else
diff --git a/fast-import.c b/fast-import.c
index 47981e6db7..37a44752a8 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3473,7 +3473,6 @@ int cmd_main(int argc, const char **argv)
rc_free[i].next = _free[i + 1];
rc_free[cmd_save - 1].next = NULL;
 
-   prepare_packed_git(the_repository);
start_packfile();
set_die_routine(die_nicely);
set_checkpoint_signal();
diff --git a/http-backend.c b/http-backend.c
index c1b1c2d557..88d2a9bc40 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -519,7 +519,6 @@ static void get_info_packs(struct strbuf *hdr, char *arg)
size_t cnt = 0;
 
select_getanyfile(hdr);
-   prepare_packed_git(the_repository);
for (p = get_packed_git(the_repository); p; p = p->next) {
if (p->pack_local)
cnt++;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 57fec38f3f..3f2dab340f 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -336,7 +336,6 @@ static int open_pack_bitmap(void)
 
assert(!bitmap_git.map && !bitmap_git.loaded);
 
-   prepare_packed_git(the_repository);
for (p = get_packed_git(the_repository); p; p = p->next) {
if (open_pack_bitmap_1(p) == 0)
ret = 0;
diff --git a/packfile.c b/packfile.c
index 89bd0d47bf..63c89ee31a 100644
--- a/packfile.c
+++ b/packfile.c
@@ -803,6 +803,7 @@ static void 

[PATCH 01/12] packfile: allow prepare_packed_git_mru to handle arbitrary repositories

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

From: 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 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 packfile.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/packfile.c b/packfile.c
index 17170fc662..8c335bdcd1 100644
--- a/packfile.c
+++ b/packfile.c
@@ -873,14 +873,14 @@ static void rearrange_packed_git(void)
set_next_packed_git, sort_pack);
 }
 
-static void prepare_packed_git_mru(void)
+static void prepare_packed_git_mru(struct repository *r)
 {
struct packed_git *p;
 
-   INIT_LIST_HEAD(_repository->objects->packed_git_mru);
+   INIT_LIST_HEAD(>objects->packed_git_mru);
 
-   for (p = the_repository->objects->packed_git; p; p = p->next)
-   list_add_tail(>mru, 
_repository->objects->packed_git_mru);
+   for (p = r->objects->packed_git; p; p = p->next)
+   list_add_tail(>mru, >objects->packed_git_mru);
 }
 
 void prepare_packed_git(void)
@@ -894,7 +894,7 @@ void prepare_packed_git(void)
for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
-   prepare_packed_git_mru();
+   prepare_packed_git_mru(the_repository);
the_repository->objects->packed_git_initialized = 1;
 }
 
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 11/12] packfile: allow find_pack_entry to handle arbitrary repositories

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

From: Stefan Beller 

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 packfile.c | 11 +--
 packfile.h |  3 +--
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/packfile.c b/packfile.c
index 26d5a17dfe..89bd0d47bf 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1845,19 +1845,18 @@ static int fill_pack_entry(const unsigned char *sha1,
return 1;
 }
 
-int find_pack_entry_the_repository(const unsigned char *sha1, struct 
pack_entry *e)
+int find_pack_entry(struct repository *r, const unsigned char *sha1, struct 
pack_entry *e)
 {
struct list_head *pos;
 
-   prepare_packed_git(the_repository);
-   if (!the_repository->objects->packed_git)
+   prepare_packed_git(r);
+   if (!r->objects->packed_git)
return 0;
 
-   list_for_each(pos, _repository->objects->packed_git_mru) {
+   list_for_each(pos, >objects->packed_git_mru) {
struct packed_git *p = list_entry(pos, struct packed_git, mru);
if (fill_pack_entry(sha1, e, p)) {
-   list_move(>mru,
- _repository->objects->packed_git_mru);
+   list_move(>mru, >objects->packed_git_mru);
return 1;
}
}
diff --git a/packfile.h b/packfile.h
index e68f790ea7..fe1a6380e6 100644
--- a/packfile.h
+++ b/packfile.h
@@ -127,8 +127,7 @@ extern const struct packed_git *has_packed_and_bad(const 
unsigned char *sha1);
  * Iff a pack file in the given repository contains the object named by sha1,
  * return true and store its location to e.
  */
-#define find_pack_entry(r, s, e) find_pack_entry_##r(s, e)
-extern int find_pack_entry_the_repository(const unsigned char *sha1, struct 
pack_entry *e);
+extern int find_pack_entry(struct repository *r, const unsigned char *sha1, 
struct pack_entry *e);
 
 extern int has_sha1_pack(const unsigned char *sha1);
 
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 10/12] packfile: add repository argument to find_pack_entry

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

From: Stefan Beller 

While at it move the documentation to the header and mention which pack
files are searched.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 packfile.c  | 8 ++--
 packfile.h  | 7 ++-
 sha1_file.c | 6 +++---
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/packfile.c b/packfile.c
index eb2dc53331..26d5a17dfe 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1845,11 +1845,7 @@ static int fill_pack_entry(const unsigned char *sha1,
return 1;
 }
 
-/*
- * Iff a pack file contains the object named by sha1, return true and
- * store its location to e.
- */
-int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
+int find_pack_entry_the_repository(const unsigned char *sha1, struct 
pack_entry *e)
 {
struct list_head *pos;
 
@@ -1871,7 +1867,7 @@ int find_pack_entry(const unsigned char *sha1, struct 
pack_entry *e)
 int has_sha1_pack(const unsigned char *sha1)
 {
struct pack_entry e;
-   return find_pack_entry(sha1, );
+   return find_pack_entry(the_repository, sha1, );
 }
 
 int has_pack_index(const unsigned char *sha1)
diff --git a/packfile.h b/packfile.h
index ee6da3a9ae..e68f790ea7 100644
--- a/packfile.h
+++ b/packfile.h
@@ -123,7 +123,12 @@ extern int packed_object_info(struct packed_git *pack, 
off_t offset, struct obje
 extern void mark_bad_packed_object(struct packed_git *p, const unsigned char 
*sha1);
 extern const struct packed_git *has_packed_and_bad(const unsigned char *sha1);
 
-extern int find_pack_entry(const unsigned char *sha1, struct pack_entry *e);
+/*
+ * Iff a pack file in the given repository contains the object named by sha1,
+ * return true and store its location to e.
+ */
+#define find_pack_entry(r, s, e) find_pack_entry_##r(s, e)
+extern int find_pack_entry_the_repository(const unsigned char *sha1, struct 
pack_entry *e);
 
 extern int has_sha1_pack(const unsigned char *sha1);
 
diff --git a/sha1_file.c b/sha1_file.c
index 9c024cd957..314ff55b47 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1266,7 +1266,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
}
 
while (1) {
-   if (find_pack_entry(real, ))
+   if (find_pack_entry(the_repository, real, ))
break;
 
/* Most likely it's a loose object. */
@@ -1275,7 +1275,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
 
/* Not a loose object; someone else may have just packed it. */
reprepare_packed_git(the_repository);
-   if (find_pack_entry(real, ))
+   if (find_pack_entry(the_repository, real, ))
break;
 
/* Check if it is a missing object */
@@ -1655,7 +1655,7 @@ static int freshen_loose_object(const unsigned char *sha1)
 static int freshen_packed_object(const unsigned char *sha1)
 {
struct pack_entry e;
-   if (!find_pack_entry(sha1, ))
+   if (!find_pack_entry(the_repository, sha1, ))
return 0;
if (e.p->freshened)
return 1;
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 07/12] packfile: allow prepare_packed_git_one to handle arbitrary repositories

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

From: Stefan Beller 

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 packfile.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/packfile.c b/packfile.c
index 1b4296277a..f49902539b 100644
--- a/packfile.c
+++ b/packfile.c
@@ -735,8 +735,7 @@ static void report_pack_garbage(struct string_list *list)
report_helper(list, seen_bits, first, list->nr);
 }
 
-#define prepare_packed_git_one(r, o, l) prepare_packed_git_one_##r(o, l)
-static void prepare_packed_git_one_the_repository(char *objdir, int local)
+static void prepare_packed_git_one(struct repository *r, char *objdir, int 
local)
 {
struct strbuf path = STRBUF_INIT;
size_t dirnamelen;
@@ -769,7 +768,7 @@ static void prepare_packed_git_one_the_repository(char 
*objdir, int local)
base_len = path.len;
if (strip_suffix_mem(path.buf, _len, ".idx")) {
/* Don't reopen a pack we already have. */
-   for (p = the_repository->objects->packed_git; p;
+   for (p = r->objects->packed_git; p;
 p = p->next) {
size_t len;
if (strip_suffix(p->pack_name, ".pack", ) &&
@@ -783,7 +782,7 @@ static void prepare_packed_git_one_the_repository(char 
*objdir, int local)
 * corresponding .pack file that we can map.
 */
(p = add_packed_git(path.buf, path.len, local)) != 
NULL)
-   install_packed_git(the_repository, p);
+   install_packed_git(r, p);
}
 
if (!report_garbage)
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 09/12] packfile: allow reprepare_packed_git to handle arbitrary repositories

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

From: Stefan Beller 

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 packfile.c | 8 
 packfile.h | 3 +--
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/packfile.c b/packfile.c
index ad90c61422..eb2dc53331 100644
--- a/packfile.c
+++ b/packfile.c
@@ -898,11 +898,11 @@ void prepare_packed_git(struct repository *r)
r->objects->packed_git_initialized = 1;
 }
 
-void reprepare_packed_git_the_repository(void)
+void reprepare_packed_git(struct repository *r)
 {
-   the_repository->objects->approximate_object_count_valid = 0;
-   the_repository->objects->packed_git_initialized = 0;
-   prepare_packed_git(the_repository);
+   r->objects->approximate_object_count_valid = 0;
+   r->objects->packed_git_initialized = 0;
+   prepare_packed_git(r);
 }
 
 struct packed_git *get_packed_git(struct repository *r)
diff --git a/packfile.h b/packfile.h
index 3fd9092472..ee6da3a9ae 100644
--- a/packfile.h
+++ b/packfile.h
@@ -35,8 +35,7 @@ extern struct packed_git *parse_pack_index(unsigned char 
*sha1, const char *idx_
 extern void (*report_garbage)(unsigned seen_bits, const char *path);
 
 extern void prepare_packed_git(struct repository *r);
-#define reprepare_packed_git(r) reprepare_packed_git_##r()
-extern void reprepare_packed_git_the_repository(void);
+extern void reprepare_packed_git(struct repository *r);
 extern void install_packed_git(struct repository *r, struct packed_git *pack);
 
 struct packed_git *get_packed_git(struct repository *r);
-- 
2.17.0.rc0.348.gd5a49e0b6f

Re: [RFC PATCH v3 3/9] Indent function git_rebase__interactive

[Johannes Schindelin]

Hi Wink,

On Thu, 22 Mar 2018, Wink Saville wrote:

> Signed-off-by: Wink Saville 
> ---
>  git-rebase--interactive.sh | 432 
> ++---

It cannot be helped (at least for now) that this indent change produces
such a large diff.

I am fine with it, of course.

Thanks,
Johannes

Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive

[Johannes Schindelin]

Hi Wink,

On Thu, 22 Mar 2018, Wink Saville wrote:

> Use initiate_action, setup_reflog_action, init_basic_state,
> init_revisions_and_shortrevisions and complete_action.
> 
> Signed-off-by: Wink Saville 
> ---
>  git-rebase--interactive.sh | 187 
> ++---

If you fold this into the previous patch, I am sure that after your 3/9
indenting the function properly, the splitting into functions will look
more or less like this:

-git_rebase__interactive () {
+initiate_action () {
+   action="$1"
 
[... unchanged code ...]
+}
+
+ () {
+   [... setting up variables ...]
 
[.. unchanged code ...]
+}
[... more of the same ...]
+
+git_rebase__interactive () {
+   initiate_action "$action" &&
+ &&
+   ...
+}

In other words, it would be easier to review and to verify that the
previous code is left unchanged (although that would have to be verified
manually by applying 3/9 and then looking at the diff with the -w option,
anyway).

Ciao,
Johannes

[ANNOUNCE] Git for Windows 2.16.3

[Johannes Schindelin]

Dear Git users,

It is my pleasure to announce that Git for Windows 2.16.3 is available from:

https://gitforwindows.org/

Changes since Git for Windows v2.16.2 (February 20th 2018)

New Features

  * Comes with Git v2.16.3.
  * When choosing to "Use Git from the Windows Command Prompt" (i.e.
add only the minimal set of Git executables to the PATH), and when
choosing the Git LFS component, Git LFS is now included in that
minimal set. This makes it possible to reuse Git for Windows' Git
LFS, say, from Visual Studio.
  * Comes with gawk v4.2.1.
  * In conjunction with the FSCache feature, git checkout is now a lot
faster when checking out a lot of files.
  * Comes with Git LFS v2.4.0.
  * Comes with Git Credential Manager v1.15.0.
  * Comes with cURL v7.59.0.
  * The Git for Windows SDK can now be "installed" via git clone
--depth=1 https://github.com/git-for-windows/git-sdk-64.
  * The tar utility (included as a courtesy, not because Git needs it)
can now unpack .tar.xz archives.

Bug Fixes

  * When a TERM is configured that Git for Windows does not know about,
Bash no longer crashes.
  * The regression where gawk stopped treating Carriage Returns as part
of the line endings was fixed.
  * When Git asks for credentials via the terminal in a Powershell
window, it no longer fails to do so.
  * The installer is now more robust when encountering files that are
in use (and can therefore not be overwritten right away).
  * The included find and rm utilities no longer have problems with
deeply nested directories on FAT drives.
  * The cygpath utility included in Git for Windows now strips trailing
slashes when normalizing paths (just like the Cygwin version of the
utility; this is different from how MSYS2 chooses to do things).
  * The certificates of HTTPS proxies configured via http.proxy are now
validated against the ca-bundle.crt correctly.

Filename | SHA-256
 | ---
Git-2.16.3-64-bit.exe | 
848f8ac7dd59817512a89a753f90ab2ff170032faf5a566badd671d65f0fb4ca
Git-2.16.3-32-bit.exe | 
9eec6c4c81e707f00adcad16cf9c005ee5df647cee5251fb854b202c321a0cf4
PortableGit-2.16.3-64-bit.7z.exe | 
b8f321d4bb9c350a9b5e58e4330d592410ac6b39df60c5c25ca2020c6e6b273e
PortableGit-2.16.3-32-bit.7z.exe | 
a8211cbe833c6eb9676ebb43cf43375484a1f9a5524009e5bf31c5df320f23c7
MinGit-2.16.3-64-bit.zip | 
74724a54a456be73df94a4ea44a62bee9b2ff00baafda2936bf5b4e61c79209d
MinGit-2.16.3-32-bit.zip | 
90d5df65004bea53e660e1d20cb374d4b3bae593bb3525c7e079c1946278f670
MinGit-2.16.3-busybox-64-bit.zip | 
436e96898e2e0bef31a0d16607d013fefcaabd5ef7f0a581a787ae0b4e8e321a
MinGit-2.16.3-busybox-32-bit.zip | 
fe12cc014cedd964d4fdd99f7fc04f941bbeee5cc4e526da0ea494c79c50e41a
Git-2.16.3-64-bit.tar.bz2 | 
8800822af28af198fc9676bf8a793193ca579252b1df2faa5dc8525cb799b95f
Git-2.16.3-32-bit.tar.bz2 | 
6c088bb0b69086932acfb05a03d2ba3781eb02217e45fed8628c2ac4e1a7be9c
pdbs-for-git-64-bit-2.16.3.1.5d726e05e4-1.zip | 
81cd1ba6c4bd76e575d9eb2c6352e207647fdad9bf44fcdf4026630d9dd2a098
pdbs-for-git-32-bit-2.16.3.1.5d726e05e4-1.zip | 
d63e97911a34d7ef3a181c3fa62366548df459dbf1b3b11fc8e424c87d80045b

Ciao,
Johannes

Re: [PATCH] completion: clear cached --options when sourcing the completion script

[Junio C Hamano]

Junio C Hamano  writes:

> SZEDER Gábor  writes:
>
>> Hang on, this test fails in the GETTEXT_POISON build.
>
> Thanks.
>
>> The thing is, we get the merge strategies with this piece of code in
>> __git_list_merge_strategies() in master:
>>
>> LANG=C LC_ALL=C git merge -s help 2>&1 |
>> sed -n -e '/[Aa]vailable strategies are: /,/^$/{
>> # a couple of s/// commands
>> }'
>>
>>
>> and that '/[Aa]vailable strategies are: /' won't match in a
>> GETTEXT_POISON-ed output, because that string is translated.
>>
>> I think for now (-rc phase) we should just drop this test, and in the
>> future we should consider adding a 'git merge --list-strategies' option.
>
> I'd say we should just add !GETTEXT_POISON prereq to the problematic
> tests.  
>
> "git merge -s help" output under forced C locale is dependable in
> the real world.  It is GETTEXT_POISON that does not get this fact
> right.  There is no need for '--list-strat' option to make this test
> pass.

IOW, this is the minumum required.

By the way, shouldn't we be running the body of these new tests
inside a subshell?  Otherwise a dot-sourcing by an earlier test of
these new ones _will_ affect all the subsequent tests.

 t/t9902-completion.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 4c86adadf2..b7f5b1e632 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1511,7 +1511,7 @@ test_expect_success 'sourcing the completion script 
clears cached porcelain comm
verbose test -z "$__git_porcelain_commands"
 '
 
-test_expect_success 'sourcing the completion script clears cached merge 
strategies' '
+test_expect_success !GETTEXT_POISON 'sourcing the completion script clears 
cached merge strategies' '
__git_compute_merge_strategies &&
verbose test -n "$__git_merge_strategies" &&
. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&

Re: [PATCH] completion: clear cached --options when sourcing the completion script

[Junio C Hamano]

SZEDER Gábor  writes:

> Hang on, this test fails in the GETTEXT_POISON build.

Thanks.

> The thing is, we get the merge strategies with this piece of code in
> __git_list_merge_strategies() in master:
>
> LANG=C LC_ALL=C git merge -s help 2>&1 |
> sed -n -e '/[Aa]vailable strategies are: /,/^$/{
> # a couple of s/// commands
> }'
>
>
> and that '/[Aa]vailable strategies are: /' won't match in a
> GETTEXT_POISON-ed output, because that string is translated.
>
> I think for now (-rc phase) we should just drop this test, and in the
> future we should consider adding a 'git merge --list-strategies' option.

I'd say we should just add !GETTEXT_POISON prereq to the problematic
tests.  

"git merge -s help" output under forced C locale is dependable in
the real world.  It is GETTEXT_POISON that does not get this fact
right.  There is no need for '--list-strat' option to make this test
pass.

[PATCH 01/27] repository: introduce raw object store field

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

From: Stefan Beller 

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

This patch introduces the raw object store and populates it with the
`objectdir`, which used to be part of the repository struct.

As the struct gains members, we'll also populate the function to clear
the memory for these members.

In a later step, we'll introduce a struct object_parser, that will
complement the object parsing in a repository struct: The raw object
parser is the layer that will provide access to raw object content,
while the higher level object parser code will parse raw objects and
keeps track of parenthood and other object relationships using 'struct
object'.  For now only add the lower level to the repository struct.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/grep.c |  3 ++-
 environment.c  |  5 +++--
 object-store.h | 18 ++
 object.c   | 14 ++
 path.c |  3 ++-
 repository.c   | 15 ++-
 repository.h   | 11 ---
 sha1_file.c|  4 +++-
 8 files changed, 56 insertions(+), 17 deletions(-)
 create mode 100644 object-store.h

diff --git a/builtin/grep.c b/builtin/grep.c
index 3ca4ac80d8..1e9cdbdf78 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -22,6 +22,7 @@
 #include "pathspec.h"
 #include "submodule.h"
 #include "submodule-config.h"
+#include "object-store.h"
 
 static char const * const grep_usage[] = {
N_("git grep [] [-e]  [...] [[--] ...]"),
@@ -432,7 +433,7 @@ static int grep_submodule(struct grep_opt *opt, struct 
repository *superproject,
 * object.
 */
grep_read_lock();
-   add_to_alternates_memory(submodule.objectdir);
+   add_to_alternates_memory(submodule.objects->objectdir);
grep_read_unlock();
 
if (oid) {
diff --git a/environment.c b/environment.c
index a5eaa97fb1..93c9fbb0ba 100644
--- a/environment.c
+++ b/environment.c
@@ -14,6 +14,7 @@
 #include "fmt-merge-msg.h"
 #include "commit.h"
 #include "argv-array.h"
+#include "object-store.h"
 
 int trust_executable_bit = 1;
 int trust_ctime = 1;
@@ -270,9 +271,9 @@ const char *get_git_work_tree(void)
 
 char *get_object_directory(void)
 {
-   if (!the_repository->objectdir)
+   if (!the_repository->objects->objectdir)
BUG("git environment hasn't been setup");
-   return the_repository->objectdir;
+   return the_repository->objects->objectdir;
 }
 
 int odb_mkstemp(struct strbuf *template, const char *pattern)
diff --git a/object-store.h b/object-store.h
new file mode 100644
index 00..abfaae059b
--- /dev/null
+++ b/object-store.h
@@ -0,0 +1,18 @@
+#ifndef OBJECT_STORE_H
+#define OBJECT_STORE_H
+
+struct raw_object_store {
+   /*
+* Path to the repository's object store.
+* Cannot be NULL after initialization.
+*/
+   char *objectdir;
+
+   /* Path to extra alternate object database if not NULL */
+   char *alternate_db;
+};
+
+struct raw_object_store *raw_object_store_new(void);
+void raw_object_store_clear(struct raw_object_store *o);
+
+#endif /* OBJECT_STORE_H */
diff --git a/object.c b/object.c
index 9e6f9ff20b..6ddd61242c 100644
--- a/object.c
+++ b/object.c
@@ -4,6 +4,7 @@
 #include "tree.h"
 #include "commit.h"
 #include "tag.h"
+#include "object-store.h"
 
 static struct object **obj_hash;
 static int nr_objs, obj_hash_size;
@@ -445,3 +446,16 @@ void clear_commit_marks_all(unsigned int flags)
obj->flags &= ~flags;
}
 }
+
+struct raw_object_store *raw_object_store_new(void)
+{
+   struct raw_object_store *o = xmalloc(sizeof(*o));
+
+   memset(o, 0, sizeof(*o));
+   return o;
+}
+void raw_object_store_clear(struct raw_object_store *o)
+{
+   FREE_AND_NULL(o->objectdir);
+   FREE_AND_NULL(o->alternate_db);
+}
diff --git a/path.c b/path.c
index da8b655730..3308b7b958 100644
--- a/path.c
+++ b/path.c
@@ -10,6 +10,7 @@
 #include "submodule-config.h"
 #include "path.h"
 #include "packfile.h"
+#include "object-store.h"
 
 static int get_st_mode_bits(const char *path, int *mode)
 {
@@ -382,7 +383,7 @@ static void adjust_git_path(const struct repository *repo,
strbuf_splice(buf, 0, buf->len,
  repo->index_file, strlen(repo->index_file));
else if (dir_prefix(base, "objects"))
-   replace_dir(buf, git_dir_len + 7, repo->objectdir);
+   replace_dir(buf, git_dir_len + 7, repo->objects->objectdir);
else if (git_hooks_path && dir_prefix(base, "hooks"))
replace_dir(buf, git_dir_len + 5, git_hooks_path);
else if (repo->different_commondir)
diff --git a/repository.c b/repository.c
index 62f52f47fc..a4848c1bd0 100644
--- a/repository.c
+++ b/repository.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include 

[PATCH 05/27] object-store: move packed_git and packed_git_mru to object store

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

From: Stefan Beller 

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

[nd: while at there, wrap access to these two fields in get_packed_git()
and get_packed_git_mru(). This allows us to lazily initialize these
fields without caller doing that explicitly]

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/count-objects.c  |  5 ++--
 builtin/fsck.c   |  6 +++--
 builtin/gc.c |  4 +++-
 builtin/index-pack.c |  1 +
 builtin/pack-objects.c   | 21 ++---
 builtin/pack-redundant.c |  6 +++--
 cache.h  | 29 ---
 fast-import.c|  8 +--
 http-backend.c   |  6 +++--
 http-push.c  |  1 +
 http-walker.c|  1 +
 http.c   |  1 +
 object-store.h   | 34 +++
 object.c |  7 ++
 pack-bitmap.c|  4 +++-
 pack-check.c |  1 +
 pack-revindex.c  |  1 +
 packfile.c   | 51 
 packfile.h   |  3 +++
 reachable.c  |  1 +
 server-info.c|  6 +++--
 sha1_name.c  |  5 ++--
 22 files changed, 128 insertions(+), 74 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index ced8958e43..b28ff00be2 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -7,6 +7,7 @@
 #include "cache.h"
 #include "config.h"
 #include "dir.h"
+#include "repository.h"
 #include "builtin.h"
 #include "parse-options.h"
 #include "quote.h"
@@ -121,9 +122,9 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
-   if (!packed_git)
+   if (!get_packed_git(the_repository))
prepare_packed_git();
-   for (p = packed_git; p; p = p->next) {
+   for (p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
if (open_pack_index(p))
diff --git a/builtin/fsck.c b/builtin/fsck.c
index c736a10a11..7707407275 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -732,7 +732,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
prepare_packed_git();
 
if (show_progress) {
-   for (p = packed_git; p; p = p->next) {
+   for (p = get_packed_git(the_repository); p;
+p = p->next) {
if (open_pack_index(p))
continue;
total += p->num_objects;
@@ -740,7 +741,8 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
 
progress = start_progress(_("Checking 
objects"), total);
}
-   for (p = packed_git; p; p = p->next) {
+   for (p = get_packed_git(the_repository); p;
+p = p->next) {
/* verify gives error messages itself */
if (verify_pack(p, fsck_obj_buffer,
progress, count))
diff --git a/builtin/gc.c b/builtin/gc.c
index 77fa720bd0..b00238cd5d 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -11,6 +11,7 @@
  */
 
 #include "builtin.h"
+#include "repository.h"
 #include "config.h"
 #include "tempfile.h"
 #include "lockfile.h"
@@ -20,6 +21,7 @@
 #include "argv-array.h"
 #include "commit.h"
 #include "packfile.h"
+#include "object-store.h"
 
 #define FAILED_RUN "failed to run %s"
 
@@ -173,7 +175,7 @@ static int too_many_packs(void)
return 0;
 
prepare_packed_git();
-   for (cnt = 0, p = packed_git; p; p = p->next) {
+   for (cnt = 0, p = get_packed_git(the_repository); p; p = p->next) {
if (!p->pack_local)
continue;
if (p->pack_keep)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 5ebd370c56..1d6bc87b76 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -13,6 +13,7 @@
 #include "streaming.h"
 #include "thread-utils.h"
 #include "packfile.h"
+#include "object-store.h"
 
 static const char index_pack_usage[] =
 "git index-pack [-v] [-o ] [--keep | --keep=] [--verify] 
[--strict] ( | --stdin [--fix-thin] [])";
diff --git a/builtin/pack-objects.c 

[PATCH 07/27] pack: move prepare_packed_git_run_once to object store

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

From: Stefan Beller 

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

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 object-store.h | 6 ++
 packfile.c | 7 +++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/object-store.h b/object-store.h
index c687ab7587..6a07a14d63 100644
--- a/object-store.h
+++ b/object-store.h
@@ -98,6 +98,12 @@ struct raw_object_store {
struct packed_git *packed_git;
/* A most-recently-used ordered version of the packed_git list. */
struct list_head packed_git_mru;
+
+   /*
+* Whether packed_git has already been populated with this repository's
+* packs.
+*/
+   unsigned packed_git_initialized : 1;
 };
 
 struct raw_object_store *raw_object_store_new(void);
diff --git a/packfile.c b/packfile.c
index f2dc084745..2a053711cf 100644
--- a/packfile.c
+++ b/packfile.c
@@ -884,12 +884,11 @@ static void prepare_packed_git_mru(void)
list_add_tail(>mru, 
_repository->objects->packed_git_mru);
 }
 
-static int prepare_packed_git_run_once = 0;
 void prepare_packed_git(void)
 {
struct alternate_object_database *alt;
 
-   if (prepare_packed_git_run_once)
+   if (the_repository->objects->packed_git_initialized)
return;
prepare_packed_git_one(get_object_directory(), 1);
prepare_alt_odb();
@@ -897,13 +896,13 @@ void prepare_packed_git(void)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
prepare_packed_git_mru();
-   prepare_packed_git_run_once = 1;
+   the_repository->objects->packed_git_initialized = 1;
 }
 
 void reprepare_packed_git(void)
 {
approximate_object_count_valid = 0;
-   prepare_packed_git_run_once = 0;
+   the_repository->objects->packed_git_initialized = 0;
prepare_packed_git();
 }
 
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 06/27] object-store: close all packs upon clearing the object store

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

From: Stefan Beller 

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/am.c   | 2 +-
 builtin/clone.c| 2 +-
 builtin/fetch.c| 2 +-
 builtin/merge.c| 2 +-
 builtin/receive-pack.c | 2 +-
 object.c   | 7 +++
 packfile.c | 4 ++--
 packfile.h | 2 +-
 8 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 5bdd2d7578..47beddbe24 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1859,7 +1859,7 @@ static void am_run(struct am_state *state, int resume)
 */
if (!state->rebasing) {
am_destroy(state);
-   close_all_packs();
+   close_all_packs(the_repository->objects);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
}
 }
diff --git a/builtin/clone.c b/builtin/clone.c
index 855947f1ab..7df5932b85 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1218,7 +1218,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
transport_disconnect(transport);
 
if (option_dissociate) {
-   close_all_packs();
+   close_all_packs(the_repository->objects);
dissociate_from_references();
}
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8ee998ea2e..a39e9d7b15 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1478,7 +1478,7 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
 
string_list_clear(, 0);
 
-   close_all_packs();
+   close_all_packs(the_repository->objects);
 
argv_array_pushl(_gc_auto, "gc", "--auto", NULL);
if (verbosity < 0)
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..96d56cbdd2 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -411,7 +411,7 @@ static void finish(struct commit *head_commit,
 * We ignore errors in 'gc --auto', since the
 * user should see them.
 */
-   close_all_packs();
+   close_all_packs(the_repository->objects);
run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
}
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b7ce7c7f52..1a298a6711 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2026,7 +2026,7 @@ int cmd_receive_pack(int argc, const char **argv, const 
char *prefix)
proc.git_cmd = 1;
proc.argv = argv_gc_auto;
 
-   close_all_packs();
+   close_all_packs(the_repository->objects);
if (!start_command()) {
if (use_sideband)
copy_to_sideband(proc.err, -1, NULL);
diff --git a/object.c b/object.c
index 04631ee841..4c2cf7ff5d 100644
--- a/object.c
+++ b/object.c
@@ -5,6 +5,7 @@
 #include "commit.h"
 #include "tag.h"
 #include "object-store.h"
+#include "packfile.h"
 
 static struct object **obj_hash;
 static int nr_objs, obj_hash_size;
@@ -483,8 +484,6 @@ void raw_object_store_clear(struct raw_object_store *o)
o->alt_odb_tail = NULL;
 
INIT_LIST_HEAD(>packed_git_mru);
-   /*
-* TODO: call close_all_packs once migrated to
-* take an object store argument
-*/
+   close_all_packs(o);
+   o->packed_git = NULL;
 }
diff --git a/packfile.c b/packfile.c
index 39f4a85200..f2dc084745 100644
--- a/packfile.c
+++ b/packfile.c
@@ -311,11 +311,11 @@ static void close_pack(struct packed_git *p)
close_pack_index(p);
 }
 
-void close_all_packs(void)
+void close_all_packs(struct raw_object_store *o)
 {
struct packed_git *p;
 
-   for (p = the_repository->objects->packed_git; p; p = p->next)
+   for (p = o->packed_git; p; p = p->next)
if (p->do_not_close)
die("BUG: want to close pack marked 'do-not-close'");
else
diff --git a/packfile.h b/packfile.h
index 76496226bb..5b1ce00f84 100644
--- a/packfile.h
+++ b/packfile.h
@@ -66,7 +66,7 @@ extern void close_pack_index(struct packed_git *);
 
 extern unsigned char *use_pack(struct packed_git *, struct pack_window **, 
off_t, unsigned long *);
 extern void close_pack_windows(struct packed_git *);
-extern void close_all_packs(void);
+extern void close_all_packs(struct raw_object_store *o);
 extern void unuse_pack(struct pack_window **);
 extern void clear_delta_base_cache(void);
 extern struct packed_git *add_packed_git(const char *path, size_t path_len, 
int local);
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 13/27] sha1_file: add repository argument to prepare_alt_odb

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

From: Stefan Beller 

See previous patch for explanation.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/fsck.c |  2 +-
 object-store.h |  3 ++-
 packfile.c |  2 +-
 sha1_file.c| 12 ++--
 sha1_name.c|  2 +-
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 7707407275..3ef25fab97 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -719,7 +719,7 @@ int cmd_fsck(int argc, const char **argv, const char 
*prefix)
 
fsck_object_dir(get_object_directory());
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
alt_odb_list = the_repository->objects->alt_odb_list;
for (alt = alt_odb_list; alt; alt = alt->next)
fsck_object_dir(alt->path);
diff --git a/object-store.h b/object-store.h
index b53e125902..79de470639 100644
--- a/object-store.h
+++ b/object-store.h
@@ -20,7 +20,8 @@ struct alternate_object_database {
 
char path[FLEX_ARRAY];
 };
-void prepare_alt_odb(void);
+#define prepare_alt_odb(r) prepare_alt_odb_##r()
+void prepare_alt_odb_the_repository(void);
 char *compute_alternate_path(const char *path, struct strbuf *err);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 int foreach_alt_odb(alt_odb_fn, void*);
diff --git a/packfile.c b/packfile.c
index b0b24ea9b8..17170fc662 100644
--- a/packfile.c
+++ b/packfile.c
@@ -890,7 +890,7 @@ void prepare_packed_git(void)
if (the_repository->objects->packed_git_initialized)
return;
prepare_packed_git_one(get_object_directory(), 1);
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next)
prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
diff --git a/sha1_file.c b/sha1_file.c
index ba4fc9103b..0fac75bd31 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -582,7 +582,7 @@ void add_to_alternates_memory(const char *reference)
 * Make sure alternates are initialized, or else our entry may be
 * overwritten when they are.
 */
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
 
link_alt_odb_entries(the_repository, reference,
 '\n', NULL, 0);
@@ -668,7 +668,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
struct alternate_object_database *ent;
int r = 0;
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (ent = the_repository->objects->alt_odb_list; ent; ent = ent->next) 
{
r = fn(ent, cb);
if (r)
@@ -677,7 +677,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
return r;
 }
 
-void prepare_alt_odb(void)
+void prepare_alt_odb_the_repository(void)
 {
if (the_repository->objects->alt_odb_tail)
return;
@@ -728,7 +728,7 @@ static int check_and_freshen_local(const unsigned char 
*sha1, int freshen)
 static int check_and_freshen_nonlocal(const unsigned char *sha1, int freshen)
 {
struct alternate_object_database *alt;
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) 
{
const char *path = alt_sha1_path(alt, sha1);
if (check_and_freshen_file(path, freshen))
@@ -887,7 +887,7 @@ static int stat_sha1_file(const unsigned char *sha1, struct 
stat *st,
if (!lstat(*path, st))
return 0;
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
errno = ENOENT;
for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) 
{
*path = alt_sha1_path(alt, sha1);
@@ -918,7 +918,7 @@ static int open_sha1_file(const unsigned char *sha1, const 
char **path)
return fd;
most_interesting_errno = errno;
 
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) 
{
*path = alt_sha1_path(alt, sha1);
fd = git_open(*path);
diff --git a/sha1_name.c b/sha1_name.c
index 7d8710..4325f74e0c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -354,7 +354,7 @@ static int init_object_disambiguation(const char *name, int 
len,
 
ds->len = len;
ds->hex_pfx[len] = '\0';
-   prepare_alt_odb();
+   prepare_alt_odb(the_repository);
return 0;
 }
 
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 09/27] sha1_file: add raw_object_store argument to alt_odb_usable

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

From: Stefan Beller 

Add a raw_object_store to alt_odb_usable to be more specific about which
repository to act on. The choice of the repository is delegated to its
only caller link_alt_odb_entry.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 sha1_file.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 84b361c125..097c372d03 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -347,7 +347,9 @@ static const char *alt_sha1_path(struct 
alternate_object_database *alt,
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
-static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
+static int alt_odb_usable(struct raw_object_store *o,
+ struct strbuf *path,
+ const char *normalized_objdir)
 {
struct alternate_object_database *alt;
 
@@ -363,7 +365,7 @@ static int alt_odb_usable(struct strbuf *path, const char 
*normalized_objdir)
 * Prevent the common mistake of listing the same
 * thing twice, or object directory itself.
 */
-   for (alt = the_repository->objects->alt_odb_list; alt; alt = alt->next) 
{
+   for (alt = o->alt_odb_list; alt; alt = alt->next) {
if (!fspathcmp(path->buf, alt->path))
return 0;
}
@@ -415,7 +417,7 @@ static int link_alt_odb_entry(const char *entry, const char 
*relative_base,
while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
strbuf_setlen(, pathbuf.len - 1);
 
-   if (!alt_odb_usable(, normalized_objdir)) {
+   if (!alt_odb_usable(the_repository->objects, , 
normalized_objdir)) {
strbuf_release();
return -1;
}
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 11/27] sha1_file: add repository argument to read_info_alternates

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

From: Stefan Beller 

See previous patch for explanation.

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 sha1_file.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 7c0ace646a..81ad2a84f2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -390,7 +390,9 @@ static int alt_odb_usable(struct raw_object_store *o,
  * SHA1, an extra slash for the first level indirection, and the
  * terminating NUL.
  */
-static void read_info_alternates(const char * relative_base, int depth);
+#define read_info_alternates(r, rb, d) read_info_alternates_##r(rb, d)
+static void read_info_alternates_the_repository(const char *relative_base,
+   int depth);
 #define link_alt_odb_entry(r, e, rb, d, n) link_alt_odb_entry_##r(e, rb, d, n)
 static int link_alt_odb_entry_the_repository(const char *entry,
const char *relative_base, int depth, const char *normalized_objdir)
@@ -431,7 +433,7 @@ static int link_alt_odb_entry_the_repository(const char 
*entry,
ent->next = NULL;
 
/* recursively add alternates */
-   read_info_alternates(pathbuf.buf, depth + 1);
+   read_info_alternates(the_repository, pathbuf.buf, depth + 1);
 
strbuf_release();
return 0;
@@ -497,7 +499,8 @@ static void link_alt_odb_entries(const char *alt, int sep,
strbuf_release();
 }
 
-static void read_info_alternates(const char * relative_base, int depth)
+static void read_info_alternates_the_repository(const char *relative_base,
+   int depth)
 {
char *path;
struct strbuf buf = STRBUF_INIT;
@@ -678,7 +681,7 @@ void prepare_alt_odb(void)
link_alt_odb_entries(the_repository->objects->alternate_db,
 PATH_SEP, NULL, 0);
 
-   read_info_alternates(get_object_directory(), 0);
+   read_info_alternates(the_repository, get_object_directory(), 0);
 }
 
 /* Returns 1 if we have successfully freshened the file, 0 otherwise. */
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH 19/27] sha1_file: add repository argument to map_sha1_file_1

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

From: Stefan Beller 

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

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

Signed-off-by: Stefan Beller 
Signed-off-by: Jonathan Nieder 
Signed-off-by: Junio C Hamano 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 sha1_file.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

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