Re: [PATCH v3 12/13] worktree.c: check whether branch is bisected in another worktree

[Eric Sunshine]

On Fri, Apr 22, 2016 at 9:01 AM, Nguyễn Thái Ngọc Duy  wrote:
> Similar to the rebase case, we want to detect if "HEAD" in some worktree
> is being bisected because
>
> 1) we do not want to checkout this branch in another worktree, after
>bisect is done it will want to go back to this branch
>
> 2) we do not want to delete the branch is either or git bisect will
>fail to return to the (long gone) branch

I'm very far behind with my reviews and I see that this series is
already in 'next', so perhaps these comments are too late...

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> @@ -263,4 +263,17 @@ test_expect_success 'check out from current worktree 
> branch ok' '
> +test_expect_success 'checkout a branch under bisect' '
> +   git worktree add under-bisect &&
> +   (
> +   cd under-bisect &&
> +   git bisect start &&
> +   git bisect bad &&
> +   git bisect good HEAD~2 &&
> +   git worktree list | grep "under-bisect.*detached HEAD" &&
> +   test_must_fail git worktree add new-bisect under-bisect &&

Nit: I realize that the checking 'worktree add' is sufficient, but
it's a bit confusing to read in the commit message about how deleting
the branch would be bad, but then see it testing only 'add'.

> +   ! test -d new-bisect
> +   )
> +'
> diff --git a/worktree.c b/worktree.c
> @@ -234,6 +234,21 @@ static int is_worktree_being_rebased(const struct 
> worktree *wt,
> +static int is_worktree_being_bisected(const struct worktree *wt,
> + const char *target)
> +{
> +   struct wt_status_state state;
> +   int found_rebase;

s/rebase/bisect/ perhaps?

> +   memset(, 0, sizeof(state));
> +   found_rebase = wt_status_check_bisect(wt, ) &&
> +   state.branch &&
> +   starts_with(target, "refs/heads/") &&
> +   !strcmp(state.branch, target + strlen("refs/heads/"));
> +   free(state.branch);
> +   return found_rebase;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 24/25] worktree move: accept destination as directory

[Eric Sunshine]

On Wed, Apr 13, 2016 at 9:15 AM, Nguyễn Thái Ngọc Duy  wrote:
> Similar to "mv a b/", which is actually "mv a b/a", we extract basename
> of source worktree and create a directory of the same name at
> destination if dst path is a directory.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -538,7 +538,13 @@ static int move_worktree(int ac, const char **av, const 
> char *prefix)
> -   if (file_exists(dst.buf))
> +   if (is_directory(dst.buf))
> +   /*
> +* keep going, dst will be appended after we get the
> +* source's absolute path
> +*/
> +   ;
> +   else if (file_exists(dst.buf))
> die(_("target '%s' already exists"), av[1]);
> @@ -558,6 +564,17 @@ static int move_worktree(int ac, const char **av, const 
> char *prefix)
> +   if (is_directory(dst.buf)) {
> +   const char *sep = strrchr(wt->path, '/');

Does this need to take Windows into account? Perhaps git_find_last_dir_sep()?

> +
> +   if (!sep)
> +   die(_("could not figure out destination name from 
> '%s'"),
> +   wt->path);
> +   strbuf_addstr(, sep);
> +   if (file_exists(dst.buf))
> +   die(_("target '%s' already exists"), dst.buf);
> +   }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 2/6] convert.c: stream and early out

[Torsten Bögershausen]

On 09.05.16 22:29, Junio C Hamano wrote:
> tbo...@web.de writes:
> 
>> +if (stats->stat_bits & earlyout)
>> +break; /* We found what we have been searching for */
> 
> Are we sure if our callers are only interested in just one bit at a
> time?  Otherwise, if we want to ensure all of the given bits are
> set,
> 
>   if ((stats->stat_bits & earlyout) == earlyout)
>   break;
> 
> would be necessary.  Otherwise, the "only one bit" assumption on the
> "earlyout" parameter somehow needs to be documented in the code.
Thanks for pointing that out.
I have changed the code a couple of times, forth and back.
I want to re-roll the series anyway (probably in the next weeks),
so something like "search_only_flags" may be a better name.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments

[Junio C Hamano]

Junio C Hamano  writes:

> Stefan Beller  writes:
>
>> +static void split_argv_pathspec_groups(int argc, const char **argv,
>> +   const char ***pathspec_argv,
>> +   struct string_list *group)
>> +{
>> +int i;
>> +struct argv_array ps = ARGV_ARRAY_INIT;
>> +for (i = 0; i < argc; i++) {
>> +if (starts_with(argv[i], "*")
>> +|| starts_with(argv[i], ":")) {
>> +string_list_insert(group, argv[i]);
>> +} else if (starts_with(argv[i], "./")) {
>> +argv_array_push(, argv[i]);
>
> I'd suggest stripping "./" when you do this.  That is, make the rule
> to be "*label is a label, :name is a name, and everything else is a
> path.  You can prefix ./ to an unfortunate path that begins with an
> asterisk or a colon and we will strip ./ disambiguator".

To clarify, "./$path and $path are the same so why strip it?" is an
understandable, even though naive, question. The reason is because
you do not want to contaminate the code that parses pathspecs with
the knowledge of this submodule-group specific prefix.  "./$path"
and "$path" may be equivalent for a literal pathspec, but I'd want
to see user be able to say

   git submodule foreach './:(icase)foo'

and find Foo, foo, FOO, etc.

I would also recommend to strip '*' and ':' from group names and
module names, and maintain two separate lists.  You would eventually
want to do "*default*" to name all groups whose name matches with
the pattern "default*", as if it were a pathspec matched against the
available group names, and that will keep the door open for future
extension like  "*:(icase)default*".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] submodule groups

[Junio C Hamano]

Stefan Beller  writes:

> I started from scratch as I think there were some sharp edges in the design.
> My thinking shifted from "submodule groups" towards "actually it's just an
> enhanced pathspec, called submodulespec".

Except for minor things I mentioned separately, overall, this seems
quite cleanly done.

Looks very promising.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments

[Junio C Hamano]

Stefan Beller  writes:

> +static void split_argv_pathspec_groups(int argc, const char **argv,
> +const char ***pathspec_argv,
> +struct string_list *group)
> +{
> + int i;
> + struct argv_array ps = ARGV_ARRAY_INIT;
> + for (i = 0; i < argc; i++) {
> + if (starts_with(argv[i], "*")
> + || starts_with(argv[i], ":")) {
> + string_list_insert(group, argv[i]);
> + } else if (starts_with(argv[i], "./")) {
> + argv_array_push(, argv[i]);

I'd suggest stripping "./" when you do this.  That is, make the rule
to be "*label is a label, :name is a name, and everything else is a
path.  You can prefix ./ to an unfortunate path that begins with an
asterisk or a colon and we will strip ./ disambiguator".

> + } else {
> + /*
> + * NEEDSWORK:
> + * Improve autodetection of items with no prefixes,
> + * roughly like
> + * if (label_or_name_exists(argv[i]))
> + *   string_list_insert(group, argv[i]);
> + * else
> + *   argv_array_push(, argv[i]);
> + */

I do not think this is desirable.  "label" that changes its meaning
between "a path ./label" to "a label *label" would force people to
give unnecessary prefix "./" when naming their path, from fear of
colliding with a label that may or may not exist.

> + argv_array_push(, argv[i]);
> + }
> + }



> + if (!group.nr) {
> + if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
> + 0, ps_matched, 1) ||
> + !S_ISGITLINK(ce->ce_mode))
> + continue;
> + } else {
> + const struct submodule *sub;
> + int ps = 0, gr = 0;
> + if (!S_ISGITLINK(ce->ce_mode))
> + continue;
> + sub = submodule_from_path(null_sha1, ce->name);
> +
> + ps = match_pathspec(pathspec, ce->name, ce_namelen(ce),
> + 0, ps_matched, 1);
> + gr = submodule_in_group(, sub);
> +
> + if (!pathspec->nr && !gr)
> + continue;
> + if (!ps && !gr)
> + continue;

Hmph, so the rule is "a submodule that is in the specified group is
chosen, even if it is outside the subset of paths narrowed by the
given pathspec."  I think that is consistent with the way how the
list of arguments given to module_list (i.e. a pathspec element plus
a group specifier OR together just like two pathspec elements do not
force a path to match both, and instead they OR together).

Is that rule (i.e. specifiers are ORed together) written down
somewhere for users?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/7] submodule-config: check if a submodule is in a group

[Junio C Hamano]

Stefan Beller  writes:

> +static int in_group(int argc, const char **argv, const char *prefix)
> + ...
> + if (!group)
> + list = git_config_get_value_multi("submodule.updateGroup");
> + else {
> + string_list_split(_list, group, ',', -1);

Is this expected to be used only by test scripts, or will it have
real scripted Porcelain as its users?  I am wondering if
concatenation with ' ' would be more natural if it is the latter (if
only used for testing, I don't care that much).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] submodule-config: keep labels around

[Junio C Hamano]

Stefan Beller  writes:

> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct 
> submodule_cache *cache,
>   submodule->update_strategy.command = NULL;
>   submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>   submodule->ignore = NULL;
> + submodule->labels = NULL;

Hmph, is there a reason to do this, instead of embedding an instance
of "struct string_list" inside submodule structure?

I am not yet claiming that embedding is better.  Just wondering if
it makes it easier to handle initialization as seen in the hunk
below, and also _clear() procedure.

> @@ -353,6 +358,17 @@ static int parse_config(const char *var, const char 
> *value, void *data)
>   else if (parse_submodule_update_strategy(value,
>>update_strategy) < 0)
>   die(_("invalid value for %s"), var);
> + } else if (!strcmp(item.buf, "label")) {
> + if (!value)
> + ret = config_error_nonbool(var);
> + else {
> + if (!submodule->labels) {
> + submodule->labels =
> + xmalloc(sizeof(*submodule->labels));
> + string_list_init(submodule->labels, 1);
> + }
> + string_list_insert(submodule->labels, value);
> + }
>   }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] submodule add: label submodules if asked to

[Junio C Hamano]

Stefan Beller  writes:

> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 814ee63..0adc4e4 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1056,6 +1056,7 @@ test_expect_success 'submodule with UTF-8 name' '
>  '
>  
>  test_expect_success 'submodule add clone shallow submodule' '
> + test_when_finished "rm -rf super" &&
>   mkdir super &&
>   pwd=$(pwd) &&
>   (
> @@ -1094,5 +1095,48 @@ test_expect_success 'submodule helper list is not 
> confused by common prefixes' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success 'submodule add records a label' '
> + test_when_finished "rm -rf super" &&
> + mkdir super &&
> + pwd=$(pwd) &&
> + (
> + cd super &&
> + git init &&
> + git submodule add --label labelA file://"$pwd"/example2 
> submodule &&
> + git config -f .gitmodules submodule."submodule".label 
> >../actual &&
> + echo labelA >../expect
> + ) &&
> + test_cmp expect actual
> +'
> +
> +cat >expect <<-EOF
> +labelA
> +labelB
> +EOF
> +
> +test_expect_success 'submodule add records multiple labels' '

The existing tests in this file may be littered with this bad
construct, but please do not add more example of running things
outside of test_expect_{success,failure} block unless there is a
good reason to do so.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] submodule--helper: add valid-label-name

[Junio C Hamano]

Stefan Beller  writes:

> +static int submodule_valid_label_name(const char *label)
> +{
> + if (!label || !strlen(label))
> + return 0;
> +
> + if (!isalnum(*label))
> + return 0;

I'd limit this one to isalpha() if I were doing this to make the
restriction similar to identifiers in traditional programming
language.

> + while (*label) {
> + if (!(isalnum(*label) ||
> + *label == '-'))

And throw in '_' to the mix while at it.

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

If the convention is "0 is good", then please signal "bad" with a
negative value, not just "non-zero".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] clone: allow specification of submodules to be cloned

[Stefan Beller]

This allows to specify a subset of all available submodules to be
initialized and cloned. It is unrelated to the `--recursive` option,
i.e. the user may still want to give `--recursive` as an option.

Originally `--recursive` implied to initialize all submodules, this
changes as well with the new option, such that only the specified
submodules are cloned, and their submodules (i.e. subsubmodules)
are cloned in full as the submodule specification is not passed on.

Signed-off-by: Stefan Beller 
---
 Documentation/git-clone.txt | 26 +++
 builtin/clone.c | 40 +--
 t/t7400-submodule-basic.sh  | 79 +
 3 files changed, 136 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 45d74be..4a9e8bb 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,7 +14,8 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--jobs ] [--] 
+ [--recursive | --recurse-submodules] [--jobs ]
+ [--init-submodule ] [--] 
  []
 
 DESCRIPTION
@@ -205,12 +206,23 @@ objects from the source repository into a pack in the 
cloned repository.
 
 --recursive::
 --recurse-submodules::
-   After the clone is created, initialize all submodules within,
-   using their default settings. This is equivalent to running
-   `git submodule update --init --recursive` immediately after
-   the clone is finished. This option is ignored if the cloned
-   repository does not have a worktree/checkout (i.e. if any of
-   `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+   After the clone is created, initialize and clone all submodules
+   within, using their default settings. This is equivalent to
+   running `git submodule update --recursive --init `
+   immediately after the clone is finished. This option is ignored
+   if the cloned repository does not have a worktree/checkout (i.e.
+   if any of `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+
+--init-submodule::
+   After the clone is cloned, initialize and clone specified
+   submodules within, using their default settings. It is possible
+   to give multiple specifications by giving this argument multiple
+   times or by giving a comma separated list. This is equivalent to
+   running `git submodule update ` immediately
+   after the clone is finished. To specify submodules you can use
+   their path, name or labels, see linkgit:git-submodules[1]. This
+   option will be recorded in the repository config as
+   `submodule.updateGroup`.
 
 --separate-git-dir=::
Instead of placing the cloned repository where it is supposed
diff --git a/builtin/clone.c b/builtin/clone.c
index 6576ecf..fa2f989 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -52,6 +52,22 @@ static struct string_list option_config;
 static struct string_list option_reference;
 static int option_dissociate;
 static int max_jobs = -1;
+static struct string_list init_submodules;
+
+static int init_submodules_cb(const struct option *opt, const char *arg, int 
unset)
+{
+   struct string_list_item *item;
+   struct string_list sl = STRING_LIST_INIT_DUP;
+
+   if (unset)
+   return -1;
+
+   string_list_split(, arg, ',', -1);
+   for_each_string_list_item(item, )
+   string_list_append((struct string_list *)opt->value, 
item->string);
+
+   return 0;
+}
 
 static struct option builtin_clone_options[] = {
OPT__VERBOSITY(_verbosity),
@@ -100,6 +116,8 @@ static struct option builtin_clone_options[] = {
TRANSPORT_FAMILY_IPV4),
OPT_SET_INT('6', "ipv6", , N_("use IPv6 addresses only"),
TRANSPORT_FAMILY_IPV6),
+   OPT_CALLBACK(0, "init-submodule", _submodules, N_("string"),
+   N_("clone specific submodules"), init_submodules_cb),
OPT_END()
 };
 
@@ -731,13 +749,22 @@ static int checkout(void)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
   sha1_to_hex(sha1), "1", NULL);
 
-   if (!err && option_recursive) {
+   if (!err && (option_recursive || init_submodules.nr > 0)) {
struct argv_array args = ARGV_ARRAY_INIT;
-   argv_array_pushl(, "submodule", "update", "--init", 
"--recursive", NULL);
+   struct string_list_item *item;
+   argv_array_pushl(, "submodule", "update", NULL);
+
+   argv_array_pushf(, "--init");
+
+   if (option_recursive)
+   argv_array_pushf(, "--recursive");
 
if (max_jobs != -1)
argv_array_pushf(, "--jobs=%d", max_jobs);
 
+   

[PATCH 4/7] submodule-config: check if a submodule is in a group

[Stefan Beller]

In later patches we need to tell if a submodule is in a group,
so expose a handy test function in both C and shell.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c  | 42 +++-
 submodule-config.c   | 50 ++
 submodule-config.h   |  3 +++
 t/t7412-submodule--helper.sh | 58 
 4 files changed, 152 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d3f4684..6ffd1c1 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -858,6 +858,45 @@ static int valid_label_name(int argc, const char **argv, 
const char *prefix)
  "and must contain alphanumeric characters or dashes only."));
 }
 
+static int in_group(int argc, const char **argv, const char *prefix)
+{
+   const struct string_list *list;
+   struct string_list actual_list = STRING_LIST_INIT_DUP;
+   const struct submodule *sub;
+   const char *group = NULL;
+
+   struct option default_group_options[] = {
+   OPT_STRING('g', "group", , N_("group"),
+   N_("comma separated group specifier for 
submodules")),
+   OPT_END()
+   };
+
+   const char *const git_submodule_helper_usage[] = {
+   N_("git submodule--helper in-group "),
+   NULL
+   };
+
+   argc = parse_options(argc, argv, prefix, default_group_options,
+git_submodule_helper_usage, 0);
+
+   gitmodules_config();
+   git_config(submodule_config, NULL);
+
+   if (argc != 1)
+   usage(git_submodule_helper_usage[0]);
+
+   sub = submodule_from_path(null_sha1, argv[0]);
+
+   if (!group)
+   list = git_config_get_value_multi("submodule.updateGroup");
+   else {
+   string_list_split(_list, group, ',', -1);
+   list = _list;
+   }
+
+   return !submodule_in_group(list, sub);
+}
+
 struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
@@ -871,7 +910,8 @@ static struct cmd_struct commands[] = {
{"resolve-relative-url", resolve_relative_url},
{"resolve-relative-url-test", resolve_relative_url_test},
{"init", module_init},
-   {"valid-label-name", valid_label_name}
+   {"valid-label-name", valid_label_name},
+   {"in-group", in_group}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/submodule-config.c b/submodule-config.c
index 0cdb47e..7f38ebd 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -522,3 +522,53 @@ void submodule_free(void)
cache_free();
is_cache_init = 0;
 }
+
+int submodule_in_group(const struct string_list *group,
+  const struct submodule *sub)
+{
+   int matched = 0;
+   struct strbuf sb = STRBUF_INIT;
+
+   if (!group)
+   /*
+* If no group is specified at all, all submodules match to
+* keep traditional behavior.
+*/
+   return 1;
+
+   if (sub->labels) {
+   struct string_list_item *item;
+   for_each_string_list_item(item, sub->labels) {
+   strbuf_reset();
+   strbuf_addf(, "*%s", item->string);
+   if (string_list_has_string(group, sb.buf)) {
+   matched = 1;
+   break;
+   }
+   }
+   }
+   if (sub->path) {
+   /*
+* NEEDSWORK: This currently works only for
+* exact paths, but we want to enable
+* inexact matches such wildcards.
+*/
+   strbuf_reset();
+   strbuf_addf(, "./%s", sub->path);
+   if (string_list_has_string(group, sb.buf))
+   matched = 1;
+   }
+   if (sub->name) {
+   /*
+* NEEDSWORK: Same as with path. Do we want to
+* support wildcards or such?
+*/
+   strbuf_reset();
+   strbuf_addf(, ":%s", sub->name);
+   if (string_list_has_string(group, sb.buf))
+   matched = 1;
+   }
+   strbuf_release();
+
+   return matched;
+}
diff --git a/submodule-config.h b/submodule-config.h
index d57da59..4c696cc 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -31,4 +31,7 @@ const struct submodule *submodule_from_path(const unsigned 
char *commit_sha1,
const char *path);
 void submodule_free(void);
 
+int submodule_in_group(const struct string_list *group,
+  const struct submodule *sub);
+
 #endif /* SUBMODULE_CONFIG_H */
diff --git a/t/t7412-submodule--helper.sh 

[PATCH 6/7] submodule update: learn partial initialization

[Stefan Beller]

The new switch `--init-default-group` updates the submodules which are
configured in `submodule.updateGroup`

Signed-off-by: Stefan Beller 
---
 Documentation/config.txt|  5 
 Documentation/git-submodule.txt |  4 ++--
 git-submodule.sh| 14 +--
 t/t7400-submodule-basic.sh  | 53 +
 4 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 59d7046..0f20019 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2735,6 +2735,11 @@ submodule.fetchJobs::
in parallel. A value of 0 will give some reasonable default.
If unset, it defaults to 1.
 
+submodule.updateGroup::
+   Specifies the group of submodules when `git submodule --init-group`
+   is called with no arguments. This setting is recorded in the initial
+   clone when `--init-submodule` was given.
+
 tag.sort::
This variable controls the sort ordering of tags when displayed by
linkgit:git-tag[1]. Without the "--sort=" option provided, the
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 35ca355..e658d15 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,9 +14,9 @@ SYNOPSIS
 'git submodule' [--quiet] status [--cached] [--recursive] [--] 
[...]
 'git submodule' [--quiet] init [--] [...]
 'git submodule' [--quiet] deinit [-f|--force] [--] ...
-'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
+'git submodule' [--quiet] update [--init[-default-group]] [--remote] 
[-N|--no-fetch]
  [-f|--force] [--rebase|--merge] [--reference ]
- [--depth ] [--recursive] [--jobs ] [--] [...]
+ [--depth ] [--recursive] [--jobs ] [--] 
[...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]
  [commit] [--] [...]
 'git submodule' [--quiet] foreach [--recursive] 
diff --git a/git-submodule.sh b/git-submodule.sh
index c8e36c5..2b0b0cb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -522,7 +522,12 @@ cmd_update()
GIT_QUIET=1
;;
-i|--init)
-   init=1
+   test -z $init || test $init = by_args || die "$(gettext 
"Only one of --init or --init-default-group may be used.")"
+   init=by_args
+   ;;
+   --init-default-group)
+   test -z $init || test $init = by_config || die 
"$(gettext "Only one of --init or --init-default-group may be used.")"
+   init=by_config
;;
--remote)
remote=1
@@ -585,7 +590,12 @@ cmd_update()
 
