GREAT NEWS!!

[Amnesty International]

We have a great news about your E-mail address!!! 

You Won  $950,500.00 USD on Amnesty International 
Kenya online E-mail Promotion. For more details 
about your prize claims, file with the following;

Names: 
Country: 
Tel:

Regards,
Mr. David Ford

[PATCH] specify encoding for sed command

[Stephon Harris]

Fixes issue with seeing `sed: RE error: illegal byte sequence` when running 
git-completion.bash
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index b09c8a23626b4..52a4ab5e2165a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -282,7 +282,7 @@ __gitcomp ()
 
 # Clear the variables caching builtins' options when (re-)sourcing
 # the completion script.
-unset $(set |sed -ne 
's/^\(__gitcomp_builtin_[a-zA-Z0-9_][a-zA-Z0-9_]*\)=.*/\1/p') 2>/dev/null
+unset $(set |LANG=C sed -ne 
's/^\(__gitcomp_builtin_[a-zA-Z0-9_][a-zA-Z0-9_]*\)=.*/\1/p') 2>/dev/null
 
 # This function is equivalent to
 #

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

[PATCH v4 3/3] 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 --type=color core.section

will be have exactly as:

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

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

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt | 10 ++
 builtin/config.c | 21 +
 t/t1300-repo-config.sh   | 30 ++
 3 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 620492d1e..bde702d2e 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -38,10 +38,8 @@ 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 <>).
 
-A type specifier may be given as an argument to `--type` to make 'git config'
-ensure that the variable(s) are of the given type and convert the value to the
-canonical form. If no type specifier is passed, no checks or transformations 
are
-performed on the value.
+`color`::
+The value is taken as an ANSI color escape sequence.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
@@ -177,6 +175,7 @@ Valid ``'s include:
   ~/` from the command line to let your shell do the expansion.)
 - 'expiry-date': canonicalize by converting from a fixed or relative 
date-string
   to a timestamp. This specifier has no effect when setting the value.
+- 'color': canonicalize by converting to an ANSI color escape sequence.
 +
 
 --bool::
@@ -223,6 +222,9 @@ Valid ``'s include:
output it as the ANSI color escape sequence to the standard
output.  The optional `default` parameter is used instead, if
there is no color configured for `name`.
++
+It is preferred to use `--type=color`, or `--type=color --default=[default]`
+instead of `--get-color`.
 
 -e::
 --edit::
diff --git a/builtin/config.c b/builtin/config.c
index 1328b568b..aa3fcabe9 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -61,6 +61,7 @@ static int show_origin;
 #define TYPE_BOOL_OR_INT   3
 #define TYPE_PATH  4
 #define TYPE_EXPIRY_DATE   5
+#define TYPE_COLOR 6
 
 static int option_parse_type(const struct option *opt, const char *arg,
 int unset)
@@ -82,6 +83,8 @@ static int option_parse_type(const struct option *opt, const 
char *arg,
*type = TYPE_PATH;
else if (!strcmp(arg, "expiry-date"))
*type = TYPE_EXPIRY_DATE;
+   else if (!strcmp(arg, "color"))
+   *type = TYPE_COLOR;
else {
die(_("unrecognized --type argument, %s"), arg);
return 1;
@@ -203,6 +206,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 (type == 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 {
@@ -348,6 +356,19 @@ static char *normalize_value(const char *key, const char 
*value)
else
return xstrdup(v ? "true" : "false");
}
+   if (type == TYPE_COLOR) {
+   char v[COLOR_MAXLEN];
+   if (!git_config_color(v, key, value))
+   /*
+* The contents of `v` now contain an ANSI escape
+* sequence, not suitable for including within a
+* configuration file. Treat the above as a
+* "sanity-check", and return the given value, which we
+* know is representable as valid color code.
+*/
+   return xstrdup(value);
+   die("cannot parse color '%s'", value);
+   }
 
die("BUG: cannot normalize type %d", type);
 }
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index b25ab7b9e..c630bdc77 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -931,6 +931,36 @@ test_expect_success 'get --expiry-date' 

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

[Taylor Blau]

In preparation for adding `--type=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.17.0

[PATCH v4 1/3] 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 variable 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
variable 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
`--type=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 variable [default] [...]

with:

  $ git config --default default --type=color variable [...]

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
configuration.

Specifically, this means that the following will work:

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

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

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt |  4 
 builtin/config.c | 12 
 t/t1310-config-default.sh| 38 
 3 files changed, 54 insertions(+)
 create mode 100755 t/t1310-config-default.sh

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index c7e56e3fd..620492d1e 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -235,6 +235,10 @@ Valid ``'s include:
using `--file`, `--global`, etc) and `on` when searching all
config files.
 
+--default value::
+  When using `--get`, and the requested slot is not found, behave as if value
+  were the value assigned to the that slot.
+
 CONFIGURATION
 -
 `pager.config` is only respected when listing configuration, i.e., when
diff --git a/builtin/config.c b/builtin/config.c
index 6e1495eac..1328b568b 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, type;
+static char *default_value;
 static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
@@ -122,6 +123,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(),
 };
 
@@ -286,6 +288,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) {
+   exit(1);
+   }
+   }
+
ret = !values.nr;
 
for (i = 0; i < values.nr; i++) {
diff --git a/t/t1310-config-default.sh b/t/t1310-config-default.sh
new file mode 100755
index 0..ab4e35f06
--- /dev/null
+++ b/t/t1310-config-default.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='Test git config in different settings (with --default)'
+
+. ./test-lib.sh
+
+test_expect_success 'uses --default when missing entry' '
+   echo quux >expect &&
+   git config -f config --default quux core.foo >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'canonicalizes --default with appropriate type' '
+   echo true >expect &&
+   git config -f config --default=true --bool core.foo >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'uses entry when available' '
+   echo bar 

[PATCH v4 0/3] Teach `--default` to `git-config(1)`

[Taylor Blau]

Hi,

Attached is a fourth re-roll of my series to add "--default" and
"--type=color" to "git config" in order to replace:

  $ git config --get-color  [default]

with

  $ git config [--default=] --type=color 

Since the last revision, I have:

  - Rebased it upon the latest changes from my series to add
"--type=" in favor of "--".

  - Fixed various incorrect hunks mentioning "--color" (which does not
exist), cc: Eric, Junio.

  - Fixed referencing "slot" when the correct spelling is "variable".
cc: Junio.

As always, thank you for your helpful feedback. This process is still
new to me, so I appreciate an extra degree of scrutiny :-).

I am hopeful that this series is ready for queueing.


Thanks,
Taylor

Taylor Blau (3):
  builtin/config: introduce `--default`
  config.c: introduce 'git_config_color' to parse ANSI colors
  builtin/config: introduce `color` type specifier

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

--
2.17.0

Re: [PATCH v3 3/3] builtin/config: introduce `color` type specifier

[Taylor Blau]

On Fri, Mar 30, 2018 at 11:09:43AM -0700, Junio C Hamano wrote:
> Taylor Blau  writes:
>
> > @@ -184,6 +183,7 @@ Valid `[type]`'s include:
> >  --bool-or-int::
> >  --path::
> >  --expiry-date::
> > +--color::
> >Historical options for selecting a type specifier. Prefer instead 
> > `--type`,
> >(see: above).
> >
> > @@ -223,6 +223,9 @@ Valid `[type]`'s include:
> > output it as the ANSI color escape sequence to the standard
> > output.  The optional `default` parameter is used instead, if
> > there is no color configured for `name`.
> > ++
> > +It is preferred to use `--type=color`, or `--type=color 
> > --default=[default]`
> > +instead of `--get-color`.
>
> Wasn't the whole point of the preliminary --type= patch to
> avoid having to add thse two hunks?

For the first hunk, yes, but not for the second. The series that adds
"--type=" was meant to make it possible to say "parse this as a
color" without squatting on the "--color" flag.

So, including "--color" in the list of historical options is a mistake.
But, using "--type=color --default=..." over "--get-color" is the
desired intention of this series.


Thanks,
Taylor

Re: [PATCH v3 2/3] config.c: introduce 'git_config_color' to parse ANSI colors

[Taylor Blau]

On Fri, Mar 30, 2018 at 04:26:09PM -0400, Eric Sunshine wrote:
> On Wed, Mar 28, 2018 at 9:16 PM, Taylor Blau  wrote:
> > 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_`.
>
> Did you mean s/--color/--type=color/ ?

I did; thanks for pointing this out. I have fixed this to mention
"--type=color" in the subsequent re-roll.

Thanks,
Taylor

Re: [PATCH v3 1/3] builtin/config: introduce `--default`

[Taylor Blau]

On Fri, Mar 30, 2018 at 04:23:56PM -0400, Eric Sunshine wrote:
> On Wed, Mar 28, 2018 at 9:16 PM, Taylor Blau  wrote:
> > 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.
>
> I'm confused. The cover letter said that this iteration no longer
> introduces a --color option (favoring instead --type=color), but this
> commit message still talks about --color. Did you mean
> s/--color/--type=color/ ?

My mistake; I think I rebased this series off of the "--type=" series
and forgot to amend this change. I have updated this and the below in
the subsequent re-roll.

> > For example, we aim to replace:
> >
> >   $ git config --get-color slot [default] [...]
> >
> > with:
> >
> >   $ git config --default default --color slot [...]
>
> Ditto: s/--color/--type=color/

Ack.

> > 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
> > configuration.
> >
> > 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`.
>
> Ditto: s/--color/--type=color/

Ack.

Thanks,
Taylor

Re: [PATCH v3 1/3] builtin/config: introduce `--default`

[Taylor Blau]

On Fri, Mar 30, 2018 at 11:06:22AM -0700, Junio C Hamano wrote:
> Taylor Blau  writes:
>
> > 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.
> > ...
> > +--default value::
> > +  When using `--get`, and the requested slot is not found, behave as if 
> > value
> > +  were the value assigned to the that slot.
>
> For "diff..color", the above is OK, but in general,
> configuration variables are not called "slot".  s/slot/variable/.

Thanks; I was unaware of this convention. I have changed "slot" to
"variable" as you suggested in the subsequent re-roll.

Thanks,
Taylor

[PATCH v5 2/5] stash: convert apply to builtin

[Joel Teichroeb]

Add a bulitin helper for performing stash commands. Converting
all at once proved hard to review, so starting with just apply
let conversion get started without the other command being
finished.

The helper is being implemented as a drop in replacement for
stash so that when it is complete it can simply be renamed and
the shell script deleted.

Delete the contents of the apply_stash shell function and replace
it with a call to stash--helper apply until pop is also
converted.

Signed-off-by: Joel Teichroeb 
---
 .gitignore  |   1 +
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/stash--helper.c | 431 
 git-stash.sh|  75 +
 git.c   |   1 +
 6 files changed, 440 insertions(+), 70 deletions(-)
 create mode 100644 builtin/stash--helper.c

diff --git a/.gitignore b/.gitignore
index 833ef3b0b7..296d5f376d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -152,6 +152,7 @@
 /git-show-ref
 /git-stage
 /git-stash
+/git-stash--helper
 /git-status
 /git-stripspace
 /git-submodule
diff --git a/Makefile b/Makefile
index 96f6138f63..6cfdbe9a32 100644
--- a/Makefile
+++ b/Makefile
@@ -1028,6 +1028,7 @@ BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-ref.o
+BUILTIN_OBJS += builtin/stash--helper.o
 BUILTIN_OBJS += builtin/stripspace.o
 BUILTIN_OBJS += builtin/submodule--helper.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
diff --git a/builtin.h b/builtin.h
index 42378f3aa4..a14fd85b0e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -219,6 +219,7 @@ extern int cmd_shortlog(int argc, const char **argv, const 
char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_status(int argc, const char **argv, const char *prefix);
+extern int cmd_stash__helper(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
 extern int cmd_submodule__helper(int argc, const char **argv, const char 
*prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
new file mode 100644
index 00..9d00a9547d
--- /dev/null
+++ b/builtin/stash--helper.c
@@ -0,0 +1,431 @@
+#include "builtin.h"
+#include "config.h"
+#include "parse-options.h"
+#include "refs.h"
+#include "lockfile.h"
+#include "cache-tree.h"
+#include "unpack-trees.h"
+#include "merge-recursive.h"
+#include "argv-array.h"
+#include "run-command.h"
+#include "dir.h"
+#include "rerere.h"
+
+static const char * const git_stash_helper_usage[] = {
+   N_("git stash--helper apply [--index] [-q|--quiet] []"),
+   NULL
+};
+
+static const char * const git_stash_helper_apply_usage[] = {
+   N_("git stash--helper apply [--index] [-q|--quiet] []"),
+   NULL
+};
+
+static const char *ref_stash = "refs/stash";
+static int quiet;
+static struct strbuf stash_index_path = STRBUF_INIT;
+
+struct stash_info {
+   struct object_id w_commit;
+   struct object_id b_commit;
+   struct object_id i_commit;
+   struct object_id u_commit;
+   struct object_id w_tree;
+   struct object_id b_tree;
+   struct object_id i_tree;
+   struct object_id u_tree;
+   struct strbuf revision;
+   int is_stash_ref;
+   int has_u;
+};
+
+static int grab_oid(struct object_id *oid, const char *fmt, const char *rev)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int ret;
+
+   strbuf_addf(, fmt, rev);
+   ret = get_oid(buf.buf, oid);
+   strbuf_release();
+   return ret;
+}
+
+static void free_stash_info(struct stash_info *info)
+{
+   strbuf_release(>revision);
+}
+
+static int get_stash_info(struct stash_info *info, int argc, const char **argv)
+{
+   struct strbuf symbolic = STRBUF_INIT;
+   int ret;
+   const char *revision;
+   const char *commit = NULL;
+   char *end_of_rev;
+   char *expanded_ref;
+   struct object_id discard;
+
+   if (argc > 1) {
+   int i;
+   struct strbuf refs_msg = STRBUF_INIT;
+   for (i = 0; i < argc; ++i)
+   strbuf_addf(_msg, " '%s'", argv[i]);
+
+   fprintf_ln(stderr, _("Too many revisions specified:%s"), 
refs_msg.buf);
+   strbuf_release(_msg);
+
+   return -1;
+   }
+
+   if (argc == 1)
+   commit = argv[0];
+
+   strbuf_init(>revision, 0);
+   if (commit == NULL) {
+   if (!ref_exists(ref_stash)) {
+   free_stash_info(info);
+   return error(_("No stash entries found."));
+   }
+
+   strbuf_addf(>revision, "%s@{0}", ref_stash);
+   } else if (strspn(commit, "0123456789") == 

[PATCH v5 5/5] stash: convert pop to builtin

[Joel Teichroeb]

Add stash pop to the helper and delete the pop_stash, drop_stash,
assert_stash_ref and pop_stash functions from the shell script
now that they are no longer needed.

Signed-off-by: Joel Teichroeb 
---
 builtin/stash--helper.c | 41 
 git-stash.sh| 50 -
 2 files changed, 45 insertions(+), 46 deletions(-)

diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 486796bb6a..7c8950bc90 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -13,6 +13,7 @@
 
 static const char * const git_stash_helper_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
+   N_("git stash--helper pop [--index] [-q|--quiet] []"),
N_("git stash--helper apply [--index] [-q|--quiet] []"),
N_("git stash--helper branch  []"),
N_("git stash--helper clear"),
@@ -24,6 +25,11 @@ static const char * const git_stash_helper_drop_usage[] = {
NULL
 };
 
+static const char * const git_stash_helper_pop_usage[] = {
+   N_("git stash--helper pop [--index] [-q|--quiet] []"),
+   NULL
+};
+
 static const char * const git_stash_helper_apply_usage[] = {
N_("git stash--helper apply [--index] [-q|--quiet] []"),
NULL
@@ -508,6 +514,39 @@ static int drop_stash(int argc, const char **argv, const 
char *prefix)
return ret;
 }
 
+static int pop_stash(int argc, const char **argv, const char *prefix)
+{
+   int index = 0, ret;
+   struct stash_info info;
+   struct option options[] = {
+   OPT__QUIET(, N_("be quiet, only report errors")),
+   OPT_BOOL(0, "index", ,
+   N_("attempt to recreate the index")),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+   git_stash_helper_pop_usage, 0);
+
+   if (get_stash_info(, argc, argv))
+   return -1;
+
+   if (assert_stash_ref()) {
+   free_stash_info();
+   return -1;
+   }
+
+   if (do_apply_stash(prefix, , index)) {
+   printf_ln(_("The stash entry is kept in case you need it 
again."));
+   free_stash_info();
+   return -1;
+   }
+
+   ret = do_drop_stash(prefix, );
+   free_stash_info();
+   return ret;
+}
+
 static int branch_stash(int argc, const char **argv, const char *prefix)
 {
const char *branch = NULL;
@@ -577,6 +616,8 @@ int cmd_stash__helper(int argc, const char **argv, const 
char *prefix)
result = clear_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "drop"))
result = drop_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "pop"))
+   result = pop_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "branch"))
result = branch_stash(argc, argv, prefix);
else {
diff --git a/git-stash.sh b/git-stash.sh
index c5fd4c6c44..8f2640fe90 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -554,50 +554,6 @@ assert_stash_like() {
}
 }
 
-is_stash_ref() {
-   is_stash_like "$@" && test -n "$IS_STASH_REF"
-}
-
-assert_stash_ref() {
-   is_stash_ref "$@" || {
-   args="$*"
-   die "$(eval_gettext "'\$args' is not a stash reference")"
-   }
-}
-
-apply_stash () {
-   cd "$START_DIR"
-   git stash--helper apply "$@"
-   res=$?
-   cd_to_toplevel
-   return $res
-}
-
-pop_stash() {
-   assert_stash_ref "$@"
-
-   if apply_stash "$@"
-   then
-   drop_stash "$@"
-   else
-   status=$?
-   say "$(gettext "The stash entry is kept in case you need it 
again.")"
-   exit $status
-   fi
-}
-
-drop_stash () {
-   assert_stash_ref "$@"
-
-   git reflog delete --updateref --rewrite "${REV}" &&
-   say "$(eval_gettext "Dropped \${REV} (\$s)")" ||
-   die "$(eval_gettext "\${REV}: Could not drop stash entry")"
-
-   # clear_stash if we just dropped the last stash entry
-   git rev-parse --verify --quiet "$ref_stash@{0}" >/dev/null ||
-   clear_stash
-}
-
 test "$1" = "-p" && set "push" "$@"
 
 PARSE_CACHE='--not-parsed'
@@ -634,7 +590,8 @@ push)
;;
 apply)
shift
-   apply_stash "$@"
+   cd "$START_DIR"
+   git stash--helper apply "$@"
;;
 clear)
shift
@@ -654,7 +611,8 @@ drop)
;;
 pop)
shift
-   pop_stash "$@"
+   cd "$START_DIR"
+   git stash--helper pop "$@"
;;
 branch)
shift
-- 
2.16.3

[PATCH v5 3/5] stash: convert drop and clear to builtin

[Joel Teichroeb]

Add the drop and clear commands to the builtin helper. These two
are each simple, but are being added together as they are quite
related.

We have to unfortunately keep the drop and clear functions in the
shell script as functions are called with parameters internally
that are not valid when the commands are called externally. Once
pop is converted they can both be removed.

Signed-off-by: Joel Teichroeb 
---
 builtin/stash--helper.c | 107 
 git-stash.sh|   4 +-
 2 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 9d00a9547d..520cd746c4 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -12,7 +12,14 @@
 #include "rerere.h"
 
 static const char * const git_stash_helper_usage[] = {
+   N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper apply [--index] [-q|--quiet] []"),
+   N_("git stash--helper clear"),
+   NULL
+};
+
+static const char * const git_stash_helper_drop_usage[] = {
+   N_("git stash--helper drop [-q|--quiet] []"),
NULL
 };
 
@@ -21,6 +28,11 @@ static const char * const git_stash_helper_apply_usage[] = {
NULL
 };
 
+static const char * const git_stash_helper_clear_usage[] = {
+   N_("git stash--helper clear"),
+   NULL
+};
+
 static const char *ref_stash = "refs/stash";
 static int quiet;
 static struct strbuf stash_index_path = STRBUF_INIT;
@@ -134,6 +146,29 @@ static int get_stash_info(struct stash_info *info, int 
argc, const char **argv)
return 0;
 }
 
+static int do_clear_stash(void)
+{
+   struct object_id obj;
+   if (get_oid(ref_stash, ))
+   return 0;
+
+   return delete_ref(NULL, ref_stash, , 0);
+}
+
+static int clear_stash(int argc, const char **argv, const char *prefix)
+{
+   struct option options[] = {
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options, 
git_stash_helper_clear_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+
+   if (argc != 0)
+   return error(_("git stash--helper clear with parameters is 
unimplemented"));
+
+   return do_clear_stash();
+}
+
 static int reset_tree(struct object_id *i_tree, int update, int reset)
 {
struct unpack_trees_options opts;
@@ -399,6 +434,74 @@ static int apply_stash(int argc, const char **argv, const 
char *prefix)
return ret;
 }
 
+static int do_drop_stash(const char *prefix, struct stash_info *info)
+{
+   struct argv_array args = ARGV_ARRAY_INIT;
+   int ret;
+   struct child_process cp = CHILD_PROCESS_INIT;
+
+   /* reflog does not provide a simple function for deleting refs. One will
+* need to be added to avoid implementing too much reflog code here
+*/
+   argv_array_pushl(, "reflog", "delete", "--updateref", "--rewrite", 
NULL);
+   argv_array_push(, info->revision.buf);
+   ret = cmd_reflog(args.argc, args.argv, prefix);
+   if (!ret) {
+   if (!quiet)
+   printf(_("Dropped %s (%s)\n"), info->revision.buf, 
oid_to_hex(>w_commit));
+   } else {
+   return error(_("%s: Could not drop stash entry"), 
info->revision.buf);
+   }
+
+   /* This could easily be replaced by get_oid, but currently it will 
throw a
+* fatal error when a reflog is empty, which we can not recover from
+*/
+   cp.git_cmd = 1;
+   /* Even though --quiet is specified, rev-parse still outputs the hash */
+   cp.no_stdout = 1;
+   argv_array_pushl(, "rev-parse", "--verify", "--quiet", NULL);
+   argv_array_pushf(, "%s@{0}", ref_stash);
+   ret = run_command();
+
+   if (ret)
+   do_clear_stash();
+
+   return 0;
+}
+
+static int assert_stash_ref(struct stash_info *info)
+{
+   if (!info->is_stash_ref)
+   return error(_("'%s' is not a stash reference"), 
info->revision.buf);
+
+   return 0;
+}
+
+static int drop_stash(int argc, const char **argv, const char *prefix)
+{
+   struct stash_info info;
+   int ret;
+   struct option options[] = {
+   OPT__QUIET(, N_("be quiet, only report errors")),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+   git_stash_helper_drop_usage, 0);
+
+   if (get_stash_info(, argc, argv))
+   return -1;
+
+   if (assert_stash_ref()) {
+   free_stash_info();
+   return -1;
+   }
+
+   ret = do_drop_stash(prefix, );
+   free_stash_info();
+   return ret;
+}
+
 int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 {
int result = 0;
@@ -421,6 +524,10 @@ int cmd_stash__helper(int argc, const char **argv, const 
char *prefix)
usage_with_options(git_stash_helper_usage, options);
else if (!strcmp(argv[0], "apply"))
  

[PATCH v5 0/5] Convert some stash functionality to a builtin

[Joel Teichroeb]

I've been working on converting all of git stash to be a
builtin, however it's hard to get it all working at once with
limited time, so I've moved around half of it to a new
stash--helper builtin and called these functions from the shell
script. Once this is stabalized, it should be easier to convert
the rest of the commands one at a time without breaking
anything.

I've sent most of this code before, but that was targetting a
full replacement of stash. The code is overall the same, but
with some code review changes and updates for internal api
changes.

Since there seems to be interest from GSOC students who want to
work on converting builtins, I figured I should finish what I
have that works now so they could build on top of it.

The code is based on next as write_cache_as_tree was changed to
take an object ID. It can easily be rebase on master by changing
the two calls to write_cache_as_tree to use tha hash.

Previous threads:
v1: https://public-inbox.org/git/20180325173916.GE10909@hank/T/
v2: https://public-inbox.org/git/20180326011426.19159-1-j...@teichroeb.net/
v3: https://public-inbox.org/git/20180327054432.26419-1-j...@teichroeb.net/
v4: https://public-inbox.org/git/20180328222129.22192-1-j...@teichroeb.net/

Changes from v4:
 - Fixed a typo (Thanks Eric)
 - Redid my test again with input from Junio
 - Cleaned up usages of get_oid (Thanks Junio)
 - Slightly reduced calls to builtin functions (rerere, rev-parse)
 - Added comments clarifying why when forking/cmd_* is used

Joel Teichroeb (5):
  stash: improve option parsing test coverage
  stash: convert apply to builtin
  stash: convert drop and clear to builtin
  stash: convert branch to builtin
  stash: convert pop to builtin

 .gitignore  |   1 +
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/stash--helper.c | 630 
 git-stash.sh| 136 +--
 git.c   |   1 +
 t/t3903-stash.sh|  35 +++
 7 files changed, 677 insertions(+), 128 deletions(-)
 create mode 100644 builtin/stash--helper.c

-- 
2.16.3

[PATCH v5 1/5] stash: improve option parsing test coverage

[Joel Teichroeb]

In preparation for converting the stash command incrementally to
a builtin command, this patch improves test coverage of the option
parsing. Both for having too many parameters, or too few.

Signed-off-by: Joel Teichroeb 
---
 t/t3903-stash.sh | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index aefde7b172..4eaa4cae9a 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -444,6 +444,36 @@ test_expect_failure 'stash file to directory' '
test foo = "$(cat file/file)"
 '
 
+test_expect_success 'giving too many ref agruments does not modify files' '
+   git stash clear &&
+   test_when_finished "git reset --hard HEAD" &&
+   echo foo >file2 &&
+   git stash &&
+   echo bar >file2 &&
+   git stash &&
+   test-chmtime =123456789 file2 &&
+   for type in apply pop "branch stash-branch"
+   do
+   test_must_fail git stash $type stash@{0} stash@{1} 2>err &&
+   test_i18ngrep "Too many" err &&
+   test 123456789 = $(test-chmtime -v +0 file2 | sed 
's/[^0-9].*$//') || return 1
+   done
+'
+
+test_expect_success 'giving too many ref agruments to drop does not drop 
anything' '
+   git stash list > stashlist1 &&
+   test_must_fail git stash drop stash@{0} stash@{1} 2>err &&
+   test_i18ngrep "Too many" err &&
+   git stash list > stashlist2 &&
+   test_cmp stashlist1 stashlist2
+'
+
+test_expect_success 'giving too many ref agruments to show does not show 
anything' '
+   test_must_fail git stash show stash@{0} stash@{1} 2>err 1>out && # show 
must not show anything
+   test_i18ngrep "Too many" err &&
+   test_must_be_empty out
+'
+
 test_expect_success 'stash create - no changes' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
@@ -479,6 +509,11 @@ test_expect_success 'stash branch - stashes on stack, 
stash-like argument' '
test $(git ls-files --modified | wc -l) -eq 1
 '
 
+test_expect_success 'stash branch complains with no arguments' '
+   test_must_fail git stash branch 2>err &&
+   test_i18ngrep "No branch name specified" err
+'
+
 test_expect_success 'stash show format defaults to --stat' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
-- 
2.16.3

[PATCH v5 4/5] stash: convert branch to builtin

[Joel Teichroeb]

Add stash branch to the helper and delete the apply_to_branch
function from the shell script.

Signed-off-by: Joel Teichroeb 
---
 builtin/stash--helper.c | 51 +
 git-stash.sh| 17 ++---
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 520cd746c4..486796bb6a 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -14,6 +14,7 @@
 static const char * const git_stash_helper_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper apply [--index] [-q|--quiet] []"),
+   N_("git stash--helper branch  []"),
N_("git stash--helper clear"),
NULL
 };
@@ -28,6 +29,11 @@ static const char * const git_stash_helper_apply_usage[] = {
NULL
 };
 
+static const char * const git_stash_helper_branch_usage[] = {
+   N_("git stash--helper branch  []"),
+   NULL
+};
+
 static const char * const git_stash_helper_clear_usage[] = {
N_("git stash--helper clear"),
NULL
@@ -502,6 +508,49 @@ static int drop_stash(int argc, const char **argv, const 
char *prefix)
return ret;
 }
 
+static int branch_stash(int argc, const char **argv, const char *prefix)
+{
+   const char *branch = NULL;
+   int ret;
+   struct argv_array args = ARGV_ARRAY_INIT;
+   struct stash_info info;
+   struct option options[] = {
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+   git_stash_helper_branch_usage, 0);
+
+   if (argc == 0)
+   return error(_("No branch name specified"));
+
+   branch = argv[0];
+
+   if (get_stash_info(, argc - 1, argv + 1))
+   return -1;
+
+   /* Checkout does not currently provide a function for checking out a 
branch
+* as cmd_checkout does a large amount of sanity checks first that we
+* require here.
+*/
+   argv_array_pushl(, "checkout", "-b", NULL);
+   argv_array_push(, branch);
+   argv_array_push(, oid_to_hex(_commit));
+   ret = cmd_checkout(args.argc, args.argv, prefix);
+   if (ret) {
+   free_stash_info();
+   return -1;
+   }
+
+   ret = do_apply_stash(prefix, , 1);
+   if (!ret && info.is_stash_ref)
+   ret = do_drop_stash(prefix, );
+
+   free_stash_info();
+
+   return ret;
+}
+
 int cmd_stash__helper(int argc, const char **argv, const char *prefix)
 {
int result = 0;
@@ -528,6 +577,8 @@ int cmd_stash__helper(int argc, const char **argv, const 
char *prefix)
result = clear_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "drop"))
result = drop_stash(argc, argv, prefix);
+   else if (!strcmp(argv[0], "branch"))
+   result = branch_stash(argc, argv, prefix);
else {
error(_("unknown subcommand: %s"), argv[0]);
usage_with_options(git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index 0b8f07b38a..c5fd4c6c44 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -598,20 +598,6 @@ drop_stash () {
clear_stash
 }
 
-apply_to_branch () {
-   test -n "$1" || die "$(gettext "No branch name specified")"
-   branch=$1
-   shift 1
-
-   set -- --index "$@"
-   assert_stash_like "$@"
-
-   git checkout -b $branch $REV^ &&
-   apply_stash "$@" && {
-   test -z "$IS_STASH_REF" || drop_stash "$@"
-   }
-}
-
 test "$1" = "-p" && set "push" "$@"
 
 PARSE_CACHE='--not-parsed'
@@ -672,7 +658,8 @@ pop)
;;
 branch)
shift
-   apply_to_branch "$@"
+   cd "$START_DIR"
+   git stash--helper branch "$@"
;;
 *)
case $# in
-- 
2.16.3

[PATCH v4 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Taylor Blau]

`git config` has long allowed the ability for callers to provide a 'type
specifier', which instructs `git config` to (1) ensure that incoming
values are satisfiable under that type, and (2) that outgoing values are
canonicalized under that type.

In another series, we propose to add extend this functionality with
`--color` and `--default` to replace `--get-color`.

However, we traditionally use `--color` to mean "colorize this output",
instead of "this value should be treated as a color".

Currently, `git config` does not support this kind of colorization, but
we should be careful to avoid inhabiting this option too soon, so that
`git config` can support `--color` (in the traditional sense) in the
future, if that is desired.

In this patch, we prefer `--type=[int|bool|bool-or-int|...]` over
`--int`, `--bool`, and etc. This allows the aforementioned upcoming
patch to support querying a color value with a default via `--type=color
--default=`

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt | 66 +++-
 builtin/config.c | 28 +++
 t/t1300-repo-config.sh   | 18 ++
 3 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e09ed5d7d..c7e56e3fd 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,13 +9,13 @@ git-config - Get and set repository or global options
 SYNOPSIS
 
 [verse]
-'git config' [] [type] [--show-origin] [-z|--null] name [value 
[value_regex]]
-'git config' [] [type] --add name value
-'git config' [] [type] --replace-all name value [value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get-all name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] [--name-only] 
--get-regexp name_regex [value_regex]
-'git config' [] [type] [-z|--null] --get-urlmatch name URL
+'git config' [] [--type] [--show-origin] [-z|--null] name [value 
[value_regex]]
+'git config' [] [--type] --add name value
+'git config' [] [--type] --replace-all name value [value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] --get name 
[value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] --get-all 
name [value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] 
[--name-only] --get-regexp name_regex [value_regex]
+'git config' [] [--type] [-z|--null] --get-urlmatch name URL
 'git config' [] --unset name [value_regex]
 'git config' [] --unset-all name [value_regex]
 'git config' [] --rename-section old_name new_name
@@ -38,12 +38,10 @@ 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 may be given as an argument to `--type` to make 'git config'
+ensure that the variable(s) are of the given type and convert the value to the
+canonical form. If no type specifier is passed, no checks or transformations 
are
+performed on the value.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
@@ -160,30 +158,34 @@ See also <>.
 --list::
List all variables set in config file, along with their values.
 
---bool::
-   'git config' will ensure that the output is "true" or "false"
+--type ::
+  'git config' will ensure that any input output is valid under the given type
+  constraint(s), and will canonicalize outgoing values in ``'s canonical
+  form.
++
+Valid ``'s include:
++
+- 'bool': canonicalize values as either "true" or "false".
+- 'int': canonicalize values as simple decimal numbers. An optional suffix of
+  'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or
+  1073741824 prior to output.
+- 'bool-or-int': canonicalize according to either 'bool' or 'int', as described
+  above.
+- 'path': canonicalize by adding a leading `~` to the value of `$HOME` and
+  `~user` to the home directory for the specified user. This specifier has no
+  effect when setting the value (but you can use `git config section.variable
+  ~/` from the command line to let your shell do the expansion.)
+- 'expiry-date': canonicalize by converting from a fixed or relative 
date-string
+  to a timestamp. This specifier has no effect when setting the value.
++
 
+--bool::
 --int::
-   'git config' will ensure that the output is a simple
-   decimal number.  

[PATCH v4 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Taylor Blau]

`git config` has long allowed the ability for callers to provide a 'type
specifier', which instructs `git config` to (1) ensure that incoming
values are satisfiable under that type, and (2) that outgoing values are
canonicalized under that type.

In another series, we propose to add extend this functionality with
`--color` and `--default` to replace `--get-color`.

However, we traditionally use `--color` to mean "colorize this output",
instead of "this value should be treated as a color".

Currently, `git config` does not support this kind of colorization, but
we should be careful to avoid inhabiting this option too soon, so that
`git config` can support `--color` (in the traditional sense) in the
future, if that is desired.

In this patch, we prefer `--type=[int|bool|bool-or-int|...]` over
`--int`, `--bool`, and etc. This allows the aforementioned upcoming
patch to support querying a color value with a default via `--type=color
--default=`

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt | 66 +++-
 builtin/config.c | 28 +++
 t/t1300-repo-config.sh   | 18 ++
 3 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e09ed5d7d..c7e56e3fd 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,13 +9,13 @@ git-config - Get and set repository or global options
 SYNOPSIS
 
 [verse]
-'git config' [] [type] [--show-origin] [-z|--null] name [value 
[value_regex]]
-'git config' [] [type] --add name value
-'git config' [] [type] --replace-all name value [value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get-all name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] [--name-only] 
--get-regexp name_regex [value_regex]
-'git config' [] [type] [-z|--null] --get-urlmatch name URL
+'git config' [] [--type] [--show-origin] [-z|--null] name [value 
[value_regex]]
+'git config' [] [--type] --add name value
+'git config' [] [--type] --replace-all name value [value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] --get name 
[value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] --get-all 
name [value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] 
[--name-only] --get-regexp name_regex [value_regex]
+'git config' [] [--type] [-z|--null] --get-urlmatch name URL
 'git config' [] --unset name [value_regex]
 'git config' [] --unset-all name [value_regex]
 'git config' [] --rename-section old_name new_name
@@ -38,12 +38,10 @@ 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 may be given as an argument to `--type` to make 'git config'
+ensure that the variable(s) are of the given type and convert the value to the
+canonical form. If no type specifier is passed, no checks or transformations 
are
+performed on the value.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
@@ -160,30 +158,34 @@ See also <>.
 --list::
List all variables set in config file, along with their values.
 
---bool::
-   'git config' will ensure that the output is "true" or "false"
+--type ::
+  'git config' will ensure that any input output is valid under the given type
+  constraint(s), and will canonicalize outgoing values in ``'s canonical
+  form.
++
+Valid ``'s include:
++
+- 'bool': canonicalize values as either "true" or "false".
+- 'int': canonicalize values as simple decimal numbers. An optional suffix of
+  'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or
+  1073741824 prior to output.
+- 'bool-or-int': canonicalize according to either 'bool' or 'int', as described
+  above.
+- 'path': canonicalize by adding a leading `~` to the value of `$HOME` and
+  `~user` to the home directory for the specified user. This specifier has no
+  effect when setting the value (but you can use `git config section.variable
+  ~/` from the command line to let your shell do the expansion.)
+- 'expiry-date': canonicalize by converting from a fixed or relative 
date-string
+  to a timestamp. This specifier has no effect when setting the value.
++
 
+--bool::
 --int::
-   'git config' will ensure that the output is a simple
-   decimal number.  

[PATCH v4 1/2] builtin/config.c: treat type specifiers singularly

[Taylor Blau]

Internally, we represent `git config`'s type specifiers as a bitset
using OPT_BIT. 'bool' is 1<<0, 'int' is 1<<1, and so on. This technique
allows for the representation of multiple type specifiers in the `int
types` field, but this multi-representation is left unused.

In fact, `git config` will not accept multiple type specifiers at a
time, as indicated by:

  $ git config --int --bool some.section
  error: only one type at a time.

This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type
specifier, so that the above command would instead be valid, and a
synonym of:

  $ git config --bool some.section

This change is motivated by two urges: (1) it does not make sense to
represent a singular type specifier internally as a bitset, only to
complain when there are multiple bits in the set. `OPT_SET_INT` is more
well-suited to this task than `OPT_BIT` is. (2) a future patch will
introduce `--type=`, and we would like not to complain in the
following situation:

  $ git config --int --type=int

Signed-off-by: Taylor Blau 
---
 builtin/config.c   | 49 +++---
 t/t1300-repo-config.sh | 11 ++
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 01169dd62..92fb8d56b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -25,7 +25,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 int actions, type;
 static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
@@ -55,11 +55,11 @@ static int show_origin;
 #define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \
ACTION_GET_REGEXP | ACTION_GET_URLMATCH)
 
-#define TYPE_BOOL (1<<0)
-#define TYPE_INT (1<<1)
-#define TYPE_BOOL_OR_INT (1<<2)
-#define TYPE_PATH (1<<3)
-#define TYPE_EXPIRY_DATE (1<<4)
+#define TYPE_BOOL  1
+#define TYPE_INT   2
+#define TYPE_BOOL_OR_INT   3
+#define TYPE_PATH  4
+#define TYPE_EXPIRY_DATE   5
 
 static struct option builtin_config_options[] = {
OPT_GROUP(N_("Config file location")),
@@ -84,11 +84,11 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "get-color", , N_("find the color configured: slot 
[default]"), ACTION_GET_COLOR),
OPT_BIT(0, "get-colorbool", , N_("find the color setting: slot 
[stdout-is-tty]"), ACTION_GET_COLORBOOL),
OPT_GROUP(N_("Type")),
-   OPT_BIT(0, "bool", , N_("value is \"true\" or \"false\""), 
TYPE_BOOL),
-   OPT_BIT(0, "int", , N_("value is decimal number"), TYPE_INT),
-   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_SET_INT(0, "bool", , N_("value is \"true\" or \"false\""), 
TYPE_BOOL),
+   OPT_SET_INT(0, "int", , N_("value is decimal number"), TYPE_INT),
+   OPT_SET_INT(0, "bool-or-int", , N_("value is --bool or --int"), 
TYPE_BOOL_OR_INT),
+   OPT_SET_INT(0, "path", , N_("value is a path (file or directory 
name)"), TYPE_PATH),
+   OPT_SET_INT(0, "expiry-date", , N_("value is an expiry date"), 
TYPE_EXPIRY_DATE),
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")),
@@ -149,26 +149,26 @@ static int format_config(struct strbuf *buf, const char 
*key_, const char *value
if (show_keys)
strbuf_addch(buf, key_delim);
 
-   if (types == TYPE_INT)
+   if (type == TYPE_INT)
strbuf_addf(buf, "%"PRId64,
git_config_int64(key_, value_ ? value_ : 
""));
-   else if (types == TYPE_BOOL)
+   else if (type == TYPE_BOOL)
strbuf_addstr(buf, git_config_bool(key_, value_) ?
  "true" : "false");
-   else if (types == TYPE_BOOL_OR_INT) {
+   else if (type == TYPE_BOOL_OR_INT) {
int is_bool, v;
v = git_config_bool_or_int(key_, value_, _bool);
if (is_bool)
strbuf_addstr(buf, v ? "true" : "false");
else
strbuf_addf(buf, "%d", v);
-   } else if (types == TYPE_PATH) {
+   } else if (type == TYPE_PATH) {
const char *v;
if (git_config_pathname(, key_, value_) < 0)
return -1;
strbuf_addstr(buf, v);
free((char 

[PATCH v4 0/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Taylor Blau]

Hi,

I have attached a fourth re-roll of my series to introduce
"--type=" in "git config", and prefer it to "--".

In particular, since the last update, I have changed the following:

  - Clearer wording in the second patch per Eric's suggestion.

  - Stopped spelling the required argument to "--type=" as "[type]", and
instead as "" (cc: Eric).

  - Changed "unexpected" to "unrecognized" in the fatal message when we
don't know how to interpret the argument to "--type".

Thanks,
Taylor

Taylor Blau (2):
  builtin/config.c: treat type specifiers singularly
  builtin/config.c: prefer `--type=bool` over `--bool`, etc.

 Documentation/git-config.txt | 66 ---
 builtin/config.c | 77 +++-
 t/t1300-repo-config.sh   | 29 ++
 3 files changed, 113 insertions(+), 59 deletions(-)

--
2.16.2.440.gc6284da4f

Re: [PATCH v3 1/2] builtin/config.c: treat type specifiers singularly

[Taylor Blau]

On Wed, Apr 04, 2018 at 03:57:12AM -0400, Eric Sunshine wrote:
> On Wed, Apr 4, 2018 at 2:07 AM, Taylor Blau  wrote:
> > [...]
> > In fact, `git config` will not accept multiple type specifiers at a
> > time, as indicated by:
> >
> >   $ git config --int --bool some.section
> >   error: only one type at a time.
> >
> > This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type
> > specifier, so that the above command would instead be valid, and a
> > synonym of:
> >
> >   $ git config --bool some.section
> >
> > This change is motivated by two urges: (1) it does not make sense to
> > represent a singular type specifier internally as a bitset, only to
> > complain when there are multiple bits in the set. `OPT_SET_INT` is more
> > well-suited to this task than `OPT_BIT` is. (2) a future patch will
> > introduce `--type=`, and we would like not to complain in the
> > following situation:
> >
> >   $ git config --int --type=int
>
> I can understand "last wins" for related options such as "--verbose
> --quiet" or "--verbose=1 --verbose=2", but I have a lot of trouble
> buying the "last wins" argument in this context where there is no
> semantic relationship between, say, --bool and --expiry-date.
> Specifying conflicting options together is likely to be unintentional,
> thus reporting it as an error seems sensible, so I'm not convinced
> that patch's removal of that error is a good idea.
>
> I understand that you're doing this to avoid complaining about "--int
> --type=int", but exactly how that case is supported should be an
> implementation detail; it doesn't need to bleed into the UI as an
> unnecessary and not-well-justified loosening of an age-old
> restriction. There are other ways to support "--int --type=int"
> without "last wins". Also, do we really need to support "--int
> --type=int"? Is that an expected use-case?

This is a fair concern, though I view the problem slightly differently.
I see this change as being motivated by the fact that we are adding
"--type", not removing an age-old restriction.

Peff's motivation for this--as I understand it--is that "--type=int"
should be a _true_ synonym for "--int". Adopting the old-style
"OPT_SET_BIT" for this purpose makes "--type=int" and "--int" _mostly_ a
synonym for one another, except that "--type=bool --type=int" will not
complain, whereas "--bool --int" would.

Loosening this restriction, in my view, brings us closer (1) to the new
semantics of multiple "--type"'s, and (2) to the existing semantics of
"--verbose=1 --verbose=2" as you mentioned above.

I would like to hear Peff's take on this as well, since he suggested the
patch originally, and would likely provide the clearest insight into
this.


Thanks,
Taylor

Re: [PATCH v3 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Taylor Blau]

On Wed, Apr 04, 2018 at 03:27:48AM -0400, Eric Sunshine wrote:
> On Wed, Apr 4, 2018 at 2:07 AM, Taylor Blau  wrote:
> > In this patch, we prefer `--type=[int|bool|bool-or-int|...]` over
> > `--int`, `--bool`, and etc. This allows the aforementioned other patch
> > to add `--color` (in the non-traditional sense) via `--type=color`,
> > instead of `--color`.
>
> I always find this last sentence confusing since it's not clear to
> which ilk of "--color" option you refer. Perhaps say instead something
> like:
>
> This normalization will allow the aforementioned upcoming patch to
> support querying a color value with a default via "--type=color
> --default=...".

I agree that this is much clearer. I have amended this patch to include
your wording here.

> > Signed-off-by: Taylor Blau 
> > ---
> > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> > @@ -160,30 +158,34 @@ See also <>.
> > +--type [type]::
> > +  'git config' will ensure that any input output is valid under the given 
> > type
> > +  constraint(s), and will canonicalize outgoing values in `[type]`'s 
> > canonical
> > +  form.
>
> Do the brackets in "[type]" mean that the argument is optional? If so,
> what does 'type' default to when not specified? The documentation
> should discuss this.

This is my mistake; I was unaware of the semantic difference between '[]'
and '<>'. If my understanding is correct that '<>' means "required" and
'[]' means "optional", than this is a misspelling of "".

I have addressed this during the re-roll.

> > diff --git a/builtin/config.c b/builtin/config.c
> > @@ -61,6 +61,33 @@ static int show_origin;
> > +static int option_parse_type(const struct option *opt, const char *arg,
> > +int unset)
> > +{
> > +   [...]
> > +   if (!strcmp(arg, "bool"))
> > +   *type = TYPE_BOOL;
> > +   else if (!strcmp(arg, "int"))
> > +   *type = TYPE_INT;
> > +   else if (!strcmp(arg, "bool-or-int"))
> > +   *type = TYPE_BOOL_OR_INT;
> > +   else if (!strcmp(arg, "path"))
> > +   *type = TYPE_PATH;
> > +   else if (!strcmp(arg, "expiry-date"))
> > +   *type = TYPE_EXPIRY_DATE;
> > +   else {
> > +   die(_("unexpected --type argument, %s"), arg);
>
> "unexpected" doesn't seem like the best word choice since an argument
> to --type _is_ expected. Perhaps you mean "unrecognized"?

Sure; I think "unrecognized" is a much better fit. I have updated this
in the re-roll.

Thanks,
Taylor

[PATCH v10] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote, based on the one from
for-each-ref. This e.g. allows ref names to be sorted by version
semantics, so that v1.2 is sorted before v1.10.

Signed-off-by: Harald Nordgren 
---

Notes:
Use 'ref_array_item_push' to fix 'REALLOC_ARRAY' call in ref-filter

 Documentation/git-ls-remote.txt | 15 ++-
 builtin/ls-remote.c | 20 ++--
 ref-filter.c| 20 ++--
 ref-filter.h|  2 ++
 t/t5512-ls-remote.sh| 41 -
 5 files changed, 92 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..fa4505fd7 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,19 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`,