if test -n "$init"
then
-   cmd_init "--" "$@" || return
+   additional_init=
+   if test "$init" = "by_config"
+   then
+   additional_init=$(git config --get-all 
submodule.updateGroup)
+   fi
+   cmd_init "--" "$@" ${additional_init:+$additional_init} || 
return
fi
 
{
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 0adc4e4..41e65c2 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1139,4 +1139,57 @@ test_expect_success 'submodule add recording wrong 
labels reports an error' '
test_i18ngrep alphanumeric actual
 '
 
+test_expect_success 'setup superproject with labeled submodules' '
+   mkdir sub1 &&
+   (
+   cd sub1 &&
+   git init &&
+   test_commit test
+   test_commit test2
+   ) &&
+   mkdir labeledsuper &&
+   (
+   cd labeledsuper &&
+   git init &&
+   git submodule add ../sub1 sub0 &&
+   git submodule add -l bit1 ../sub1 sub1 &&
+   git submodule add -l bit2 ../sub1 sub2 &&
+   git submodule add -l bit2 -l bit1 ../sub1 sub3 &&
+   git commit -m "add labeled submodules"
+   )
+'
+
+cat >expect <<-EOF
+-sub0
+ sub1 (test2)
+ sub2 (test2)
+ sub3 (test2)
+EOF
+
+test_expect_success 'submodule update --init with a group' '
+   test_when_finished "rm -rf labeledsuper_clone" &&
+   pwd=$(pwd) &&
+   git clone file://"$pwd"/labeledsuper labeledsuper_clone &&
+   (
+   cd labeledsuper_clone &&
+   git submodule update --init \*bit1 ./sub2 &&
+   git submodule status |cut -c 1,43- >../actual
+   ) &&
+   test_cmp expect actual
+'
+
+test_expect_success 'submodule update --init-default-group' '
+   test_when_finished "rm -rf labeledsuper_clone" &&
+   pwd=$(pwd) &&
+   git clone file://"$pwd"/labeledsuper labeledsuper_clone &&
+   (
+   cd labeledsuper_clone &&
+   git config 

[PATCH 3/7] submodule-config: keep labels around

[Stefan Beller]

We need the submodule labels in a later patch.

Signed-off-by: Stefan Beller 
---
 submodule-config.c | 16 
 submodule-config.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/submodule-config.c b/submodule-config.c
index b82d1fb..0cdb47e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -60,6 +60,10 @@ static void free_one_config(struct submodule_entry *entry)
free((void *) entry->config->path);
free((void *) entry->config->name);
free((void *) entry->config->update_strategy.command);
+   if (entry->config->labels) {
+   string_list_clear(entry->config->labels, 0);
+   free(entry->config->labels);
+   }
free(entry->config);
 }
 
@@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
submodule->update_strategy.command = NULL;
submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
submodule->ignore = NULL;
+   submodule->labels = NULL;
 
hashcpy(submodule->gitmodules_sha1, gitmodules_sha1);
 
@@ -353,6 +358,17 @@ static int parse_config(const char *var, const char 
*value, void *data)
else if (parse_submodule_update_strategy(value,
 >update_strategy) < 0)
die(_("invalid value for %s"), var);
+   } else if (!strcmp(item.buf, "label")) {
+   if (!value)
+   ret = config_error_nonbool(var);
+   else {
+   if (!submodule->labels) {
+   submodule->labels =
+   xmalloc(sizeof(*submodule->labels));
+   string_list_init(submodule->labels, 1);
+   }
+   string_list_insert(submodule->labels, value);
+   }
}
 
strbuf_release();
diff --git a/submodule-config.h b/submodule-config.h
index e4857f5..d57da59 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -18,6 +18,8 @@ struct submodule {
struct submodule_update_strategy update_strategy;
/* the sha1 blob id of the responsible .gitmodules file */
unsigned char gitmodules_sha1[20];
+   /* sorted, not as on disk */
+   struct string_list *labels;
 };
 
 int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
-- 
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments

[Stefan Beller]

Additionally to taking a pathspec, `module_list_compute` will also take
labels and submodule names, when these are prefixed by '*' and ':'
respectively.

`module_list_compute` is used by other functions in the submodule helper:
* module_list, used by `submodule {deinit, status, sync, foreach}`
* module_init, used by `submodule init`
* update_clone, used by `submodule update`

{foreach, update} do not pass on command line arguments to the listing
function such that these are not affected. The rest of them {deinit,
status, sync, init} will be exercised in additional tests.

As the labeling requires lookup of .gitmodules files, we need to make sure
the submodule config cache is initialized in all the functions, that's why
there are additional calls to gitmodules_config() and   git_config(...);

Signed-off-by: Stefan Beller 
---
 Documentation/git-submodule.txt | 22 +--
 builtin/submodule--helper.c | 76 +++-
 git-submodule.sh|  8 ++--
 t/t7412-submodule--helper.sh| 86 +
 4 files changed, 175 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 9ba8895..35ca355 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -11,16 +11,16 @@ SYNOPSIS
 [verse]
 'git submodule' [--quiet] add [-b ] [-f|--force] [-l|--label ]
  [--reference ] [--depth ] [--]  
[]
-'git submodule' [--quiet] status [--cached] [--recursive] [--] [...]
-'git submodule' [--quiet] init [--] [...]
-'git submodule' [--quiet] deinit [-f|--force] [--] ...
+'git submodule' [--quiet] status [--cached] [--recursive] [--] 
[...]
+'git submodule' [--quiet] init [--] [...]
+'git submodule' [--quiet] deinit [-f|--force] [--] ...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
  [-f|--force] [--rebase|--merge] [--reference ]
  [--depth ] [--recursive] [--jobs ] [--] [...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]
  [commit] [--] [...]
 'git submodule' [--quiet] foreach [--recursive] 
-'git submodule' [--quiet] sync [--recursive] [--] [...]
+'git submodule' [--quiet] sync [--recursive] [--] [...]
 
 
 DESCRIPTION
@@ -37,6 +37,20 @@ these will not be checked out by default; the 'init' and 
'update'
 subcommands will maintain submodules checked out and at
 appropriate revision in your working tree.
 
+When working on submodules you can specify the desired submodule
+group or give no specification at all to apply the command to the
+default group of submodules, which includes all submodules.
+To group submodules you can make use of
+ . a pathspec,
+ . their name,
+ . labels.
+It is undefined behavior to give a spec that is ambigious for a name,
+pathspec or label. To make the specification unambigous, you can prefix
+ . pathspecs with './',
+ . names with ':',
+ . labels with '*'.
+Labels must consist of alphanumeric characters or the dash character.
+
 Submodules are composed from a so-called `gitlink` tree entry
 in the main repository that refers to a particular commit object
 within the inner repository that is completely separate.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6ffd1c1..ba43c80 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -12,6 +12,7 @@
 #include "remote.h"
 #include "refs.h"
 #include "connect.h"
+#include "argv-array.h"
 
 static char *get_default_remote(void)
 {
@@ -215,6 +216,36 @@ static int resolve_relative_url_test(int argc, const char 
**argv, const char *pr
return 0;
 }
 
+static void split_argv_pathspec_groups(int argc, const char **argv,
+  const char ***pathspec_argv,
+  struct string_list *group)
+{
+   int i;
+   struct argv_array ps = ARGV_ARRAY_INIT;
+   for (i = 0; i < argc; i++) {
+   if (starts_with(argv[i], "*")
+   || starts_with(argv[i], ":")) {
+   string_list_insert(group, argv[i]);
+   } else if (starts_with(argv[i], "./")) {
+   argv_array_push(, argv[i]);
+   } else {
+   /*
+   * NEEDSWORK:
+   * Improve autodetection of items with no prefixes,
+   * roughly like
+   * if (label_or_name_exists(argv[i]))
+   *   string_list_insert(group, argv[i]);
+   * else
+   *   argv_array_push(, argv[i]);
+   */
+   argv_array_push(, argv[i]);
+   }
+   }
+
+   *pathspec_argv = argv_array_detach();
+   argv_array_clear();
+}
+
 struct module_list {
const struct cache_entry **entries;
int alloc, nr;
@@ -228,10 +259,15 @@ 

[PATCH 1/7] submodule--helper: add valid-label-name

[Stefan Beller]

We could allow more than just alphanumeric and dash characters
for submodule labels. As a precaution we'll first allow only this
subset and later on we can extend it once we have more experience
with them.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c  | 30 ++-
 t/t7412-submodule--helper.sh | 49 
 2 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100755 t/t7412-submodule--helper.sh

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7f0941d..d3f4684 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -831,6 +831,33 @@ static int update_clone(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+static int submodule_valid_label_name(const char *label)
+{
+   if (!label || !strlen(label))
+   return 0;
+
+   if (!isalnum(*label))
+   return 0;
+
+   while (*label) {
+   if (!(isalnum(*label) ||
+   *label == '-'))
+   return 0;
+   label++;
+   }
+
+   return 1;
+}
+
+static int valid_label_name(int argc, const char **argv, const char *prefix)
+{
+   if (argc == 2 && submodule_valid_label_name(argv[1]))
+   return 0;
+
+   die(_("submodule label must start with an alphanumeric character"
+ "and must contain alphanumeric characters or dashes only."));
+}
+
 struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
@@ -843,7 +870,8 @@ static struct cmd_struct commands[] = {
{"update-clone", update_clone},
{"resolve-relative-url", resolve_relative_url},
{"resolve-relative-url-test", resolve_relative_url_test},
-   {"init", module_init}
+   {"init", module_init},
+   {"valid-label-name", valid_label_name}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh
new file mode 100755
index 000..3af315c
--- /dev/null
+++ b/t/t7412-submodule--helper.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='Basic plumbing support of submodule--helper
+
+This test verifies the submodule--helper plumbing command used to implement
+git-submodule.
+'
+
+. ./test-lib.sh
+
+
+test_expect_success 'valid-label-name tests empty label' '
+   test_must_fail git submodule--helper valid-label-name 2>actual &&
+   test_i18ngrep alphanumeric actual &&
+   test_must_fail git submodule--helper valid-label-name "" 2>actual &&
+   test_i18ngrep alphanumeric actual
+'
+
+test_expect_success 'valid-label-name tests correct label asdf' '
+   git submodule--helper valid-label-name asdf 2>actual &&
+   test_must_be_empty actual
+'
+
+test_expect_success 'valid-label-name tests correct label a' '
+   git submodule--helper valid-label-name a 2>actual &&
+   test_must_be_empty actual
+'
+
+test_expect_success 'valid-label-name tests correct label a-b' '
+   git submodule--helper valid-label-name a-b 2>actual &&
+   test_must_be_empty actual
+'
+
+test_expect_success 'valid-label-name fails with multiple arguments' '
+   test_must_fail git submodule--helper valid-label-name a b 2>actual &&
+   test_i18ngrep alphanumeric actual
+'
+
+test_expect_success 'valid-label-name fails with white spaced arguments' '
+   test_must_fail git submodule--helper valid-label-name "a b" 2>actual &&
+   test_i18ngrep alphanumeric actual
+'
+
+test_expect_success 'valid-label-name fails with utf8 characters' '
+   test_must_fail git submodule--helper valid-label-name ☺ 2>actual &&
+   test_i18ngrep alphanumeric actual
+'
+
+test_done
-- 
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] submodule add: label submodules if asked to

[Stefan Beller]

When adding new submodules, you can specify the labels the submodule
belongs to by giving one or more --label arguments. This will record
each label in the .gitmodules file as a value of the key
"submodule.$NAME.label".

Signed-off-by: Stefan Beller 
---
 Documentation/git-submodule.txt |  4 +++-
 git-submodule.sh| 16 ++-
 t/t7400-submodule-basic.sh  | 44 +
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 13adebf..9ba8895 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 
 [verse]
-'git submodule' [--quiet] add [-b ] [-f|--force] [--name ]
+'git submodule' [--quiet] add [-b ] [-f|--force] [-l|--label ]
  [--reference ] [--depth ] [--]  
[]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...]
 'git submodule' [--quiet] init [--] [...]
@@ -101,6 +101,8 @@ is the superproject and submodule repositories will be kept
 together in the same relative location, and only the
 superproject's URL needs to be provided: git-submodule will correctly
 locate the submodule using the relative URL in .gitmodules.
++
+All labels are recorded in the .gitmodules file in the label fields.
 
 status::
Show the status of the submodules. This will print the SHA-1 of the
diff --git a/git-submodule.sh b/git-submodule.sh
index 82e95a9..c1213d8 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] add [-b ] [-f|--force] [--name ] [--reference 
] [--]  []
+USAGE="[--quiet] add [-b ] [-f|--force] [--name ] [--reference 
] [-l|--label ][--]  []
or: $dashless [--quiet] status [--cached] [--recursive] [--] [...]
or: $dashless [--quiet] init [--] [...]
or: $dashless [--quiet] deinit [-f|--force] [--] ...
@@ -130,6 +130,7 @@ cmd_add()
 {
# parse $args after "submodule ... add".
reference_path=
+   labels=
while test $# -ne 0
do
case "$1" in
@@ -165,6 +166,15 @@ cmd_add()
--depth=*)
depth=$1
;;
+   -l|--label)
+   git submodule--helper valid-label-name "$2" || exit
+   labels="${labels} $2"
+   shift
+   ;;
+   --label=*)
+   git submodule--helper valid-label-name "${1#--label=}" 
|| exit
+   labels="${labels} ${1#--label=}"
+   ;;
--)
shift
break
@@ -292,6 +302,10 @@ Use -f if you really want to add it." >&2
 
git config -f .gitmodules submodule."$sm_name".path "$sm_path" &&
git config -f .gitmodules submodule."$sm_name".url "$repo" &&
+   for label in $labels
+   do
+   git config --add -f .gitmodules submodule."$sm_name".label 
"$label"
+   done &&
if test -n "$branch"
then
git config -f .gitmodules submodule."$sm_name".branch "$branch"
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 814ee63..0adc4e4 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1056,6 +1056,7 @@ test_expect_success 'submodule with UTF-8 name' '
 '
 
 test_expect_success 'submodule add clone shallow submodule' '
+   test_when_finished "rm -rf super" &&
mkdir super &&
pwd=$(pwd) &&
(
@@ -1094,5 +1095,48 @@ test_expect_success 'submodule helper list is not 
confused by common prefixes' '
test_cmp expect actual
 '
 
+test_expect_success 'submodule add records a label' '
+   test_when_finished "rm -rf super" &&
+   mkdir super &&
+   pwd=$(pwd) &&
+   (
+   cd super &&
+   git init &&
+   git submodule add --label labelA file://"$pwd"/example2 
submodule &&
+   git config -f .gitmodules submodule."submodule".label 
>../actual &&
+   echo labelA >../expect
+   ) &&
+   test_cmp expect actual
+'
+
+cat >expect <<-EOF
+labelA
+labelB
+EOF
+
+test_expect_success 'submodule add records multiple labels' '
+   test_when_finished "rm -rf super" &&
+   mkdir super &&
+   pwd=$(pwd) &&
+   (
+   cd super &&
+   git init &&
+   git submodule add --label=labelA -l labelB 
file://"$pwd"/example2 submodule &&
+   git config --get-all -f .gitmodules submodule."submodule".label 
>../actual
+   ) &&
+   test_cmp expect actual
+'
+
+test_expect_success 'submodule add recording wrong labels reports an error' '
+   test_when_finished "rm -rf super" &&
+   mkdir super &&
+  

[PATCH 0/7] submodule groups

[Stefan Beller]

I started from scratch as I think there were some sharp edges in the design.
My thinking shifted from "submodule groups" towards "actually it's just an
enhanced pathspec, called submodulespec".

The meat is found in the last 3 patches.

What is this series about?
==

If you have lots of submodules, you probably don't need all of them at once,
but you have functional units. Some submodules are absolutely required,
some are optional and only for very specific purposes.

This patch series adds labels to submodules in the .gitmodules file.

So you could have a .gitmodules file such as:

[submodule "gcc"]
path = gcc
url = git://...
label = default
label = devel
[submodule "linux"]
path = linux
url = git://...
label = default
[submodule "nethack"]
path = nethack
url = git://...
label = optional
label = games

and by this series you can work on an arbitrary group of these submodules
composed by the labels, names or paths of the submodules.

git clone --init-submodule=\* --init-submodule=: git://...
# will clone the superproject checkout
# any submodule being labeled  or named 

git submodule add --label  git://... ..
# record a label while adding a submodule

git submodule update [--init] \*label2
# update only the submodules labeled "label2"

git config submodule.updateGroups default
git config --add submodule.updateGroups devel
# configure which submodules you are interested in
git submodule update [--init-default-group] 
# ... and update them

git status 
git diff
git submodule summary
# unlike the last series, these are not touched

git submodule status \*label2
# only show information about "label2" submodules.


Any feedback welcome, specially on the design level!

Thanks,
Stefan

Prior series found here: 
http://thread.gmane.org/gmane.comp.version-control.git/292666

Stefan Beller (7):
  submodule--helper: add valid-label-name
  submodule add: label submodules if asked to
  submodule-config: keep labels around
  submodule-config: check if a submodule is in a group
  submodule--helper module_list_compute: allow label or name arguments
  submodule update: learn partial initialization
  clone: allow specification of submodules to be cloned

 Documentation/config.txt|   5 ++
 Documentation/git-clone.txt |  26 --
 Documentation/git-submodule.txt |  30 +--
 builtin/clone.c |  40 -
 builtin/submodule--helper.c | 146 +++---
 git-submodule.sh|  38 ++--
 submodule-config.c  |  66 ++
 submodule-config.h  |   5 ++
 t/t7400-submodule-basic.sh  | 176 
 t/t7412-submodule--helper.sh| 193 
 10 files changed, 692 insertions(+), 33 deletions(-)
 create mode 100755 t/t7412-submodule--helper.sh

-- 
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Yaroslav Halchenko]