+   except that because `ls-remote` deals only with remotes, keys like
+   `committerdate` that require access to the objects themselves will
+   not work.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..f87b2657c 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
int show_symref_target = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
+   int i;
 
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -108,9 +116,17 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
continue;
if (!tail_match(pattern, ref->name))
continue;
+   ref_array_push(, ref);
+   }
+
+   if (sorting)
+   ref_array_sort(sorting, );
+
+   for (i = 0; i < array.nr; i++) {
+   const struct ref_array_item *ref = array.items[i];
if (show_symref_target && ref->symref)
-   printf("ref: %s\t%s\n", ref->symref, ref->name);
-   printf("%s\t%s\n", oid_to_hex(>old_oid), ref->name);
+   printf("ref: %s\t%s\n", ref->symref, ref->refname);
+   printf("%s\t%s\n", oid_to_hex(>objectname), ref->refname);
status = 0; /* we found something */
}
return status;
diff --git a/ref-filter.c b/ref-filter.c
index 45fc56216..6dbafba07 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1861,6 +1861,23 @@ static int ref_kind_from_refname(const char *refname)
return FILTER_REFS_OTHERS;
 }
 
+void ref_array_item_push(struct ref_array 

[PATCH v9] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote, based on the one from
for-each-ref. This e.g. allows ref names to be sorted by version
semantics, so that v1.2 is sorted before v1.10.

Signed-off-by: Harald Nordgren 
---

Notes:
Create 'ref_array_push' function in ref-filter

 Documentation/git-ls-remote.txt | 15 ++-
 builtin/ls-remote.c | 20 ++--
 ref-filter.c| 12 
 ref-filter.h|  2 ++
 t/t5512-ls-remote.sh| 41 -
 5 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..fa4505fd7 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,19 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`,
+   except that because `ls-remote` deals only with remotes, keys like
+   `committerdate` that require access to the objects themselves will
+   not work.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..f87b2657c 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
int show_symref_target = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
+   int i;
 
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -108,9 +116,17 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
continue;
if (!tail_match(pattern, ref->name))
continue;
+   ref_array_push(, ref);
+   }
+
+   if (sorting)
+   ref_array_sort(sorting, );
+
+   for (i = 0; i < array.nr; i++) {
+   const struct ref_array_item *ref = array.items[i];
if (show_symref_target && ref->symref)
-   printf("ref: %s\t%s\n", ref->symref, ref->name);
-   printf("%s\t%s\n", oid_to_hex(>old_oid), ref->name);
+   printf("ref: %s\t%s\n", ref->symref, ref->refname);
+   printf("%s\t%s\n", oid_to_hex(>objectname), ref->refname);
status = 0; /* we found something */
}
return status;
diff --git a/ref-filter.c b/ref-filter.c
index 45fc56216..a5686dacd 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1861,6 +1861,18 @@ static int ref_kind_from_refname(const char *refname)
return FILTER_REFS_OTHERS;
 }
 
+void ref_array_push(struct ref_array *array, const struct ref *ref)
+{
+  

Re: [PATCH v8] ls-remote: create '--sort' option

[Harald Nordgren]

Without digging to much into the `ref-filter` code itself, it seems
like there is an opportunity to generalize and unfify the logic
between these two cases. As well as using `ALLOC_GROW`. But maybe that
is best left as a follow-up task? Especially since this patch focuses
on `ls-remote`. Seems possibly like too big of a change to start
changing a different sub-command.

Wouldn't a `ref_array_push()` also require `ref->symref`, maybe then
we could pass the whole ref? It needs to be very clear that it's a
`ref` and not a `ref_array_item` that is being pushed. Much of my
logic here deals specifically with trying to treat a ref as
ref_array_item.

>From my viewpoint as implementer, I was very happy that I could
implement the feature *without* invoking `filter_refs` since that
`filter->kind` switching looks a pretty daunting. I'm not exactly sure
what a `git ls-remote --contains HEAD` would do, maybe you could
explain a bit more?

On Thu, Apr 5, 2018 at 1:01 AM, Harald Nordgren
 wrote:
> Create a '--sort' option for ls-remote, based on the one from
> for-each-ref. This e.g. allows ref names to be sorted by version
> semantics, so that v1.2 is sorted before v1.10.
>
> Signed-off-by: Harald Nordgren 
> ---
>
> Notes:
> Partial fixes from Jeff King's comments
>
>  Documentation/git-ls-remote.txt | 15 ++-
>  builtin/ls-remote.c | 27 +--
>  t/t5512-ls-remote.sh| 41 
> -
>  3 files changed, 79 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 5f2628c8f..fa4505fd7 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  
>  [verse]
>  'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
> - [-q | --quiet] [--exit-code] [--get-url]
> + [-q | --quiet] [--exit-code] [--get-url] [--sort=]
>   [--symref] [ [...]]
>
>  DESCRIPTION
> @@ -60,6 +60,19 @@ OPTIONS
> upload-pack only shows the symref HEAD, so it will be the only
> one shown by ls-remote.
>
> +--sort=::
> +   Sort based on the key given.  Prefix `-` to sort in
> +   descending order of the value. You may use the --sort= option
> +   multiple times, in which case the last key becomes the primary
> +   key. Also supports "version:refname" or "v:refname" (tag
> +   names are treated as versions). The "version:refname" sort
> +   order can also be affected by the "versionsort.suffix"
> +   configuration variable.
> +   The keys supported are the same as those in `git for-each-ref`,
> +   except that because `ls-remote` deals only with remotes, keys like
> +   `committerdate` that require access to the objects themselves will
> +   not work.
> +
>  ::
> The "remote" repository to query.  This parameter can be
> either a URL or the name of a remote (see the GIT URLS and
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 540d56429..fbec2bc95 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -1,6 +1,7 @@
>  #include "builtin.h"
>  #include "cache.h"
>  #include "transport.h"
> +#include "ref-filter.h"
>  #include "remote.h"
>
>  static const char * const ls_remote_usage[] = {
> @@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
> int show_symref_target = 0;
> const char *uploadpack = NULL;
> const char **pattern = NULL;
> +   int i;
>
> struct remote *remote;
> struct transport *transport;
> const struct ref *ref;
> +   struct ref_array array;
> +   static struct ref_sorting *sorting = NULL, **sorting_tail = 
>
> struct option options[] = {
> OPT__QUIET(, N_("do not print remote URL")),
> @@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
> OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
> REF_NORMAL),
> OPT_BOOL(0, "get-url", _url,
>  N_("take url..insteadOf into account")),
> +   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
> +N_("field name to sort on"), 
> _opt_ref_sorting),
> OPT_SET_INT_F(0, "exit-code", ,
>   N_("exit with exit code 2 if no matching refs 
> are found"),
>   2, PARSE_OPT_NOCOMPLETE),
> @@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
> OPT_END()
> };
>
> +   memset(, 0, sizeof(array));
> +
> argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
>  PARSE_OPT_STOP_AT_NON_OPTION);
> dest = argv[0];
> @@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const 

[PATCH v8] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote, based on the one from
for-each-ref. This e.g. allows ref names to be sorted by version
semantics, so that v1.2 is sorted before v1.10.

Signed-off-by: Harald Nordgren 
---

Notes:
Partial fixes from Jeff King's comments

 Documentation/git-ls-remote.txt | 15 ++-
 builtin/ls-remote.c | 27 +--
 t/t5512-ls-remote.sh| 41 -
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..fa4505fd7 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,19 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`,
+   except that because `ls-remote` deals only with remotes, keys like
+   `committerdate` that require access to the objects themselves will
+   not work.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..fbec2bc95 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
int show_symref_target = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
+   int i;
 
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
for ( ; ref; ref = ref->next) {
+   struct ref_array_item *item;
if (!check_ref_type(ref, flags))
continue;
if (!tail_match(pattern, ref->name))
continue;
+
+   FLEX_ALLOC_STR(item, refname, ref->name);
+   item->symref = xstrdup_or_null(ref->symref);
+   oidcpy(>objectname, >old_oid);
+
+   ALLOC_GROW(array.items, array.nr + 1, array.alloc);
+   array.items[array.nr++] = item;
+   }
+
+   if (sorting)
+   ref_array_sort(sorting, );
+
+   for (i = 0; i < array.nr; i++) {
+   const struct ref_array_item *ref = array.items[i];
if (show_symref_target && ref->symref)
-   printf("ref: %s\t%s\n", ref->symref, ref->name);
-   printf("%s\t%s\n", oid_to_hex(>old_oid), ref->name);
+   printf("ref: %s\t%s\n", ref->symref, ref->refname);
+   printf("%s\t%s\n", oid_to_hex(>objectname), ref->refname);

GREAT NEWS!!

[Amnesty International]

We have a great news about your E-mail address!!! 

You Won  $950,500.00 USD on Amnesty International 
Kenya online E-mail Promotion. For more details 
about your prize claims, file with the following;

Names: 
Country: 
Tel:

Regards,
Mr. David Ford

GREAT NEWS!!

[Amnesty International]

We have a great news about your E-mail address!!! 

You Won  $950,500.00 USD on Amnesty International 
Kenya online E-mail Promotion. For more details 
about your prize claims, file with the following;

Names: 
Country: 
Tel:

Regards,
Mr. David Ford

Re: Timing issue in t5570 "daemon log records all attributes"

[Jan Palus]

On 03.04.2018 16:32, Jeff King wrote:
> I'm tempted to say we should just scrap this test. It was added
> relatively recently and only shows the fix for a pretty minor bug.
> It's probably not worth the contortions to make it race-proof.

Thanks for your reply Jeff.

It appears race condition in reading/writing daemon.log is not the only issue of
t5570. On a different machine I've just randomly got:

t5570-git-daemon.sh[163]: can't create git_daemon_output: Interrupted system 
call

which I believe might also be associated with concurrent processing of piped
data connected with a fact that test restarts daemon few times. I can barely
wrap my head around it but I guess it's somewhat around "shell still tries to
read fifo while attempt to create new one is made".


Regards
Jan

Re: [RFC PATCH v3 2/2] t7406: add test for non-default branch in submodule

[Stefan Beller]

On Wed, Apr 4, 2018 at 1:37 PM, Eddy Petrișor  wrote:

>> This patch only contains the test, I presume this goes on top of
>> https://public-inbox.org/git/20180403222053.23132-1-eddy.petri...@codeaurora.org/
>> which you plan to later submit as one patch including both the change as 
>> well as
>> the test.
>
> Yes, not sure why the emails were not linked together, I specified the
> in-reply-to mesage id when I "git format-patch"-ed the patches.

They are linked in public-inbox, but I noticed too late. (in gmail
they were not)

> Thanks for the pointer, I only looked at the post-failure state after
> adding --debug -i --verbose, but having "test_pause" to stop and
> inspect the interim stage should help a lot with debugging.

I also like the -x flag that stops at the failing test, but as this is the
last test of this script it is of limited use here.

> After looking at other tests I was under the impression that the setup
> phase (e.g. creating the "server" side repos) of the test was done in
> the main context, while the test case (i.e. the client side, or what
> the user would do) is in subshells.

This roughly coincides, as the client is usually in a new clone. :)

>> style: The Git test suite prefers to have the redirect adjacent to the
>> file name:
>>   echo hello >world
>
> I wasn't aware of that, it seems there are some inconsistencies,
> including in the modified test:
>
> eddy@feodora:~/usr/src/git/t$ grep '> ' -c t* 2>/dev/null | grep -v
> ':0$' | grep 7406
> t7406-submodule-update.sh:24
> eddy@feodora:~/usr/src/git/t$ grep '> ' -c t* 2>/dev/null | grep -v
> ':0$' | wc -l
> 325

Thanks for digging up numbers!

> I suspect that a minor patch correcting these inconsistencies would be
> faily fast reviewed adn accepted, of course, disconnected from this
> modification.

There are two schools of thought, one of them is to fix things up
whenever you see fit. I am very sympathetic to this one.

The other is found in Documentation/CodingGuidelines:

 - Fixing style violations while working on a real change as a
   preparatory clean-up step is good, but otherwise avoid useless code
   churn for the sake of conforming to the style.

   "Once it _is_ in the tree, it's not really worth the patch noise to
   go and fix it up."
   Cf. http://lkml.iu.edu/hypermail/linux/kernel/1001.3/01069.html

> Yes, I was trying to replicate the redox-os case which has this structure:
>
> redox-os (master branch)
>+ rust (@some commit on a non-default)
>   + src/llvm (@some commit on a non-default branch)
>
> This section is making sure the b2 branch has other content than the
> default one and setting the expectation for level2 submodule branch,
> later.

Oh, cool!

>
 +   git rev-parse --verify HEAD >../expectl1 &&
 +   git checkout master &&
>>
>> We go back to master, which doesn't have the nested submodule?
>
> exactly, since the desired behaviour is to have in the nested
> submodule the b2 branch.

Well if the nested submodule doesn't exist at HEAD, then git should
not change branches in there, only on checking out/ updating to a state
that has the nested sub? (I may missunderstand things here)

> I guess I could have added the level2 submodule@master on l1's master
> branch, but I didn't see much point in that since it should be enough
> now.
>
> It might make more sense if master and b2, and, respectively master
> and l1 have diverging histories, but for now that is probably overkill
> and it will make sense in the context of shallow cloning of
> submodules, which I haven't yet tackled except presenting the idea.

ok.

>
 +   cd ../super5 &&
 +   echo super_with_2_chained_modules > super5 &&
 +   git add super5 &&
 +   test_tick &&
 +   git commit -m "commit on default branch in super5" &&
 +   git submodule add ../submodl1b1 submodl1b1 &&
 +   git config -f .gitmodules submodule."submodl1b1".branch b1 &&
 +   git add .gitmodules &&
 +   test_tick &&
 +   git commit -m "add l1 module with branch b1 in super5" &&
>>
>> now we do this again without the nested submodule, just one repo
>> as a submodule?
>
> My intention was to have super5 -> submodl1b1@b1 -> submodl2b2@b2 on
> the "server" side.
> But are you saying I just implemented super5 -> sbmodl1b1@master due
> to the previous master checkout in submodl1b1?

No. I was a little confused about the code.

>
 +   git submodule init submodl1b1 &&
 +   git clone super5 super &&
>>
>> does super exist here already? (I did not check, but IIRC
>> super and super{1-4} are there as we count upwards to
>> find a name that is ok.
>
> I created it in the first step of the test with the intention to have
> super5 as the "server" and "super" as the client clone.

oh, ok.

>
>>
 +   (
 +   cd super &&
 +   git submodule update --recursive --init
 +   ) &&
 +   (
 +

Re: [PATCH] send-email: fix docs regarding storing password with git credential

[Jeff King]

On Tue, Apr 03, 2018 at 12:23:48AM +0100, Michal Nazarewicz wrote:

> First of all, ‘git credential fill’ does not store credentials
> but is used to *read* them.  The command which adds credentials
> to the helper’s store is ‘git credential approve’.

Yep, makes sense (I wish we had just called these consistently "get",
"store", and "erase" as they are in the git<->helper interface).

> Second of all, git-send-email will include port number in host
> parameter when getting the password so it has to be set when
> storing the password as well.
> 
> Apply the two above to fix the Gmail example in git-send-email
> documentation.

Makes sense. This is an interesting counter-example to my earlier "well,
it usually works out in the long run" statement. Because usually you're
relying on some part of Git to issue the "fill" and the "approve", so
whatever it uses, it will be the same. But here we're trying to
pre-seed, so we have to match what the tool will do.

On the other hand, I'm not sure why we need to pre-seed here. Wouldn't
it be sufficient to just issue a "git send-email", which would then
prompt for the password? And then you'd input your generated token,
which would get saved via the approve mechanism?

-Peff

Re: [PATCH] send-email: report host and port separately when calling git credential

[Jeff King]

On Mon, Apr 02, 2018 at 03:05:35PM -0700, Junio C Hamano wrote:

> "git help credential" mentions protocol, host, path, username and
> password (and also url which is a short-hand for setting protocol
> and host), but not "port".  And common sense tells me, when a system
> allows setting host but not port, that it would expect host:port to
> be given when the service is running a non-standard port, so from
> that point of view, I suspect that the current code is working as
> expected.  In fact, credential.h, which defines the API, does not
> have any "port" field, either, so I am not sure how this is expected
> to change anything without touching the back-end that talks over the
> pipe via _credential_run-->credential_write callchain.
> 
> Now, it is a separate matter if it were a better design if the
> credential API had 'host' and 'port' defined as separate keys to the
> authentication information.  Such an alternative design would have
> made certain things harder while some other things easier (e.g. "use
> this credential to the host no matter what port the service runs"
> may be easier to implement if 'host' and 'port' are separate).

I don't recall giving a huge amount of thought to alternate ports when
writing the credential code. But at least the osxkeychain helper does
parse "host:port" from the host field and feed it to the appropriate
keychain arguments. And I think more oblivious helpers like
credential-cache would just treat the "host" field as an opaque blob,
making the port part of the matching.

I suspect there are some corner cases, though. Reading the osxkeychain
code, I think that asking for "http://example.com:80; and
"http://example.com; would probably not get you to the same key, as we
feed port==0 in the second case. In practice, it's probably not a _huge_
deal to be overly picky, as the worst case is that you get prompted and
store the credential in a second slot (which then works going forward).

So in general I think it's OK for the whole system to err on the side of
being picky about whether two things are "the same" (which in this case
is including the port). It usually works itself out in the long run, and
we would not surprise the user with "example.com:8080 is the same as
example.com:80".

-Peff

Re: commit -> public-inbox link helper

[Johannes Schindelin]

Hi Mike,

as I said here:

On Wed, 4 Apr 2018, Mike Rappazzo wrote:

> On Wed, Apr 4, 2018 at 12:35 PM, Johannes Schindelin
>  wrote:
> >
> > [...]
> >
> > With --open, it opens the mail in a browser (macOS support is missing,
> > mainly because I cannot test: just add an `open` alternative to
> > `xdg-open`).
> >
> > [...]
> > open)
> > url=https://public-inbox.org/git/$messageid
> > case "$(uname -s)" in
> > Linux) xdg-open "$url";;
> > MINGW*|MSYS*) start "$url";;
> 
>  Darwin*) open "$url";;

I am aware of this alternative, but as I do not currently develop on macOS
apart from headless build agents, I did not add support for that.

Feel free to adopt the script, publish it in a GitHub repository, adapt it
to use refs/notes/amlog instead of a public-inbox mirror, and then tell me
where I can clone it ;-)

Ciao,
Johannes

Re: commit -> public-inbox link helper

[Johannes Schindelin]

Hi Peff,

On Wed, 4 Apr 2018, Jeff King wrote:

> On Wed, Apr 04, 2018 at 06:35:59PM +0200, Johannes Schindelin wrote:
> 
> > I found myself in dear need to quickly look up mails in the public-inbox
> > mail archive corresponding to any given commit in git.git. Some time ago,
> > I wrote a shell script to help me with that, and I found myself using it a
> > couple of times, so I think it might be useful for others, too.
> > 
> > This script (I call it lookup-commit.sh) needs to be dropped into a *bare*
> > clone of http://public-inbox.org/git, and called with its absolute or
> > relative path from a git.git worktree, e.g.
> > 
> > ~/public-inbox-git.git/lookup-commit.sh \
> > fea16b47b603e7e4fa7fca198bd49229c0e5da3d
> > 
> > This will take a while initially, or when the `master` branch of the
> > public-inbox mirror was updated, as it will generate two files with
> > plain-text mappings.
> 
> Junio publishes a git-notes ref with the mapping you want. So you can
> do:
> 
>   git fetch git://github.com/gitster/git.git refs/notes/amlog:refs/notes/amlog
>   mid=$(git notes --ref amlog show $commit | perl -lne '/<(.*)>/ and print 
> $1')
>   echo "https://public-inbox.org/git/$mid;
> 
> without having to download the gigantic list archive repo at all (though
> I do keep my own copy of the archive and index it with mairix, so I can
> use "mairix -t m:$mid" and then view the whole thing locally in mutt).

Good to know! Thanks for the script.

And thanks also for the `--ref` trick: I had a look at the man page of
git-notes, and it was not immediately obvious that it supports options
before the sub-subcommand. The `--ref` description is buried pretty deep
in there.

Thanks,
Dscho

Re: What's cooking in git.git (Mar 2018, #06; Fri, 30)

[Jeff King]

On Sat, Mar 31, 2018 at 08:41:11PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > [...]
> > * jk/drop-ancient-curl (2017-08-09) 5 commits
> >  - http: #error on too-old curl
> >  - curl: remove ifdef'd code never used with curl >=7.19.4
> >  - http: drop support for curl < 7.19.4
> >  - http: drop support for curl < 7.16.0
> >  - http: drop support for curl < 7.11.1
> >
> >  Some code in http.c that has bitrot is being removed.
> >
> >  Expecting a reroll.
> 
> This has been idle for a long time. Peff: What's left to be done for it?

It wasn't clear to me we actually wanted to do this. It got some
complaints, and then somebody showed up to actually fix the compilation
problems with the old versions.

It also isn't that much of a burden to carry the #ifdefs. The main
question is whether we're doing a disservice to users, since those old
setups likely aren't well tested.

Even if we do proceed, I'm not sure if we'd want the top #error patch,
given the reports that distros will sometimes backport features.

-Peff

Re: [RFC PATCH v3 2/2] t7406: add test for non-default branch in submodule

[Eddy Petrișor]

2018-04-04 21:36 GMT+03:00 Stefan Beller :
> On Tue, Apr 3, 2018 at 3:26 PM, Eddy Petrișor  wrote:
>> Notes:
>> I am aware this test is not probably the best one and maybe I
>> should have first one test that does a one level non-default, before
>> trying a test with 2 levels of submodules, but I wanted to express the
>> goal of the patch.
>
> This patch only contains the test, I presume this goes on top of
> https://public-inbox.org/git/20180403222053.23132-1-eddy.petri...@codeaurora.org/
> which you plan to later submit as one patch including both the change as well 
> as
> the test.

Yes, not sure why the emails were not linked together, I specified the
in-reply-to mesage id when I "git format-patch"-ed the patches.

I wasn't actually planning to squash them in a single patch, will do,
but I guess for now it helps to focus the discussion around the test
since the implementation is still lacking, see below 2 lines comment.

>> Currently the test fails, so I am obviously missing something.
>> Help would be appreciated.
>>
>>
>> 2018-04-04 1:20 GMT+03:00 Eddy Petrișor :
>>> From: Eddy Petrișor 
[..]
>>> --- a/t/t7406-submodule-update.sh
>>> +++ b/t/t7406-submodule-update.sh
>>> @@ -259,6 +259,60 @@ test_expect_success 'submodule update --remote should 
>>> fetch upstream changes wit
>>> )
>>>  '
>>>
>>> +test_expect_success 'submodule update --remote --recursive --init should 
>>> fetch module branch from .gitmodules' '
>>> +   git clone . super5 &&
>
> I found adding "test_pause &&"
> to be a great helper as it will drop you in a shell where
> you can inspect the repository.

Thanks for the pointer, I only looked at the post-failure state after
adding --debug -i --verbose, but having "test_pause" to stop and
inspect the interim stage should help a lot with debugging.

>
>>> +   git clone super5 submodl2b2 &&
>>> +   git clone super5 submodl1b1 &&
>
> We may want to cleanup after the test is done:
>
>   test_when_finished "rm submodl2b2" &&
>   test_when_finished "rm submodl1b2" &&

Sure, will do.

>>> +   cd submodl2b2 &&
>
> I'd think the test suite prefers subshells to operate in other dirs
> instead of direct cd's, just like at the end of the test.

After looking at other tests I was under the impression that the setup
phase (e.g. creating the "server" side repos) of the test was done in
the main context, while the test case (i.e. the client side, or what
the user would do) is in subshells.

> For short parts, we make heavy use of the -C option,
> So for example the code below
>(
>cd super &&
>git submodule update --recursive --init
>) &&
>
> can be written as
>
> git -C super submodule update --recursive --init
>
> which is shorter.

Nice.

>>> +   echo linel2b2 > l2b2 &&
>
> style: The Git test suite prefers to have the redirect adjacent to the
> file name:
>   echo hello >world

I wasn't aware of that, it seems there are some inconsistencies,
including in the modified test:

eddy@feodora:~/usr/src/git/t$ grep '> ' -c t* 2>/dev/null | grep -v
':0$' | grep 7406
t7406-submodule-update.sh:24
eddy@feodora:~/usr/src/git/t$ grep '> ' -c t* 2>/dev/null | grep -v
':0$' | wc -l
325

I suspect that a minor patch correcting these inconsistencies would be
faily fast reviewed adn accepted, of course, disconnected from this
modification.

>>> +   git checkout -b b2 &&
>>> +   git add l2b2 &&
>>> +   test_tick &&
>>> +   git commit -m "commit on b2 branch in l2" &&
>>> +   git rev-parse --verify HEAD >../expectl2 &&
>
> So until now we made a commit in a submodule on branch b2
> and wrote it out to an expect file.

Yes, I was trying to replicate the redox-os case which has this structure:

redox-os (master branch)
   + rust (@some commit on a non-default)
  + src/llvm (@some commit on a non-default branch)

This section is making sure the b2 branch has other content than the
default one and setting the expectation for level2 submodule branch,
later.

>>> +   git checkout master &&
>>> +   cd ../submodl1b1 &&
>>> +   git checkout -b b1 &&
>>> +   echo linel1b1 > l1b1 &&
>>> +   git add l1b1 &&
>>> +   test_tick &&
>>> +   git commit -m "commit on b1 branch in l1" &&
>
> very similar to above, just in another repo
> instead of making a commit yourself, you may want to use
> test_commit 
> then you don't need to call echo/add/commit yourself.

thanks for the pointer, will change that since my purpose for the
commit was to make sure default branch (master) and b1 branch are
different

>>> +   git submodule add ../submodl2b2 submodl2b2 &&
>>> +   git config -f .gitmodules submodule."submodl2b2".branch b2 &&
>>> +   git add .gitmodules &&
>>> +   test_tick &&
>>> +   git commit -m "add l2 module with branch b2 in l1 module in branch 
>>> b1" &&
>
> So one 

Re: [PATCH] Change permissions on git-completion.bash

[Ævar Arnfjörð Bjarmason]

On Wed, Apr 04 2018, Stephon Harris wrote:

> Updating git-completion.bash to include instructions to change the
> permissions on the file when sourcing it in your .bashrc or .zshrc
> file.

But why is this needed? Files sourced by shells don't need to be
executable, and quoting the bash manual "The file searched for in PATH
need not be executable.".

What breaks for you / doesn't work because it doesn't have the +x bit?

[PATCH] Change permissions on git-completion.bash

[Stephon Harris]

Updating  git-completion.bash to include instructions to change the permissions 
on the file when sourcing it in your .bashrc or .zshrc file.
---
 contrib/completion/git-completion.bash | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index b09c8a23626b4..fcd76cf169559 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -18,9 +18,10 @@
 # To use these routines:
 #
 #1) Copy this file to somewhere (e.g. ~/.git-completion.bash).
-#2) Add the following line to your .bashrc/.zshrc:
+#2) Change the permissions of the file to be executable (e.g. chmod u+x  
~/.git-completion.bash)
+#3) Add the following line to your .bashrc/.zshrc:
 #source ~/.git-completion.bash
-#3) Consider changing your PS1 to also show the current branch,
+#4) Consider changing your PS1 to also show the current branch,
 #   see git-prompt.sh for details.
 #
 # If you use complex aliases of form '!f() { ... }; f', you can use the null

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

Re: commit -> public-inbox link helper

[Mike Rappazzo]

On Wed, Apr 4, 2018 at 12:35 PM, Johannes Schindelin
 wrote:
> Hi team,
>
> I found myself in dear need to quickly look up mails in the public-inbox
> mail archive corresponding to any given commit in git.git. Some time ago,
> I wrote a shell script to help me with that, and I found myself using it a
> couple of times, so I think it might be useful for others, too.
>
> This script (I call it lookup-commit.sh) needs to be dropped into a *bare*
> clone of http://public-inbox.org/git, and called with its absolute or
> relative path from a git.git worktree, e.g.
>
> ~/public-inbox-git.git/lookup-commit.sh \
> fea16b47b603e7e4fa7fca198bd49229c0e5da3d
>
> This will take a while initially, or when the `master` branch of the
> public-inbox mirror was updated, as it will generate two files with
> plain-text mappings.
>
> In its default mode, it will print the Message-ID of the mail (if found).
>
> With --open, it opens the mail in a browser (macOS support is missing,
> mainly because I cannot test: just add an `open` alternative to
> `xdg-open`).
>
> With --reply, it puts the mail into the `from-public-inbox` folder of a
> maildir-formatted ~/Mail/, so it is pretty specific to my setup here.
>
> Note: the way mails are matched is by timestamp. In practice, this works
> amazingly often (although not always, I reported findings short after
> GitMerge 2017). My plan was to work on this when/as needed.
>
> Note also: the script is very much in a 'works-for-me' state, and I could
> imagine that others might want to modify it to their needs. I would be
> delighted if somebody would take custody of it (as in: start a repository
> on GitHub, adding a README.md, adding a config setting to point to the
> location of the public-inbox mirror without having to copy the script,
> adding an option to install an alias to run the script, etc).
>
> And now, without further ado, here it is, the script, in its current glory:
>
> -- snipsnap --
> #!/bin/sh
>
> # This is a very simple helper to assist with finding the mail (if any)
> # corresponding to a given commit in git.git.
>
> die () {
> echo "$*" >&2
> exit 1
> }
>
> mode=print
> while case "$1" in
> --open) mode=open;;
> --reply) mode=reply;;
> -*) die "Unknown option: $1";;
> *) break;;
> esac; do shift; done
>
> test $# = 1 ||
> die "Usage: $0 ( [--open] | [--reply] ) "
>
> test reply != $mode ||
> test -d "$HOME/Mail" ||
> die "Need $HOME/Mail to reply"
>
> commit="$1"
> tae="$(git show -s --format='%at %an <%ae>' "$1")" ||
> die "Could not get Timestamp/Author/Email triplet from $1"
>
> # We try to match the timestamp first; the author name and author email are
> # not as reliable: they might have been overridden via a "From:" line in the
> # mail's body
> timestamp=${tae%% *}
>
> cd "$(dirname "$0")" ||
> die "Could not cd to the public-inbox directory"
>
> git rev-parse --quiet --verify \
> b60d038730d2c2bb8ab2b48c117db917ad529cf7 >/dev/null 2>&1 ||
> die "Not a public-inbox directory: $(pwd)"
>
> head="$(git rev-parse --verify master)" ||
> die "Could not determine tip of master"
>
> prevhead=
> test ! -f map.latest-rev ||
> prevhead="$(cat map.latest-rev)"
>
> if test $head != "$prevhead"
> then
> range=${prevhead:+$prevhead..}$head
> echo "Inserting records for $range" >&2
> git log --format="%at %h %an <%ae>" $range >map.txt.add ||
> die "Could not enumerate $range"
>
> cat map.txt map.txt.add 2>/dev/null | sort -n >map.txt.new &&
> mv -f map.txt.new map.txt ||
> die "Could not insert new records"
>
> echo $head >map.latest-rev
> fi
>
> lines="$(grep "^$timestamp "  if test 1 != $(echo "$lines" | wc -l)
> then
> test -n "$lines" ||
> die "No records found for timestamp $timestamp"
>
> echo "Multiple records found:"
>
> for h in $(echo "$lines" | cut -d ' ' -f 2)
> do
> git show -s --format="%nOn %ad, %an <%ae> sent" $h
> git show $h |
> sed -n -e 's/^+Message-Id: <\(.*\)>/\1/ip' \
> -e 's/^+Subject: //ip'
> done
>
> exit 1
> fi
>
> # We found exactly one record: print the message ID
> h=${lines#$timestamp }
> h=${h%% *}
> messageid="$(git show $h | sed -n 's/^+Message-Id: <\(.*\)>/\1/ip')" ||
> die "Could not determine Message-Id from $h"
>
> case $mode in
> print) echo $messageid;;
> open)
> url=https://public-inbox.org/git/$messageid
> case "$(uname -s)" in
> Linux) xdg-open "$url";;
> MINGW*|MSYS*) start "$url";;

 Darwin*) open "$url";;

> *) die "Need to learn how to open URLs on $(uname -s)";;
> esac
> ;;
> reply)
> mkdir -p "$HOME/Mail/from-public-inbox/new" &&
> mkdir -p "$HOME/Mail/from-public-inbox/cur" &&
> mkdir -p "$HOME/Mail/from-public-inbox/tmp" ||
> die "Could not set up mail folder 

Re: [PATCH 7/6] ref-filter: use generation number for --contains

[Jeff King]

On Wed, Apr 04, 2018 at 03:45:30PM -0400, Derrick Stolee wrote:

> On 4/4/2018 3:42 PM, Jeff King wrote:
> > On Wed, Apr 04, 2018 at 03:22:01PM -0400, Derrick Stolee wrote:
> > 
> > > That is the idea. I should make this clearer in all of my commit messages.
> > Yes, please. :) And maybe in the documentation of the file format, if
> > it's not there (I didn't check). It's a very useful property, and we
> > want to make sure people making use of the graph know they can depend on
> > it.
> 
> For v2, I'll expand on the roles of _UNDEF and _NONE in the discussion of
> generation numbers in Documentation/technical/commit-graph.txt (the design
> doc instead of the file format).

Yeah, that makes sense. Thanks, and thanks for a thoughtful discussion.
The performance numbers are very exciting.

-Peff

Re: [PATCH 7/6] ref-filter: use generation number for --contains

[Derrick Stolee]

On 4/4/2018 3:42 PM, Jeff King wrote:

On Wed, Apr 04, 2018 at 03:22:01PM -0400, Derrick Stolee wrote:


That is the idea. I should make this clearer in all of my commit messages.

Yes, please. :) And maybe in the documentation of the file format, if
it's not there (I didn't check). It's a very useful property, and we
want to make sure people making use of the graph know they can depend on
it.


For v2, I'll expand on the roles of _UNDEF and _NONE in the discussion 
of generation numbers in Documentation/technical/commit-graph.txt (the 
design doc instead of the file format).


Thanks,
-Stolee

Re: [PATCH 7/6] ref-filter: use generation number for --contains