On Tue, 10 May 2016, Junio C Hamano wrote:
> >> The necessary update to the client might be as simple as using
> >> $GIVEN_URL/.git/ and attempting the request again after seeing the
> >> probe for $GIVEN_URL/info/refs fails.

> > Sure -- workarounds are possible,...

> Just so that there is no misunderstanding, the above was not a
> workaround but is an outline of an implementation of updated http
> client shipped with Git.

ah, sorry, I have indeed might have misread it.  So we are on the same
page -- that is I saw also as the tentative implementation ;)

-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] worktree: simplify prefixing paths

[Duy Nguyen]

On Wed, May 11, 2016 at 6:07 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>
> This changes semantics, doesn't it?  prefix_filename() seems to do a
> lot more than just strbuf_vadd("%s%s", prefix, filename); would do.
>
> It may be a good change (e.g. turn '\' into '/' on Windows), but
> this is way more than "simplify prefixing".  It is something else
> whose effect needs to be explained.

On Wed, May 11, 2016 at 6:07 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>
> This changes semantics, doesn't it?  prefix_filename() seems to do a
> lot more than just strbuf_vadd("%s%s", prefix, filename); would do.
>
> It may be a good change (e.g. turn '\' into '/' on Windows), but
> this is way more than "simplify prefixing".  It is something else
> whose effect needs to be explained.

I admit I forgot about Windows part in prefix_filename(). For
non-Windows code, it's exactly the same behavior. Maybe we should do
this to emphasize that prefix_filename() is no-op when pfx_len is
zero? The same pattern is used elsewhere too (or I'm spreading it in
my local tree..) Unless of course if convert_slashes() is a good thing
to always do, then I need to update my commit message (+Johannes for
this question).

diff --git a/abspath.c b/abspath.c
index 2825de8..bf454e0 100644
--- a/abspath.c
+++ b/abspath.c
@@ -160,8 +160,11 @@ const char *absolute_path(const char *path)
 const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
 {
  static struct strbuf path = STRBUF_INIT;
+
+ if (!pfx_len)
+ return arg;
 #ifndef GIT_WINDOWS_NATIVE
- if (!pfx_len || is_absolute_path(arg))
+ if (is_absolute_path(arg))
  return arg;
  strbuf_reset();
  strbuf_add(, pfx, pfx_len);

>
>>  builtin/worktree.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index b53f802..f9dac37 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -337,7 +337,7 @@ static int add(int ac, const char **av, const char 
>> *prefix)
>>   if (ac < 1 || ac > 2)
>>   usage_with_options(worktree_usage, options);
>>
>> - path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];
>> + path = prefix_filename(prefix, strlen(prefix), av[0]);
>>   branch = ac < 2 ? "HEAD" : av[1];
>>
>>   opts.force_new_branch = !!new_branch_force;
>> @@ -467,6 +467,8 @@ int cmd_worktree(int ac, const char **av, const char 
>> *prefix)
>>
>>   if (ac < 2)
>>   usage_with_options(worktree_usage, options);
>> + if (!prefix)
>> + prefix = "";
>>   if (!strcmp(av[1], "add"))
>>   return add(ac - 1, av + 1, prefix);
>>   if (!strcmp(av[1], "prune"))
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 05/13] worktree.c: mark current worktree

[Duy Nguyen]

On Wed, May 11, 2016 at 6:03 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On second thought, why hold patches back, lengthen the worktree-move
>> series and make it a pain to review? I moved a few patches from
>> worktree-move into this series and I took two other out to create
>> nd/error-errno. So I'm going to take more patches out of it, creating
>> two bite-sized series, to be sent shortly.
>>
>> The first one is purely cleanup (ok, 1/7 is not exactly cleanup)
>>
>>   [1/7] completion: support git-worktree
>>   [2/7] worktree.c: rewrite mark_current_worktree() to avoid
>>   [3/7] git-worktree.txt: keep subcommand listing in alphabetical
>>   [4/7] worktree.c: use is_dot_or_dotdot()
>>   [5/7] worktree.c: add clear_worktree()
>>   [6/7] worktree: avoid 0{40}, too many zeroes, hard to read
>>   [7/7] worktree: simplify prefixing paths
>
> Where are these patches designed to apply?
>
> It appears that this depends on something in 'next' (probably
> nd/worktree-various-heads topic?)

Yes. Sorry I forgot to mention that. Though if you move 2/7 to
nd/worktree-various-heads (and deal with some conflicts in worktree.c)
then it may become independent.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] worktree: simplify prefixing paths

[Junio C Hamano]

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

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---

This changes semantics, doesn't it?  prefix_filename() seems to do a
lot more than just strbuf_vadd("%s%s", prefix, filename); would do.

It may be a good change (e.g. turn '\' into '/' on Windows), but
this is way more than "simplify prefixing".  It is something else
whose effect needs to be explained.

>  builtin/worktree.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index b53f802..f9dac37 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -337,7 +337,7 @@ static int add(int ac, const char **av, const char 
> *prefix)
>   if (ac < 1 || ac > 2)
>   usage_with_options(worktree_usage, options);
>  
> - path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];
> + path = prefix_filename(prefix, strlen(prefix), av[0]);
>   branch = ac < 2 ? "HEAD" : av[1];
>  
>   opts.force_new_branch = !!new_branch_force;
> @@ -467,6 +467,8 @@ int cmd_worktree(int ac, const char **av, const char 
> *prefix)
>  
>   if (ac < 2)
>   usage_with_options(worktree_usage, options);
> + if (!prefix)
> + prefix = "";
>   if (!strcmp(av[1], "add"))
>   return add(ac - 1, av + 1, prefix);
>   if (!strcmp(av[1], "prune"))
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 05/13] worktree.c: mark current worktree

[Junio C Hamano]

Duy Nguyen  writes:

> On second thought, why hold patches back, lengthen the worktree-move
> series and make it a pain to review? I moved a few patches from
> worktree-move into this series and I took two other out to create
> nd/error-errno. So I'm going to take more patches out of it, creating
> two bite-sized series, to be sent shortly.
>
> The first one is purely cleanup (ok, 1/7 is not exactly cleanup)
>
>   [1/7] completion: support git-worktree
>   [2/7] worktree.c: rewrite mark_current_worktree() to avoid
>   [3/7] git-worktree.txt: keep subcommand listing in alphabetical
>   [4/7] worktree.c: use is_dot_or_dotdot()
>   [5/7] worktree.c: add clear_worktree()
>   [6/7] worktree: avoid 0{40}, too many zeroes, hard to read
>   [7/7] worktree: simplify prefixing paths

Where are these patches designed to apply?

It appears that this depends on something in 'next' (probably
nd/worktree-various-heads topic?)

>
> And the second one adds "git worktree lock" and "git worktree
> unlock". This series is built on top of the first one, it needs 1/7.
>
>   [1/5] worktree.c: add find_worktree_by_path()
>   [2/5] worktree.c: add is_main_worktree()
>   [3/5] worktree.c: add is_worktree_locked()
>   [4/5] worktree: add "lock" command
>   [5/5] worktree: add "unlock" command
>
> After this, worktree-move becomes ~10 patch series.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug?] log -p -W showing the whole file for a patch that adds to the end?

[Junio C Hamano]

Junio C Hamano  writes:

> Just noticed a curiosity:
>
> $ git show -W 3e3ceaa58 quote.c
>
> shows the entire file.  The commit in question adds a whole new
> function at the end of the file.  If I move that addition to just
> before the last function the file already had before the change and
> amend the commit, "show -W" would work as expected, i.e. addition of
> the function, with three lines before and three lines after context.
>
> The -W feature was added by 14937c2c (diff: add option to show whole
> functions as context, 2011-10-09).

A workaround (atEnd) seems to give me a slightly better result, but
I am not sure if this breaks other corner cases.

The real problem is that get_func_line() is called with (start,
limit) but a hunk that adds to the end does not even enter the loop
that goes back (i.e. step = -1) to find the funcline:

for (l = start; l != limit && 0 <= l && l < xe->xdf1.nrec; l += step) {
const char *rec;
long reclen = xdl_get_rec(>xdf1, l, );
long len = ff(rec, reclen, buf, size, xecfg->find_func_priv);

because start == xe->xdf1.nrec in that case.  We could instead force
it to enter the loop by decrementing start when it is xe->xdf1.nrec
and we are going down, but that would show two functions in the
resulting hunk (one that is the existing function at the end of the
file, the other that is added by the patch), which is not better than
showing the patch unmodified.

-- >8 --
Subject: diff -W: do not show the whole file when punting

The -W option to "diff" family of commands allows the pre- and post-
context of hunks extended to cover the previous and the next
"function header line", so that the whole function that is patched
can be seen in the hunk.

The helper function get_func_line() however gets confused when a
hunk adds a new function at the very end, and returns -1 to signal
that it did not find a suitable "function header line", i.e. the
beginning of previous function.  The caller then takes this signal
and shows from the very beginning of the file.  We end up showing
the entire file, starting from the very beginning to the end of the
newly added lines.

We can avoid this by using the original hunk boundary when the
helper gives up.

Signed-off-by: Junio C Hamano 
---
 xdiff/xemit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 993724b..4e58482 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -166,7 +166,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, 
xdemitcb_t *ecb,
if (xecfg->flags & XDL_EMIT_FUNCCONTEXT) {
long fs1 = get_func_line(xe, xecfg, NULL, xch->i1, -1);
if (fs1 < 0)
-   fs1 = 0;
+   fs1 = xch->i1;
if (fs1 < s1) {
s2 -= s1 - fs1;
s1 = fs1;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Junio C Hamano]

Yaroslav Halchenko  writes:

>> The necessary update to the client might be as simple as using
>> $GIVEN_URL/.git/ and attempting the request again after seeing the
>> probe for $GIVEN_URL/info/refs fails.
>
> Sure -- workarounds are possible,...

Just so that there is no misunderstanding, the above was not a
workaround but is an outline of an implementation of updated http
client shipped with Git.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Yaroslav Halchenko]

On Tue, 10 May 2016, Jacob Keller wrote:
> > The necessary update to the client might as simple as using
> > $GIVEN_URL/.git/ and attempting the request again after seeing the
> > probe for $GIVEN_URL/info/refs fails.
> I know at least Jenkin's Git plugin has a workaround to solve this
> issue that is quite similar.

On Tue, 10 May 2016, Junio C Hamano wrote:

> >> traverse website since could lead to dangerous places.  But .git is under
> >> originating url directory, as well as info/ or HEAD or any other object
> >> accessed by git, so IMHO this concern is not a concern.

> I am afraid that the reason why you saw no response is primarily
> because nobody is interested in extending dumb commit-walker HTTP
> transport after the world has largely moved on and abandoned it.

> The necessary update to the client might as simple as using
> $GIVEN_URL/.git/ and attempting the request again after seeing the
> probe for $GIVEN_URL/info/refs fails.

Sure -- workarounds are possible, and we are at the state that many dependent
projects seems are doing that already (as above noted Jenkin's Git plugin, smth
along the lines probably done by github for https as well).  In my case I even
have managed to erect a lovely apache rewrite rule which seems to work, so I
can just 'git clone --recursive' a collection of 30 submodules without a hiccup
(we are also interested in .git/annex part here ;) ).  Citing here if it
comes handy for anyone

# To overcome http://thread.gmane.org/gmane.comp.version-control.git/293777
# we need to rewrite urls so that there is no need for explicit .git/
RewriteEngine On
RewriteCond "!.*/\.git/.*"
RewriteRule 
"(.*?/)((?http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/19] index-helper/watchman

[Ramsay Jones]

On 10/05/16 21:30, Junio C Hamano wrote:
> Ramsay Jones  writes:
> 
>> On 10/05/16 12:52, Dennis Kaarsemaker wrote:
>>> On ma, 2016-05-09 at 15:22 -0700, Junio C Hamano wrote:
 It passes on one box and fails on another.  They both run the same
 Ubuntu 14.04 derivative, with same ext3 filesystem.  The failing one
 is on a VM.
>>>
>>> Same here, except ext4 instead of ext3. Failing on a virtual machine,
>>> not failing on a physical one.
>>
>> I can confirm the trend:
>>
>> Linux Mint 17.3, ext4 - bare-metal pass, (Virtual Box) VM fail.
>>
> I do not think there is anything VM specific, though.  If it is
> SIGPIPE, it is very much understandable it is timing dependent, and
> VMness may be a cause of timing differences, nothing more.

Yeah, I had previously noted that this was probably a timing related
issue - this was only meant as confirmation that David could probably
reproduce the issue if he tested in a VM. ;-)

However, it appears Dennis has found a way to force a failure, even
on bare-metal (at least on v8, I think).

> I seem to getting the failure on my physical box today, by the way.

Ah, interesting.

ATB,
Ramsay Jones

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] test-lib: set BASH_XTRACEFD automatically

[Junio C Hamano]

Jeff King  writes:

> The patch itself is a trivial-looking one-liner, but there
> are a few subtleties worth mentioning:
>
>   - the variable is _not_ exported; the "set -x" is local to
> our process, and so the tracefd should match
>
>   - this line has to come after we do the redirection for fd
> 4, as bash will otherwise complain during the variable
> assignment
>
>   - likewise, we cannot ever unset this variable, as it
> would close descriptor 4
>
>   - setting it once here is sufficient to cover both the
> regular "-x" case (which implies "--verbose"), as well
> as "--verbose-only=1". This works because the latter
> flips "set -x" off and on for particular tests (if it
> didn't, we would get tracing for all tests, as going to
> descriptor 4 effectively circumvents the verbose flag).

Thanks.  Some of them may deserve to be next to the one-liner to
prevent people from making changes that tickle them?

> Signed-off-by: Jeff King 
> ---
>  t/README  | 6 +++---
>  t/test-lib.sh | 1 +
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/t/README b/t/README
> index 1dc908e..76a0daa 100644
> --- a/t/README
> +++ b/t/README
> @@ -84,9 +84,9 @@ appropriately before running "make".
>  
>  -x::
>   Turn on shell tracing (i.e., `set -x`) during the tests
> - themselves. Implies `--verbose`. Note that this can cause
> - failures in some tests which redirect and test the
> - output of shell functions. Use with caution.
> + themselves. Implies `--verbose`. Note that in non-bash shells,
> + this can cause failures in some tests which redirect and test
> + the output of shell functions. Use with caution.
>  
>  -d::
>  --debug::
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 286c5f3..482ec11 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -321,6 +321,7 @@ then
>  else
>   exec 4>/dev/null 3>/dev/null
>  fi
> +BASH_XTRACEFD=4
>  
>  test_failure=0
>  test_count=0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug?] log -p -W showing the whole file for a patch that adds to the end?

[Junio C Hamano]

Just noticed a curiosity:

$ git show -W 3e3ceaa58 quote.c

shows the entire file.  The commit in question adds a whole new
function at the end of the file.  If I move that addition to just
before the last function the file already had before the change and
amend the commit, "show -W" would work as expected, i.e. addition of
the function, with three lines before and three lines after context.

The -W feature was added by 14937c2c (diff: add option to show whole
functions as context, 2011-10-09).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/33] Yet more preparation for reference backends

[Junio C Hamano]

Michael Haggerty  writes:

> ... I think I have addressed all of the points that were
> brought up. Plus I fixed a pre-existing bug that I noticed myself
> while adding some more tests; see the first bullet point below for
> more information.
>
> Changes between v1 and v2:
>
> * Prefixed the patch series with three new patches that demonstrate
>   and fix some bugs in reference resolution that I noticed while
>   inspecting this series:

I'd propose to wait for further comments a few more days and merge
this to 'next' by the end of the week.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] test-lib: set BASH_XTRACEFD automatically

[Jeff King]

On Tue, May 10, 2016 at 02:13:26PM -0700, Junio C Hamano wrote:

> > I don't think there is a scalable, portable way to do so. "-x" output is
> > going to stderr, and is inherited by any functions or subshells. So
> > either we have to ask "-x" output to go somewhere else, or we have to
> > turn it off inside the functions and subshells. The latter requires
> > tweaking each site, which isn't scalable. And there is no way to do the
> > former in a portable way (AFAIK).
> 
> Yeah, that was the conclusion I was coming to; the same "unscalable"
> argument applies to the patch under discussion, too.

I think munging the tests themselves is even more horrible than tweaking
test_must_fail, but even the latter is not very scalable (test_must_fail
is undoubtedly the most common function, but it's not the only one; and
one may actually want to see its trace output anyway).

> > +BASH_XTRACEFD=4
> [...]
> 
> Yeah, something like that I would greatly appreciate.

Here it is in patch form. It's sad that we can't automatically help
people using dash (which includes myself). I suppose we could
auto-reinvoke ourselves using bash if it is available, but that feels a
bit too magical. I'm content to let "try running with bash" be a tool in
our toolbox.

-- >8 --
Subject: [PATCH] test-lib: set BASH_XTRACEFD automatically

Passing "-x" to a test script enables the shell's "set -x"
tracing, which can help with tracking down the command that
is causing a failure. Unfortunately, it can also _cause_
failures in some tests that redirect the stderr of a shell
function.  Inside the function the shell continues to
respect "set -x", and the trace output is collected along
with whatever stderr is generated normally by the function.

You can see an example of this by running:

  ./t0040-parse-options.sh -x -i

which will fail immediately in the first test, as it
expects:

  test_must_fail some-cmd 2>output.err

to leave output.err empty (but with "-x" it has our trace
output).

Unfortunately there isn't a portable or scalable solution to
this. We could teach test_must_fail to disable "set -x", but
that doesn't help any of the other functions or subshells.

However, we can work around it by pointing the "set -x"
output to our descriptor 4, which always points to the
original stderr of the test script. Unfortunately this only
works for bash, but it's better than nothing (and other
shells will just ignore the BASH_XTRACEFD variable).

The patch itself is a trivial-looking one-liner, but there
are a few subtleties worth mentioning:

  - the variable is _not_ exported; the "set -x" is local to
our process, and so the tracefd should match

  - this line has to come after we do the redirection for fd
4, as bash will otherwise complain during the variable
assignment

  - likewise, we cannot ever unset this variable, as it
would close descriptor 4

  - setting it once here is sufficient to cover both the
regular "-x" case (which implies "--verbose"), as well
as "--verbose-only=1". This works because the latter
flips "set -x" off and on for particular tests (if it
didn't, we would get tracing for all tests, as going to
descriptor 4 effectively circumvents the verbose flag).

Signed-off-by: Jeff King 
---
 t/README  | 6 +++---
 t/test-lib.sh | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/README b/t/README
index 1dc908e..76a0daa 100644
--- a/t/README
+++ b/t/README
@@ -84,9 +84,9 @@ appropriately before running "make".
 
 -x::
Turn on shell tracing (i.e., `set -x`) during the tests
-   themselves. Implies `--verbose`. Note that this can cause
-   failures in some tests which redirect and test the
-   output of shell functions. Use with caution.
+   themselves. Implies `--verbose`. Note that in non-bash shells,
+   this can cause failures in some tests which redirect and test
+   the output of shell functions. Use with caution.
 
 -d::
 --debug::
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 286c5f3..482ec11 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -321,6 +321,7 @@ then
 else
exec 4>/dev/null 3>/dev/null
 fi
+BASH_XTRACEFD=4
 
 test_failure=0
 test_count=0
-- 
2.8.2.660.ge43c418

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Eric Sunshine]