[Jeff King]

On Wed, Apr 04, 2018 at 03:22:01PM -0400, Derrick Stolee wrote:

> > I don't know that the reachability property is explicitly promised by
> > your work, but it seems like it would be a natural fallout (after all,
> > you have to know the generation of each ancestor in order to compute the
> > later ones, so you're really just promising that you've actually stored
> > all the ones you've computed).
> 
> The commit-graph is closed under reachability, so if a commit has a
> generation number then it is in the graph and so are all its ancestors.

OK, if we assume that it's closed, then I think we can effectively
ignore the UNDEF cases. They'll just work out. And then yes I'd agree
that the:

  if (cutoff == UNDEF)
cutoff = NONE;

code is wrong. We'd want to keep it at UNDEF so we stop traversing at
any generation number.

> The reason for GENERATION_NUMBER_NONE is that the commit-graph file stores
> "0" for generation number until this patch. It still satisfies the condition
> that gen(A) < gen(B) if B can reach A, but also gives us a condition for
> "this commit still needs its generation number computed".

OK. I thought at first that would yield wrong results when comparing
UNDEF to NONE, but I think for this kind of --contains traversal, it's
still OK (NONE is less than UNDEF, but we know that the UNDEF thing
cannot be found by traversing from a NONE).

> > If you could make the reachability assumption, I think this question
> > just goes away. As soon as you hit a commit with _any_ generation
> > number, you could quit traversing down that path.
> That is the idea. I should make this clearer in all of my commit messages.

Yes, please. :) And maybe in the documentation of the file format, if
it's not there (I didn't check). It's a very useful property, and we
want to make sure people making use of the graph know they can depend on
it.

-Peff

Re: [PATCH 7/6] ref-filter: use generation number for --contains

[Derrick Stolee]

On 4/4/2018 3:16 PM, Jeff King wrote:

On Wed, Apr 04, 2018 at 03:06:26PM -0400, Derrick Stolee wrote:


@@ -1615,8 +1619,20 @@ static enum contains_result contains_tag_algo(struct 
commit *candidate,
  struct contains_cache *cache)
   {
struct contains_stack contains_stack = { 0, 0, NULL };
-   enum contains_result result = contains_test(candidate, want, cache);
+   enum contains_result result;
+   uint32_t cutoff = GENERATION_NUMBER_UNDEF;
+   const struct commit_list *p;
+
+   for (p = want; p; p = p->next) {
+   struct commit *c = p->item;
+   parse_commit_or_die(c);
+   if (c->generation < cutoff)
+   cutoff = c->generation;
+   }

Now that you mention it, let me split out the portion you are probably
talking about as incorrect:


+   if (cutoff == GENERATION_NUMBER_UNDEF)
+   cutoff = GENERATION_NUMBER_NONE;

You're right, we don't want this. Since GENERATION_NUMBER_NONE == 0, we get
no benefit from this. If we keep it GENERATION_NUMBER_UNDEF, then our walk
will be limited to commits NOT in the commit-graph (which we hope is small
if proper hygiene is followed).

I think it's more than that. If we leave it at UNDEF, that's wrong,
because contains_test() compares:

   candidate->generation < cutoff

which would _always_ be true. In other words, we're saying that our
"want" has an insanely high generation number, and traversing can never
find it. Which is clearly wrong.


That condition is not always true (which is why we use strict comparison 
instead of <=). If a commit is not in the commit-graph file, then its 
generation is equal to GENERATION_NUMBER_UNDEF, as shown in alloc.c:


void *alloc_commit_node(void)
{
    struct commit *c = alloc_node(_state, sizeof(struct 
commit));

    c->object.type = OBJ_COMMIT;
    c->index = alloc_commit_index();
    c->graph_pos = COMMIT_NOT_FROM_GRAPH;
    c->generation = GENERATION_NUMBER_UNDEF;
    return c;
}



So we have to put it at "0", to say "you should always traverse, we
can't tell you that this is a dead end". So that part of the logic is
currently correct.

But what I was getting at is that the loop behavior can't just pick the
min cutoff. The min is effectively "0" if there's even a single ref for
which we don't have a generation number, because we cannot ever stop
traversing (we might get to that commit if we kept going).

(It's also possible I'm confused about how UNDEF and NONE are used; I'm
assuming commits for which we don't have a generation number available
would get UNDEF in their commit->generation field).


I think it is this case.


If you could make the assumption that when we have a generation for
commit X, then we have a generation for all of its ancestors, things get
easier. Because then if you hit commit X with a generation number and
want to compare it to a cutoff, you know that either:

   1. The cutoff is defined, in which case you can stop traversing if
  we've gone past the cutoff.

   2. The cutoff is undefined, in which case we cannot possibly reach
  our "want" by traversing. Even if it has a smaller generation
  number than us, it's on an unrelated line of development.

I don't know that the reachability property is explicitly promised by
your work, but it seems like it would be a natural fallout (after all,
you have to know the generation of each ancestor in order to compute the
later ones, so you're really just promising that you've actually stored
all the ones you've computed).


The commit-graph is closed under reachability, so if a commit has a 
generation number then it is in the graph and so are all its ancestors.


The reason for GENERATION_NUMBER_NONE is that the commit-graph file 
stores "0" for generation number until this patch. It still satisfies 
the condition that gen(A) < gen(B) if B can reach A, but also gives us a 
condition for "this commit still needs its generation number computed".





I wonder to what degree it's worth traversing to come up with a
generation number for the "want" commits. If we walked, say, 50 commits
to do it, you'd probably save a lot of work (since the alternative is
walking thousands of commits until you realize that some ancient "v1.0"
tag is not useful).

I'd actually go so far as to say that any amount of traversal is
generally going to be worth it to come up with the correct generation
cutoff here. You can come up with pathological cases where you only have
one really recent tag or something, but in practice every repository
where performance is a concern is going to end up with refs much further
back than it would take to reach the cutoff condition.

Perhaps there is some value in walking to find the correct cutoff value, but
it is difficult to determine how far we are from commits with correct
generation numbers _a priori_. I'd rather rely on the commit-graph being in
a good state, not too 

Re: [PATCH 7/6] ref-filter: use generation number for --contains

[Jeff King]

On Wed, Apr 04, 2018 at 03:06:26PM -0400, Derrick Stolee wrote:

> > > @@ -1615,8 +1619,20 @@ static enum contains_result 
> > > contains_tag_algo(struct commit *candidate,
> > > struct contains_cache 
> > > *cache)
> > >   {
> > >   struct contains_stack contains_stack = { 0, 0, NULL };
> > > - enum contains_result result = contains_test(candidate, want, cache);
> > > + enum contains_result result;
> > > + uint32_t cutoff = GENERATION_NUMBER_UNDEF;
> > > + const struct commit_list *p;
> > > +
> > > + for (p = want; p; p = p->next) {
> > > + struct commit *c = p->item;
> > > + parse_commit_or_die(c);
> > > + if (c->generation < cutoff)
> > > + cutoff = c->generation;
> > > + }
> 
> Now that you mention it, let me split out the portion you are probably
> talking about as incorrect:
> 
> > > + if (cutoff == GENERATION_NUMBER_UNDEF)
> > > + cutoff = GENERATION_NUMBER_NONE;
> 
> You're right, we don't want this. Since GENERATION_NUMBER_NONE == 0, we get
> no benefit from this. If we keep it GENERATION_NUMBER_UNDEF, then our walk
> will be limited to commits NOT in the commit-graph (which we hope is small
> if proper hygiene is followed).

I think it's more than that. If we leave it at UNDEF, that's wrong,
because contains_test() compares:

  candidate->generation < cutoff

which would _always_ be true. In other words, we're saying that our
"want" has an insanely high generation number, and traversing can never
find it. Which is clearly wrong.

So we have to put it at "0", to say "you should always traverse, we
can't tell you that this is a dead end". So that part of the logic is
currently correct.

But what I was getting at is that the loop behavior can't just pick the
min cutoff. The min is effectively "0" if there's even a single ref for
which we don't have a generation number, because we cannot ever stop
traversing (we might get to that commit if we kept going).

(It's also possible I'm confused about how UNDEF and NONE are used; I'm
assuming commits for which we don't have a generation number available
would get UNDEF in their commit->generation field).

If you could make the assumption that when we have a generation for
commit X, then we have a generation for all of its ancestors, things get
easier. Because then if you hit commit X with a generation number and
want to compare it to a cutoff, you know that either:

  1. The cutoff is defined, in which case you can stop traversing if
 we've gone past the cutoff.

  2. The cutoff is undefined, in which case we cannot possibly reach
 our "want" by traversing. Even if it has a smaller generation
 number than us, it's on an unrelated line of development.

I don't know that the reachability property is explicitly promised by
your work, but it seems like it would be a natural fallout (after all,
you have to know the generation of each ancestor in order to compute the
later ones, so you're really just promising that you've actually stored
all the ones you've computed).

> > I wonder to what degree it's worth traversing to come up with a
> > generation number for the "want" commits. If we walked, say, 50 commits
> > to do it, you'd probably save a lot of work (since the alternative is
> > walking thousands of commits until you realize that some ancient "v1.0"
> > tag is not useful).
> > 
> > I'd actually go so far as to say that any amount of traversal is
> > generally going to be worth it to come up with the correct generation
> > cutoff here. You can come up with pathological cases where you only have
> > one really recent tag or something, but in practice every repository
> > where performance is a concern is going to end up with refs much further
> > back than it would take to reach the cutoff condition.
> 
> Perhaps there is some value in walking to find the correct cutoff value, but
> it is difficult to determine how far we are from commits with correct
> generation numbers _a priori_. I'd rather rely on the commit-graph being in
> a good state, not too far behind the refs. An added complexity of computing
> generation numbers dynamically is that we would need to add a dependence on
> the commit-graph file's existence at all.

If you could make the reachability assumption, I think this question
just goes away. As soon as you hit a commit with _any_ generation
number, you could quit traversing down that path.

-Peff

Re: [PATCH 7/6] ref-filter: use generation number for --contains

[Derrick Stolee]

On 4/4/2018 2:22 PM, Jeff King wrote:

On Wed, Apr 04, 2018 at 11:45:53AM -0400, Derrick Stolee wrote:


@@ -1615,8 +1619,20 @@ static enum contains_result contains_tag_algo(struct 
commit *candidate,
  struct contains_cache *cache)
  {
struct contains_stack contains_stack = { 0, 0, NULL };
-   enum contains_result result = contains_test(candidate, want, cache);
+   enum contains_result result;
+   uint32_t cutoff = GENERATION_NUMBER_UNDEF;
+   const struct commit_list *p;
+
+   for (p = want; p; p = p->next) {
+   struct commit *c = p->item;
+   parse_commit_or_die(c);
+   if (c->generation < cutoff)
+   cutoff = c->generation;
+   }


Now that you mention it, let me split out the portion you are probably 
talking about as incorrect:



+   if (cutoff == GENERATION_NUMBER_UNDEF)
+   cutoff = GENERATION_NUMBER_NONE;


You're right, we don't want this. Since GENERATION_NUMBER_NONE == 0, we 
get no benefit from this. If we keep it GENERATION_NUMBER_UNDEF, then 
our walk will be limited to commits NOT in the commit-graph (which we 
hope is small if proper hygiene is followed).



Hmm, on reflection, I'm not sure if this is right in the face of
multiple "want" commits, only some of which have generation numbers.  We
probably want to disable the cutoff if _any_ "want" commit doesn't have
a number.

There's also an obvious corner case where this won't kick in, and you'd
really like it to: recently added commits. E.g,. if I do this:

   git gc ;# imagine this writes generation numbers
   git pull
   git tag --contains HEAD

then HEAD isn't going to have a generation number. But this is the case
where we have the most to gain, since we could throw away all of the
ancient tags immediately upon seeing that their generation numbers are
way less than that of HEAD.

I wonder to what degree it's worth traversing to come up with a
generation number for the "want" commits. If we walked, say, 50 commits
to do it, you'd probably save a lot of work (since the alternative is
walking thousands of commits until you realize that some ancient "v1.0"
tag is not useful).

I'd actually go so far as to say that any amount of traversal is
generally going to be worth it to come up with the correct generation
cutoff here. You can come up with pathological cases where you only have
one really recent tag or something, but in practice every repository
where performance is a concern is going to end up with refs much further
back than it would take to reach the cutoff condition.


Perhaps there is some value in walking to find the correct cutoff value, 
but it is difficult to determine how far we are from commits with 
correct generation numbers _a priori_. I'd rather rely on the 
commit-graph being in a good state, not too far behind the refs. An 
added complexity of computing generation numbers dynamically is that we 
would need to add a dependence on the commit-graph file's existence at all.


Thanks,
-Stolee

Re: [PATCH 8/6] commit: use generation numbers for in_merge_bases()

[Jeff King]

On Wed, Apr 04, 2018 at 02:53:45PM -0400, Derrick Stolee wrote:

> > I'd have to do some timings, but I suspect we may want to switch to the
> > "tag --contains" algorithm anyway. This still does N independent
> > merge-base operations, one per ref. So with enough refs, you're still
> > better off throwing it all into one big traversal.
> 
> ...and I suppose your timings are to find out if there are data shapes where
> the branch algorithm is faster. Perhaps that is impossible now that we have
> the generation number cutoff for the tag algorithm.

Well, I wanted to show the opposite: that the branch algorithm can still
perform quite poorly. :)

I think with generation numbers that the tag algorithm should always
perform better, since you can't walk past a merge base when using a
cutoff. But it could definitely perform worse in a case where you don't
have generation numbers.

> Patches 7 and 8 seem to me like simple changes with no downside UNLESS we
> are deciding instead to delete the code I'm changing.

Yeah, I think they are strict improvements modulo the inverted UNDEF
logic I mentioned.

-Peff

Re: [PATCH v7] ls-remote: create '--sort' option

[Jeff King]

On Wed, Apr 04, 2018 at 07:18:42PM +0200, Harald Nordgren wrote:

> Jeff, you are right that 'git ls-remote --sort=committerdate' will not
> work. Do you think we need to do something about this, or it's fine
> that it fails the way you showed?

It's a reasonable-sized footgun, but one that I think most people would
be unlikely to trigger. I think it's probably OK as long as we warn the
user in the documentation (see my other response).

-Peff

Re: [PATCH v7] ls-remote: create '--sort' option

[Jeff King]

On Wed, Apr 04, 2018 at 07:11:53PM +0200, Harald Nordgren wrote:

> @@ -60,6 +60,16 @@ OPTIONS
>   upload-pack only shows the symref HEAD, so it will be the only
>   one shown by ls-remote.
>  
> +--sort=::
> + Sort based on the key given.  Prefix `-` to sort in
> + descending order of the value. You may use the --sort= option
> + multiple times, in which case the last key becomes the primary
> + key. Also supports "version:refname" or "v:refname" (tag
> + names are treated as versions). The "version:refname" sort
> + order can also be affected by the "versionsort.suffix"
> + configuration variable.
> + The keys supported are the same as those in `git for-each-ref`.

We probably ought to warn the user in that final sentence that keys
which actually look at the objects may not work, since we don't
necessarily have the objects.

There's one other subtlety, which is that things like %(HEAD) assume
we're talking about local refs, not the remote HEAD. So that wouldn't
work (of course it seems unlikely that anybody woudl _sort_ on that).

> @@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const char **argv, const 
> char *prefix)
>   if (!dest && !quiet)
>   fprintf(stderr, "From %s\n", *remote->url);
>   for ( ; ref; ref = ref->next) {
> + struct ref_array_item *item;
>   if (!check_ref_type(ref, flags))
>   continue;
>   if (!tail_match(pattern, ref->name))
>   continue;
> +
> + FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));

I think this can use the slightly-simpler FLEX_ALLOC_STR().

> + item->symref = ref->symref;

Normally a ref_array_item's symref is an allocated string owned by the
item. I don't think it actually matters now, but in the spirit of
least-surprise for the future, should this be xstrdup_or_null(ref->symref)?

> + item->objectname = ref->old_oid;

This is actually a struct assignment. Which does work, but our usual
mechanism would be to use "oidcpy(>objectname, >old_oid)".

All of this might be a little nicer if ref-filter provided a function to
allocate a new item. We're pushing the boundaries of ref-filter was
meant to be used here, as it was assumed you'd always start with a call
to filter_refs().

> + ALLOC_GROW(array.items, array.nr + 1, array.alloc);
> + array.items[array.nr++] = item;

The existing ref-filter code fails to use ALLOC_GROW() correctly. I
don't think it actually matters, since we don't intermingle this with
allocations done there. But perhaps we should be fixing that one while
we're looking at it. Or again, maybe it would be nicer still if there
were a ref-filter function to do this, and the whole call here could
just be:

  ref_array_push(, ref->name, >old_oid);

One more drastic alternative is to actually use the existing
filter_refs(), and just teach it to populate the array from a list of
refs. As you can see from its implementation, it does a few other setup
steps. I don't think they matter now, but if you eventually wanted to be
able to do "git ls-remote --contains HEAD", you'd need that setup.

-Peff

Re: [PATCH 8/6] commit: use generation numbers for in_merge_bases()

[Derrick Stolee]

On 4/4/2018 2:24 PM, Jeff King wrote:

On Wed, Apr 04, 2018 at 11:48:42AM -0400, Derrick Stolee wrote:


diff --git a/commit.c b/commit.c
index 858f4fdbc9..2566cba79f 100644
--- a/commit.c
+++ b/commit.c
@@ -1059,12 +1059,19 @@ int in_merge_bases_many(struct commit *commit, int 
nr_reference, struct commit *
   {
struct commit_list *bases;
int ret = 0, i;
+   uint32_t min_generation = GENERATION_NUMBER_UNDEF;
if (parse_commit(commit))
return ret;
-   for (i = 0; i < nr_reference; i++)
+   for (i = 0; i < nr_reference; i++) {
if (parse_commit(reference[i]))
return ret;
+   if (min_generation > reference[i]->generation)
+   min_generation = reference[i]->generation;
+   }
+
+   if (commit->generation > min_generation)
+   return 0;
bases = paint_down_to_common(commit, nr_reference, reference);
if (commit->object.flags & PARENT2)

This patch may suffice to speed up 'git branch --contains' instead of
needing to always use the 'git tag --contains' algorithm as considered in
[1].


I guess I want to specify: the only reason to NOT switch to the tags 
algorithm is because it _may_ hurt existing cases in certain data shapes...



I'd have to do some timings, but I suspect we may want to switch to the
"tag --contains" algorithm anyway. This still does N independent
merge-base operations, one per ref. So with enough refs, you're still
better off throwing it all into one big traversal.


...and I suppose your timings are to find out if there are data shapes 
where the branch algorithm is faster. Perhaps that is impossible now 
that we have the generation number cutoff for the tag algorithm.


Since the branch algorithm checks generation numbers before triggering 
pain_down_to_common(), we will do N independent merge-base calculations, 
where N is the number of branches with large enough generation numbers 
(which is why my test does so well: most are below the target generation 
number). This doesn't help at all if none of the refs are in the graph.


The other thing to do is add a minimum generation for the walk in 
paint_down_to_common() so even if commit->generation <= min_generation 
we still only walk down to commit->generation instead of all merge 
bases. This is something we could change in a later patch.


Patches 7 and 8 seem to me like simple changes with no downside UNLESS 
we are deciding instead to delete the code I'm changing.


Thanks,
-Stolee

Re: [RFC PATCH v3 2/2] t7406: add test for non-default branch in submodule

[Stefan Beller]

On Tue, Apr 3, 2018 at 3:26 PM, Eddy Petrișor  wrote:
> Notes:
> I am aware this test is not probably the best one and maybe I
> should have first one test that does a one level non-default, before
> trying a test with 2 levels of submodules, but I wanted to express the
> goal of the patch.

This patch only contains the test, I presume this goes on top of
https://public-inbox.org/git/20180403222053.23132-1-eddy.petri...@codeaurora.org/
which you plan to later submit as one patch including both the change as well as
the test.

> Currently the test fails, so I am obviously missing something.
> Help would be appreciated.
>
>
> 2018-04-04 1:20 GMT+03:00 Eddy Petrișor :
>> From: Eddy Petrișor 
>>
>> If a submodule uses a non-default branch and the branch info is versioned, on
>> submodule update --recursive --init the correct branch should be checked out.
>>
>> Signed-off-by: Eddy Petrișor 
>> ---
>>  t/t7406-submodule-update.sh | 54 
>> +
>>  1 file changed, 54 insertions(+)
>>
>> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
>> index 6f083c4d6..7b65f1dd1 100755
>> --- a/t/t7406-submodule-update.sh
>> +++ b/t/t7406-submodule-update.sh
>> @@ -259,6 +259,60 @@ test_expect_success 'submodule update --remote should 
>> fetch upstream changes wit
>> )
>>  '
>>
>> +test_expect_success 'submodule update --remote --recursive --init should 
>> fetch module branch from .gitmodules' '
>> +   git clone . super5 &&

I found adding "test_pause &&"
to be a great helper as it will drop you in a shell where
you can inspect the repository.


>> +   git clone super5 submodl2b2 &&
>> +   git clone super5 submodl1b1 &&

We may want to cleanup after the test is done:

  test_when_finished "rm submodl2b2" &&
  test_when_finished "rm submodl1b2" &&

>> +   cd submodl2b2 &&

I'd think the test suite prefers subshells to operate in other dirs
instead of direct cd's, just like at the end of the test.
For short parts, we make heavy use of the -C option,
So for example the code below
   (
   cd super &&
   git submodule update --recursive --init
   ) &&

can be written as

git -C super submodule update --recursive --init

which is shorter.

>> +   echo linel2b2 > l2b2 &&

style: The Git test suite prefers to have the redirect adjacent to the
file name:
  echo hello >world

>> +   git checkout -b b2 &&
>> +   git add l2b2 &&
>> +   test_tick &&
>> +   git commit -m "commit on b2 branch in l2" &&
>> +   git rev-parse --verify HEAD >../expectl2 &&

So until now we made a commit in a submodule on branch b2
and wrote it out to an expect file.


>> +   git checkout master &&
>> +   cd ../submodl1b1 &&
>> +   git checkout -b b1 &&
>> +   echo linel1b1 > l1b1 &&
>> +   git add l1b1 &&
>> +   test_tick &&
>> +   git commit -m "commit on b1 branch in l1" &&

very similar to above, just in another repo
instead of making a commit yourself, you may want to use
test_commit 
then you don't need to call echo/add/commit yourself.

>> +   git submodule add ../submodl2b2 submodl2b2 &&
>> +   git config -f .gitmodules submodule."submodl2b2".branch b2 &&
>> +   git add .gitmodules &&
>> +   test_tick &&
>> +   git commit -m "add l2 module with branch b2 in l1 module in branch 
>> b1" &&

So one submodule is made to be a submodule of the other
with a specific branch (b2)

>> +   git submodule init submodl2b2 &&

git submodule add ought to have initialized that submodule
already, I am not sure we need the explicit init here.

>> +   git rev-parse --verify HEAD >../expectl1 &&
>> +   git checkout master &&

We go back to master, which doesn't have the nested submodule?

>> +   cd ../super5 &&
>> +   echo super_with_2_chained_modules > super5 &&
>> +   git add super5 &&
>> +   test_tick &&
>> +   git commit -m "commit on default branch in super5" &&
>> +   git submodule add ../submodl1b1 submodl1b1 &&
>> +   git config -f .gitmodules submodule."submodl1b1".branch b1 &&
>> +   git add .gitmodules &&
>> +   test_tick &&
>> +   git commit -m "add l1 module with branch b1 in super5" &&

now we do this again without the nested submodule, just one repo
as a submodule?

>> +   git submodule init submodl1b1 &&
>> +   git clone super5 super &&

does super exist here already? (I did not check, but IIRC
super and super{1-4} are there as we count upwards to
find a name that is ok.

>> +   (
>> +   cd super &&
>> +   git submodule update --recursive --init
>> +   ) &&
>> +   (
>> +   cd submodl1b1 &&
>> +   git rev-parse --verify HEAD >../../actuall1 &&
>> +   test_cmp ../../expectl1 ../../actuall1
>> +   ) &&
>> +   (
>> +   

Re: commit -> public-inbox link helper

[Jeff King]

On Wed, Apr 04, 2018 at 06:35:59PM +0200, Johannes Schindelin wrote:

> Hi team,
> 
> I found myself in dear need to quickly look up mails in the public-inbox
> mail archive corresponding to any given commit in git.git. Some time ago,
> I wrote a shell script to help me with that, and I found myself using it a
> couple of times, so I think it might be useful for others, too.
> 
> This script (I call it lookup-commit.sh) needs to be dropped into a *bare*
> clone of http://public-inbox.org/git, and called with its absolute or
> relative path from a git.git worktree, e.g.
> 
>   ~/public-inbox-git.git/lookup-commit.sh \
>   fea16b47b603e7e4fa7fca198bd49229c0e5da3d
> 
> This will take a while initially, or when the `master` branch of the
> public-inbox mirror was updated, as it will generate two files with
> plain-text mappings.

Junio publishes a git-notes ref with the mapping you want. So you can
do:

  git fetch git://github.com/gitster/git.git refs/notes/amlog:refs/notes/amlog
  mid=$(git notes --ref amlog show $commit | perl -lne '/<(.*)>/ and print $1')
  echo "https://public-inbox.org/git/$mid;

without having to download the gigantic list archive repo at all (though
I do keep my own copy of the archive and index it with mairix, so I can
use "mairix -t m:$mid" and then view the whole thing locally in mutt).

-Peff

Re: [PATCH 8/6] commit: use generation numbers for in_merge_bases()

[Jeff King]

On Wed, Apr 04, 2018 at 11:48:42AM -0400, Derrick Stolee wrote:

> > diff --git a/commit.c b/commit.c
> > index 858f4fdbc9..2566cba79f 100644
> > --- a/commit.c
> > +++ b/commit.c
> > @@ -1059,12 +1059,19 @@ int in_merge_bases_many(struct commit *commit, int 
> > nr_reference, struct commit *
> >   {
> > struct commit_list *bases;
> > int ret = 0, i;
> > +   uint32_t min_generation = GENERATION_NUMBER_UNDEF;
> > if (parse_commit(commit))
> > return ret;
> > -   for (i = 0; i < nr_reference; i++)
> > +   for (i = 0; i < nr_reference; i++) {
> > if (parse_commit(reference[i]))
> > return ret;
> > +   if (min_generation > reference[i]->generation)
> > +   min_generation = reference[i]->generation;
> > +   }
> > +
> > +   if (commit->generation > min_generation)
> > +   return 0;
> > bases = paint_down_to_common(commit, nr_reference, reference);
> > if (commit->object.flags & PARENT2)
> 
> This patch may suffice to speed up 'git branch --contains' instead of
> needing to always use the 'git tag --contains' algorithm as considered in
> [1].

I'd have to do some timings, but I suspect we may want to switch to the
"tag --contains" algorithm anyway. This still does N independent
merge-base operations, one per ref. So with enough refs, you're still
better off throwing it all into one big traversal.

-Peff

Re: [PATCH 7/6] ref-filter: use generation number for --contains

[Jeff King]

On Wed, Apr 04, 2018 at 11:45:53AM -0400, Derrick Stolee wrote:

> @@ -1615,8 +1619,20 @@ static enum contains_result contains_tag_algo(struct 
> commit *candidate,
> struct contains_cache *cache)
>  {
>   struct contains_stack contains_stack = { 0, 0, NULL };
> - enum contains_result result = contains_test(candidate, want, cache);
> + enum contains_result result;
> + uint32_t cutoff = GENERATION_NUMBER_UNDEF;
> + const struct commit_list *p;
> +
> + for (p = want; p; p = p->next) {
> + struct commit *c = p->item;
> + parse_commit_or_die(c);
> + if (c->generation < cutoff)
> + cutoff = c->generation;
> + }
> + if (cutoff == GENERATION_NUMBER_UNDEF)
> + cutoff = GENERATION_NUMBER_NONE;

Hmm, on reflection, I'm not sure if this is right in the face of
multiple "want" commits, only some of which have generation numbers.  We
probably want to disable the cutoff if _any_ "want" commit doesn't have
a number.

There's also an obvious corner case where this won't kick in, and you'd
really like it to: recently added commits. E.g,. if I do this:

  git gc ;# imagine this writes generation numbers
  git pull
  git tag --contains HEAD

then HEAD isn't going to have a generation number. But this is the case
where we have the most to gain, since we could throw away all of the
ancient tags immediately upon seeing that their generation numbers are
way less than that of HEAD.

I wonder to what degree it's worth traversing to come up with a
generation number for the "want" commits. If we walked, say, 50 commits
to do it, you'd probably save a lot of work (since the alternative is
walking thousands of commits until you realize that some ancient "v1.0"
tag is not useful).

I'd actually go so far as to say that any amount of traversal is
generally going to be worth it to come up with the correct generation
cutoff here. You can come up with pathological cases where you only have
one really recent tag or something, but in practice every repository
where performance is a concern is going to end up with refs much further
back than it would take to reach the cutoff condition.

-Peff

Re: [PATCH v7] ls-remote: create '--sort' option

[Harald Nordgren]

Links to previous revisions:

[1] 
https://public-inbox.org/git/20180402174614.ga28...@sigill.intra.peff.net/T/#m108fe8c83f3558afaea8e317e680f7eaa136e9a9
[2] 
https://public-inbox.org/git/20180402211920.ga32...@sigill.intra.peff.net/T/#ma9ec4e0ce664160086e535c012e20d76822c60e5
...
[4] 
https://public-inbox.org/git/20180402174614.ga28...@sigill.intra.peff.net/T/#maa02c40c87b192e56c370c312098d469c9fce757
[5] 
https://public-inbox.org/git/20180402174614.ga28...@sigill.intra.peff.net/T/#m52cda3f359e1257e7bdfe19cd9a26f55fa20
[6] 
https://public-inbox.org/git/20180402174614.ga28...@sigill.intra.peff.net/T/#m6d3ce17f0f6dabeddaf03336c92512b7c413422b

On Wed, Apr 4, 2018 at 7:18 PM, Harald Nordgren
 wrote:
> I updated the code to use 'ALLOC_GROW'. I makes sense, I now I realize
> why array.alloc is there ;)
>
> Jeff, you are right that 'git ls-remote --sort=committerdate' will not
> work. Do you think we need to do something about this, or it's fine
> that it fails the way you showed?
>
> On Wed, Apr 4, 2018 at 7:11 PM, Harald Nordgren
>  wrote:
>> Create a '--sort' option for ls-remote, based on the one from
>> for-each-ref. This e.g. allows ref names to be sorted by version
>> semantics, so that v1.2 is sorted before v1.10.
>>
>> Signed-off-by: Harald Nordgren 
>> ---
>>
>> Notes:
>> Started using 'ALLOC_GROW'
>>
>>  Documentation/git-ls-remote.txt | 12 +++-
>>  builtin/ls-remote.c | 27 +--
>>  t/t5512-ls-remote.sh| 41 
>> -
>>  3 files changed, 76 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/git-ls-remote.txt 
>> b/Documentation/git-ls-remote.txt
>> index 5f2628c8f..17fae7218 100644
>> --- a/Documentation/git-ls-remote.txt
>> +++ b/Documentation/git-ls-remote.txt
>> @@ -10,7 +10,7 @@ SYNOPSIS
>>  
>>  [verse]
>>  'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
>> - [-q | --quiet] [--exit-code] [--get-url]
>> + [-q | --quiet] [--exit-code] [--get-url] [--sort=]
>>   [--symref] [ [...]]
>>
>>  DESCRIPTION
>> @@ -60,6 +60,16 @@ OPTIONS
>> upload-pack only shows the symref HEAD, so it will be the only
>> one shown by ls-remote.
>>
>> +--sort=::
>> +   Sort based on the key given.  Prefix `-` to sort in
>> +   descending order of the value. You may use the --sort= option
>> +   multiple times, in which case the last key becomes the primary
>> +   key. Also supports "version:refname" or "v:refname" (tag
>> +   names are treated as versions). The "version:refname" sort
>> +   order can also be affected by the "versionsort.suffix"
>> +   configuration variable.
>> +   The keys supported are the same as those in `git for-each-ref`.
>> +
>>  ::
>> The "remote" repository to query.  This parameter can be
>> either a URL or the name of a remote (see the GIT URLS and
>> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
>> index 540d56429..5bb8ee68a 100644
>> --- a/builtin/ls-remote.c
>> +++ b/builtin/ls-remote.c
>> @@ -1,6 +1,7 @@
>>  #include "builtin.h"
>>  #include "cache.h"
>>  #include "transport.h"
>> +#include "ref-filter.h"
>>  #include "remote.h"
>>
>>  static const char * const ls_remote_usage[] = {
>> @@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const 
>> char *prefix)
>> int show_symref_target = 0;
>> const char *uploadpack = NULL;
>> const char **pattern = NULL;
>> +   int i;
>>
>> struct remote *remote;
>> struct transport *transport;
>> const struct ref *ref;
>> +   struct ref_array array;
>> +   static struct ref_sorting *sorting = NULL, **sorting_tail = 
>>
>> struct option options[] = {
>> OPT__QUIET(, N_("do not print remote URL")),
>> @@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
>> *prefix)
>> OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
>> REF_NORMAL),
>> OPT_BOOL(0, "get-url", _url,
>>  N_("take url..insteadOf into account")),
>> +   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
>> +N_("field name to sort on"), 
>> _opt_ref_sorting),
>> OPT_SET_INT_F(0, "exit-code", ,
>>   N_("exit with exit code 2 if no matching refs 
>> are found"),
>>   2, PARSE_OPT_NOCOMPLETE),
>> @@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
>> *prefix)
>> OPT_END()
>> };
>>
>> +   memset(, 0, sizeof(array));
>> +
>> argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
>>  PARSE_OPT_STOP_AT_NON_OPTION);
>> dest = argv[0];
>> @@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const char 

Re: js/runtime-prefix-windows, was Re: What's cooking in git.git (Mar 2018, #06; Fri, 30)

[Johannes Sixt]

Am 03.04.2018 um 15:12 schrieb Johannes Schindelin:

On Fri, 30 Mar 2018, Junio C Hamano wrote:


* js/runtime-prefix-windows (2018-03-27) 2 commits
  - mingw/msvc: use the new-style RUNTIME_PREFIX helper
  - exec_cmd: provide a new-style RUNTIME_PREFIX helper for Windows
  (this branch uses dj/runtime-prefix.)

  The Windows port was the first that allowed Git to be installed
  anywhere by having its components refer to each other with relative
  pathnames.  The recent dj/runtime-prefix topic extends the idea to
  other platforms, and its approach has been adopted back in the
  Windows port.

  Is this, together with the dj/runtime-prefix topic, ready for
  'next'?


As far as I am concerned: yes!


Works in my environment, too. No objections.

-- Hannes

Re: [PATCH v7] ls-remote: create '--sort' option

[Harald Nordgren]

I updated the code to use 'ALLOC_GROW'. I makes sense, I now I realize
why array.alloc is there ;)

Jeff, you are right that 'git ls-remote --sort=committerdate' will not
work. Do you think we need to do something about this, or it's fine
that it fails the way you showed?

On Wed, Apr 4, 2018 at 7:11 PM, Harald Nordgren
 wrote:
> Create a '--sort' option for ls-remote, based on the one from
> for-each-ref. This e.g. allows ref names to be sorted by version
> semantics, so that v1.2 is sorted before v1.10.
>
> Signed-off-by: Harald Nordgren 
> ---
>
> Notes:
> Started using 'ALLOC_GROW'
>
>  Documentation/git-ls-remote.txt | 12 +++-
>  builtin/ls-remote.c | 27 +--
>  t/t5512-ls-remote.sh| 41 
> -
>  3 files changed, 76 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 5f2628c8f..17fae7218 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  
>  [verse]
>  'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
> - [-q | --quiet] [--exit-code] [--get-url]
> + [-q | --quiet] [--exit-code] [--get-url] [--sort=]
>   [--symref] [ [...]]
>
>  DESCRIPTION
> @@ -60,6 +60,16 @@ OPTIONS
> upload-pack only shows the symref HEAD, so it will be the only
> one shown by ls-remote.
>
> +--sort=::
> +   Sort based on the key given.  Prefix `-` to sort in
> +   descending order of the value. You may use the --sort= option
> +   multiple times, in which case the last key becomes the primary
> +   key. Also supports "version:refname" or "v:refname" (tag
> +   names are treated as versions). The "version:refname" sort
> +   order can also be affected by the "versionsort.suffix"
> +   configuration variable.
> +   The keys supported are the same as those in `git for-each-ref`.
> +
>  ::
> The "remote" repository to query.  This parameter can be
> either a URL or the name of a remote (see the GIT URLS and
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 540d56429..5bb8ee68a 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -1,6 +1,7 @@
>  #include "builtin.h"
>  #include "cache.h"
>  #include "transport.h"
> +#include "ref-filter.h"
>  #include "remote.h"
>
>  static const char * const ls_remote_usage[] = {
> @@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
> int show_symref_target = 0;
> const char *uploadpack = NULL;
> const char **pattern = NULL;
> +   int i;
>
> struct remote *remote;
> struct transport *transport;
> const struct ref *ref;
> +   struct ref_array array;
> +   static struct ref_sorting *sorting = NULL, **sorting_tail = 
>
> struct option options[] = {
> OPT__QUIET(, N_("do not print remote URL")),
> @@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
> OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
> REF_NORMAL),
> OPT_BOOL(0, "get-url", _url,
>  N_("take url..insteadOf into account")),
> +   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
> +N_("field name to sort on"), 
> _opt_ref_sorting),
> OPT_SET_INT_F(0, "exit-code", ,
>   N_("exit with exit code 2 if no matching refs 
> are found"),
>   2, PARSE_OPT_NOCOMPLETE),
> @@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
> *prefix)
> OPT_END()
> };
>
> +   memset(, 0, sizeof(array));
> +
> argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
>  PARSE_OPT_STOP_AT_NON_OPTION);
> dest = argv[0];
> @@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const char **argv, const 
> char *prefix)
> if (!dest && !quiet)
> fprintf(stderr, "From %s\n", *remote->url);
> for ( ; ref; ref = ref->next) {
> +   struct ref_array_item *item;
> if (!check_ref_type(ref, flags))
> continue;
> if (!tail_match(pattern, ref->name))
> continue;
> +
> +   FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
> +   item->symref = ref->symref;
> +   item->objectname = ref->old_oid;
> +
> +   ALLOC_GROW(array.items, array.nr + 1, array.alloc);
> +   array.items[array.nr++] = item;
> +   }
> +
> +   if (sorting)
> +   ref_array_sort(sorting, );
> +
> +   for (i = 0; i < array.nr; i++) {
> +   const 

[PATCH v7] ls-remote: create '--sort' option

[Harald Nordgren]

Create a '--sort' option for ls-remote, based on the one from
for-each-ref. This e.g. allows ref names to be sorted by version
semantics, so that v1.2 is sorted before v1.10.

Signed-off-by: Harald Nordgren 
---

Notes:
Started using 'ALLOC_GROW'

 Documentation/git-ls-remote.txt | 12 +++-
 builtin/ls-remote.c | 27 +--
 t/t5512-ls-remote.sh| 41 -
 3 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f..17fae7218 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=]
- [-q | --quiet] [--exit-code] [--get-url]
+ [-q | --quiet] [--exit-code] [--get-url] [--sort=]
  [--symref] [ [...]]
 
 DESCRIPTION
@@ -60,6 +60,16 @@ OPTIONS
upload-pack only shows the symref HEAD, so it will be the only
one shown by ls-remote.
 
+--sort=::
+   Sort based on the key given.  Prefix `-` to sort in
+   descending order of the value. You may use the --sort= option
+   multiple times, in which case the last key becomes the primary
+   key. Also supports "version:refname" or "v:refname" (tag
+   names are treated as versions). The "version:refname" sort
+   order can also be affected by the "versionsort.suffix"
+   configuration variable.
+   The keys supported are the same as those in `git for-each-ref`.
+
 ::
The "remote" repository to query.  This parameter can be
either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 540d56429..5bb8ee68a 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "transport.h"
+#include "ref-filter.h"
 #include "remote.h"
 
 static const char * const ls_remote_usage[] = {
@@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
int show_symref_target = 0;
const char *uploadpack = NULL;
const char **pattern = NULL;
+   int i;
 
struct remote *remote;
struct transport *transport;
const struct ref *ref;
+   struct ref_array array;
+   static struct ref_sorting *sorting = NULL, **sorting_tail = 
 
struct option options[] = {
OPT__QUIET(, N_("do not print remote URL")),
@@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, "refs", , N_("do not show peeled tags"), 
REF_NORMAL),
OPT_BOOL(0, "get-url", _url,
 N_("take url..insteadOf into account")),
+   OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
+N_("field name to sort on"), 
_opt_ref_sorting),
OPT_SET_INT_F(0, "exit-code", ,
  N_("exit with exit code 2 if no matching refs are 
found"),
  2, PARSE_OPT_NOCOMPLETE),
@@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
OPT_END()
};
 