On Tue, May 10, 2016 at 5:11 PM, Junio C Hamano  wrote:
> SZEDER Gábor  writes:
>> I wonder if is it really necessary to specify the path to the .git
>> directory via $GIT_DIR.  Would 'git --git-dir=/over/there' be just as
>> good?
>
> Then you are testing two different things that may go through
> different codepaths.
>
> Adding yet another test to check "git --git-dir=" in addition is
> fine, but that is not a replacement.  We do want to make sure that
> "GIT_DIR=there git" form keeps giving us the expected outcome.

When working on this, I did test with --git-dir= in place of GIT_DIR
and some tests failed, but I didn't follow through to see what the
actual problem was, partly because the code was in flux and I may have
messed up something else, but primarily for the reason Junio gives
above: I wanted this modernization series to be faithful to the
original; additional testing can be added later.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] t3404: be resilient against running with the -x flag

[Junio C Hamano]

Jeff King  writes:

> On Tue, May 10, 2016 at 12:49:42PM -0700, Junio C Hamano wrote:
>
>> I wonder if we can fix "-x" instead so that we do not have to
>> butcher tests like this patch does.  It was quite clear what it
>> expected to see before this patch, and it is sad that the workaround
>> makes less readable (and relies on the real output we are looking
>> for never begins with '+').
>
> I don't think there is a scalable, portable way to do so. "-x" output is
> going to stderr, and is inherited by any functions or subshells. So
> either we have to ask "-x" output to go somewhere else, or we have to
> turn it off inside the functions and subshells. The latter requires
> tweaking each site, which isn't scalable. And there is no way to do the
> former in a portable way (AFAIK).

Yeah, that was the conclusion I was coming to; the same "unscalable"
argument applies to the patch under discussion, too.

> That being said, bash supports BASH_XTRACEFD, so maybe something like
> this:
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 286c5f3..482ec11 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -321,6 +321,7 @@ then
>  else
>   exec 4>/dev/null 3>/dev/null
>  fi
> +BASH_XTRACEFD=4
>  
>  test_failure=0
>  test_count=0
>
> would help Dscho's case (and people on other shells aren't helped, but
> they are not hurt either).

Yeah, something like that I would greatly appreciate.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Junio C Hamano]

SZEDER Gábor  writes:

> I wonder if is it really necessary to specify the path to the .git
> directory via $GIT_DIR.  Would 'git --git-dir=/over/there' be just as
> good?

Then you are testing two different things that may go through
different codepaths.

Adding yet another test to check "git --git-dir=" in addition is
fine, but that is not a replacement.  We do want to make sure that
"GIT_DIR=there git" form keeps giving us the expected outcome.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: syntax error in git-rebase while running t34* tests

[Jeff King]

On Tue, May 10, 2016 at 01:53:56PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I think it is clear why it works. If $strategy_opts is empty, then the
> > code we generate looks like:
> >
> >   for strategy_opt in
> >   do
> >   ...
> >   done
> 
> Ah, of course.  Thanks.

Here it is as a patch and commit message.

-- >8 --
Subject: [PATCH] rebase--interactive: avoid empty list in shell for-loop

The $strategy_opts variable contains a space-separated list
of strategy options, each individually shell-quoted. To loop
over each, we "unwrap" them by doing an eval like:

  eval '
for opt in '"$strategy_opts"'
do
   ...
done
  '

Note the quoting that means we expand $strategy_opts inline
in the code to be evaluated (which is the right thing
because we want the IFS-split and de-quoting). If the
variable is empty, however, we ask the shell to eval the
following code:

  for opt in
  do
 ...
  done

without anything between "in" and "do".  Most modern shells
are happy to treat that like a noop, but reportedly ksh88 on
AIX considers it a syntax error. So let's catch the case
that the variable is empty and skip the eval altogether
(since we know the loop would be a noop anyway).

Reported-by: Armin Kunaschik 
Signed-off-by: Jeff King 
---
 git-rebase--interactive.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 9ea3075..1c6dfb6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -82,6 +82,7 @@ rewritten_pending="$state_dir"/rewritten-pending
 cr=$(printf "\015")
 
 strategy_args=${strategy:+--strategy=$strategy}
+test -n "$strategy_opts" &&
 eval '
for strategy_opt in '"$strategy_opts"'
do
-- 
2.8.2.660.ge43c418

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[SZEDER Gábor]

Quoting Eric Sunshine :

On Tue, May 10, 2016 at 3:12 PM, Eric Sunshine  
 wrote:

On Tue, May 10, 2016 at 2:39 PM, Jeff King  wrote:

On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote:

  while :
  do
  case "$1" in
  -C) dir="-C $2"; shift; shift ;;
  -b) bare="$2"; shift; shift ;;
+ -g) env="GIT_DIR=$2; export GIT_DIR"; shift; shift ;;
@@ -36,6 +38,8 @@ test_rev_parse () {
  do
  expect="$1"
  test_expect_success "$name: $o" '
+ test_when_finished "sane_unset GIT_DIR" &&
+ eval $env &&


This will set up the sane_unset regardless of whether $env does
anything. Would it make more sense to stick the test_when_finished
inside $env? You could use regular unset then, too, since you know the
variable would be set.


I didn't worry about it too much because the end result is effectively
the same and, with all the 'case' arms being short one-liners, I think
the code is a bit easier to read as-is; bundling 'test_when_finished'
into the 'env' assignment line would probably require wrapping the
line. I do like the improved encapsulation of your suggestion but
don't otherwise feel strongly about it.


Actually, I think we can have improved encapsulation and maintain
readability like this:

case "$1" in
...
-g) env="$2"; shift;  shift ;;
...
esac

...
test_expect_success "..." '
if test -n "$env"
do
test_when_finished "unset GIT_DIR"
GIT_DIR="$env"
export GIT_DIR
fi
...
'


I wonder if is it really necessary to specify the path to the .git
directory via $GIT_DIR.  Would 'git --git-dir=/over/there' be just as
good?  If yes, then how about

  git ${gitdir:+--git-dir="$gitdir"} rev-parse ...


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] t3404: be resilient against running with the -x flag

[Jeff King]

On Tue, May 10, 2016 at 12:49:42PM -0700, Junio C Hamano wrote:

> I wonder if we can fix "-x" instead so that we do not have to
> butcher tests like this patch does.  It was quite clear what it
> expected to see before this patch, and it is sad that the workaround
> makes less readable (and relies on the real output we are looking
> for never begins with '+').

I don't think there is a scalable, portable way to do so. "-x" output is
going to stderr, and is inherited by any functions or subshells. So
either we have to ask "-x" output to go somewhere else, or we have to
turn it off inside the functions and subshells. The latter requires
tweaking each site, which isn't scalable. And there is no way to do the
former in a portable way (AFAIK).

That being said, bash supports BASH_XTRACEFD, so maybe something like
this:

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 286c5f3..482ec11 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -321,6 +321,7 @@ then
 else
exec 4>/dev/null 3>/dev/null
 fi
+BASH_XTRACEFD=4
 
 test_failure=0
 test_count=0

would help Dscho's case (and people on other shells aren't helped, but
they are not hurt either).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: syntax error in git-rebase while running t34* tests

[Jeff King]

On Tue, May 10, 2016 at 12:11:59PM -0700, Junio C Hamano wrote:

> Armin Kunaschik  writes:
> 
> > I fail to see why eval is really necessary here.
> 
> It is necessary to work correctly with any strategy option with $IFS
> in it, I would think.  The calling script "git-rebase" accumulates
> --strategy-option values after passing each of them through
> "rev-parse --sq-quote" for that reason.
> 
> which means that eval'ed string here:
> 
> > My quick and dirty hotfix is to place a
> > test -n "$strategy_opts" &&
> > in front of the eval.
> > The tests run fine after this change.
> >
> > What do you think?
> 
> I do not see why "test -n &&" is necessary here, and would be very
> hesitant to accept a change that nobody understands why it works.

I think it is clear why it works. If $strategy_opts is empty, then the
code we generate looks like:

  for strategy_opt in
  do
  ...
  done

and some shells (apparently) are not happy with no content after the
"in". If the variable is empty, there is no need to run the loop at all,
so it is OK to skip it the eval entirely.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: syntax error in git-rebase while running t34* tests

[Junio C Hamano]

Jeff King  writes:

> I think it is clear why it works. If $strategy_opts is empty, then the
> code we generate looks like:
>
>   for strategy_opt in
>   do
>   ...
>   done

Ah, of course.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Jeff King]

On Tue, May 10, 2016 at 03:59:42PM -0400, Eric Sunshine wrote:

> On Tue, May 10, 2016 at 3:48 PM, Eric Sunshine  
> wrote:
> > Actually, I think we can have improved encapsulation and maintain
> > readability like this:
> >
> > case "$1" in
> > ...
> > -g) env="$2"; shift;  shift ;;
> > ...
> > esac
> >
> > ...
> > test_expect_success "..." '
> > if test -n "$env"
> > do
> > test_when_finished "unset GIT_DIR"
> > GIT_DIR="$env"
> > export GIT_DIR
> > fi
> > ...
> > '
> 
> At this point, I'd also rename 'env' to 'gitdir' to be more meaningful.

Yeah, I like this even better. As much as I enjoy eval tricks, I think
this case is more readable with the condition written out.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Jacob Keller]

On Tue, May 10, 2016 at 1:11 PM, Junio C Hamano  wrote:
> Yaroslav Halchenko  writes:
>
>> On Fri, 06 May 2016, Yaroslav Halchenko wrote:
>>
>>> Dear Git Folks,
>>
>>> Originally this issue was mentioned in previous thread [1], and I have 
>>> decided
>>> to bring it into a separate thread.  ATM there is a dichotomy in git 
>>> behavior
>>> between cloning non-bare repos:  if I clone over ssh or just locally by
>>> providing url without trailing /.git, git senses for /.git and works just 
>>> fine
>>> with ssh or local repositories, but fails for "dummy" http ones, the demo
>>> script is here [2] which produces output listed below.  From which you can 
>>> see
>>> that  cloning using http URL to the repository without /.git fails (git 
>>> version
>>> 2.8.1, Debian).  As it was noted in [1], concern could have been to not
>>> traverse website since could lead to dangerous places.  But .git is under
>>> originating url directory, as well as info/ or HEAD or any other object
>>> accessed by git, so IMHO this concern is not a concern.
>
> I am afraid that the reason why you saw no response is primarily
> because nobody is interested in extending dumb commit-walker HTTP
> transport after the world has largely moved on and abandoned it.
>
> The necessary update to the client might as simple as using
> $GIVEN_URL/.git/ and attempting the request again after seeing the
> probe for $GIVEN_URL/info/refs fails.
> --

I know at least Jenkin's Git plugin has a workaround to solve this
issue that is quite similar.

Thanks,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/19] index-helper/watchman

[Junio C Hamano]

Ramsay Jones  writes:

> On 10/05/16 12:52, Dennis Kaarsemaker wrote:
>> On ma, 2016-05-09 at 15:22 -0700, Junio C Hamano wrote:
>>> It passes on one box and fails on another.  They both run the same
>>> Ubuntu 14.04 derivative, with same ext3 filesystem.  The failing one
>>> is on a VM.
>> 
>> Same here, except ext4 instead of ext3. Failing on a virtual machine,
>> not failing on a physical one.
>
> I can confirm the trend:
>
> Linux Mint 17.3, ext4 - bare-metal pass, (Virtual Box) VM fail.
>
> ATB,
> Ramsay Jones

I do not think there is anything VM specific, though.  If it is
SIGPIPE, it is very much understandable it is timing dependent, and
VMness may be a cause of timing differences, nothing more.

I seem to getting the failure on my physical box today, by the way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] perf: make the tests work in worktrees

[Junio C Hamano]

Johannes Schindelin  writes:

> This patch makes perf-lib.sh more robust so that it can run correctly
> even inside a worktree. For example, it assumed that $GIT_DIR/objects is
> the objects directory (which is not the case for worktrees) and it used
> the commondir file verbatim, even if it contained a relative path.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  t/perf/perf-lib.sh | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
> index e9020d0..e5682f7 100644
> --- a/t/perf/perf-lib.sh
> +++ b/t/perf/perf-lib.sh
> @@ -80,22 +80,22 @@ test_perf_create_repo_from () {
>   error "bug in the test script: not 2 parameters to test-create-repo"
>   repo="$1"
>   source="$2"
> - source_git=$source/$(cd "$source" && git rev-parse --git-dir)
> + source_git="$(cd "$source" && git rev-parse --git-dir)"
> + objects_dir="$(git rev-parse --git-path objects)"

I do not quite understand this change.  Whose object_dir is this
looking into?  The original wanted to peek into $source/.git/objects/
which may have been wrong when $source is borrowing from some other
repository, but the new invocation of rev-parse --git-path objects
is done inside what repository?  It does not seem to pay any attention
to $source and the change below just copies from there into $repo.

Confused.

>   mkdir -p "$repo/.git"
>   (
> - cd "$repo/.git" &&
> - { cp -Rl "$source_git/objects" . 2>/dev/null ||
> - cp -R "$source_git/objects" .; } &&
> + { cp -Rl "$objects_dir" "$repo/.git/" 2>/dev/null ||
> + cp -R "$objects_dir" "$repo/.git/"; } &&
>   for stuff in "$source_git"/*; do
>   case "$stuff" in
> - */objects|*/hooks|*/config)
> + */objects|*/hooks|*/config|*/commondir)
>   ;;
>   *)
> - cp -R "$stuff" . || exit 1
> + cp -R "$stuff" "$repo/.git/" || exit 1
>   ;;
>   esac
>   done &&
> - cd .. &&
> + cd "$repo" &&
>   git init -q &&
>   if test_have_prereq MINGW
>   then
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Move test-* to t/helper/ subdirectory

[Junio C Hamano]

Duy Nguyen  writes:

> Or a simpler, more-to-the-point patch like this?

I am OK with that, even though I find it a bit too "cute" for my
taste.



> -- 8< --
> Subject: [PATCH] wrap-for-bin.sh: regenerate bin-wrappers when switching 
> branches
>
> Commit e6e7530 (test helpers: move test-* to t/helper/ subdirectory -
> 2016-04-13) moves test-* to t/helper. However because bin-wrappers/*
> only depend on wrap-for-bin.sh, when switching between a branch that has
> this commit and one that does not, bin-wrappers/* may not be regenerated
> and point to the old/outdated test programs.
>
> This commit makes a non-functional change in wrap-for-bin.sh, just
> enough for 'make' to detect and re-execute wrap-for-bin.sh. When
> switching between a branch containing both this commit and e6e7530 and
> one containing neither, bin-wrappers/*, we should get fresh bin-wrappers/*.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  wrap-for-bin.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
> index db0ec6a..22b6e49 100644
> --- a/wrap-for-bin.sh
> +++ b/wrap-for-bin.sh
> @@ -17,6 +17,7 @@ fi
>  GITPERLLIB='@@BUILD_DIR@@/perl/blib/lib'"${GITPERLLIB:+:$GITPERLLIB}"
>  GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
>  PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
> +
>  export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
>  
>  if test -n "$GIT_TEST_GDB"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Junio C Hamano]

Yaroslav Halchenko  writes:

> On Fri, 06 May 2016, Yaroslav Halchenko wrote:
>
>> Dear Git Folks,
>
>> Originally this issue was mentioned in previous thread [1], and I have 
>> decided
>> to bring it into a separate thread.  ATM there is a dichotomy in git behavior
>> between cloning non-bare repos:  if I clone over ssh or just locally by
>> providing url without trailing /.git, git senses for /.git and works just 
>> fine
>> with ssh or local repositories, but fails for "dummy" http ones, the demo
>> script is here [2] which produces output listed below.  From which you can 
>> see
>> that  cloning using http URL to the repository without /.git fails (git 
>> version
>> 2.8.1, Debian).  As it was noted in [1], concern could have been to not
>> traverse website since could lead to dangerous places.  But .git is under
>> originating url directory, as well as info/ or HEAD or any other object
>> accessed by git, so IMHO this concern is not a concern.

I am afraid that the reason why you saw no response is primarily
because nobody is interested in extending dumb commit-walker HTTP
transport after the world has largely moved on and abandoned it.

The necessary update to the client might as simple as using
$GIVEN_URL/.git/ and attempting the request again after seeing the
probe for $GIVEN_URL/info/refs fails.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Junio C Hamano]

Eric Sunshine  writes:

> On Tue, May 10, 2016 at 3:12 PM, Eric Sunshine  
> wrote:
>> On Tue, May 10, 2016 at 2:39 PM, Jeff King  wrote:
>>> On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote:
   while :
   do
   case "$1" in
   -C) dir="-C $2"; shift; shift ;;
   -b) bare="$2"; shift; shift ;;
 + -g) env="GIT_DIR=$2; export GIT_DIR"; shift; shift ;;
 @@ -36,6 +38,8 @@ test_rev_parse () {
   do
   expect="$1"
   test_expect_success "$name: $o" '
 + test_when_finished "sane_unset GIT_DIR" &&
 + eval $env &&
>>>
>>> This will set up the sane_unset regardless of whether $env does
>>> anything. Would it make more sense to stick the test_when_finished
>>> inside $env? You could use regular unset then, too, since you know the
>>> variable would be set.
>>
>> I didn't worry about it too much because the end result is effectively
>> the same and, with all the 'case' arms being short one-liners, I think
>> the code is a bit easier to read as-is; bundling 'test_when_finished'
>> into the 'env' assignment line would probably require wrapping the
>> line. I do like the improved encapsulation of your suggestion but
>> don't otherwise feel strongly about it.
>
> Actually, I think we can have improved encapsulation and maintain
> readability like this:
>
> case "$1" in
> ...
> -g) env="$2"; shift;  shift ;;
> ...
> esac
>
> ...
> test_expect_success "..." '
> if test -n "$env"
> do
> test_when_finished "unset GIT_DIR"
> GIT_DIR="$env"
> export GIT_DIR
> fi
> ...
> '

That's even better.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Eric Sunshine]

On Tue, May 10, 2016 at 3:48 PM, Eric Sunshine  wrote:
> Actually, I think we can have improved encapsulation and maintain
> readability like this:
>
> case "$1" in
> ...
> -g) env="$2"; shift;  shift ;;
> ...
> esac
>
> ...
> test_expect_success "..." '
> if test -n "$env"
> do
> test_when_finished "unset GIT_DIR"
> GIT_DIR="$env"
> export GIT_DIR
> fi
> ...
> '

At this point, I'd also rename 'env' to 'gitdir' to be more meaningful.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] perf: let's disable symlinks on Windows

[Junio C Hamano]

Johannes Schindelin  writes:

> In Git for Windows' SDK, Git's source code is always checked out
> with symlinks disabled. The reason is that POSIX symlinks have no
> accurate equivalent on Windows [*1*]. More precisely, though, it is
> not just Git's source code but *all* source code that is checked
> out with symlinks disabled: core.symlinks is set to false in the
> system-wide gitconfig.
>
> Since the perf tests are run with the system-wide gitconfig *disabled*,
> we have to make sure that the Git repository is initialized correctly
> by configuring core.symlinks explicitly.

Is MINGW the right prerequisite to use here, or is SIMLINKS more
appropriate?

>
> Footnote *1*:
> https://github.com/git-for-windows/git/wiki/Symbolic-Links
>
> Signed-off-by: Johannes Schindelin 
> ---
>  t/perf/perf-lib.sh | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
> index 5cf74ed..e9020d0 100644
> --- a/t/perf/perf-lib.sh
> +++ b/t/perf/perf-lib.sh
> @@ -97,6 +97,10 @@ test_perf_create_repo_from () {
>   done &&
>   cd .. &&
>   git init -q &&
> + if test_have_prereq MINGW
> + then
> + git config core.symlinks false
> + fi &&
>   mv .git/hooks .git/hooks-disabled 2>/dev/null
>   ) || error "failed to copy repository '$source' to '$repo'"
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] t3404: be resilient against running with the -x flag

[Junio C Hamano]

Johannes Schindelin  writes:

> To: Junio C Hamano 
> Cc: git@vger.kernel.org

Probably the above is the other way around.

> The -x flag (trace commands) is a priceless tool when hunting down bugs
> that trigger test failures. It is a worthless tool if the -x flag
> *itself* triggers test failures.

True.

I wonder if we can fix "-x" instead so that we do not have to
butcher tests like this patch does.  It was quite clear what it
expected to see before this patch, and it is sad that the workaround
makes less readable (and relies on the real output we are looking
for never begins with '+').

I do agree the change from 1d to //d in this patch
is a very good thing; it makes it clear that we are excluding the
"error: ", and expect that after removing the message what follows
is the help text.  And in the spirit of that change, I think it is
better to match /^error: / instead of /option .exec. requires.../.

> So let's change the offending tests so that they are a bit less
> stringent and do not stumble over the "+..." lines generated by the -x
> flag.
>
> Signed-off-by: Johannes Schindelin 
> ---
>  t/t3404-rebase-interactive.sh | 67 
> ++-
>  1 file changed, 15 insertions(+), 52 deletions(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 66348f1..25f1118 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -882,7 +882,8 @@ test_expect_success 'rebase -i --exec without ' '
>   git reset --hard execute &&
>   set_fake_editor &&
>   test_must_fail git rebase -i --exec 2>tmp &&
> - sed -e "1d" tmp >actual &&
> + sed -e "/option .exec. requires a value/d" -e '/^+/d' \
> + tmp >actual &&
>   test_must_fail git rebase -h >expected &&
>   test_cmp expected actual &&
>   git checkout master
> @@ -1149,10 +1150,6 @@ test_expect_success 'drop' '
>   test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
>  '
>  
> -cat >expect < -Successfully rebased and updated refs/heads/missing-commit.
> -EOF
> -
>  test_expect_success 'rebase -i respects rebase.missingCommitsCheck = ignore' 
> '
>   test_config rebase.missingCommitsCheck ignore &&
>   rebase_setup_and_clean missing-commit &&
> @@ -1160,52 +1157,33 @@ test_expect_success 'rebase -i respects 
> rebase.missingCommitsCheck = ignore' '
>   FAKE_LINES="1 2 3 4" \
>   git rebase -i --root 2>actual &&
>   test D = $(git cat-file commit HEAD | sed -ne \$p) &&
> - test_cmp expect actual
> + test_i18ngrep \
> + "Successfully rebased and updated refs/heads/missing-commit." \
> + actual

Is this string going to be i18n'ed?  If so this change is good, but
it probably wants to be a separate "prepare for i18n" patch, not
this "work around -x that pollutes 2>actual output" patch.

>  test_expect_success 'rebase -i respects rebase.missingCommitsCheck = warn' '
> + line="$(git rev-list --pretty=oneline --abbrev-commit -1 master)" &&
>   test_config rebase.missingCommitsCheck warn &&
>   rebase_setup_and_clean missing-commit &&
>   set_fake_editor &&
>   FAKE_LINES="1 2 3 4" \
>   git rebase -i --root 2>actual &&
> - test_cmp expect actual &&
> + test_i18ngrep "Warning: some commits may have been dropped" actual &&
> + test_i18ngrep "^ - $line" actual &&

The former is good in "prepare for i18n" patch.  The latter is not.

test_i18ngrep is primarily to make sure what is *not* supposed to be
localized are not localized.  GETTEXT_POISON build-time option
builds Git with garbage translations for strings marked for
localization, and test_i18ngrep knows to pretend the test always
passes in POISON build.

We test things that are _not_ to be localized with "grep", so a test
with POISON build will catch if a string (like plumbing output) that
are not supposed to be localized is marked for localization by
mistake.

I stop quoting here, but I think the remainder has the same
over-eager use of i18ngrep.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Eric Sunshine]

On Tue, May 10, 2016 at 3:12 PM, Eric Sunshine  wrote:
> On Tue, May 10, 2016 at 2:39 PM, Jeff King  wrote:
>> On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote:
>>>   while :
>>>   do
>>>   case "$1" in
>>>   -C) dir="-C $2"; shift; shift ;;
>>>   -b) bare="$2"; shift; shift ;;
>>> + -g) env="GIT_DIR=$2; export GIT_DIR"; shift; shift ;;
>>> @@ -36,6 +38,8 @@ test_rev_parse () {
>>>   do
>>>   expect="$1"
>>>   test_expect_success "$name: $o" '
>>> + test_when_finished "sane_unset GIT_DIR" &&
>>> + eval $env &&
>>
>> This will set up the sane_unset regardless of whether $env does
>> anything. Would it make more sense to stick the test_when_finished
>> inside $env? You could use regular unset then, too, since you know the
>> variable would be set.
>
> I didn't worry about it too much because the end result is effectively
> the same and, with all the 'case' arms being short one-liners, I think
> the code is a bit easier to read as-is; bundling 'test_when_finished'
> into the 'env' assignment line would probably require wrapping the
> line. I do like the improved encapsulation of your suggestion but
> don't otherwise feel strongly about it.

Actually, I think we can have improved encapsulation and maintain
readability like this:

case "$1" in
...
-g) env="$2"; shift;  shift ;;
...
esac

...
test_expect_success "..." '
if test -n "$env"
do
test_when_finished "unset GIT_DIR"
GIT_DIR="$env"
export GIT_DIR
fi
...
'
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Junio C Hamano]

Eric Sunshine  writes:

>> I don't know if it's worth worrying about or not. The usual solution is
>> something like:
>>
>>   env_git_dir=$2
>>   env='GIT_DIR=$env_git_dir; export GIT_DIR'
>>   ...
>>   eval "$env"
>
> Makes sense; I wasn't quite happy with having $2 interpolated
> unquoted. Like you, though, I don't know if it's worth worrying
> about...

This is a good change, including the quoting of $env.

> I flip-flopped on this one several times, quoting, and not quoting.
> Documentation for 'eval' says:
>
> The args are read and concatenated together into a single
> command.

Which means the two eval's give different results:

$ e='echo "a  b"'
$ eval $e
$ eval "$e"

>> This will set up the sane_unset regardless of whether $env does
>> anything. Would it make more sense to stick the test_when_finished
>> inside $env? You could use regular unset then, too, since you know the
>> variable would be set.
>
> I didn't worry about it too much because the end result is effectively
> the same and, with all the 'case' arms being short one-liners, I think
> the code is a bit easier to read as-is.

Yeah, this is OK.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: diff --break-rewrites for just a part of a file

[Junio C Hamano]

Sascha Silbe  writes:

> A combination of --break-rewrites and --inter-hunk-context that merges
> changes with less than the given number of unchanged lines between them
> into a single delete/insert change would be even better. But just
> ignoring the unchanged header and footer of a file for --break-rewrites
> would already go a long way.

That's interesting.  Break-rewrites as originally designed and
implemented is only about "When the entire file been rewritten
completely, it is easier to read when the diff did not pay any
attention to those accidentally common lines".

We recently had a discussion on tweaking the logic used to merge and
minimize hunks; the topic was about where to start the hunk boundary
for maximum readability when the beginning and end of a hunk can be
shifted around, but this smells like a similar issue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: syntax error in git-rebase while running t34* tests

[Junio C Hamano]

Armin Kunaschik  writes:

> I fail to see why eval is really necessary here.

It is necessary to work correctly with any strategy option with $IFS
in it, I would think.  The calling script "git-rebase" accumulates
--strategy-option values after passing each of them through
"rev-parse --sq-quote" for that reason.

which means that eval'ed string here:

> My quick and dirty hotfix is to place a
> test -n "$strategy_opts" &&
> in front of the eval.
> The tests run fine after this change.
>
> What do you think?

I do not see why "test -n &&" is necessary here, and would be very
hesitant to accept a change that nobody understands why it works.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Eric Sunshine]

On Tue, May 10, 2016 at 2:39 PM, Jeff King  wrote:
> On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote:
>> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
>> @@ -7,11 +7,13 @@ test_description='test git rev-parse'
>>   while :
>>   do
>>   case "$1" in
>>   -C) dir="-C $2"; shift; shift ;;
>>   -b) bare="$2"; shift; shift ;;
>> + -g) env="GIT_DIR=$2; export GIT_DIR"; shift; shift ;;
>
> This will expand $2 inside $env, which is later eval'd. So funny things
> happen if there are spaces or metacharacters. It looks like you only use
> it with short relative paths ("../repo.git", etc), which is OK, but this
> would probably break badly if we ever used absolute paths.
>
> I don't know if it's worth worrying about or not. The usual solution is
> something like:
>
>   env_git_dir=$2
>   env='GIT_DIR=$env_git_dir; export GIT_DIR'
>   ...
>   eval "$env"

Makes sense; I wasn't quite happy with having $2 interpolated
unquoted. Like you, though, I don't know if it's worth worrying
about...

>> @@ -36,6 +38,8 @@ test_rev_parse () {
>>   do
>>   expect="$1"
>>   test_expect_success "$name: $o" '
>> + test_when_finished "sane_unset GIT_DIR" &&
>> + eval $env &&
>
> I was surprised not to see quoting around $env here, but it probably
> doesn't matter (I think it may affect how some whitespace is treated,
> but the contents of $env are pretty tame).

I flip-flopped on this one several times, quoting, and not quoting.
Documentation for 'eval' says:

The args are read and concatenated together into a single
command.

so, I ultimately left it unquoted, but don't feel strongly about it.

> This will set up the sane_unset regardless of whether $env does
> anything. Would it make more sense to stick the test_when_finished
> inside $env? You could use regular unset then, too, since you know the
> variable would be set.

I didn't worry about it too much because the end result is effectively
the same and, with all the 'case' arms being short one-liners, I think
the code is a bit easier to read as-is; bundling 'test_when_finished'
into the 'env' assignment line would probably require wrapping the
line. I do like the improved encapsulation of your suggestion but
don't otherwise feel strongly about it.

Nevertheless, I can re-roll with these changes if you feel more
strongly than I about it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Yaroslav Halchenko]

On Fri, 06 May 2016, Yaroslav Halchenko wrote:

> Dear Git Folks,

> Originally this issue was mentioned in previous thread [1], and I have decided
> to bring it into a separate thread.  ATM there is a dichotomy in git behavior
> between cloning non-bare repos:  if I clone over ssh or just locally by
> providing url without trailing /.git, git senses for /.git and works just fine
> with ssh or local repositories, but fails for "dummy" http ones, the demo
> script is here [2] which produces output listed below.  From which you can see
> that  cloning using http URL to the repository without /.git fails (git 
> version
> 2.8.1, Debian).  As it was noted in [1], concern could have been to not
> traverse website since could lead to dangerous places.  But .git is under
> originating url directory, as well as info/ or HEAD or any other object
> accessed by git, so IMHO this concern is not a concern.
> ...

If there is a better venue (bug tracker?) to spark the interest
and discussion, please let me know ;)

-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] t1500: avoid setting environment variables outside of tests

[Jeff King]

On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote:

> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index c058aa4..525e6d3 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -7,11 +7,13 @@ test_description='test git rev-parse'
>  test_rev_parse () {
>   dir=
>   bare=
> + env=
>   while :
>   do
>   case "$1" in
>   -C) dir="-C $2"; shift; shift ;;
>   -b) bare="$2"; shift; shift ;;
> + -g) env="GIT_DIR=$2; export GIT_DIR"; shift; shift ;;

This will expand $2 inside $env, which is later eval'd. So funny things
happen if there are spaces or metacharacters. It looks like you only use
it with short relative paths ("../repo.git", etc), which is OK, but this
would probably break badly if we ever used absolute paths.

I don't know if it's worth worrying about or not. The usual solution is
something like:

  env_git_dir=$2
  env='GIT_DIR=$env_git_dir; export GIT_DIR'
  ...
  eval "$env"

> @@ -36,6 +38,8 @@ test_rev_parse () {
>   do
>   expect="$1"
>   test_expect_success "$name: $o" '
> + test_when_finished "sane_unset GIT_DIR" &&
> + eval $env &&

I was surprised not to see quoting around $env here, but it probably
doesn't matter (I think it may affect how some whitespace is treated,
but the contents of $env are pretty tame).

This will set up the sane_unset regardless of whether $env does
anything. Would it make more sense to stick the test_when_finished
inside $env? You could use regular unset then, too, since you know the
variable would be set.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] modernize t1500

[Junio C Hamano]

Eric Sunshine  writes:

>   t1500: test_rev_parse: facilitate future test enhancements
>   t1500: reduce dependence upon global state
>   t1500: avoid changing working directory outside of tests
>   t1500: avoid setting configuration options outside of tests
>   t1500: avoid setting environment variables outside of tests
>   t1500: be considerate to future potential tests

When you reroll sg/completion-updates series (87a213f^..2be685a, 21
patches), please pay attention to this series, as it changes the way
you would check the output from your "--absolute-git-dir" option.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] modernize t1500

[Junio C Hamano]

Junio C Hamano  writes:

> Junio C Hamano  writes:
>
>> Eric Sunshine  writes:
>>
>>> This series modernizes t1500; it takes an entirely different approach
>>> than [1][2] and is intended to replace that series.
>>
>> Turns out that it wasn't so painful after all.
>>
>> The only small niggle I have is on 6/6; my preference would be,
>> because once we set up .git we do not add objects and refs to it, to
>> make a copy a lot earlier before the tests start.
>
> -- >8 --
> Subject: [PATCH 7/6] t1500: finish preparation upfront
>
> The earlier tests do not attempt to modify the contents of .git (by
> creating objects or refs, for example), which means that even if
> some earlier tests before "cp -R" fail, the tests in repo.git will
> run in an environment that we can expect them to succeed in.
>
> Make it clear that tests in repo.git will be independent from the
> results of earlier tests done in .git by moving its initialization
> "cp -R .git repo.git" much higher in the test sequence.
>
> Signed-off-by: Junio C Hamano 
> ---

I think the same logic applies to other preparatory things like
creation of sub/dir in the working tree etc.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: is ORIG_HEAD allowed to point to a non-existing object?

[Junio C Hamano]

Christian Halstrick  writes:

> If I do a "git-rebase -i ..." followed by "git reflog expire ..." and
> "git gc ..." then I can end up with a repo which has a ref ORIG_HEAD
> which points to a non-existing object.
>
> - Is this intended?

Yes.

HEAD is a ref, but other things like MERGE_HEAD, ORIG_HEAD,
FETCH_HEAD are not considered as refs and they are intended to be
temporary.  This does mean that they will become invalid if you
prune objects that are only reachable from them, but your "reflog
expire && gc" falls into "if it hurts, don't do it".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/6] modernize t1500

[Junio C Hamano]

Junio C Hamano  writes:

> Eric Sunshine  writes:
>
>> This series modernizes t1500; it takes an entirely different approach
>> than [1][2] and is intended to replace that series.
>
> Turns out that it wasn't so painful after all.
>
> The only small niggle I have is on 6/6; my preference would be,
> because once we set up .git we do not add objects and refs to it, to
> make a copy a lot earlier before the tests start.

-- >8 --
Subject: [PATCH 7/6] t1500: finish preparation upfront

The earlier tests do not attempt to modify the contents of .git (by
creating objects or refs, for example), which means that even if
some earlier tests before "cp -R" fail, the tests in repo.git will
run in an environment that we can expect them to succeed in.

Make it clear that tests in repo.git will be independent from the
results of earlier tests done in .git by moving its initialization
"cp -R .git repo.git" much higher in the test sequence.

Signed-off-by: Junio C Hamano 
---
 t/t1500-rev-parse.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 811c70f2..cb08677 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -51,6 +51,7 @@ test_rev_parse () {
 }
 
 ROOT=$(pwd)
+test_expect_success 'setup non-local database ../repo.git' 'cp -R .git 
repo.git'
 
 test_rev_parse toplevel false false true '' .git
 
@@ -72,8 +73,6 @@ test_rev_parse -C work -g ../.git -b t 'GIT_DIR=../.git, 
core.bare = true' true
 
 test_rev_parse -C work -g ../.git -b u 'GIT_DIR=../.git, core.bare undefined' 
false false true ''
 
-test_expect_success 'setup non-local database ../repo.git' 'cp -R .git 
repo.git'
-
 test_rev_parse -C work -g ../repo.git -b f 'GIT_DIR=../repo.git, core.bare = 
false' false false true ''
 
 test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = 
true' true false false ''
-- 
2.8.2-623-gacdd3f1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] t1500: avoid setting configuration options outside of tests

[Junio C Hamano]

Eric Sunshine  writes:

>> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
>> @@ -6,15 +6,25 @@ test_description='test git rev-parse'
>> +   case "$bare" in
>> ...
>> +   u*) bare="test_unconfig $dir core.bare" ;;
>> +   *) error "test_rev_parse: unrecognized core.bare value '$bare'"
>
> Oops, this line lost its ;; at some point while refining the code.
>
>> +   esac
>> +

Strictly speaking, you do not have to have one.  I'll squeeze it in
anyways.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Windows: only add a no-op pthread_sigmask() when needed

[Junio C Hamano]

Johannes Schindelin  writes:

> In f924b52 (Windows: add pthread_sigmask() that does nothing,
> 2016-05-01), we introduced a no-op for Windows. However, this breaks
> building Git in Git for Windows' SDK because pthread_sigmask() is
> already a no-op there, #define'd in the pthread_signal.h header in
> /mingw64/x86_64-w64-mingw32/include/.
>
> Let's guard the definition of pthread_sigmask() in #ifndef...#endif to
> make the code compile both with modern MinGW-w64 as well as with the
> previously common MinGW headers.
>
> Signed-off-by: Johannes Schindelin 
> ---
>
>   This patch is obviously based on 'next' (because 'master' does not
>   have the referenced commit yet).

One thing that makes me wonder is what would happen when
/mingw64/x86_64-w64-mingw32/include/pthread_signal.h changes its
mind and uses "static inline" instead of "#define".  How much
control does Git-for-Windows folks have over that header file?

Also, could you explain "However" part a bit?  Obviously in _some_
environment other than "Git for Windows' SDK", the previous patch
helped you compile.  And you need to #ifdef it out, because "Git for
Windows' SDK" already has its own support.  What is that other
environment that lacks the support (hence we need our own "static
inline") and is there a way to tell "Git for Windows' SDK" from that
other environment?

What I am trying to get at is:

 (1) If the answer is "we have total control", then I am perfectly
 fine with using "#ifdef pthread_sigmask" here.

 (2) If not, i.e. "they can change the implementation to 'static
 inline' themselves", then I do not think it is prudent to use
 "#ifndef pthread_sigmask" as the conditional here--using a
 symbol that lets you check for that "other" environment and
 doing "#ifdef THAT_OTHER_ONE_THAT_LACKS_SIGMASK" would be
 safer.

Also is https://lists.gnu.org/archive/html/bug-gnulib/2015-04/msg00068.html
relevant?  Does /mingw64/x86_64-w64-mingw32/include/ implement "macro only
without function"?

> Published-As: https://github.com/dscho/git/releases/tag/mingw-sigmask-v1
>  compat/win32/pthread.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
> index d336451..8df702c 100644
> --- a/compat/win32/pthread.h
> +++ b/compat/win32/pthread.h
> @@ -104,9 +104,11 @@ static inline void *pthread_getspecific(pthread_key_t 
> key)
>   return TlsGetValue(key);
>  }
>  
> +#ifndef pthread_sigmask
>  static inline int pthread_sigmask(int how, const sigset_t *set, sigset_t 
> *oset)
>  {
>   return 0;
>  }
> +#endif
>  
>  #endif /* PTHREAD_H */
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/19] index-helper/watchman