+   memset(, 0, sizeof(array));
+
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
 PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
@@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const char **argv, const char 
*prefix)
if (!dest && !quiet)
fprintf(stderr, "From %s\n", *remote->url);
for ( ; ref; ref = ref->next) {
+   struct ref_array_item *item;
if (!check_ref_type(ref, flags))
continue;
if (!tail_match(pattern, ref->name))
continue;
+
+   FLEX_ALLOC_MEM(item, refname, ref->name, strlen(ref->name));
+   item->symref = ref->symref;
+   item->objectname = ref->old_oid;
+
+   ALLOC_GROW(array.items, array.nr + 1, array.alloc);
+   array.items[array.nr++] = item;
+   }
+
+   if (sorting)
+   ref_array_sort(sorting, );
+
+   for (i = 0; i < array.nr; i++) {
+   const struct ref_array_item *ref = array.items[i];
if (show_symref_target && ref->symref)
-   printf("ref: %s\t%s\n", ref->symref, ref->name);
-   printf("%s\t%s\n", oid_to_hex(>old_oid), ref->name);
+   printf("ref: %s\t%s\n", ref->symref, ref->refname);
+   printf("%s\t%s\n", oid_to_hex(>objectname), ref->refname);
status = 0; /* we found something */
}
return status;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 02106c922..66370cd88 100755
--- 

Errors testing on macOS High Sierra version 10.13.4

[Wink Saville]

I built git on a mac osx laptop and got some errors when testing.
I ran ./ci/run-build-and-tests.sh and three of the tests had failures
that appear to be associated with character encoding:
...
BUILTIN git-whatchanged
SUBDIR git-gui
SUBDIR gitk-git
SUBDIR templates
+ make --quiet test
*** prove ***
[07:58:38] t0204-gettext-reencode-sanity.sh ...
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/8 subtests
[07:58:39] t0050-filesystem.sh 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 2/10 subtests
[07:58:42] t9822-git-p4-path-encoding.sh ..
Dubious, test returned 1 (wstat 256, 0x100)
Failed 3/6 subtests
[08:00:55] t9001-send-email.sh  ok   132492 ms
[08:01:00] t3421-rebase-topology-linear.sh  ok   139911 ms
[08:01:08] t3404-rebase-interactive.sh  ok   146923 ms
[08:02:42] t3903-stash.sh . ok   101289 ms
...

And here is one of the errors from t0204:

$ cat t0204-gettext-reencode-sanity.out
Initialized empty Git repository in /Users/wink/prgs/git/git/t/trash
directory.t0204-gettext-reencode-sanity/.git/
# lib-gettext: Found 'is_IS.UTF-8' as an is_IS UTF-8 locale
# lib-gettext: Found 'is_IS.ISO8859-1' as an is_IS ISO-8859-1 locale
...
++ eval_ret=0
++ :
ok 7 - gettext.c: git init UTF-8 -> UTF-8

expecting success:
printf "Bjó til tóma Git lind" >expect &&
LANGUAGE=is LC_ALL="$is_IS_iso_locale" git init repo >actual &&
test_when_finished "rm -rf repo" &&
grep "^$(cat expect | iconv -f UTF-8 -t ISO8859-1) " actual
++ printf 'Bjó til tóma Git lind'
++ LANGUAGE=is
++ LC_ALL=is_IS.ISO8859-1
++ git init repo
++ test_when_finished 'rm -rf repo'
++ test 0 = 0
++ test_cleanup='{ rm -rf repo
} && (exit "$eval_ret"); eval_ret=$?; :'
+++ cat expect
+++ iconv -f UTF-8 -t ISO8859-1
++ grep '^Bj? til t?ma Git lind ' actual
error: last command exited with $?=1
++ rm -rf repo
++ exit 1
++ eval_ret=1
++ :
not ok 8 - gettext.c: git init UTF-8 -> ISO-8859-1
#
# printf "Bjó til tóma Git lind" >expect &&
# LANGUAGE=is LC_ALL="$is_IS_iso_locale" git init repo >actual &&
# test_when_finished "rm -rf repo" &&
# grep "^$(cat expect | iconv -f UTF-8 -t ISO8859-1) " actual
#

# failed 1 among 8 test(s)
1..8


Of course on travis-ci there are no failures so I dug deeper and found
that travis-ci is running 10.12.6 (I added a call to system_profier in
ci/run-build-and-tests.sh) where as I'm running is 10.13.4:

+system_profiler SPSoftwareDataType
Software:
System Software Overview:
  System Version: macOS 10.12.6 (16G29)
  Kernel Version: Darwin 16.7.0
  Boot Volume: Macintosh HD
  Boot Mode: Normal
  Computer Name: Travis’s Mac (294)
  User Name: Travis (travis)
  Secure Virtual Memory: Enabled
  System Integrity Protection: Enabled
  Time since boot: 5 minutes

Not sure, but maybe I've got something configured incorrectly.

Suggestions anyone?

-- Wink

Re: [PATCH 8/6] commit: use generation numbers for in_merge_bases()

[Brandon Williams]

On 04/04, Derrick Stolee wrote:
> On 4/4/2018 11:45 AM, Derrick Stolee wrote:
> > The containment algorithm for 'git branch --contains' is different
> > from that for 'git tag --contains' in that it uses is_descendant_of()
> > instead of contains_tag_algo(). The expensive portion of the branch
> > algorithm is computing merge bases.
> > 
> > When a commit-graph file exists with generation numbers computed,
> > we can avoid this merge-base calculation when the target commit has
> > a larger generation number than the target commits.
> > 
> > Performance tests were run on a copy of the Linux repository where
> > HEAD is contained in v4.13 but no earlier tag. Also, all tags were
> > copied to branches and 'git branch --contains' was tested:
> > 
> > Before: 60.0s
> > After:   0.4s
> > Rel %: -99.3%

Now that is an impressive speedup.

-- 
Brandon Williams

Re: Socket activation for GitWeb FastCGI with systemd?

[Alex Ivanov]

03.04.2018, 23:04, "Jacob Keller" :
> On Tue, Apr 3, 2018 at 11:53 AM, Alex Ivanov  wrote:
>>  Hi.
>>  I want to use systemd as fastcgi spawner for gitweb + nginx.
>>  The traffic is low and number of users is limited + traversal bots. For 
>> that reason I've decided to use following mimimal services
>>
>>  gitweb.socket
>>  [Unit]
>>  Description=GitWeb Socket
>>
>>  [Socket]
>>  ListenStream=/run/gitweb.sock
>>  Accept=false
>>
>>  [Install]
>>  WantedBy=sockets.target
>>
>>  gitweb.service
>>  [Unit]
>>  Description=GitWeb Service
>>
>>  [Service]
>>  Type=simple
>>  ExecStart=/path/to/gitweb.cgi --fcgi
>>  StandardInput=socket
>>
>>  However this scheme is not resistant to simple DDOS.
>>  E.g. traversal bots often kill the service by opening non existing path 
>> (e.g http://host/?p=repo;a=blob;f=nonexisting/path;hb=HEAD showing in 
>> browser 404 - Cannot find file) many times consecutively, which leads to
>>  Apr 03 21:32:10 host systemd[1]: gitweb.service: Start request repeated too 
>> quickly.
>>  Apr 03 21:32:10 host systemd[1]: gitweb.service: Failed with result 
>> 'start-limit-hit'.
>>  Apr 03 21:32:10 host systemd[1]: Failed to start GitWeb service.
>>  and 502 Bad Gateway in browser. I believe the reason is that gitweb.service 
>> dies on failure and if it happens too often, systemd declines to restart the 
>> service due to start limit hit.
>>  So my question is how to correct systemd services for GitWeb to be 
>> resistant to such issue? I prefer to use single process to process all 
>> clients.
>>  Thanks.
>
> This sounds like a systemd specific question that might get a better
> answer from the systemd mailing list.

Thanks I will try that too.

>
> That being said, I believe if in this case gitweb is dying due to the
> path not existing? You might be able to configure systemd to
> understand that the particular exit code for when the path doesn't
> exist is a "valid" exit, and not a failure case..

I will try to do that, but I'm afraid that there may be other ways to remotely 
abuse the service.

>
> I'm not entirely understanding your goal.. you want each request to
> launch the gitweb process, and when it's done you want it to exit? But
> if there are multiple connections at once you want it to stay alive
> until it services them all? I think the best answer is configure
> systemd to understand that the exit code for when the path is invalid
> will be counted as a success.

I want a single process for all connections too keep RAM usage at minimal. I 
also though it fits my case since number of users is low.

>
> Thanks,
> Jake

commit -> public-inbox link helper

[Johannes Schindelin]

Hi team,

I found myself in dear need to quickly look up mails in the public-inbox
mail archive corresponding to any given commit in git.git. Some time ago,
I wrote a shell script to help me with that, and I found myself using it a
couple of times, so I think it might be useful for others, too.

This script (I call it lookup-commit.sh) needs to be dropped into a *bare*
clone of http://public-inbox.org/git, and called with its absolute or
relative path from a git.git worktree, e.g.

~/public-inbox-git.git/lookup-commit.sh \
fea16b47b603e7e4fa7fca198bd49229c0e5da3d

This will take a while initially, or when the `master` branch of the
public-inbox mirror was updated, as it will generate two files with
plain-text mappings.

In its default mode, it will print the Message-ID of the mail (if found).

With --open, it opens the mail in a browser (macOS support is missing,
mainly because I cannot test: just add an `open` alternative to
`xdg-open`).

With --reply, it puts the mail into the `from-public-inbox` folder of a
maildir-formatted ~/Mail/, so it is pretty specific to my setup here.

Note: the way mails are matched is by timestamp. In practice, this works
amazingly often (although not always, I reported findings short after
GitMerge 2017). My plan was to work on this when/as needed.

Note also: the script is very much in a 'works-for-me' state, and I could
imagine that others might want to modify it to their needs. I would be
delighted if somebody would take custody of it (as in: start a repository
on GitHub, adding a README.md, adding a config setting to point to the
location of the public-inbox mirror without having to copy the script,
adding an option to install an alias to run the script, etc).

And now, without further ado, here it is, the script, in its current glory:

-- snipsnap --
#!/bin/sh

# This is a very simple helper to assist with finding the mail (if any)
# corresponding to a given commit in git.git.

die () {
echo "$*" >&2
exit 1
}

mode=print
while case "$1" in
--open) mode=open;;
--reply) mode=reply;;
-*) die "Unknown option: $1";;
*) break;;
esac; do shift; done

test $# = 1 ||
die "Usage: $0 ( [--open] | [--reply] ) "

test reply != $mode ||
test -d "$HOME/Mail" ||
die "Need $HOME/Mail to reply"

commit="$1"
tae="$(git show -s --format='%at %an <%ae>' "$1")" ||
die "Could not get Timestamp/Author/Email triplet from $1"

# We try to match the timestamp first; the author name and author email are
# not as reliable: they might have been overridden via a "From:" line in the
# mail's body
timestamp=${tae%% *}

cd "$(dirname "$0")" ||
die "Could not cd to the public-inbox directory"

git rev-parse --quiet --verify \
b60d038730d2c2bb8ab2b48c117db917ad529cf7 >/dev/null 2>&1 ||
die "Not a public-inbox directory: $(pwd)"

head="$(git rev-parse --verify master)" ||
die "Could not determine tip of master"

prevhead=
test ! -f map.latest-rev ||
prevhead="$(cat map.latest-rev)"

if test $head != "$prevhead"
then
range=${prevhead:+$prevhead..}$head
echo "Inserting records for $range" >&2
git log --format="%at %h %an <%ae>" $range >map.txt.add ||
die "Could not enumerate $range"

cat map.txt map.txt.add 2>/dev/null | sort -n >map.txt.new &&
mv -f map.txt.new map.txt ||
die "Could not insert new records"

echo $head >map.latest-rev
fi

lines="$(grep "^$timestamp " /\1/ip' \
-e 's/^+Subject: //ip'
done

exit 1
fi

# We found exactly one record: print the message ID
h=${lines#$timestamp }
h=${h%% *}
messageid="$(git show $h | sed -n 's/^+Message-Id: <\(.*\)>/\1/ip')" ||
die "Could not determine Message-Id from $h"

case $mode in
print) echo $messageid;;
open)
url=https://public-inbox.org/git/$messageid
case "$(uname -s)" in
Linux) xdg-open "$url";;
MINGW*|MSYS*) start "$url";;
*) die "Need to learn how to open URLs on $(uname -s)";;
esac
;;
reply)
mkdir -p "$HOME/Mail/from-public-inbox/new" &&
mkdir -p "$HOME/Mail/from-public-inbox/cur" &&
mkdir -p "$HOME/Mail/from-public-inbox/tmp" ||
die "Could not set up mail folder 'from-public-inbox'"

path=$(git diff --name-only $h^!) &&
mail="$(printf "%s_%09d.%s:2," $(date +%s.%N) $$ $(hostname -f))"
&&
git show $h:$path >"$HOME/Mail/from-public-inbox/new/$mail" ||
die "Could not write mail"
;;
*)
die "Unhandled mode: $mode"
;;
esac

Re: [PATCH] completion: improve ls-files filter performance

[Johannes Schindelin]

Hi drizzd,

On Wed, 4 Apr 2018, Clemens Buchacher wrote:

> From the output of ls-files, we remove all but the leftmost path
> component and then we eliminate duplicates. We do this in a while loop,
> which is a performance bottleneck when the number of iterations is large
> (e.g. for 6 files in linux.git).
> 
> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git
> 
> real0m11.876s
> user0m4.685s
> sys 0m6.808s
> 
> Replacing the loop with the cut command improves performance
> significantly:
> 
> $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git
> 
> real0m1.372s
> user0m0.263s
> sys 0m0.167s
> 
> The measurements were done with Msys2 bash, which is used by Git for
> Windows.

Those are nice numbers right there, so I am eager to get this into Git for
Windows as quickly as it stabilizes (i.e. when it hits `next` or so).

I was wondering about one thing, though:

> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 6da95b8..69a2d41 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -384,12 +384,7 @@ __git_index_files ()
>   local root="${2-.}" file
>  
>   __git_ls_files_helper "$root" "$1" |
> - while read -r file; do
> - case "$file" in
> - ?*/*) echo "${file%%/*}" ;;

This is a bit different from the `cut -f1 -d/` logic, as it does *not
necessarily* strip a leading slash: for `/abc` the existing code would
return the string unmodified, for `/abc/def` it would return an empty
string!

Now, I think that this peculiar behavior is most likely bogus as `git
ls-files` outputs only relative paths (that I know of). In any case,
reducing paths to an empty string seems fishy.

I looked through the history of that code and tracked it all the way back
to
https://public-inbox.org/git/1357930123-26310-1-git-send-email-manlio.peri...@gmail.com/
(that is the reason why you are Cc:ed, Manlio). Manlio, do you remember
why you put the `?` in front of `?*/*` here? I know, it's been more than
five years...

Out of curiosity, would the numbers change a lot if you replaced the `cut
-f1 -d/` call by a `sed -e 's/^\//' -e 's/\/.*//'` one?

I am not proposing to change the patch, though, because we really do not
need to expect `ls-files` to print lines with leading slashes.

> - *) echo "$file" ;;
> - esac
> - done | sort | uniq
> + cut -f1 -d/ | sort | uniq
>  }
>  
>  # Lists branches from the local repository.

Ciao,
Dscho

Re: [PATCH 8/6] commit: use generation numbers for in_merge_bases()

[Derrick Stolee]

On 4/4/2018 11:45 AM, Derrick Stolee wrote:

The containment algorithm for 'git branch --contains' is different
from that for 'git tag --contains' in that it uses is_descendant_of()
instead of contains_tag_algo(). The expensive portion of the branch
algorithm is computing merge bases.

When a commit-graph file exists with generation numbers computed,
we can avoid this merge-base calculation when the target commit has
a larger generation number than the target commits.

Performance tests were run on a copy of the Linux repository where
HEAD is contained in v4.13 but no earlier tag. Also, all tags were
copied to branches and 'git branch --contains' was tested:

Before: 60.0s
After:   0.4s
Rel %: -99.3%

Reported-by: Jeff King 
Signed-off-by: Derrick Stolee 
---
  commit.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/commit.c b/commit.c
index 858f4fdbc9..2566cba79f 100644
--- a/commit.c
+++ b/commit.c
@@ -1059,12 +1059,19 @@ int in_merge_bases_many(struct commit *commit, int 
nr_reference, struct commit *
  {
struct commit_list *bases;
int ret = 0, i;
+   uint32_t min_generation = GENERATION_NUMBER_UNDEF;
  
  	if (parse_commit(commit))

return ret;
-   for (i = 0; i < nr_reference; i++)
+   for (i = 0; i < nr_reference; i++) {
if (parse_commit(reference[i]))
return ret;
+   if (min_generation > reference[i]->generation)
+   min_generation = reference[i]->generation;
+   }
+
+   if (commit->generation > min_generation)
+   return 0;
  
  	bases = paint_down_to_common(commit, nr_reference, reference);

if (commit->object.flags & PARENT2)


This patch may suffice to speed up 'git branch --contains' instead of 
needing to always use the 'git tag --contains' algorithm as considered 
in [1].


Thanks,
-Stolee

[1] 
https://public-inbox.org/git/20180303051516.ge27...@sigill.intra.peff.net/

    Re: [PATCH 0/4] Speed up git tag --contains

[PATCH 7/6] ref-filter: use generation number for --contains

[Derrick Stolee]

A commit A can reach a commit B only if the generation number of A
is strictly larger than the generation number of B. This condition
allows significantly short-circuiting commit-graph walks.

Use generation number for '--contains' type queries.

On a copy of the Linux repository where HEAD is containd in v4.13
but no earlier tag, the command 'git tag --contains HEAD' had the
following peformance improvement:

Before: 0.81s
After:  0.04s
Rel %:  -95%

Helped-by: Jeff King 
Signed-off-by: Derrick Stolee 
---
 ref-filter.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 45fc56216a..b147b1d0ee 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1584,7 +1584,8 @@ static int in_commit_list(const struct commit_list *want, 
struct commit *c)
  */
 static enum contains_result contains_test(struct commit *candidate,
  const struct commit_list *want,
- struct contains_cache *cache)
+ struct contains_cache *cache,
+ uint32_t cutoff)
 {
enum contains_result *cached = contains_cache_at(cache, candidate);
 
@@ -1598,8 +1599,11 @@ static enum contains_result contains_test(struct commit 
*candidate,
return CONTAINS_YES;
}
 
-   /* Otherwise, we don't know; prepare to recurse */
parse_commit_or_die(candidate);
+
+   if (candidate->generation < cutoff)
+   return CONTAINS_NO;
+
return CONTAINS_UNKNOWN;
 }
 
@@ -1615,8 +1619,20 @@ static enum contains_result contains_tag_algo(struct 
commit *candidate,
  struct contains_cache *cache)
 {
struct contains_stack contains_stack = { 0, 0, NULL };
-   enum contains_result result = contains_test(candidate, want, cache);
+   enum contains_result result;
+   uint32_t cutoff = GENERATION_NUMBER_UNDEF;
+   const struct commit_list *p;
+
+   for (p = want; p; p = p->next) {
+   struct commit *c = p->item;
+   parse_commit_or_die(c);
+   if (c->generation < cutoff)
+   cutoff = c->generation;
+   }
+   if (cutoff == GENERATION_NUMBER_UNDEF)
+   cutoff = GENERATION_NUMBER_NONE;
 
+   result = contains_test(candidate, want, cache, cutoff);
if (result != CONTAINS_UNKNOWN)
return result;
 
@@ -1634,7 +1650,7 @@ static enum contains_result contains_tag_algo(struct 
commit *candidate,
 * If we just popped the stack, parents->item has been marked,
 * therefore contains_test will return a meaningful yes/no.
 */
-   else switch (contains_test(parents->item, want, cache)) {
+   else switch (contains_test(parents->item, want, cache, cutoff)) 
{
case CONTAINS_YES:
*contains_cache_at(cache, commit) = CONTAINS_YES;
contains_stack.nr--;
@@ -1648,7 +1664,7 @@ static enum contains_result contains_tag_algo(struct 
commit *candidate,
}
}
free(contains_stack.contains_stack);
-   return contains_test(candidate, want, cache);
+   return contains_test(candidate, want, cache, cutoff);
 }
 
 static int commit_contains(struct ref_filter *filter, struct commit *commit,
-- 
2.17.0.rc0

[PATCH 8/6] commit: use generation numbers for in_merge_bases()

[Derrick Stolee]

The containment algorithm for 'git branch --contains' is different
from that for 'git tag --contains' in that it uses is_descendant_of()
instead of contains_tag_algo(). The expensive portion of the branch
algorithm is computing merge bases.

When a commit-graph file exists with generation numbers computed,
we can avoid this merge-base calculation when the target commit has
a larger generation number than the target commits.

Performance tests were run on a copy of the Linux repository where
HEAD is contained in v4.13 but no earlier tag. Also, all tags were
copied to branches and 'git branch --contains' was tested:

Before: 60.0s
After:   0.4s
Rel %: -99.3%

Reported-by: Jeff King 
Signed-off-by: Derrick Stolee 
---
 commit.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/commit.c b/commit.c
index 858f4fdbc9..2566cba79f 100644
--- a/commit.c
+++ b/commit.c
@@ -1059,12 +1059,19 @@ int in_merge_bases_many(struct commit *commit, int 
nr_reference, struct commit *
 {
struct commit_list *bases;
int ret = 0, i;
+   uint32_t min_generation = GENERATION_NUMBER_UNDEF;
 
if (parse_commit(commit))
return ret;
-   for (i = 0; i < nr_reference; i++)
+   for (i = 0; i < nr_reference; i++) {
if (parse_commit(reference[i]))
return ret;
+   if (min_generation > reference[i]->generation)
+   min_generation = reference[i]->generation;
+   }
+
+   if (commit->generation > min_generation)
+   return 0;
 
bases = paint_down_to_common(commit, nr_reference, reference);
if (commit->object.flags & PARENT2)
-- 
2.17.0.rc0

Re: [PATCH 0/3] Lazy-load trees when reading commit-graph

[Derrick Stolee]

On 4/3/2018 4:20 PM, Jeff King wrote:

On Tue, Apr 03, 2018 at 09:14:50AM -0400, Derrick Stolee wrote:


I'm not sure what the exact solution would be, but I imagine something
like variable-sized "struct commit"s with the parent pointers embedded,
with some kind of flag to indicate the number of parents (and probably
some fallback to break out to a linked list for extreme cases of more
than 2 parents).  It may end up pretty invasive, though, as there's a
lot of open-coded traversals of that parent list.

Anyway, not anything to do with this patch, but food for thought as you
micro-optimize these traversals.

One other thing that I've been thinking about is that 'struct commit' is so
much bigger than the other structs in 'union any_object'. This means that
the object cache, which I think creates blocks of 'union any_object' for
memory-alignment reasons, is overly bloated. This would be especially true
when walking many more trees than commits.

Perhaps there are other memory concerns in that case (such as cached
buffers) that the 'union any_object' is not a concern, but it is worth
thinking about as we brainstorm how to reduce the parent-list memory.

It definitely bloats any_object, but I don't think we typically allocate
too many of those. Those should only come from lookup_unknown_object(),
but typically we'll come across objects by traversing the graph, in
which case we have an expectation of the type (and use the appropriate
lookup_foo() function, which uses the type-specific block allocators).


Thanks for the clarification here. I'm glad I was wrong about how often 
any_object is used.


Thanks,
-Stolee

Hey

[Nina Granzotto]

Apply for a loan at 3% reply to this Email for more Info

Gruß

[Vladimir Volf]

Gruß

In einer kurzen Einleitung bin ich der Anwalt Vladimir Volf aus
zamberk Tschechische Republik, aber jetzt lebe ich in London, ich habe
dir eine E-Mail über meinen verstorbenen Klienten geschickt, aber ich
habe keine Antwort von dir erhalten, der Verstorbene ist ein Bürger
Von deinem Land mit dem gleichen Nachnamen mit dir, er ist ein
Exporteur von Gold hier in London.

Er starb vor ein paar Jahren mit der Familie und verließ seine
Firma,und riesige Geldbeträge in UBS Investment Bank hier in London
hinterlegt, Ich bin sein persönlicher Anwalt und ich brauche deine
Mitarbeit, damit wir das Geld bekommen können. Das Geld wird in Höhe
von £ 5,7 Millionen geschätzt, bevor die Regierung das Geld endgültig
beschlagnahmt hat, aber ich sage dir weitere Details darüber, wenn ich
von dir höre.

Re: [PATCH v3 2/2] t3200: verify "branch --list" sanity when rebasing from detached HEAD

[Eric Sunshine]

On Tue, Apr 3, 2018 at 10:47 AM, Kaartic Sivaraam
 wrote:
> From: Eric Sunshine 
>
> "git branch --list" shows an in-progress rebase as:
>
>   * (no branch, rebasing )
> master
> ...
>
> However, if the rebase is started from a detached HEAD, then there is no
> , and it would attempt to print a NULL pointer. The previous
> commit fixed this problem, so add a test to verify that the output is
> sane in this situation.
>
> Signed-off-by: Eric Sunshine 
> Signed-off-by: Kaartic Sivaraam 

Thanks. This re-roll looks fine.

> ---
>  t/t3200-branch.sh | 24 
>  1 file changed, 24 insertions(+)
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 503a88d02..89fff3fa9 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -6,6 +6,7 @@
>  test_description='git branch assorted tests'
>
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
>
>  test_expect_success 'prepare a trivial repository' '
> echo Hello >A &&
> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with 
> --no-merged' '
> test_must_fail git branch --merged HEAD --no-merged HEAD
>  '
>
> +test_expect_success '--list during rebase' '
> +   test_when_finished "reset_rebase" &&
> +   git checkout master &&
> +   FAKE_LINES="1 edit 2" &&
> +   export FAKE_LINES &&
> +   set_fake_editor &&
> +   git rebase -i HEAD~2 &&
> +   git branch --list >actual &&
> +   test_i18ngrep "rebasing master" actual
> +'
> +
> +test_expect_success '--list during rebase from detached HEAD' '
> +   test_when_finished "reset_rebase && git checkout master" &&
> +   git checkout master^0 &&
> +   oid=$(git rev-parse --short HEAD) &&
> +   FAKE_LINES="1 edit 2" &&
> +   export FAKE_LINES &&
> +   set_fake_editor &&
> +   git rebase -i HEAD~2 &&
> +   git branch --list >actual &&
> +   test_i18ngrep "rebasing detached HEAD $oid" actual
> +'
> +
>  test_expect_success 'tracking with unexpected .fetch refspec' '
> rm -rf a b c d &&
> git init a &&
> --
> 2.17.0.484.g0c8726318

Re: [PATCH v3 1/2] builtin/config.c: treat type specifiers singularly

[Eric Sunshine]

On Wed, Apr 4, 2018 at 2:07 AM, Taylor Blau  wrote:
> [...]
> In fact, `git config` will not accept multiple type specifiers at a
> time, as indicated by:
>
>   $ git config --int --bool some.section
>   error: only one type at a time.
>
> This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type
> specifier, so that the above command would instead be valid, and a
> synonym of:
>
>   $ git config --bool some.section
>
> This change is motivated by two urges: (1) it does not make sense to
> represent a singular type specifier internally as a bitset, only to
> complain when there are multiple bits in the set. `OPT_SET_INT` is more
> well-suited to this task than `OPT_BIT` is. (2) a future patch will
> introduce `--type=`, and we would like not to complain in the
> following situation:
>
>   $ git config --int --type=int

I can understand "last wins" for related options such as "--verbose
--quiet" or "--verbose=1 --verbose=2", but I have a lot of trouble
buying the "last wins" argument in this context where there is no
semantic relationship between, say, --bool and --expiry-date.
Specifying conflicting options together is likely to be unintentional,
thus reporting it as an error seems sensible, so I'm not convinced
that patch's removal of that error is a good idea.

I understand that you're doing this to avoid complaining about "--int
--type=int", but exactly how that case is supported should be an
implementation detail; it doesn't need to bleed into the UI as an
unnecessary and not-well-justified loosening of an age-old
restriction. There are other ways to support "--int --type=int"
without "last wins". Also, do we really need to support "--int
--type=int"? Is that an expected use-case?

[PATCH] completion: improve ls-files filter performance

[Clemens Buchacher]

>From the output of ls-files, we remove all but the leftmost path
component and then we eliminate duplicates. We do this in a while loop,
which is a performance bottleneck when the number of iterations is large
(e.g. for 6 files in linux.git).

$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git

real0m11.876s
user0m4.685s
sys 0m6.808s

Replacing the loop with the cut command improves performance
significantly:

$ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git

real0m1.372s
user0m0.263s
sys 0m0.167s

The measurements were done with Msys2 bash, which is used by Git for
Windows.

When filtering the ls-files output we take care not to touch absolute
paths. This is redundant, because ls-files will never output absolute
paths. Remove the unnecessary operations.

The issue was reported here:
https://github.com/git-for-windows/git/issues/1533

Signed-off-by: Clemens Buchacher 
---

On Sun, Mar 18, 2018 at 02:26:18AM +0100, SZEDER Gábor wrote:
> 
> You didn't run the test suite, did you? ;)

My bad. I put the sort back in. Test t9902 is now pass. I did not run
the other tests. I think the completion script is not used there.

I also considered Junio's and Johannes' comments.

> I have a short patch series collecting dust somewhere for a long
> while, [...]
> Will try to dig up those patches.

Cool. Bash completion can certainly use more performance improvements.

 contrib/completion/git-completion.bash | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 6da95b8..69a2d41 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -384,12 +384,7 @@ __git_index_files ()
local root="${2-.}" file
 
__git_ls_files_helper "$root" "$1" |
-   while read -r file; do
-   case "$file" in
-   ?*/*) echo "${file%%/*}" ;;
-   *) echo "$file" ;;
-   esac
-   done | sort | uniq
+   cut -f1 -d/ | sort | uniq
 }
 
 # Lists branches from the local repository.
-- 
2.7.4

Re: [PATCH v3 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Eric Sunshine]

On Wed, Apr 4, 2018 at 2:07 AM, Taylor Blau  wrote:
> `git config` has long allowed the ability for callers to provide a 'type
> specifier', which instructs `git config` to (1) ensure that incoming
> values are satisfiable under that type, and (2) that outgoing values are
> canonicalized under that type.
>
> In another series, we propose to add extend this functionality with
> `--color` and `--default` to replace `--get-color`.
>
> However, we traditionally use `--color` to mean "colorize this output",
> instead of "this value should be treated as a color".
>
> Currently, `git config` does not support this kind of colorization, but
> we should be careful to avoid inhabiting this option too soon, so that
> `git config` can support `--color` (in the traditional sense) in the
> future, if that is desired.
>
> In this patch, we prefer `--type=[int|bool|bool-or-int|...]` over
> `--int`, `--bool`, and etc. This allows the aforementioned other patch
> to add `--color` (in the non-traditional sense) via `--type=color`,
> instead of `--color`.

I always find this last sentence confusing since it's not clear to
which ilk of "--color" option you refer. Perhaps say instead something
like:

This normalization will allow the aforementioned upcoming patch to
support querying a color value with a default via "--type=color
--default=...".

> Signed-off-by: Taylor Blau 
> ---
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> @@ -160,30 +158,34 @@ See also <>.
> +--type [type]::
> +  'git config' will ensure that any input output is valid under the given 
> type
> +  constraint(s), and will canonicalize outgoing values in `[type]`'s 
> canonical
> +  form.

Do the brackets in "[type]" mean that the argument is optional? If so,
what does 'type' default to when not specified? The documentation
should discuss this.

> diff --git a/builtin/config.c b/builtin/config.c
> @@ -61,6 +61,33 @@ static int show_origin;
> +static int option_parse_type(const struct option *opt, const char *arg,
> +int unset)
> +{
> +   [...]
> +   if (!strcmp(arg, "bool"))
> +   *type = TYPE_BOOL;
> +   else if (!strcmp(arg, "int"))
> +   *type = TYPE_INT;
> +   else if (!strcmp(arg, "bool-or-int"))
> +   *type = TYPE_BOOL_OR_INT;
> +   else if (!strcmp(arg, "path"))
> +   *type = TYPE_PATH;
> +   else if (!strcmp(arg, "expiry-date"))
> +   *type = TYPE_EXPIRY_DATE;
> +   else {
> +   die(_("unexpected --type argument, %s"), arg);

"unexpected" doesn't seem like the best word choice since an argument
to --type _is_ expected. Perhaps you mean "unrecognized"?

> +   return 1;
> +   }
> +   return 0;
> +}

[PATCH v3 1/2] builtin/config.c: treat type specifiers singularly

[Taylor Blau]

Internally, we represent `git config`'s type specifiers as a bitset
using OPT_BIT. 'bool' is 1<<0, 'int' is 1<<1, and so on. This technique
allows for the representation of multiple type specifiers in the `int
types` field, but this multi-representation is left unused.

In fact, `git config` will not accept multiple type specifiers at a
time, as indicated by:

  $ git config --int --bool some.section
  error: only one type at a time.

This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type
specifier, so that the above command would instead be valid, and a
synonym of:

  $ git config --bool some.section

This change is motivated by two urges: (1) it does not make sense to
represent a singular type specifier internally as a bitset, only to
complain when there are multiple bits in the set. `OPT_SET_INT` is more
well-suited to this task than `OPT_BIT` is. (2) a future patch will
introduce `--type=`, and we would like not to complain in the
following situation:

  $ git config --int --type=int

Signed-off-by: Taylor Blau 
---
 builtin/config.c   | 49 +++---
 t/t1300-repo-config.sh | 11 ++
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 01169dd62..92fb8d56b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -25,7 +25,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 int actions, type;
 static int end_null;
 static int respect_includes_opt = -1;
 static struct config_options config_options;
@@ -55,11 +55,11 @@ static int show_origin;
 #define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \
ACTION_GET_REGEXP | ACTION_GET_URLMATCH)
 
-#define TYPE_BOOL (1<<0)
-#define TYPE_INT (1<<1)
-#define TYPE_BOOL_OR_INT (1<<2)
-#define TYPE_PATH (1<<3)
-#define TYPE_EXPIRY_DATE (1<<4)
+#define TYPE_BOOL  1
+#define TYPE_INT   2
+#define TYPE_BOOL_OR_INT   3
+#define TYPE_PATH  4
+#define TYPE_EXPIRY_DATE   5
 
 static struct option builtin_config_options[] = {
OPT_GROUP(N_("Config file location")),
@@ -84,11 +84,11 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "get-color", , N_("find the color configured: slot 
[default]"), ACTION_GET_COLOR),
OPT_BIT(0, "get-colorbool", , N_("find the color setting: slot 
[stdout-is-tty]"), ACTION_GET_COLORBOOL),
OPT_GROUP(N_("Type")),
-   OPT_BIT(0, "bool", , N_("value is \"true\" or \"false\""), 
TYPE_BOOL),
-   OPT_BIT(0, "int", , N_("value is decimal number"), TYPE_INT),
-   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_SET_INT(0, "bool", , N_("value is \"true\" or \"false\""), 
TYPE_BOOL),
+   OPT_SET_INT(0, "int", , N_("value is decimal number"), TYPE_INT),
+   OPT_SET_INT(0, "bool-or-int", , N_("value is --bool or --int"), 
TYPE_BOOL_OR_INT),
+   OPT_SET_INT(0, "path", , N_("value is a path (file or directory 
name)"), TYPE_PATH),
+   OPT_SET_INT(0, "expiry-date", , N_("value is an expiry date"), 
TYPE_EXPIRY_DATE),
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")),
@@ -149,26 +149,26 @@ static int format_config(struct strbuf *buf, const char 
*key_, const char *value
if (show_keys)
strbuf_addch(buf, key_delim);
 
-   if (types == TYPE_INT)
+   if (type == TYPE_INT)
strbuf_addf(buf, "%"PRId64,
git_config_int64(key_, value_ ? value_ : 
""));
-   else if (types == TYPE_BOOL)
+   else if (type == TYPE_BOOL)
strbuf_addstr(buf, git_config_bool(key_, value_) ?
  "true" : "false");
-   else if (types == TYPE_BOOL_OR_INT) {
+   else if (type == TYPE_BOOL_OR_INT) {
int is_bool, v;
v = git_config_bool_or_int(key_, value_, _bool);
if (is_bool)
strbuf_addstr(buf, v ? "true" : "false");
else
strbuf_addf(buf, "%d", v);
-   } else if (types == TYPE_PATH) {
+   } else if (type == TYPE_PATH) {
const char *v;
if (git_config_pathname(, key_, value_) < 0)
return -1;
strbuf_addstr(buf, v);
free((char 

[PATCH v3 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Taylor Blau]

`git config` has long allowed the ability for callers to provide a 'type
specifier', which instructs `git config` to (1) ensure that incoming
values are satisfiable under that type, and (2) that outgoing values are
canonicalized under that type.

In another series, we propose to add extend this functionality with
`--color` and `--default` to replace `--get-color`.

However, we traditionally use `--color` to mean "colorize this output",
instead of "this value should be treated as a color".

Currently, `git config` does not support this kind of colorization, but
we should be careful to avoid inhabiting this option too soon, so that
`git config` can support `--color` (in the traditional sense) in the
future, if that is desired.

In this patch, we prefer `--type=[int|bool|bool-or-int|...]` over
`--int`, `--bool`, and etc. This allows the aforementioned other patch
to add `--color` (in the non-traditional sense) via `--type=color`,
instead of `--color`.

Signed-off-by: Taylor Blau 
---
 Documentation/git-config.txt | 66 +++-
 builtin/config.c | 28 +++
 t/t1300-repo-config.sh   | 18 ++
 3 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e09ed5d7d..86c9b 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,13 +9,13 @@ git-config - Get and set repository or global options
 SYNOPSIS
 
 [verse]
-'git config' [] [type] [--show-origin] [-z|--null] name [value 
[value_regex]]
-'git config' [] [type] --add name value
-'git config' [] [type] --replace-all name value [value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get-all name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] [--name-only] 
--get-regexp name_regex [value_regex]
-'git config' [] [type] [-z|--null] --get-urlmatch name URL
+'git config' [] [--type] [--show-origin] [-z|--null] name [value 
[value_regex]]
+'git config' [] [--type] --add name value
+'git config' [] [--type] --replace-all name value [value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] --get name 
[value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] --get-all 
name [value_regex]
+'git config' [] [--type] [--show-origin] [-z|--null] 
[--name-only] --get-regexp name_regex [value_regex]
+'git config' [] [--type] [-z|--null] --get-urlmatch name URL
 'git config' [] --unset name [value_regex]
 'git config' [] --unset-all name [value_regex]
 'git config' [] --rename-section old_name new_name
@@ -38,12 +38,10 @@ 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 may be given as an argument to `--type` to make 'git config'
+ensure that the variable(s) are of the given type and convert the value to the
+canonical form. If no type specifier is passed, no checks or transformations 
are
+performed on the value.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
@@ -160,30 +158,34 @@ See also <>.
 --list::
List all variables set in config file, along with their values.
 
---bool::
-   'git config' will ensure that the output is "true" or "false"
+--type [type]::
+  'git config' will ensure that any input output is valid under the given type
+  constraint(s), and will canonicalize outgoing values in `[type]`'s canonical
+  form.
++
+Valid `[type]`'s include:
++
+- 'bool': canonicalize values as either "true" or "false".
+- 'int': canonicalize values as simple decimal numbers. An optional suffix of
+  'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or
+  1073741824 prior to output.
+- 'bool-or-int': canonicalize according to either 'bool' or 'int', as described
+  above.
+- 'path': canonicalize by adding a leading `~` to the value of `$HOME` and
+  `~user` to the home directory for the specified user. This specifier has no
+  effect when setting the value (but you can use `git config section.variable
+  ~/` from the command line to let your shell do the expansion.)
+- 'expiry-date': canonicalize by converting from a fixed or relative 
date-string
+  to a timestamp. This specifier has no effect when setting the value.
++
 
+--bool::
 --int::
-   'git config' will ensure that the output is a simple
-  

[PATCH v3 0/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Taylor Blau]

Hi,

Here is a v3 of a series to (1) treat type specifiers in `git config` as
singulars rather than a collection of bits, and (2) to prefer --type= over
--.

I have made the following changes since v2:

  - Fix some misspellings in Documentation/git-config.txt (cc: René).
  - Handle --no-type correctly (cc: Peff).
  - No longer prefer (1 << x) style for enumerating type specifiers (cc: Peff).
  - Fix awkward rebase (cc: Peff).

Thanks as always for your review :-).


Thanks,
Taylor

Taylor Blau (2):
  builtin/config.c: treat type specifiers singularly
  builtin/config.c: prefer `--type=bool` over `--bool`, etc.

 Documentation/git-config.txt | 66 ---
 builtin/config.c | 77 +++-
 t/t1300-repo-config.sh   | 29 ++
 3 files changed, 113 insertions(+), 59 deletions(-)

--
2.16.2.440.gc6284da4f