[Dennis Kaarsemaker]

On di, 2016-05-10 at 19:28 +0200, Dennis Kaarsemaker wrote:
> On ma, 2016-05-09 at 15:32 -0700, Junio C Hamano wrote:
> > 
> > Junio C Hamano  writes:
> > 
> > > 
> > > 
> > > David Turner  writes:
> > > 
> > > > 
> > > > 
> > > > On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
> > > > > 
> > > > > 
> > > > > Hmmm, I seem to be getting
> > > > > 
> > > > > $ cat t/trash*7900*/err
> > > > > fatal: Already running
> > > > > 
> > > > > after running t7900 and it fails at #5, after applying
> > > > > "index-helper: optionally automatically run"
> > The symptom looks pretty similar to $gmane/293461 reported earlier.
> > Here is how "t7900-index-helper.sh -i -v -x -d" ends.
> > 
> > 
> > expecting success:
> > test_when_finished "git index-helper --kill" &&
> > rm -f .git/index-helper.sock &&
> > git status &&
> > test_path_is_missing .git/index-helper.sock &&
> > test_config indexhelper.autorun true &&
> > git status &&
> > test -S .git/index-helper.sock &&
> > git status 2>err &&
> > test -S .git/index-helper.sock &&
> > test_must_be_empty err &&
> > git index-helper --kill &&
> > test_config indexhelper.autorun false &&
> > git status &&
> > test_path_is_missing .git/index-helper.sock
> > 
> > + test_when_finished git index-helper --kill
> > + test 0 = 0
> > + test_cleanup={ git index-helper --kill
> > } && (exit "$eval_ret"); eval_ret=$?; :
> > + rm -f .git/index-helper.sock
> > + git status
> > On branch master
> > Untracked files:
> >   (use "git add ..." to include in what will be committed)
> > 
> > err
> > 
> > nothing added to commit but untracked files present (use "git add"
> > to
> > track)
> > + test_path_is_missing .git/index-helper.sock
> > + test -e .git/index-helper.sock
> > + test_config indexhelper.autorun true
> > + config_dir=
> > + test indexhelper.autorun = -C
> > + test_when_finished test_unconfig  'indexhelper.autorun'
> > + test 0 = 0
> > + test_cleanup={ test_unconfig  'indexhelper.autorun'
> > } && (exit "$eval_ret"); eval_ret=$?; { git index-
> > helper --kill
> > } && (exit "$eval_ret"); eval_ret=$?; :
> > + git config indexhelper.autorun true
> > + git status
> > error: last command exited with $?=141
> > not ok 5 - index-helper autorun works
> > #
> > #   test_when_finished "git index-helper --kill" &&
> > #   rm -f .git/index-helper.sock &&
> > #   git status &&
> > #   test_path_is_missing .git/index-helper.sock &&
> > #   test_config indexhelper.autorun true &&
> > #   git status &&
> > #   test -S .git/index-helper.sock &&
> > #   git status 2>err &&
> > #   test -S .git/index-helper.sock &&
> > #   test_must_be_empty err &&
> > #   git index-helper --kill &&
> > #   test_config indexhelper.autorun false &&
> > #   git status &&
> > #   test_path_is_missing .git/index-helper.sock
> > #
> Here are the relevant bits of a strace, pid 22200 is the second git
> status, 222197 is the index helper. 22122 is the test script
> 
> 22200 socket(PF_LOCAL, SOCK_STREAM, 0)  = 7
> 22200 connect(7, {sa_family=AF_LOCAL, sun_path=".git/index-
> helper.sock"}, 110 
> 22197 <... poll resumed> )  = 1
> 22197 accept(7, 0, NULL)= 8
> 22197 fcntl(8, F_GETFL) = 0x2 (flags O_RDWR)
> 22197 fcntl(8, F_SETFL, O_RDWR) = 0
> 22197 read(8,  
> 22200 <... connect resumed> )   = 0
> 22200 rt_sigaction(SIGPIPE, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, 8) = 0
> 22200 write(7, "000fpoke 22200 ", 15 
> 22197 <... read resumed> 0x7ffc4e4b9b20, 4) = 4
> 22197 read(8, 0x7ffc4e4b9c70, 11)   = 11
> 22197 write(8, 0x18b08b0, 6)= 6
> 22197 close(8)  = 0
> 22197 poll([?] 0x7ffc4e4b9b80, 1, 60 
> 22200 <... write resumed> ) = 15
> 22200 write(7, "", 4)   = -1 EPIPE (Broken pipe)
> 22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=22200,
> si_uid=1000} ---
> 22200 rt_sigaction(SIGPIPE, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, 8) = 0
> 22200 tgkill(22200, 22200, SIGPIPE) = 0   
> 22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_TKILL, si_pid=22200,
> si_uid=1000} ---
> 22200 +++ killed by SIGPIPE +++
> 22122 <... wait4 resumed> [{WIFSIGNALED(s) && WTERMSIG(s) ==
> SIGPIPE}], 0, NULL) = 22200 
> 22122 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_KILLED,
> si_pid=22200, si_status=SIGPIPE, si_utime=0, si_stime=0} ---
> 
> Looks like the index helper closes the socket, but git status still
> wants to write to it. The index-helper also 

Re: [PATCH v8 3/3] submodule: ensure that -c http.extraheader is heeded

[Junio C Hamano]

Johannes Schindelin  writes:

> To support this developer's use case of allowing build agents token-based
> access to private repositories, we introduced the http.extraheader
> feature, allowing extra HTTP headers to be sent along with every HTTP
> request.
>
> This patch verifies that we can configure these extra HTTP headers via the
> command-line for use with `git submodule update`, too. Example: git -c
> http.extraheader="Secret: Sauce" submodule update --init
>
> Signed-off-by: Johannes Schindelin 
> ---

Applying this directly on top of the other two fails, and when
merged with jk/submodule-c-credential, the test passes.

Which is exactly what we expect to see.  Nice.

I'll merge jk/submodule-c-credential into js/http-custom-headers
that already has 1 & 2, and then apply this.  An alternative would
be to hold this and wait until both jk/submodule-c-credential and
js/http-custom-headers with 1 & 2 graduates and then apply this,
which is a lot inferior option.

Thanks.

>  t/t5551-http-fetch-smart.sh | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index 43b257e..2f375eb 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -287,7 +287,16 @@ test_expect_success 'custom http headers' '
>   fetch "$HTTPD_URL/smart_headers/repo.git" &&
>   git -c http.extraheader="x-magic-one: abra" \
>   -c http.extraheader="x-magic-two: cadabra" \
> - fetch "$HTTPD_URL/smart_headers/repo.git"
> + fetch "$HTTPD_URL/smart_headers/repo.git" &&
> + git update-index --add --cacheinfo 16,$(git rev-parse HEAD),sub &&
> + git config -f .gitmodules submodule.sub.path sub &&
> + git config -f .gitmodules submodule.sub.url \
> + "$HTTPD_URL/smart_headers/repo.git" &&
> + git submodule init sub &&
> + test_must_fail git submodule update sub &&
> + git -c http.extraheader="x-magic-one: abra" \
> + -c http.extraheader="x-magic-two: cadabra" \
> + submodule update sub
>  '
>  
>  stop_httpd
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 2/3] t5551: make the test for extra HTTP headers more robust

[Junio C Hamano]

Johannes Schindelin  writes:

> To test that extra HTTP headers are passed correctly, t5551 verifies that
> a fetch succeeds when two required headers are passed, and that the fetch
> does not succeed when those headers are not passed.
>
> However, this test would also succeed if the configuration required only
> one header. As Apache's configuration is notoriously tricky (this
> developer frequently requires StackOverflow's help to understand Apache's
> documentation), especially when still supporting the 2.2 line, let's just
> really make sure that the test verifies what we want it to verify.
>
> Signed-off-by: Johannes Schindelin 
> ---

Matches the previous one I queued with Reviewed-by: from Peff; good.

>  t/t5551-http-fetch-smart.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index e44fe72..43b257e 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -283,7 +283,8 @@ test_expect_success EXPENSIVE 'http can handle enormous 
> ref negotiation' '
>  '
>  
>  test_expect_success 'custom http headers' '
> - test_must_fail git fetch "$HTTPD_URL/smart_headers/repo.git" &&
> + test_must_fail git -c http.extraheader="x-magic-two: cadabra" \
> + fetch "$HTTPD_URL/smart_headers/repo.git" &&
>   git -c http.extraheader="x-magic-one: abra" \
>   -c http.extraheader="x-magic-two: cadabra" \
>   fetch "$HTTPD_URL/smart_headers/repo.git"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 05/13] worktree.c: mark current worktree

[Junio C Hamano]

Duy Nguyen  writes:

> On second thought, why hold patches back, lengthen the worktree-move
> series and make it a pain to review? I moved a few patches from
> worktree-move into this series and I took two other out to create
> nd/error-errno. So I'm going to take more patches out of it, creating
> two bite-sized series, to be sent shortly.
>
> The first one is purely cleanup (ok, 1/7 is not exactly cleanup)
>
>   [1/7] completion: support git-worktree
>   [2/7] worktree.c: rewrite mark_current_worktree() to avoid
>   [3/7] git-worktree.txt: keep subcommand listing in alphabetical
>   [4/7] worktree.c: use is_dot_or_dotdot()
>   [5/7] worktree.c: add clear_worktree()
>   [6/7] worktree: avoid 0{40}, too many zeroes, hard to read
>   [7/7] worktree: simplify prefixing paths
>
> And the second one adds "git worktree lock" and "git worktree
> unlock". This series is built on top of the first one, it needs 1/7.
>
>   [1/5] worktree.c: add find_worktree_by_path()
>   [2/5] worktree.c: add is_main_worktree()
>   [3/5] worktree.c: add is_worktree_locked()
>   [4/5] worktree: add "lock" command
>   [5/5] worktree: add "unlock" command
>
> After this, worktree-move becomes ~10 patch series.

Yay.  Thanks; will take a look.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 1/3] tests: adjust the configuration for Apache 2.2

[Junio C Hamano]

Johannes Schindelin  writes:

> Lars Schneider noticed that the configuration introduced to test the
> extra HTTP headers cannot be used with Apache 2.2 (which is still
> actively maintained, as pointed out by Junio Hamano).
>
> To let the tests pass with Apache 2.2 again, let's substitute the
> offending  and `expr` by using old school RewriteCond
> statements.
>
> As RewriteCond does not allow testing for *non*-matches, we simply match
> the desired case first and let it pass by marking the RewriteRule as
> '[L]' ("last rule, do not process any other matching RewriteRules after
> this"), and then have another RewriteRule that matches all other cases
> and lets them fail via '[F]' ("fail").
>
> Signed-off-by: Johannes Schindelin 
> Signed-off-by: Junio C Hamano 
> ---

I applied this and compared what was queued from the previous round.
It turns out that it is the same, except that I amended it with
"Tested-by:" from Lars', so I'll skip this and nothing is lost ;-)

Thanks for being thorough (the above is not a suggestion to omit
unchanged ones next time--quite the contrary, being able to verify
by comparing is a good thing).

>  t/lib-httpd/apache.conf | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index b8ed96f..018a83a 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -103,10 +103,6 @@ Alias /auth/dumb/ www/auth/dumb/
>   Header set Set-Cookie name=value
>  
>  
> - 
> - Require expr %{HTTP:x-magic-one} == 'abra'
> - Require expr %{HTTP:x-magic-two} == 'cadabra'
> - 
>   SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
>   SetEnv GIT_HTTP_EXPORT_ALL
>  
> @@ -136,6 +132,18 @@ RewriteRule ^/ftp-redir/(.*)$ ftp://localhost:1000/$1 
> [R=302]
>  RewriteRule ^/loop-redir/x-x-x-x-x-x-x-x-x-x-x-x-x-x-x-x-x-x-x-x-(.*) /$1 
> [R=302]
>  RewriteRule ^/loop-redir/(.*)$ /loop-redir/x-$1 [R=302]
>  
> +# Apache 2.2 does not understand , so we use RewriteCond.
> +# And as RewriteCond does not allow testing for non-matches, we match
> +# the desired case first (one has abra, two has cadabra), and let it
> +# pass by marking the RewriteRule as [L], "last rule, do not process
> +# any other matching RewriteRules after this"), and then have another
> +# RewriteRule that matches all other cases and lets them fail via '[F]',
> +# "fail the request".
> +RewriteCond %{HTTP:x-magic-one} =abra
> +RewriteCond %{HTTP:x-magic-two} =cadabra
> +RewriteRule ^/smart_headers/.* - [L]
> +RewriteRule ^/smart_headers/.* - [F]
> +
>  
>  LoadModule ssl_module modules/mod_ssl.so
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

syntax error in git-rebase while running t34* tests

[Armin Kunaschik]

Hello,

I noticed in my environment (AIX, ksh) that all rebase tests don't work.
git-rebase terminates with
git-rebase[6]: Syntax error at line 3 : `newline or ;' is not expected.
Digging deeper I found the problem in git-rebase--interactive line 85


strategy_args=${strategy:+--strategy=$strategy}
eval '
for strategy_opt in '"$strategy_opts"'
do
strategy_args="$strategy_args -X$(git rev-parse
--sq-quote "${strategy_opt#--}")"
done
'


This snippet fails when $strategy_opts is empty or undefined... and my
eval seems not to like this.
The for loop runs fine, even with empty strategy_opts, but not inside eval.
I fail to see why eval is really necessary here. Things can probably
be done without eval, but I
don't feel to see the big picture around this code.

My quick and dirty hotfix is to place a
test -n "$strategy_opts" &&
in front of the eval.
The tests run fine after this change.

What do you think?

Armin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/19] index-helper/watchman

[Dennis Kaarsemaker]

On ma, 2016-05-09 at 15:32 -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > 
> > David Turner  writes:
> > 
> > > 
> > > On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
> > > > 
> > > > Hmmm, I seem to be getting
> > > > 
> > > > $ cat t/trash*7900*/err
> > > > fatal: Already running
> > > > 
> > > > after running t7900 and it fails at #5, after applying
> > > > "index-helper: optionally automatically run"
> The symptom looks pretty similar to $gmane/293461 reported earlier.
> Here is how "t7900-index-helper.sh -i -v -x -d" ends.
> 
> 
> expecting success:
> test_when_finished "git index-helper --kill" &&
> rm -f .git/index-helper.sock &&
> git status &&
> test_path_is_missing .git/index-helper.sock &&
> test_config indexhelper.autorun true &&
> git status &&
> test -S .git/index-helper.sock &&
> git status 2>err &&
> test -S .git/index-helper.sock &&
> test_must_be_empty err &&
> git index-helper --kill &&
> test_config indexhelper.autorun false &&
> git status &&
> test_path_is_missing .git/index-helper.sock
> 
> + test_when_finished git index-helper --kill
> + test 0 = 0
> + test_cleanup={ git index-helper --kill
> } && (exit "$eval_ret"); eval_ret=$?; :
> + rm -f .git/index-helper.sock
> + git status
> On branch master
> Untracked files:
>   (use "git add ..." to include in what will be committed)
> 
> err
> 
> nothing added to commit but untracked files present (use "git add" to
> track)
> + test_path_is_missing .git/index-helper.sock
> + test -e .git/index-helper.sock
> + test_config indexhelper.autorun true
> + config_dir=
> + test indexhelper.autorun = -C
> + test_when_finished test_unconfig  'indexhelper.autorun'
> + test 0 = 0
> + test_cleanup={ test_unconfig  'indexhelper.autorun'
> } && (exit "$eval_ret"); eval_ret=$?; { git index-
> helper --kill
> } && (exit "$eval_ret"); eval_ret=$?; :
> + git config indexhelper.autorun true
> + git status
> error: last command exited with $?=141
> not ok 5 - index-helper autorun works
> #
> #   test_when_finished "git index-helper --kill" &&
> #   rm -f .git/index-helper.sock &&
> #   git status &&
> #   test_path_is_missing .git/index-helper.sock &&
> #   test_config indexhelper.autorun true &&
> #   git status &&
> #   test -S .git/index-helper.sock &&
> #   git status 2>err &&
> #   test -S .git/index-helper.sock &&
> #   test_must_be_empty err &&
> #   git index-helper --kill &&
> #   test_config indexhelper.autorun false &&
> #   git status &&
> #   test_path_is_missing .git/index-helper.sock
> #

Here are the relevant bits of a strace, pid 22200 is the second git
status, 222197 is the index helper. 22122 is the test script

22200 socket(PF_LOCAL, SOCK_STREAM, 0)  = 7
22200 connect(7, {sa_family=AF_LOCAL, sun_path=".git/index-helper.sock"}, 110 

22197 <... poll resumed> )  = 1
22197 accept(7, 0, NULL)= 8
22197 fcntl(8, F_GETFL) = 0x2 (flags O_RDWR)
22197 fcntl(8, F_SETFL, O_RDWR) = 0
22197 read(8,  
22200 <... connect resumed> )   = 0
22200 rt_sigaction(SIGPIPE, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART, 
0x7fcc463fdd40}, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART, 0x7fcc463fdd40}, 8) 
= 0
22200 write(7, "000fpoke 22200 ", 15 
22197 <... read resumed> 0x7ffc4e4b9b20, 4) = 4
22197 read(8, 0x7ffc4e4b9c70, 11)   = 11
22197 write(8, 0x18b08b0, 6)= 6
22197 close(8)  = 0
22197 poll([?] 0x7ffc4e4b9b80, 1, 60 
22200 <... write resumed> ) = 15
22200 write(7, "", 4)   = -1 EPIPE (Broken pipe)
22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=22200, 
si_uid=1000} ---
22200 rt_sigaction(SIGPIPE, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART, 
0x7fcc463fdd40}, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART, 0x7fcc463fdd40}, 8) 
= 0
22200 tgkill(22200, 22200, SIGPIPE) = 0   
22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_TKILL, si_pid=22200, 
si_uid=1000} ---
22200 +++ killed by SIGPIPE +++
22122 <... wait4 resumed> [{WIFSIGNALED(s) && WTERMSIG(s) == SIGPIPE}], 0, 
NULL) = 22200 
22122 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_KILLED, si_pid=22200, 
si_status=SIGPIPE, si_utime=0, si_stime=0} ---

Looks like the index helper closes the socket, but git status still
wants to write to it. The index-helper also doesn't seem to read all
data from the socket.

-- 
Dennis Kaarsemaker
www.kaarsemaker.net


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] Support marking .git/ (or all files) as hidden on Windows

[Junio C Hamano]

Johannes Schindelin  writes:

> Hi Junio,
>
> On Mon, 9 May 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin  writes:
>> 
>> > This is a heavily version of patches we carried in Git for Windows for
>
> s/patches/patched/
>
> I wish I had a penny for each time I wrote this particular typo.

This is a heavily version of patched we carried...

does not sound all that grammatical.

These are heavily modified version of patches...

is a possibility, but perhaps you meant

This is a heavily patched version of what we carried...

>> OK, so what do you want me to do with this "heavily modified
>> version"?  Earlier you responded:
>> 
>> > I have a huge preference for a code that has been production for
>> > years over a new code that would cook at most two weeks in 'next'.
>> 
>> I agree. However, it does not fill me with confidence that we did not
>> catch those two bugs earlier. Even one round of reviews (including a
>> partial rewrite) was better than all that time since the regressions
>> were introduced.
>> 
>> So do we want to follow the regular "a few days in 'pu' in case
>> somebody finds 'oops this trivial change is needed', a week or two
>> in 'next' for simmering as everybody else, and finally down to
>> 'master'" schedule?
>
> Well, I plan to include this patch (replacing the original
> version) in whatever Git for Windows version I release next.  I
> guess that we can go with the regular way in git.git. You could
> just as well merge it to master right away, it won't matter much
> as far as Git for Windows is concerned.

OK.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff

[David Turner]

Will do, thanks.

On Tue, 2016-05-10 at 12:13 +0200, SZEDER Gábor wrote:
> This patch adds a new plumbing command, which then will show up in
> completion after 'git '.  Could you please squash in this
> oneliner to exclude index-helper from porcelain commands in the
> completion script?
> 
> 
> ---
>  contrib/completion/git-completion.bash | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 34024754d929..9264ab919a6a 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -684,6 +684,7 @@ __git_list_porcelain_commands ()
>   for-each-ref) : plumbing;;
>   hash-object)  : plumbing;;
>   http-*)   : transport;;
> + index-helper) : plumbing;;
>   index-pack)   : plumbing;;
>   init-db)  : deprecated;;
>   local-fetch)  : plumbing;;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] mingw: introduce the 'core.hideDotFiles' setting

[Junio C Hamano]

Johannes Schindelin  writes:

> On Mon, 9 May 2016, Junio C Hamano wrote:
>
>> Johannes Schindelin  writes:
>> 
>> > +core.hideDotFiles::
>> > +  (Windows-only) If true, mark newly-created directories and files whose
>> > +  name starts with a dot as hidden.  If 'dotGitOnly', only the `.git/`
>> > +  directory is hidden, but no other files starting with a dot.  The
>> > +  default mode is to mark only the `.git/` directory as hidden.
>> 
>> I think "The default mode is 'dotGitOnly'" is sufficient, given that
>> it is described just one sentence before, which is still likely to
>> be in readers' mind.  But I'll let it pass without tweaking.
>
> Yeah, when rewriting this after Ramsay pointed out the erroneous
> documentation, I wanted to fail on the crystal-clear side.

Saying the same thing twice in rapid succession does not make it
any clearer, though.  If you are rerolling anyway, then I think it
would be better to shorten it to clarify.

>> The other is why you do not have to retry creation in a similar way
>> when !needs_hiding(filename).  I didn't see anything in the function
>> before reaching this point that does anything differently based on
>> needs_hiding().  Can't the same 'ERROR_ACCESS_DENIED' error trigger
>> if CREATE_ALWAYS was specified and file attributes of an existing
>> file match, and if it can, don't you want to retry that too, even if
>> you are not going to hide the filename?
>
> The attributes that can trigger the ERROR_ACCESS_DENIED error are the
> hidden flag and the system flag. Since Git *never* marks any file as
> system file, we should be safe.

I was just wondering Git's trying to open it via this codepath
trigger the "if CREATE_ALWAYS was specified and an already existing
file's attributes do not match *exactly*.", if the path is already
hidden.  But again, I am clueless on Windows API, expected use
pattern of Git for Windows, and your future project plans
(e.g. basing future work on Karsten's clean-up patch), so I defer to
your judgment.

> Anyway, the next iteration of this patch is on its way.

OK.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] travis-ci: build documentation

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> From: Lars Schneider 
>
> Build documentation as separate Travis CI job to check for
> documentation errors.
>
> Signed-off-by: Lars Schneider 
> ---
>  .travis.yml  | 15 +++
>  ci/test-documentation.sh | 14 ++
>  2 files changed, 29 insertions(+)
>  create mode 100755 ci/test-documentation.sh
>
> diff --git a/.travis.yml b/.travis.yml
> index 78e433b..55299bd 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -32,6 +32,21 @@ env:
>  # t9816 occasionally fails with "TAP out of sequence errors" on Travis 
> CI OS X
>  - GIT_SKIP_TESTS="t9810 t9816"

Completely offtopic, but this looks like this is made to apply to
all archs, not limited to OSX?  It of course would be ideal to see
why they fail only on OSX and fix them, but shouldn't the blacklist
at least limited to the platform with the problem?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (May 2016, #02; Fri, 6)

[Junio C Hamano]

Lars Schneider  writes:

> A version with at least one section error is here:
> http://article.gmane.org/gmane.comp.version-control.git/293521
> Should I fix and reroll this patch?

I just compared this with jc/linkgit-fix, which was done until the
script introduced by jc/doc-lint gets happy.  293521 covers the same
spots, but misses the incorrect section numbers, so your reroll of it
would end up looking exactly like jc/linkgit-fix, I would think.

> The Travis-CI documentation check build step did not make it in:
> http://article.gmane.org/gmane.comp.version-control.git/293523
> Is there something I can improve?

Let me take another look later.

Thanks for carefully going through the "What's cooking" report.  It
really helps to make sure we move forward topics that need to.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

diff --break-rewrites for just a part of a file

[Sascha Silbe]

Hello,

the other day I was reviewing a patch that replaced a large chunk in a
Makefile with completely different logic. No matter what diff algorithm
and options I threw at it, the diff would always synchronise at the
empty lines between individual targets and thus show the rewrite of a
larger section as complete replacements of many smaller, but directly
adjacent sections (only separated by a blank line).

--break-rewrites would be nicely suited for this case, but once I dialed
down the parameters enough for the option to apply at all, it showed the
entire file as being replaced rather than just the section in between
that actually changed. Is there a way to have --break-rewrites leave out
the unchanged lines at beginning and end of the file?

A combination of --break-rewrites and --inter-hunk-context that merges
changes with less than the given number of unchanged lines between them
into a single delete/insert change would be even better. But just
ignoring the unchanged header and footer of a file for --break-rewrites
would already go a long way.

Sascha


pgpsryiLpPJZE.pgp
Description: PGP signature

[PATCH 3/3] Add a perf test for rebase -i

[Johannes Schindelin]

This developer spent a lot of time trying to speed up the interactive
rebase, in particular on Windows. And will continue to do so.

To make it easier to demonstrate the performance improvement, let's have
a reproducible performance test.

The topic branch we use to test performance was found using these shell
commands (essentially searching for a long-enough topic branch in Git's
own history that touched the same file multiple times):

git rev-list --parents origin/master |
grep ' .* ' |
while read commit rest
do
patch_count=$(git rev-list --count $commit^..$commit^2)
test $patch_count -gt 20 || continue

merges="$(git rev-list --parents $commit^..$commit^2 |
grep ' .* ')"
test -z "$merges" || continue

patches_per_file="$(git log --pretty=%H --name-only \
$commit^..$commit^2 |
grep -v '^$' |
sort |
uniq -c -d |
sort -n -r)"
test -n "$patches_per_file" &&
test 20 -lt $(echo "$patches_per_file" |
sed -n '1s/^ *\([0-9]*\).*/\1/p') || continue

printf 'commit %s\n%s\n' "$commit" "$patches_per_file"
done

Note that we can get away with *not* having to reset to the original
branch tip before rebasing: we switch the first two "pick" lines every
time, so we end up with the same patch order after two rebases, and the
complexity of both rebases is equivalent.

Signed-off-by: Johannes Schindelin 
---
 t/perf/p3404-rebase-interactive.sh | 31 +++
 1 file changed, 31 insertions(+)
 create mode 100755 t/perf/p3404-rebase-interactive.sh

diff --git a/t/perf/p3404-rebase-interactive.sh 
b/t/perf/p3404-rebase-interactive.sh
new file mode 100755
index 000..382163c
--- /dev/null
+++ b/t/perf/p3404-rebase-interactive.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+test_description='Tests rebase -i performance'
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+# This commit merges a sufficiently long topic branch for reasonable
+# performance testing
+branch_merge=ba5312d
+export branch_merge
+
+write_script swap-first-two.sh <<\EOF
+case "$1" in
+*/COMMIT_EDITMSG)
+   mv "$1" "$1".bak &&
+   sed -e '1{h;d}' -e 2G <"$1".bak >"$1"
+   ;;
+esac
+EOF
+
+test_expect_success 'setup' '
+   git config core.editor "\"$PWD"/swap-first-two.sh\" &&
+   git checkout -f $branch_merge^2
+'
+
+test_perf 'rebase -i' '
+   git rebase -i $branch_merge^
+'
+
+test_done
-- 
2.8.2.465.gb077790
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] perf: make the tests work in worktrees

[Johannes Schindelin]

This patch makes perf-lib.sh more robust so that it can run correctly
even inside a worktree. For example, it assumed that $GIT_DIR/objects is
the objects directory (which is not the case for worktrees) and it used
the commondir file verbatim, even if it contained a relative path.

Signed-off-by: Johannes Schindelin 
---
 t/perf/perf-lib.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index e9020d0..e5682f7 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -80,22 +80,22 @@ test_perf_create_repo_from () {
error "bug in the test script: not 2 parameters to test-create-repo"
repo="$1"
source="$2"
-   source_git=$source/$(cd "$source" && git rev-parse --git-dir)
+   source_git="$(cd "$source" && git rev-parse --git-dir)"
+   objects_dir="$(git rev-parse --git-path objects)"
mkdir -p "$repo/.git"
(
-   cd "$repo/.git" &&
-   { cp -Rl "$source_git/objects" . 2>/dev/null ||
-   cp -R "$source_git/objects" .; } &&
+   { cp -Rl "$objects_dir" "$repo/.git/" 2>/dev/null ||
+   cp -R "$objects_dir" "$repo/.git/"; } &&
for stuff in "$source_git"/*; do
case "$stuff" in
-   */objects|*/hooks|*/config)
+   */objects|*/hooks|*/config|*/commondir)
;;
*)
-   cp -R "$stuff" . || exit 1
+   cp -R "$stuff" "$repo/.git/" || exit 1
;;
esac
done &&
-   cd .. &&
+   cd "$repo" &&
git init -q &&
if test_have_prereq MINGW
then
-- 
2.8.2.465.gb077790


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] perf: let's disable symlinks on Windows

[Johannes Schindelin]

In Git for Windows' SDK, Git's source code is always checked out
with symlinks disabled. The reason is that POSIX symlinks have no
accurate equivalent on Windows [*1*]. More precisely, though, it is
not just Git's source code but *all* source code that is checked
out with symlinks disabled: core.symlinks is set to false in the
system-wide gitconfig.

Since the perf tests are run with the system-wide gitconfig *disabled*,
we have to make sure that the Git repository is initialized correctly
by configuring core.symlinks explicitly.

Footnote *1*:
https://github.com/git-for-windows/git/wiki/Symbolic-Links

Signed-off-by: Johannes Schindelin 
---
 t/perf/perf-lib.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 5cf74ed..e9020d0 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -97,6 +97,10 @@ test_perf_create_repo_from () {
done &&
cd .. &&
git init -q &&
+   if test_have_prereq MINGW
+   then
+   git config core.symlinks false
+   fi &&
mv .git/hooks .git/hooks-disabled 2>/dev/null
) || error "failed to copy repository '$source' to '$repo'"
 }
-- 
2.8.2.465.gb077790


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] Introduce a perf test for interactive rebase

[Johannes Schindelin]

This is the second preparatory patch series for my rebase--helper work
(i.e. moving parts of the interactive rebase into a builtin).

It simply introduces a perf test (and ensures that it runs in my
environment) so as to better determine how much the performance changes,
really.


Johannes Schindelin (3):
  perf: let's disable symlinks on Windows
  perf: make the tests work in worktrees
  Add a perf test for rebase -i

 t/perf/p3404-rebase-interactive.sh | 31 +++
 t/perf/perf-lib.sh | 18 +++---
 2 files changed, 42 insertions(+), 7 deletions(-)
 create mode 100755 t/perf/p3404-rebase-interactive.sh

Published-As: https://github.com/dscho/git/releases/tag/perf-rebase-i-v1
-- 
2.8.2.465.gb077790

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/19] index-helper/watchman

[Ramsay Jones]

On 10/05/16 12:52, Dennis Kaarsemaker wrote:
> On ma, 2016-05-09 at 15:22 -0700, Junio C Hamano wrote:
>> It passes on one box and fails on another.  They both run the same
>> Ubuntu 14.04 derivative, with same ext3 filesystem.  The failing one
>> is on a VM.
> 
> Same here, except ext4 instead of ext3. Failing on a virtual machine,
> not failing on a physical one.

I can confirm the trend:

Linux Mint 17.3, ext4 - bare-metal pass, (Virtual Box) VM fail.

ATB,
Ramsay Jones


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] worktree.c: add is_worktree_locked()

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

This provides an API for checking if a worktree is locked. We need to
check this to avoid double locking a worktree, or try to unlock one when
it's not even locked.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 worktree.c | 18 ++
 worktree.h |  6 ++
 2 files changed, 24 insertions(+)

diff --git a/worktree.c b/worktree.c
index 37fea3d..6805deb 100644
--- a/worktree.c
+++ b/worktree.c
@@ -243,6 +243,24 @@ int is_main_worktree(const struct worktree *wt)
return wt && !wt->id;
 }
 
+const char *is_worktree_locked(const struct worktree *wt)
+{
+   static struct strbuf sb = STRBUF_INIT;
+
+   if (!file_exists(git_common_path("worktrees/%s/locked", wt->id)))
+   return NULL;
+
+   strbuf_reset();
+   if (strbuf_read_file(,
+git_common_path("worktrees/%s/locked", wt->id),
+0) < 0)
+   die_errno(_("failed to read '%s'"),
+ git_common_path("worktrees/%s/locked", wt->id));
+
+   strbuf_rtrim();
+   return sb.buf;
+}
+
 int is_worktree_being_rebased(const struct worktree *wt,
  const char *target)
 {
diff --git a/worktree.h b/worktree.h
index 77a9e09..12906be 100644
--- a/worktree.h
+++ b/worktree.h
@@ -40,6 +40,12 @@ extern struct worktree *find_worktree_by_path(struct 
worktree **list,
  */
 extern int is_main_worktree(const struct worktree *wt);
 
+/*
+ * Return the reason string if the given worktree is locked. Return
+ * NULL otherwise.
+ */
+extern const char *is_worktree_locked(const struct worktree *wt);
+
 /*
  * Free up the memory for worktree
  */
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] worktree: add "lock" command

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-worktree.txt | 12 --
 builtin/worktree.c | 41 ++
 contrib/completion/git-completion.bash |  5 -
 t/t2028-worktree-move.sh (new +x)  | 34 
 4 files changed, 89 insertions(+), 3 deletions(-)
 create mode 100755 t/t2028-worktree-move.sh

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 27feff6..74583c1 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git worktree add' [-f] [--detach] [--checkout] [-b ]  
[]
 'git worktree list' [--porcelain]
+'git worktree lock' [--reason ] 
 'git worktree prune' [-n] [-v] [--expire ]
 
 DESCRIPTION
@@ -61,6 +62,12 @@ each of the linked worktrees.  The output details include if 
the worktree is
 bare, the revision currently checked out, and the branch currently checked out
 (or 'detached HEAD' if none).
 
+lock::
+
+When a worktree is locked, it cannot be pruned, moved or deleted. For
+example, if the worktree is on portable device that is not available
+when "git worktree " is executed.
+
 prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
@@ -110,6 +117,9 @@ OPTIONS
 --expire ::
With `prune`, only expire unused working trees older than .
 
+--reason :
+   An explanation why the worktree is locked.
+
 DETAILS
 ---
 Each linked working tree has a private sub-directory in the repository's
@@ -226,8 +236,6 @@ performed manually, such as:
 - `remove` to remove a linked working tree and its administrative files (and
   warn if the working tree is dirty)
 - `mv` to move or rename a working tree and update its administrative files
-- `lock` to prevent automatic pruning of administrative files (for instance,
-  for a working tree on a portable device)
 
 GIT
 ---
diff --git a/builtin/worktree.c b/builtin/worktree.c
index f9dac37..51fa103 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -14,6 +14,7 @@
 static const char * const worktree_usage[] = {
N_("git worktree add []  []"),
N_("git worktree list []"),
+   N_("git worktree lock [] "),
N_("git worktree prune []"),
NULL
 };
@@ -459,6 +460,44 @@ static int list(int ac, const char **av, const char 
*prefix)
return 0;
 }
 
+static int lock_worktree(int ac, const char **av, const char *prefix)
+{
+   const char *reason = "", *old_reason;
+   struct option options[] = {
+   OPT_STRING(0, "reason", , N_("string"),
+  N_("reason for locking")),
+   OPT_END()
+   };
+   struct worktree **worktrees, *wt;
+   struct strbuf dst = STRBUF_INIT;
+
+   ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+   if (ac != 1)
+   usage_with_options(worktree_usage, options);
+
+   strbuf_addstr(, prefix_filename(prefix,
+   strlen(prefix),
+   av[0]));
+
+   worktrees = get_worktrees();
+   wt = find_worktree_by_path(worktrees, dst.buf);
+   if (!wt)
+   die(_("'%s' is not a working directory"), av[0]);
+   if (is_main_worktree(wt))
+   die(_("'%s' is a main working directory"), av[0]);
+
+   old_reason = is_worktree_locked(wt);
+   if (old_reason) {
+   if (*old_reason)
+   die(_("already locked, reason: %s"), old_reason);
+   die(_("already locked, no reason"));
+   }
+
+   write_file(git_common_path("worktrees/%s/locked", wt->id),
+  "%s", reason);
+   return 0;
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
struct option options[] = {
@@ -475,5 +514,7 @@ int cmd_worktree(int ac, const char **av, const char 
*prefix)
return prune(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "list"))
return list(ac - 1, av + 1, prefix);
+   if (!strcmp(av[1], "lock"))
+   return lock_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
 }
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index d3ac391..6c321aa 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2597,7 +2597,7 @@ _git_whatchanged ()
 
 _git_worktree ()
 {
-   local subcommands="add list prune"
+   local subcommands="add list lock prune"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
@@ -2609,6 +2609,9 @@ _git_worktree ()
list,--*)
__gitcomp "--porcelain"
;;
+   lock,--*)
+   __gitcomp "--reason"
+   ;;

[PATCH 5/5] worktree: add "unlock" command

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-worktree.txt |  5 +
 builtin/worktree.c | 31 +++
 contrib/completion/git-completion.bash |  2 +-
 t/t2028-worktree-move.sh   | 14 ++
 4 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 74583c1..9ac1129 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 'git worktree list' [--porcelain]
 'git worktree lock' [--reason ] 
 'git worktree prune' [-n] [-v] [--expire ]
+'git worktree unlock' 
 
 DESCRIPTION
 ---
@@ -72,6 +73,10 @@ prune::
 
 Prune working tree information in $GIT_DIR/worktrees.
 
+unlock::
+
+Unlock a worktree, allowing it to be pruned, moved or deleted.
+
 OPTIONS
 ---
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 51fa103..53e5f5a 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -16,6 +16,7 @@ static const char * const worktree_usage[] = {
N_("git worktree list []"),
N_("git worktree lock [] "),
N_("git worktree prune []"),
+   N_("git worktree unlock "),
NULL
 };
 
@@ -498,6 +499,34 @@ static int lock_worktree(int ac, const char **av, const 
char *prefix)
return 0;
 }
 
+static int unlock_worktree(int ac, const char **av, const char *prefix)
+{
+   struct option options[] = {
+   OPT_END()
+   };
+   struct worktree **worktrees, *wt;
+   struct strbuf dst = STRBUF_INIT;
+
+   ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+   if (ac != 1)
+   usage_with_options(worktree_usage, options);
+
+   strbuf_addstr(, prefix_filename(prefix,
+   strlen(prefix),
+   av[0]));
+
+   worktrees = get_worktrees();
+   wt = find_worktree_by_path(worktrees, dst.buf);
+   if (!wt)
+   die(_("'%s' is not a working directory"), av[0]);
+   if (is_main_worktree(wt))
+   die(_("'%s' is a main working directory"), av[0]);
+   if (!is_worktree_locked(wt))
+   die(_("not locked"));
+
+   return unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
+}
+
 int cmd_worktree(int ac, const char **av, const char *prefix)
 {
struct option options[] = {
@@ -516,5 +545,7 @@ int cmd_worktree(int ac, const char **av, const char 
*prefix)
return list(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "lock"))
return lock_worktree(ac - 1, av + 1, prefix);
+   if (!strcmp(av[1], "unlock"))
+   return unlock_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
 }
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 6c321aa..db4b0e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2597,7 +2597,7 @@ _git_whatchanged ()
 
 _git_worktree ()
 {
-   local subcommands="add list lock prune"
+   local subcommands="add list lock prune unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 97434be..f4b2816 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -31,4 +31,18 @@ test_expect_success 'lock worktree twice' '
test_cmp expected .git/worktrees/source/locked
 '
 
+test_expect_success 'unlock main worktree' '
+   test_must_fail git worktree unlock .
+'
+
+test_expect_success 'unlock linked worktree' '
+   git worktree unlock source &&
+   test_path_is_missing .git/worktrees/source/locked
+'
+
+test_expect_success 'unlock worktree twice' '
+   test_must_fail git worktree unlock source &&
+   test_path_is_missing .git/worktrees/source/locked
+'
+
 test_done
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] worktree.c: add is_main_worktree()

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

Main worktree _is_ different. You can lock a linked worktree but not the
main one, for example. Provide an API for checking that.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 worktree.c | 5 +
 worktree.h | 5 +
 2 files changed, 10 insertions(+)

diff --git a/worktree.c b/worktree.c
index a6d1ad1..37fea3d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -238,6 +238,11 @@ struct worktree *find_worktree_by_path(struct worktree 
**list,
return wt;
 }
 
+int is_main_worktree(const struct worktree *wt)
+{
+   return wt && !wt->id;
+}
+
 int is_worktree_being_rebased(const struct worktree *wt,
  const char *target)
 {
diff --git a/worktree.h b/worktree.h
index 7775d1b..77a9e09 100644
--- a/worktree.h
+++ b/worktree.h
@@ -35,6 +35,11 @@ extern const char *get_worktree_git_dir(const struct 
worktree *wt);
 extern struct worktree *find_worktree_by_path(struct worktree **list,
  const char *path_);
 
+/*
+ * Return true if the given worktree is the main one.
+ */
+extern int is_main_worktree(const struct worktree *wt);
+
 /*
  * Free up the memory for worktree
  */
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] worktree.c: add find_worktree_by_path()

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

So far we haven't needed to identify an existing worktree from command
line. Future commands such as lock or move will need it. There are of
course other options for identifying a worktree, for example by branch
or even by internal id. They may be added later if proved useful.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 worktree.c | 16 
 worktree.h |  6 ++
 2 files changed, 22 insertions(+)

diff --git a/worktree.c b/worktree.c
index 335c58f..a6d1ad1 100644
--- a/worktree.c
+++ b/worktree.c
@@ -222,6 +222,22 @@ const char *get_worktree_git_dir(const struct worktree *wt)
return git_common_path("worktrees/%s", wt->id);
 }
 
+struct worktree *find_worktree_by_path(struct worktree **list,
+  const char *path_)
+{
+   char *path = xstrdup(real_path(path_));
+   struct worktree *wt = NULL;
+
+   while (*list) {
+   wt = *list++;
+   if (!fspathcmp(path, real_path(wt->path)))
+   break;
+   wt = NULL;
+   }
+   free(path);
+   return wt;
+}
+
 int is_worktree_being_rebased(const struct worktree *wt,
  const char *target)
 {
diff --git a/worktree.h b/worktree.h
index 7430a4e..7775d1b 100644
--- a/worktree.h
+++ b/worktree.h
@@ -29,6 +29,12 @@ extern struct worktree **get_worktrees(void);
  */
 extern const char *get_worktree_git_dir(const struct worktree *wt);
 
+/*
+ * Search a worktree by its path. Paths are normalized internally.
+ */
+extern struct worktree *find_worktree_by_path(struct worktree **list,
+ const char *path_);
+
 /*
  * Free up the memory for worktree
  */
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] completion: support git-worktree

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 contrib/completion/git-completion.bash | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 3402475..d3ac391 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2595,6 +2595,29 @@ _git_whatchanged ()
_git_log
 }
 
+_git_worktree ()
+{
+   local subcommands="add list prune"
+   local subcommand="$(__git_find_on_cmdline "$subcommands")"
+   if [ -z "$subcommand" ]; then
+   __gitcomp "$subcommands"
+   else
+   case "$subcommand,$cur" in
+   add,--*)
+   __gitcomp "--detach --force"
+   ;;
+   list,--*)
+   __gitcomp "--porcelain"
+   ;;
+   prune,--*)
+   __gitcomp "--dry-run --expire --verbose"
+   ;;
+   *)
+   ;;
+   esac
+   fi
+}
+
 __git_main ()
 {
local i c=1 command __git_dir
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/7] worktree: avoid 0{40}, too many zeroes, hard to read

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/worktree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index aaee0e2..b53f802 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -262,7 +262,7 @@ static int add_worktree(const char *path, const char 
*refname,
 */
strbuf_reset();
strbuf_addf(, "%s/HEAD", sb_repo.buf);
-   write_file(sb.buf, "");
+   write_file(sb.buf, sha1_to_hex(null_sha1));
strbuf_reset();
strbuf_addf(, "%s/commondir", sb_repo.buf);
write_file(sb.buf, "../..");
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] worktree.c: use is_dot_or_dotdot()

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/worktree.c | 2 +-
 worktree.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index bf80111..aaee0e2 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -95,7 +95,7 @@ static void prune_worktrees(void)
if (!dir)
return;
while ((d = readdir(dir)) != NULL) {
-   if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+   if (is_dot_or_dotdot(d->d_name))
continue;
strbuf_reset();
if (!prune_worktree(d->d_name, ))
diff --git a/worktree.c b/worktree.c
index 6a11611..f4a4f38 100644
--- a/worktree.c
+++ b/worktree.c
@@ -187,7 +187,7 @@ struct worktree **get_worktrees(void)
if (dir) {
while ((d = readdir(dir)) != NULL) {
struct worktree *linked = NULL;
-   if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+   if (is_dot_or_dotdot(d->d_name))
continue;
 
if ((linked = get_linked_worktree(d->d_name))) {
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] worktree.c: add clear_worktree()

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

The use case is keep some worktree and discard the rest of the worktree
list.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 worktree.c | 14 +++---
 worktree.h |  5 +
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/worktree.c b/worktree.c
index f4a4f38..335c58f 100644
--- a/worktree.c
+++ b/worktree.c
@@ -5,14 +5,22 @@
 #include "dir.h"
 #include "wt-status.h"
 
+void clear_worktree(struct worktree *wt)
+{
+   if (!wt)
+   return;
+   free(wt->path);
+   free(wt->id);
+   free(wt->head_ref);
+   memset(wt, 0, sizeof(*wt));
+}
+
 void free_worktrees(struct worktree **worktrees)
 {
int i = 0;
 
for (i = 0; worktrees[i]; i++) {
-   free(worktrees[i]->path);
-   free(worktrees[i]->id);
-   free(worktrees[i]->head_ref);
+   clear_worktree(worktrees[i]);
free(worktrees[i]);
}
free (worktrees);
diff --git a/worktree.h b/worktree.h
index 1394909..7430a4e 100644
--- a/worktree.h
+++ b/worktree.h
@@ -29,6 +29,11 @@ extern struct worktree **get_worktrees(void);
  */
 extern const char *get_worktree_git_dir(const struct worktree *wt);
 
+/*
+ * Free up the memory for worktree
+ */
+extern void clear_worktree(struct worktree *);
+
 /*
  * Free up the memory for worktree(s)
  */
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] worktree.c: rewrite mark_current_worktree() to avoid strbuf

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

strbuf is a bit overkill for this function. What we need is call
absolute_path() twice and make sure the second call does not destroy the
result of the first. One buffer allocation is enough.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 worktree.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/worktree.c b/worktree.c
index 4817d60..6a11611 100644
--- a/worktree.c
+++ b/worktree.c
@@ -153,21 +153,19 @@ done:
 
 static void mark_current_worktree(struct worktree **worktrees)
 {
-   struct strbuf git_dir = STRBUF_INIT;
-   struct strbuf path = STRBUF_INIT;
+   char *git_dir = xstrdup(absolute_path(get_git_dir()));
int i;
 
-   strbuf_addstr(_dir, absolute_path(get_git_dir()));
for (i = 0; worktrees[i]; i++) {
struct worktree *wt = worktrees[i];
-   strbuf_addstr(, absolute_path(get_worktree_git_dir(wt)));
-   wt->is_current = !fspathcmp(git_dir.buf, path.buf);
-   strbuf_reset();
-   if (wt->is_current)
+   const char *wt_git_dir = get_worktree_git_dir(wt);
+
+   if (!fspathcmp(git_dir, absolute_path(wt_git_dir))) {
+   wt->is_current = 1;
break;
+   }
}
-   strbuf_release(_dir);
-   strbuf_release();
+   free(git_dir);
 }
 
 struct worktree **get_worktrees(void)
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/7] git-worktree.txt: keep subcommand listing in alphabetical order

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

This is probably not the best order. But it makes it no-brainer to know
where to insert new commands. At some point we might want to reorder at
least the synopsis part again, grouping commonly use subcommands together.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-worktree.txt | 10 +-
 builtin/worktree.c |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index c622345..27feff6 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -10,8 +10,8 @@ SYNOPSIS
 
 [verse]
 'git worktree add' [-f] [--detach] [--checkout] [-b ]  
[]
-'git worktree prune' [-n] [-v] [--expire ]
 'git worktree list' [--porcelain]
+'git worktree prune' [-n] [-v] [--expire ]
 
 DESCRIPTION
 ---
@@ -54,10 +54,6 @@ If `` is omitted and neither `-b` nor `-B` nor 
`--detached` used,
 then, as a convenience, a new branch based at HEAD is created automatically,
 as if `-b $(basename )` was specified.
 
-prune::
-
-Prune working tree information in $GIT_DIR/worktrees.
-
 list::
 
 List details of each worktree.  The main worktree is listed first, followed by
@@ -65,6 +61,10 @@ each of the linked worktrees.  The output details include if 
the worktree is
 bare, the revision currently checked out, and the branch currently checked out
 (or 'detached HEAD' if none).
 
+prune::
+
+Prune working tree information in $GIT_DIR/worktrees.
+
 OPTIONS
 ---
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 12c0af7..bf80111 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -13,8 +13,8 @@
 
 static const char * const worktree_usage[] = {
N_("git worktree add []  []"),
-   N_("git worktree prune []"),
N_("git worktree list []"),
+   N_("git worktree prune []"),
NULL
 };
 
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] worktree: simplify prefixing paths

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/worktree.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index b53f802..f9dac37 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -337,7 +337,7 @@ static int add(int ac, const char **av, const char *prefix)
if (ac < 1 || ac > 2)
usage_with_options(worktree_usage, options);
 
-   path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];
+   path = prefix_filename(prefix, strlen(prefix), av[0]);
branch = ac < 2 ? "HEAD" : av[1];
 
opts.force_new_branch = !!new_branch_force;
@@ -467,6 +467,8 @@ int cmd_worktree(int ac, const char **av, const char 
*prefix)
 
if (ac < 2)
usage_with_options(worktree_usage, options);
+   if (!prefix)
+   prefix = "";
if (!strcmp(av[1], "add"))
return add(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "prune"))
-- 
2.8.2.524.g6ff3d78

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] t3404: be resilient against running with the -x flag

[Johannes Schindelin]

The -x flag (trace commands) is a priceless tool when hunting down bugs
that trigger test failures. It is a worthless tool if the -x flag
*itself* triggers test failures.

So let's change the offending tests so that they are a bit less
stringent and do not stumble over the "+..." lines generated by the -x
flag.

Signed-off-by: Johannes Schindelin 
---
 t/t3404-rebase-interactive.sh | 67 ++-
 1 file changed, 15 insertions(+), 52 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 66348f1..25f1118 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -882,7 +882,8 @@ test_expect_success 'rebase -i --exec without ' '
git reset --hard execute &&
set_fake_editor &&
test_must_fail git rebase -i --exec 2>tmp &&
-   sed -e "1d" tmp >actual &&
+   sed -e "/option .exec. requires a value/d" -e '/^+/d' \
+   tmp >actual &&
test_must_fail git rebase -h >expected &&
test_cmp expected actual &&
git checkout master
@@ -1149,10 +1150,6 @@ test_expect_success 'drop' '
test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
 '
 
-cat >expect expect expect expect expect 

Re: [PATCH v3 05/13] worktree.c: mark current worktree

[Duy Nguyen]

On Fri, May 06, 2016 at 05:21:05PM +0700, Duy Nguyen wrote:
> > Similarly, it looks like 'path' doesn't need to be a strbuf at all
> > since the result of absolute_path() should remain valid long enough
> > for fspathcmp(). It could just be:
> >
> > const char *path = absolute_path(...);
> > wt->is_current = !fspathcmp(git_dir, path);
> >
> > But these are very minor; probably not worth a re-roll.
> 
> Yeah. I think the use of strbuf is influenced by the code in
> get_worktrees(). But since this code is now in a separate function, it
> makes little sense to go with the strbuf hammer. If there's no big
> change in this series, I'll just do this as a cleanup step in my next
> series, worktree-move.

On second thought, why hold patches back, lengthen the worktree-move
series and make it a pain to review? I moved a few patches from
worktree-move into this series and I took two other out to create
nd/error-errno. So I'm going to take more patches out of it, creating
two bite-sized series, to be sent shortly.

The first one is purely cleanup (ok, 1/7 is not exactly cleanup)

  [1/7] completion: support git-worktree
  [2/7] worktree.c: rewrite mark_current_worktree() to avoid
  [3/7] git-worktree.txt: keep subcommand listing in alphabetical
  [4/7] worktree.c: use is_dot_or_dotdot()
  [5/7] worktree.c: add clear_worktree()
  [6/7] worktree: avoid 0{40}, too many zeroes, hard to read
  [7/7] worktree: simplify prefixing paths

And the second one adds "git worktree lock" and "git worktree
unlock". This series is built on top of the first one, it needs 1/7.

  [1/5] worktree.c: add find_worktree_by_path()
  [2/5] worktree.c: add is_main_worktree()
  [3/5] worktree.c: add is_worktree_locked()
  [4/5] worktree: add "lock" command
  [5/5] worktree: add "unlock" command

After this, worktree-move becomes ~10 patch series.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] t3404: fix typo

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin 
---
 t/t3404-rebase-interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d96d0e4..66348f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -62,7 +62,7 @@ test_expect_success 'setup' '
 
 # "exec" commands are ran with the user shell by default, but this may
 # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
-# to create a file. Unseting SHELL avoids such non-portable behavior
+# to create a file. Unsetting SHELL avoids such non-portable behavior
 # in tests. It must be exported for it to take effect where needed.
 SHELL=
 export SHELL
-- 
2.8.2.463.g99156ee


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] Work on t3404 in preparation for rebase--helper

[Johannes Schindelin]

This is the first patch series in preparation for a faster interactive
rebase.

It actually only prepares the test script that I mainly used to develop
the rebase--helper, and the resilience against running with -x proved to
be invaluable in keeping my sanity.


Johannes Schindelin (2):
  t3404: fix typo
  t3404: be resilient against running with the -x flag

 t/t3404-rebase-interactive.sh | 69 ++-
 1 file changed, 16 insertions(+), 53 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/t3404-fixes-v1
-- 
2.8.2.463.g99156ee

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: t4151 missing quotes

[Jeff King]

On Tue, May 10, 2016 at 03:44:32PM +0200, Armin Kunaschik wrote:

> I'm building on a quite current AIX 6.1 where /bin/sh defaults to /bin/ksh
> which is a posix shell (ksh88).
> Using /bin/bash doesn't work because SHELL_PATH is only used in
> git scripts but not in any t* test scripts.

If you run "make test" (or just "make" inside "t/") the test scripts
will be executed with SHELL_PATH. If you run:

  ./t1234-whatever.sh

then obviously no, they will not be. Don't do that. Either use:

  make t1234-whatever.sh

or:

  $YOUR_SHELL_PATH t1234-whatever.sh

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: t4151 missing quotes

[Armin Kunaschik]

Sorry for any duplicate mails, the list blocked my html mail.
Note to self: Don't use GMail on a tablet.

On Mon, May 9, 2016 at 11:35 PM, Eric Sunshine  wrote:
>>
>> Hmph, do we have a broken &&-chain?
>
> I don't know. Unfortunately, Armin didn't provide much information in
> his initial email, saying only "skipping through some failed tests",
> which doesn't necessarily indicate if those tests failed or if he
> somehow manually skipped them.

In t4151 there was only a problem with this test. All other tests
inside t4151 were ok.
Skipping through the tests referred to all git tests, not just t4151.

>> If an earlier test fails and leaves an unmerged path, "ls-files -u"
>> would give some output, so "test -z" would get one or more non-empty
>> strings; if we feed multiple, this would fail.  But we would not have
>> even run "test -z" as long as we properly &&-chain these tests.
>>
>> I think the real issue is when the earlier step succeeds and does
>> not leave any unmerged path.  In that case, we would run "test -z"
>> without anything else on the command line, which would lead to an
>> syntax error.

Yes. While debugging the test, I saw a syntax error. I did not try to find out
why the test argument is empty. It seems not necessary.. the test logic
is still the same.

>> Side Note: /usr/bin/test and test (built into bash and dash)
>> seem not to care about the lack of string in "test -z "
>> and "test -n ".  It appears to me that they just take
>> "-z" and "-n" without "" as a special case of "test
>> " that is fed "-z" or "-n" as .  Apparently, the
>> platform Armin is working on doesn't.
>
> I also tested on Mac OS X and BSD, and they happily accept bare "test
> -n", as well (though, I don't doubt that there are old shells which
> complain).

I'm building on a quite current AIX 6.1 where /bin/sh defaults to /bin/ksh
which is a posix shell (ksh88).
Using /bin/bash doesn't work because SHELL_PATH is only used in
git scripts but not in any t* test scripts.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Windows: only add a no-op pthread_sigmask() when needed

[Johannes Schindelin]

In f924b52 (Windows: add pthread_sigmask() that does nothing,
2016-05-01), we introduced a no-op for Windows. However, this breaks
building Git in Git for Windows' SDK because pthread_sigmask() is
already a no-op there, #define'd in the pthread_signal.h header in
/mingw64/x86_64-w64-mingw32/include/.

Let's guard the definition of pthread_sigmask() in #ifndef...#endif to
make the code compile both with modern MinGW-w64 as well as with the
previously common MinGW headers.

Signed-off-by: Johannes Schindelin 
---

This patch is obviously based on 'next' (because 'master' does not
have the referenced commit yet).

Published-As: https://github.com/dscho/git/releases/tag/mingw-sigmask-v1
 compat/win32/pthread.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index d336451..8df702c 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -104,9 +104,11 @@ static inline void *pthread_getspecific(pthread_key_t key)
return TlsGetValue(key);
 }
 
+#ifndef pthread_sigmask
 static inline int pthread_sigmask(int how, const sigset_t *set, sigset_t *oset)
 {
return 0;
 }
+#endif
 
 #endif /* PTHREAD_H */
-- 
2.8.2.463.g99156ee
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/19] index-helper/watchman

[Duy Nguyen]

On Tue, May 10, 2016 at 7:45 PM, Duy Nguyen  wrote:
> If --detach is used, log_warning() can't cover die(),
> warning() or error(), most importantly die() for example because of
> bugs.

A case for redirecting warning() is because watchman-support.c uses
it. But because this file is only used by index-helper, you have
another option, simply convert it to using log_warning().
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html