Re: bug report

[李本超]

git version 2.6.4 (Apple Git-63)
system version: OS X EI Capitan 10.11.4

below is the steps:
$ mkdir test_repo
$ cd test_repo
$ git init
$ echo "hello" > README.md
$ git commit -a -m 'Add README.md'

$ git checkout -b A
$ echo "world" > README.md
$ git commit -a -m 'Add one line'

$ git checkout master
$ git checkout -b B
$ echo "world" > README.md
$ git commit -a -m 'Add one line too'
$ [midify 'world' line to other things like 'git' using vi]
$ git commit -a -m 'Modify one line'

$ git checkout master
$ git merge A

$ git checkout B
$ git rebase master [problem is here, cat README.rd we will get :
hello and git instead of hello world git]

2016-05-13 13:23 GMT+08:00 Pranit Bauva :
> Please mention the version no of git you are using and your system.
> I am answering according to git 2.8.1 Lubuntu 15.04
>
> On Fri, May 13, 2016 at 10:34 AM, 李本超  wrote:
>> Hi all,
>>
>>   Yestoday when I worked using Git, I found a bug. It's about
>> rebase. Or I don't know if it is a bug, maybe that is Git. Below is my
>> problem:
>>
>>   There is a master branch, and we develop in our own branch.
>> Let's simplify this: there are two branches created at the same commit
>> point at master. Then branch A add a function X. Branch B add funciton
>> X too (yes, they are very same). Then branch B modify function X to
>> function Y.
>
> What do you mean by this? Did you amend the previous commit, or
> introduced another separate commit ?
>
>>   Branch A finishes it's job first and merged to master
>> successfully and happily without any conflicts. When branch B wants to
>> merge to master, he finds that master has updated. So branch B must
>> rebase to the current master. Then problem happends: git rebase
>> successfully without any conflicts. But branch B cannot see function X
>> from master (or branch A), only its own function Y.
>>   I think that's because Git is based on file instead of patch.
>> But I think Git can report it in this situation.
>>   How do you think ? Thank you anyway for maintaining this amazing 
>> software.
>
> Well I tried to reproduce the problem. I did the following steps:
> $ mdkir test_repo
> $ cd test_repo
> $ git init
> $ echo Hello >hi
> $ git commit -a -m "C1"
> $ git checkout -b A
> $ echo Bye >hi
> $ git commit -a -m "C2 - A"
> $ git checkout -
> $ git checkout -b B
> $ echo "Bye." >hi
> $ git commit -a -m "C3 - B"
> $ git checkout -
> $ git merge A
> $ git checkout B
> $ git rebase master
>
> This shows that some merge conflicts needs resolving. Did I follow
> your steps or I missed something? It would be better if you could
> reproduce your steps like I did so as to make things more clear to us.
>
> Regards,
> Pranit Bauva



-- 
Benchao Li
School of Electronics Engineering and Computer Science, Peking University
Tel:+86-15650713730
Email: libenc...@gmail.com; libenc...@pku.edu.cn
--
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/4] pathspec: record labels

[Stefan Beller]

On Thu, May 12, 2016 at 10:26 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> +
>> +label:;;
>> + Labels can be assigned to pathspecs in the .gitattributes file.
>> + By specifying a list of labels the pattern will match only
>> + files which have all of the listed labels.
>>  +
>
> Attributes are given to paths, not pathspecs.  You specify which paths
> the entry applies to by matching the pattern (which is at the beginning
> of the line) against each path, and take the matching entries.
>
> In any case, what the first sentence tries to explain applies to
> each and every attribute .gitattributes file can define, and
> explaining it should be better left for the first sentence in the
> DESCRIPTION section.
>
> As to the second sentence, I would say "When specifying the paths to
> work on to various Git commands, the :(label=,...)  magic
> pathspec can be used to select paths that have all the labels given
> by the 'label' attribute.", or something like that, to clarify where
> that "specifying" and "match" happens (they do not happen here, but
> happen when using magic pathspec).

Reminder for myself: add a test for

":(label=a) :(exclude,label=b)"

>
>> +void load_labels(const char *name, int namelen, struct string_list *list)
>
> This must be file scope static, no?

Yes. It seems like I forget these statics often. :(

>
>> +{
>> + static struct git_attr *attr;
>> + struct git_attr_check check;
>> + char *path = xmemdupz(name, namelen);
>> +
>> + if (!attr)
>> + attr = git_attr("label");
>> + check.attr = attr;
>> +
>> + if (git_check_attr(path, 1, ))
>> + die("git_check_attr died");
>> +
>> + if (ATTR_CUSTOM(check.value)) {
>> + string_list_split(list, check.value, ',', -1);
>> + string_list_sort(list);
>> + }
>> +
>> + free(path);
>> +}
>
> I am wondering where we should sanity check (and die, pointing out
> an error in .gitattributes file).  Is this function a good place to
> reject TRUE and FALSE?

Would it make sense to mark a file as

"follows the labeling system, but has no label" (TRUE)
"doesn't follow the labeling system at all" (FALSE)

This may be a useful addition later on to say

"I want to talk only about labeled files, but not label 'a'"

I would think.


>
> By the way, ATTR_CUSTOM() is probably a bad name.  gitattributes.txt
> calls them Set, Unset, Set to a value and Unspecified, and this is
> trying to single out "Set to a value" case.  ATTR_STRING() may be
> more appropriate.

Ok.

>
>> @@ -263,6 +285,15 @@ static int match_pathspec_item(const struct 
>> pathspec_item *item, int prefix,
>>   strncmp(item->match, name - prefix, item->prefix))
>>   return 0;
>>
>> + if (item->group) {
>> + struct string_list has_labels = STRING_LIST_INIT_DUP;
>> + struct string_list_item *si;
>> + load_labels(name, namelen, _labels);
>> + for_each_string_list_item(si, item->group)
>> + if (!string_list_has_string(_labels, si->string))
>> + return 0;
>> + }
>> +
>
> Is this the right place, before we even check if the prefix already
> covered everything?
>
> We are looking at one element in the pathspec here.  If that element
> is known to be a "group", and the path has all the required labels,
> is it correct to fall through?
>
> Ahh, you are making ":(label=...)makefile" to say "paths must
> match the string 'makefile' in some way, and further the paths
> must have all these labels?  Then falling through is correct.

This is how I understood your initial design idea.

:(label=C_code)contrib/

gives all the retired C programs.

> The placement before the "prefix covered" looks still a bit
> strange, but understandable.
>
> Is this code leaking has_labels every time it is called?

It does.

>
> By the way, we should pick one between label and group and stick to
> it, no?  Perhaps call the new field "item->label"?

will do.

>
>>   /* If the match was just the prefix, we matched */
>>   if (!*match)
>>   return MATCHED_RECURSIVELY;
>> diff --git a/pathspec.c b/pathspec.c
>> index 4dff252..c227c25 100644
>> --- a/pathspec.c
>> +++ b/pathspec.c
>> @@ -94,6 +94,7 @@ static void eat_long_magic(struct pathspec_item *item, 
>> const char *elt,
>>  {
>>   int i;
>>   const char *copyfrom = *copyfrom_;
>> + const char *out;
>
> It probably is meant as "output", but it looks more like the "body" or
> the "contents" of the named magic to me.

ok.

>
>>   /* longhand */
>>   const char *nextat;
>>   for (copyfrom = elt + 2;
>> @@ -117,6 +118,20 @@ static void eat_long_magic(struct pathspec_item *item, 
>> const char *elt,
>>   continue;
>>   }
>>
>> + if (skip_prefix(copyfrom, "label:", )) {
>
> This is good, but can you fix 

Re: [PATCH 4/4] pathspec: record labels

[Stefan Beller]

On Thu, May 12, 2016 at 9:32 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> diff --git a/Documentation/glossary-content.txt 
>> b/Documentation/glossary-content.txt
>> index 8ad29e6..a1fc9e0 100644
>> --- a/Documentation/glossary-content.txt
>> +++ b/Documentation/glossary-content.txt
>> @@ -362,6 +362,11 @@ glob;;
>>   For example, "Documentation/{asterisk}.html" matches
>>   "Documentation/git.html" but not "Documentation/ppc/ppc.html"
>>   or "tools/perf/Documentation/perf.html".
>> +
>> +label:;;
>> + Labels can be assigned to pathspecs in the .gitattributes file.
>> + By specifying a list of labels the pattern will match only
>> + files which have all of the listed labels.
>>  +
>>  Two consecutive asterisks ("`**`") in patterns matched against
>>  full pathname may have special meaning:
>
> I think the new text is stuffed in the middle of the "glob;;" entry.

It is. Will fix in a reroll.

Thanks,
Stefan
--
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/4] pathspec: record labels

[Junio C Hamano]

Stefan Beller  writes:

> +
> +label:;;
> + Labels can be assigned to pathspecs in the .gitattributes file.
> + By specifying a list of labels the pattern will match only
> + files which have all of the listed labels.
>  +

Attributes are given to paths, not pathspecs.  You specify which paths
the entry applies to by matching the pattern (which is at the beginning
of the line) against each path, and take the matching entries.

In any case, what the first sentence tries to explain applies to
each and every attribute .gitattributes file can define, and
explaining it should be better left for the first sentence in the
DESCRIPTION section.

As to the second sentence, I would say "When specifying the paths to
work on to various Git commands, the :(label=,...)  magic
pathspec can be used to select paths that have all the labels given
by the 'label' attribute.", or something like that, to clarify where
that "specifying" and "match" happens (they do not happen here, but
happen when using magic pathspec).

> +void load_labels(const char *name, int namelen, struct string_list *list)

This must be file scope static, no?

> +{
> + static struct git_attr *attr;
> + struct git_attr_check check;
> + char *path = xmemdupz(name, namelen);
> +
> + if (!attr)
> + attr = git_attr("label");
> + check.attr = attr;
> +
> + if (git_check_attr(path, 1, ))
> + die("git_check_attr died");
> +
> + if (ATTR_CUSTOM(check.value)) {
> + string_list_split(list, check.value, ',', -1);
> + string_list_sort(list);
> + }
> +
> + free(path);
> +}

I am wondering where we should sanity check (and die, pointing out
an error in .gitattributes file).  Is this function a good place to
reject TRUE and FALSE?

By the way, ATTR_CUSTOM() is probably a bad name.  gitattributes.txt
calls them Set, Unset, Set to a value and Unspecified, and this is
trying to single out "Set to a value" case.  ATTR_STRING() may be
more appropriate.

> @@ -263,6 +285,15 @@ static int match_pathspec_item(const struct 
> pathspec_item *item, int prefix,
>   strncmp(item->match, name - prefix, item->prefix))
>   return 0;
>  
> + if (item->group) {
> + struct string_list has_labels = STRING_LIST_INIT_DUP;
> + struct string_list_item *si;
> + load_labels(name, namelen, _labels);
> + for_each_string_list_item(si, item->group)
> + if (!string_list_has_string(_labels, si->string))
> + return 0;
> + }
> +

Is this the right place, before we even check if the prefix already
covered everything?

We are looking at one element in the pathspec here.  If that element
is known to be a "group", and the path has all the required labels,
is it correct to fall through?

Ahh, you are making ":(label=...)makefile" to say "paths must
match the string 'makefile' in some way, and further the paths
must have all these labels?  Then falling through is correct.
The placement before the "prefix covered" looks still a bit
strange, but understandable.

Is this code leaking has_labels every time it is called?

By the way, we should pick one between label and group and stick to
it, no?  Perhaps call the new field "item->label"?

>   /* If the match was just the prefix, we matched */
>   if (!*match)
>   return MATCHED_RECURSIVELY;
> diff --git a/pathspec.c b/pathspec.c
> index 4dff252..c227c25 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -94,6 +94,7 @@ static void eat_long_magic(struct pathspec_item *item, 
> const char *elt,
>  {
>   int i;
>   const char *copyfrom = *copyfrom_;
> + const char *out;

It probably is meant as "output", but it looks more like the "body" or
the "contents" of the named magic to me.

>   /* longhand */
>   const char *nextat;
>   for (copyfrom = elt + 2;
> @@ -117,6 +118,20 @@ static void eat_long_magic(struct pathspec_item *item, 
> const char *elt,
>   continue;
>   }
>  
> + if (skip_prefix(copyfrom, "label:", )) {

This is good, but can you fix the "prefix:" one we see a few lines
above, too, using this to lose "copyfrom + 7" there?
--
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 report

[Pranit Bauva]

Please mention the version no of git you are using and your system.
I am answering according to git 2.8.1 Lubuntu 15.04

On Fri, May 13, 2016 at 10:34 AM, 李本超  wrote:
> Hi all,
>
>   Yestoday when I worked using Git, I found a bug. It's about
> rebase. Or I don't know if it is a bug, maybe that is Git. Below is my
> problem:
>
>   There is a master branch, and we develop in our own branch.
> Let's simplify this: there are two branches created at the same commit
> point at master. Then branch A add a function X. Branch B add funciton
> X too (yes, they are very same). Then branch B modify function X to
> function Y.

What do you mean by this? Did you amend the previous commit, or
introduced another separate commit ?

>   Branch A finishes it's job first and merged to master
> successfully and happily without any conflicts. When branch B wants to
> merge to master, he finds that master has updated. So branch B must
> rebase to the current master. Then problem happends: git rebase
> successfully without any conflicts. But branch B cannot see function X
> from master (or branch A), only its own function Y.
>   I think that's because Git is based on file instead of patch.
> But I think Git can report it in this situation.
>   How do you think ? Thank you anyway for maintaining this amazing 
> software.

Well I tried to reproduce the problem. I did the following steps:
$ mdkir test_repo
$ cd test_repo
$ git init
$ echo Hello >hi
$ git commit -a -m "C1"
$ git checkout -b A
$ echo Bye >hi
$ git commit -a -m "C2 - A"
$ git checkout -
$ git checkout -b B
$ echo "Bye." >hi
$ git commit -a -m "C3 - B"
$ git checkout -
$ git merge A
$ git checkout B
$ git rebase master

This shows that some merge conflicts needs resolving. Did I follow
your steps or I missed something? It would be better if you could
reproduce your steps like I did so as to make things more clear to us.

Regards,
Pranit Bauva
--
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 report

[李本超]

Hi all,

  Yestoday when I worked using Git, I found a bug. It's about
rebase. Or I don't know if it is a bug, maybe that is Git. Below is my
problem:

  There is a master branch, and we develop in our own branch.
Let's simplify this: there are two branches created at the same commit
point at master. Then branch A add a function X. Branch B add funciton
X too (yes, they are very same). Then branch B modify function X to
function Y.
  Branch A finishes it's job first and merged to master
successfully and happily without any conflicts. When branch B wants to
merge to master, he finds that master has updated. So branch B must
rebase to the current master. Then problem happends: git rebase
successfully without any conflicts. But branch B cannot see function X
from master (or branch A), only its own function Y.
  I think that's because Git is based on file instead of patch.
But I think Git can report it in this situation.
  How do you think ? Thank you anyway for maintaining this amazing software.

-- 

Benchao Li
School of Electronics Engineering and Computer Science, Peking University
Tel:+86-15650713730
Email: libenc...@gmail.com; libenc...@pku.edu.cn
--
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/4] pathspec: move prefix check out of the inner loop

[Junio C Hamano]

Stefan Beller  writes:

> The prefix check is not related the check of pathspec magic; also there
> is no code that is relevant after we'd break the loop on a match for
> "prefix:". So move the check before the loop and shortcircuit the outer
> loop.
>
> Signed-off-by: Stefan Beller 
> ---

What were we thinking back when we added this in the middle of the
loop at 233c3e6c (parse_pathspec: preserve prefix length via
PATHSPEC_PREFIX_ORIGIN, 2013-07-14)?  I am a bit embarrassed.

>  pathspec.c | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/pathspec.c b/pathspec.c
> index eba37c2..4dff252 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -107,21 +107,22 @@ static void eat_long_magic(struct pathspec_item *item, 
> const char *elt,
>   nextat = copyfrom + len;
>   if (!len)
>   continue;
> +
> + if (starts_with(copyfrom, "prefix:")) {
> + char *endptr;
> + *pathspec_prefix = strtol(copyfrom + 7,
> +   , 10);
> + if (endptr - copyfrom != len)
> + die(_("invalid parameter for pathspec magic 
> 'prefix'"));
> + continue;
> + }
> +
>   for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
>   if (strlen(pathspec_magic[i].name) == len &&
>   !strncmp(pathspec_magic[i].name, copyfrom, len)) {
>   *magic |= pathspec_magic[i].bit;
>   break;
>   }
> - if (starts_with(copyfrom, "prefix:")) {
> - char *endptr;
> - *pathspec_prefix = strtol(copyfrom + 7,
> -   , 10);
> - if (endptr - copyfrom != len)
> - die(_("invalid parameter for pathspec 
> magic 'prefix'"));
> - /* "i" would be wrong, but it does not matter */
> - break;
> - }
>   }
>   if (ARRAY_SIZE(pathspec_magic) <= i)
>   die(_("Invalid pathspec magic '%.*s' in '%s'"),
--
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/4] pathspec: record labels

[Junio C Hamano]

Stefan Beller  writes:

> diff --git a/Documentation/glossary-content.txt 
> b/Documentation/glossary-content.txt
> index 8ad29e6..a1fc9e0 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -362,6 +362,11 @@ glob;;
>   For example, "Documentation/{asterisk}.html" matches
>   "Documentation/git.html" but not "Documentation/ppc/ppc.html"
>   or "tools/perf/Documentation/perf.html".
> +
> +label:;;
> + Labels can be assigned to pathspecs in the .gitattributes file.
> + By specifying a list of labels the pattern will match only
> + files which have all of the listed labels.
>  +
>  Two consecutive asterisks ("`**`") in patterns matched against
>  full pathname may have special meaning:

I think the new text is stuffed in the middle of the "glob;;" entry.
--
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] Documentation: fix a typo

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 Documentation/gitattributes.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e3b1de8..af2c682 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -86,7 +86,7 @@ is either not set or empty, $HOME/.config/git/attributes is 
used instead.
 Attributes for all users on a system should be placed in the
 `$(prefix)/etc/gitattributes` file.
 
-Sometimes you would need to override an setting of an attribute
+Sometimes you would need to override a setting of an attribute
 for a path to `Unspecified` state.  This can be done by listing
 the name of the attribute prefixed with an exclamation point `!`.
 
-- 
2.8.2.400.g8bfb85c.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

[RFC PATCH 0/4] pathspec labels [WAS: submodule groups]

[Stefan Beller]

After some fruitful discussion[1] in which Junio suggested trying a very
different route[2] that is more general and not submodule related, I considered
doing a mock for this.

This lets you label arbitrary pathspecs, e.g. in git.git we may want to have:

t/t[0-9]*.sh label=tests

such that

$ git log --author=Beller ":(label=tests)"

would show all commits in which I touched tests.

This has suprisingly few lines of code, as the first 3 patches are refactoring.
The actual new feature is in the last patch.

This would solve the submodule issues I want to solve, as I can produce a
.gitattributes like:

./submodule1 label=default
./submodule2 label=default
./submodule3 label=optional-feature1

and then I'd instruct the users to clone like this:

git clone .. superproject
cd superproject
git submodule update --init :(label:default)

The second part of the submodule series to collapse these three commands
will come as an extra series later, then.

What annoys me here:
Attributes can only be set once at the moment, there is no way to collect all
attributes. If we'd have such a collection feature we would be able to
configure this:

*.[ch] label=C,code
contrib/** label=oldstuff

and run this:

git status ":(label:C oldstuff)"

which would be the equivalent to

contrib/**.[ch]

as in this proposed implementation the labels which are given within one
pathspec item are logical AND. To get the logical OR, you'd have to give 
multiple
pathspec items, i.e. ":(label:C)" ":(label:oldstuff)"

Feedback welcome!

Thanks,
Stefan

[1] http://thread.gmane.org/gmane.comp.version-control.git/294212
[2] http://thread.gmane.org/gmane.comp.version-control.git/294212/focus=294391

Stefan Beller (4):
  Documentation: correct typo in example for querying attributes
  pathspec: move long magic parsing out of prefix_pathspec
  pathspec: move prefix check out of the inner loop
  pathspec: record labels

 Documentation/glossary-content.txt|   5 ++
 Documentation/technical/api-gitattributes.txt |   2 +-
 attr.h|   1 +
 dir.c |  31 
 pathspec.c| 109 +-
 pathspec.h|   1 +
 t/t6134-pathspec-with-labels.sh   |  91 +
 7 files changed, 201 insertions(+), 39 deletions(-)
 create mode 100755 t/t6134-pathspec-with-labels.sh

-- 
2.8.2.400.g66c4903.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 4/4] pathspec: record labels

[Stefan Beller]

Labels were originally designed to manage large amount of
submodules, the discussion steered this in a more general
direction, such that other files can be labeled as well.

Labels are meant to describe arbitrary set of files, which
is not described via the tree layout.

Signed-off-by: Stefan Beller 
---
 Documentation/glossary-content.txt |  5 +++
 attr.h |  1 +
 dir.c  | 31 +
 pathspec.c | 24 +-
 pathspec.h |  1 +
 t/t6134-pathspec-with-labels.sh| 91 ++
 6 files changed, 152 insertions(+), 1 deletion(-)
 create mode 100755 t/t6134-pathspec-with-labels.sh

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 8ad29e6..a1fc9e0 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -362,6 +362,11 @@ glob;;
For example, "Documentation/{asterisk}.html" matches
"Documentation/git.html" but not "Documentation/ppc/ppc.html"
or "tools/perf/Documentation/perf.html".
+
+label:;;
+   Labels can be assigned to pathspecs in the .gitattributes file.
+   By specifying a list of labels the pattern will match only
+   files which have all of the listed labels.
 +
 Two consecutive asterisks ("`**`") in patterns matched against
 full pathname may have special meaning:
diff --git a/attr.h b/attr.h
index 8b08d33..f6fc7c3 100644
--- a/attr.h
+++ b/attr.h
@@ -18,6 +18,7 @@ extern const char git_attr__false[];
 #define ATTR_TRUE(v) ((v) == git_attr__true)
 #define ATTR_FALSE(v) ((v) == git_attr__false)
 #define ATTR_UNSET(v) ((v) == NULL)
+#define ATTR_CUSTOM(v) (!(ATTR_UNSET(v) || ATTR_FALSE(v) || ATTR_TRUE(v)))
 
 /*
  * Send one or more git_attr_check to git_check_attr(), and
diff --git a/dir.c b/dir.c
index 656f272..51d5965 100644
--- a/dir.c
+++ b/dir.c
@@ -9,6 +9,7 @@
  */
 #include "cache.h"
 #include "dir.h"
+#include "attr.h"
 #include "refs.h"
 #include "wildmatch.h"
 #include "pathspec.h"
@@ -208,6 +209,27 @@ int within_depth(const char *name, int namelen,
return 1;
 }
 
+void load_labels(const char *name, int namelen, struct string_list *list)
+{
+   static struct git_attr *attr;
+   struct git_attr_check check;
+   char *path = xmemdupz(name, namelen);
+
+   if (!attr)
+   attr = git_attr("label");
+   check.attr = attr;
+
+   if (git_check_attr(path, 1, ))
+   die("git_check_attr died");
+
+   if (ATTR_CUSTOM(check.value)) {
+   string_list_split(list, check.value, ',', -1);
+   string_list_sort(list);
+   }
+
+   free(path);
+}
+
 #define DO_MATCH_EXCLUDE   1
 #define DO_MATCH_DIRECTORY 2
 
@@ -263,6 +285,15 @@ static int match_pathspec_item(const struct pathspec_item 
*item, int prefix,
strncmp(item->match, name - prefix, item->prefix))
return 0;
 
+   if (item->group) {
+   struct string_list has_labels = STRING_LIST_INIT_DUP;
+   struct string_list_item *si;
+   load_labels(name, namelen, _labels);
+   for_each_string_list_item(si, item->group)
+   if (!string_list_has_string(_labels, si->string))
+   return 0;
+   }
+
/* If the match was just the prefix, we matched */
if (!*match)
return MATCHED_RECURSIVELY;
diff --git a/pathspec.c b/pathspec.c
index 4dff252..c227c25 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -94,6 +94,7 @@ static void eat_long_magic(struct pathspec_item *item, const 
char *elt,
 {
int i;
const char *copyfrom = *copyfrom_;
+   const char *out;
/* longhand */
const char *nextat;
for (copyfrom = elt + 2;
@@ -117,6 +118,20 @@ static void eat_long_magic(struct pathspec_item *item, 
const char *elt,
continue;
}
 
+   if (skip_prefix(copyfrom, "label:", )) {
+   struct strbuf sb = STRBUF_INIT;
+   size_t l = nextat - out;
+   strbuf_add(, out, l);
+   if (!item->group) {
+   item->group = xmalloc(sizeof(*item->group));
+   string_list_init(item->group, 1);
+   }
+   string_list_split(item->group, sb.buf, ' ', -1);
+   string_list_remove_empty_items(item->group, 0);
+   strbuf_release();
+   continue;
+   }
+
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
if (strlen(pathspec_magic[i].name) == len &&
!strncmp(pathspec_magic[i].name, copyfrom, len)) {
@@ -425,7 +440,7 @@ void parse_pathspec(struct pathspec *pathspec,
for (i = 0; i < n; i++) {
  

[PATCH 1/4] Documentation: correct typo in example for querying attributes

[Stefan Beller]

The introductory text of the example has a typo in the
specification which makes it harder to follow that example.

Signed-off-by: Stefan Beller 
---
 Documentation/technical/api-gitattributes.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/technical/api-gitattributes.txt 
b/Documentation/technical/api-gitattributes.txt
index 2602668..36fc9af 100644
--- a/Documentation/technical/api-gitattributes.txt
+++ b/Documentation/technical/api-gitattributes.txt
@@ -61,7 +61,7 @@ Querying Specific Attributes
 Example
 ---
 
-To see how attributes "crlf" and "indent" are set for different paths.
+To see how attributes "crlf" and "ident" are set for different paths.
 
 . Prepare an array of `struct git_attr_check` with two elements (because
   we are checking two attributes).  Initialize their `attr` member with
-- 
2.8.2.400.g66c4903.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 3/4] pathspec: move prefix check out of the inner loop

[Stefan Beller]

The prefix check is not related the check of pathspec magic; also there
is no code that is relevant after we'd break the loop on a match for
"prefix:". So move the check before the loop and shortcircuit the outer
loop.

Signed-off-by: Stefan Beller 
---
 pathspec.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index eba37c2..4dff252 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -107,21 +107,22 @@ static void eat_long_magic(struct pathspec_item *item, 
const char *elt,
nextat = copyfrom + len;
if (!len)
continue;
+
+   if (starts_with(copyfrom, "prefix:")) {
+   char *endptr;
+   *pathspec_prefix = strtol(copyfrom + 7,
+ , 10);
+   if (endptr - copyfrom != len)
+   die(_("invalid parameter for pathspec magic 
'prefix'"));
+   continue;
+   }
+
for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
if (strlen(pathspec_magic[i].name) == len &&
!strncmp(pathspec_magic[i].name, copyfrom, len)) {
*magic |= pathspec_magic[i].bit;
break;
}
-   if (starts_with(copyfrom, "prefix:")) {
-   char *endptr;
-   *pathspec_prefix = strtol(copyfrom + 7,
- , 10);
-   if (endptr - copyfrom != len)
-   die(_("invalid parameter for pathspec 
magic 'prefix'"));
-   /* "i" would be wrong, but it does not matter */
-   break;
-   }
}
if (ARRAY_SIZE(pathspec_magic) <= i)
die(_("Invalid pathspec magic '%.*s' in '%s'"),
-- 
2.8.2.400.g66c4903.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/4] pathspec: move long magic parsing out of prefix_pathspec

[Stefan Beller]

`prefix_pathspec` is quite a lengthy function and we plan on adding more.
Split it up for better readability. As we want to add code into the
inner loop of the long magic parsing, we also benefit from lower
indentation.

Signed-off-by: Stefan Beller 
---
 pathspec.c | 84 +++---
 1 file changed, 47 insertions(+), 37 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index c9e9b6c..eba37c2 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -88,6 +88,52 @@ static void prefix_short_magic(struct strbuf *sb, int 
prefixlen,
strbuf_addf(sb, ",prefix:%d)", prefixlen);
 }
 
+static void eat_long_magic(struct pathspec_item *item, const char *elt,
+   unsigned *magic, int *pathspec_prefix,
+   const char **copyfrom_, const char **long_magic_end)
+{
+   int i;
+   const char *copyfrom = *copyfrom_;
+   /* longhand */
+   const char *nextat;
+   for (copyfrom = elt + 2;
+*copyfrom && *copyfrom != ')';
+copyfrom = nextat) {
+   size_t len = strcspn(copyfrom, ",)");
+   if (copyfrom[len] == ',')
+   nextat = copyfrom + len + 1;
+   else
+   /* handle ')' and '\0' */
+   nextat = copyfrom + len;
+   if (!len)
+   continue;
+   for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+   if (strlen(pathspec_magic[i].name) == len &&
+   !strncmp(pathspec_magic[i].name, copyfrom, len)) {
+   *magic |= pathspec_magic[i].bit;
+   break;
+   }
+   if (starts_with(copyfrom, "prefix:")) {
+   char *endptr;
+   *pathspec_prefix = strtol(copyfrom + 7,
+ , 10);
+   if (endptr - copyfrom != len)
+   die(_("invalid parameter for pathspec 
magic 'prefix'"));
+   /* "i" would be wrong, but it does not matter */
+   break;
+   }
+   }
+   if (ARRAY_SIZE(pathspec_magic) <= i)
+   die(_("Invalid pathspec magic '%.*s' in '%s'"),
+   (int) len, copyfrom, elt);
+   }
+   if (*copyfrom != ')')
+   die(_("Missing ')' at the end of pathspec magic in '%s'"), elt);
+   *long_magic_end = copyfrom;
+   copyfrom++;
+   *copyfrom_ = copyfrom;
+}
+
 /*
  * Take an element of a pathspec and check for magic signatures.
  * Append the result to the prefix. Return the magic bitmap.
@@ -150,43 +196,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
(flags & PATHSPEC_LITERAL_PATH)) {
; /* nothing to do */
} else if (elt[1] == '(') {
-   /* longhand */
-   const char *nextat;
-   for (copyfrom = elt + 2;
-*copyfrom && *copyfrom != ')';
-copyfrom = nextat) {
-   size_t len = strcspn(copyfrom, ",)");
-   if (copyfrom[len] == ',')
-   nextat = copyfrom + len + 1;
-   else
-   /* handle ')' and '\0' */
-   nextat = copyfrom + len;
-   if (!len)
-   continue;
-   for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
-   if (strlen(pathspec_magic[i].name) == len &&
-   !strncmp(pathspec_magic[i].name, copyfrom, 
len)) {
-   magic |= pathspec_magic[i].bit;
-   break;
-   }
-   if (starts_with(copyfrom, "prefix:")) {
-   char *endptr;
-   pathspec_prefix = strtol(copyfrom + 7,
-, 10);
-   if (endptr - copyfrom != len)
-   die(_("invalid parameter for 
pathspec magic 'prefix'"));
-   /* "i" would be wrong, but it does not 
matter */
-   break;
-   }
-   }
-   if (ARRAY_SIZE(pathspec_magic) <= i)
-   die(_("Invalid pathspec magic '%.*s' in '%s'"),
-   (int) len, copyfrom, elt);
-   }
-   if (*copyfrom != ')')
-   die(_("Missing ')' at the end of 

Re: [PATCH] i18n: unpack-trees: avoid substituting only a verb in sentences

[Junio C Hamano]

Junio C Hamano  writes:

> Thanks.  I think we should squash the following in, as all these
> messages are now i18ned and without being marked with test_i18ncmp,
> GETTEXT_POISON build would fail to pass these tests.

... this was not a request for you to re-send your patch with the
update to the test.  I'll just squash it in.

Thanks for the i18n updates.
--
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] i18n: unpack-trees: avoid substituting only a verb in sentences

[Junio C Hamano]

Vasco Almeida  writes:

> Instead of reusing the same set of message templates for checkout
> and other actions and substituting the verb with "%s", prepare
> separate message templates for each known action. That would make
> it easier for translation into languages where the same verb may
> conjugate differently depending on the message we are giving.
>
> See gettext documentation for details:
> http://www.gnu.org/software/gettext/manual/html_node/Preparing-Strings.html
>
> Helped-by: Junio C Hamano 
> Signed-off-by: Vasco Almeida 
> ---
>
> I removed "Please," in favor of "Please" as Junio C Hamano sugested.

Thanks.  I think we should squash the following in, as all these
messages are now i18ned and without being marked with test_i18ncmp,
GETTEXT_POISON build would fail to pass these tests.

 t/t7609-merge-co-error-msgs.sh | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/t/t7609-merge-co-error-msgs.sh b/t/t7609-merge-co-error-msgs.sh
index 0e4a682..6729cb3 100755
--- a/t/t7609-merge-co-error-msgs.sh
+++ b/t/t7609-merge-co-error-msgs.sh
@@ -37,14 +37,14 @@ EOF
 
 test_expect_success 'untracked files overwritten by merge (fast and non-fast 
forward)' '
test_must_fail git merge branch 2>out &&
-   test_cmp out expect &&
+   test_i18ncmp out expect &&
git commit --allow-empty -m empty &&
(
GIT_MERGE_VERBOSITY=0 &&
export GIT_MERGE_VERBOSITY &&
test_must_fail git merge branch 2>out2
) &&
-   test_cmp out2 expect &&
+   test_i18ncmp out2 expect &&
git reset --hard HEAD^
 '
 
@@ -53,7 +53,7 @@ error: Your local changes to the following files would be 
overwritten by merge:
four
three
two
-Please, commit your changes or stash them before you can merge.
+Please commit your changes or stash them before you can merge.
 error: The following untracked working tree files would be overwritten by 
merge:
five
 Please move or remove them before you can merge.
@@ -65,14 +65,14 @@ test_expect_success 'untracked files or local changes 
ovewritten by merge' '
git add three &&
git add four &&
test_must_fail git merge branch 2>out &&
-   test_cmp out expect
+   test_i18ncmp out expect
 '
 
 cat >expect <<\EOF
 error: Your local changes to the following files would be overwritten by 
checkout:
rep/one
rep/two
-Please, commit your changes or stash them before you can switch branches.
+Please commit your changes or stash them before you can switch branches.
 Aborting
 EOF
 
@@ -87,21 +87,21 @@ test_expect_success 'cannot switch branches because of 
local changes' '
echo uno >rep/one &&
echo dos >rep/two &&
test_must_fail git checkout branch 2>out &&
-   test_cmp out expect
+   test_i18ncmp out expect
 '
 
 cat >expect <<\EOF
 error: Your local changes to the following files would be overwritten by 
checkout:
rep/one
rep/two
-Please, commit your changes or stash them before you can switch branches.
+Please commit your changes or stash them before you can switch branches.
 Aborting
 EOF
 
 test_expect_success 'not uptodate file porcelain checkout error' '
git add rep/one rep/two &&
test_must_fail git checkout branch 2>out &&
-   test_cmp out expect
+   test_i18ncmp out expect
 '
 
 cat >expect <<\EOF
@@ -132,7 +132,7 @@ test_expect_success 'not_uptodate_dir porcelain checkout 
error' '
>rep/untracked-file &&
>rep2/untracked-file &&
test_must_fail git checkout branch 2>out &&
-   test_cmp out ../expect
+   test_i18ncmp out ../expect
 '
 
 test_done
--
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] i18n: unpack-trees: avoid substituting only a verb in sentences

[Vasco Almeida]

Instead of reusing the same set of message templates for checkout
and other actions and substituting the verb with "%s", prepare
separate message templates for each known action. That would make
it easier for translation into languages where the same verb may
conjugate differently depending on the message we are giving.

See gettext documentation for details:
http://www.gnu.org/software/gettext/manual/html_node/Preparing-Strings.html

Helped-by: Junio C Hamano 
Signed-off-by: Vasco Almeida 
---

I removed "Please," in favor of "Please" as Junio C Hamano sugested.

 unpack-trees.c | 60 +-
 1 file changed, 47 insertions(+), 13 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 630a8cf..6bc9512 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -58,27 +58,61 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
int i;
const char **msgs = opts->msgs;
const char *msg;
-   const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
 
-   if (advice_commit_before_merge)
-   msg = _("Your local changes to the following files would be 
overwritten by %s:\n%%s"
-   "Please, commit your changes or stash them before you 
can %s.");
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by checkout:\n%%s"
+ "Please commit your changes or stash them before you 
can switch branches.")
+ : _("Your local changes to the following files would be 
overwritten by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by merge:\n%%s"
+ "Please commit your changes or stash them before you 
can merge.")
+ : _("Your local changes to the following files would be 
overwritten by merge:\n%%s");
else
-   msg = _("Your local changes to the following files would be 
overwritten by %s:\n%%s");
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by %s:\n%%s"
+ "Please commit your changes or stash them before you 
can %s.")
+ : _("Your local changes to the following files would be 
overwritten by %s:\n%%s");
msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] =
-   xstrfmt(msg, cmd, cmd2);
+   xstrfmt(msg, cmd, cmd);
 
msgs[ERROR_NOT_UPTODATE_DIR] =
_("Updating the following directories would lose untracked 
files in it:\n%s");
 
-   if (advice_commit_before_merge)
-   msg = _("The following untracked working tree files would be %s 
by %s:\n%%s"
-   "Please move or remove them before you can %s.");
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by checkout:\n%%s"
+ "Please move or remove them before you can switch 
branches.")
+ : _("The following untracked working tree files would be 
removed by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by merge:\n%%s"
+ "Please move or remove them before you can merge.")
+ : _("The following untracked working tree files would be 
removed by merge:\n%%s");
else
-   msg = _("The following untracked working tree files would be %s 
by %s:\n%%s");
-
-   msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, "removed", cmd, 
cmd2);
-   msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = xstrfmt(msg, 
"overwritten", cmd, cmd2);
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by %s:\n%%s"
+ "Please move or remove them before you can %s.")
+ : _("The following untracked working tree files would be 
removed by %s:\n%%s");
+   msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, cmd, cmd);
+
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
overwritten by checkout:\n%%s"
+ "Please move or remove them before you can switch 
branches.")
+ : _("The following untracked working tree files would be 
overwritten by checkout:\n%%s");
+   

Re: [PATCH v10 11/20] index-helper: use watchman to avoid refreshing index with lstat()

[Ramsay Jones]

On 12/05/16 21:20, David Turner wrote:
> From: Nguyễn Thái Ngọc Duy 
[snip]

>  
> +/* in ms */
> +#define WATCHMAN_TIMEOUT 1000
> +
> +static int poke_and_wait_for_reply(int fd)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + int ret = -1;
> + struct pollfd pollfd;
> + int bytes_read;
> + char reply_buf[4096];
> + const char *requested_capabilities = "";
> +
> +#ifdef USE_WATCHMAN
> + requested_capabilities = "watchman";
> +#endif
> +
> + if (fd < 0)
> + return -1;
> +
> + strbuf_addf(, "poke %d %s", getpid(), requested_capabilities);
> + if (packet_write_gently(fd, buf.buf, buf.len))

This is not causing a problem or bug, but is none the less not
correct - as you know, packet_write_gently() takes a 'printf' like
variable argument list. (So, buf.buf is the format specifier and
buf.len is an unused arg).

I think I would write this as:

strbuf_addf(, "poke %d", getpid());
if (requested_capabilities && *requested_capabilities)
strbuf_addf(, " %s", requested_capabilities);
if (packet_write_gently(fd, "%s", buf.buf))

... or something similar. [Note, just typing into my email client, so
it's not been close to a compiler.]

> + return -1;
> + if (packet_flush_gently(fd))
> + return -1;

Why are you sending a flush packet - doesn't the index-helper
simply ignore it?

I haven't tried this yet BTW, just reading patches as they float
on past... ;-)

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 v4 2/7] i18n: unpack-trees: mark strings for translation

[Junio C Hamano]

I think this patch is better than what is already in 'next', so let
me see if I can make it into an incremental update.

We'd need your sign-off, of course.

-- >8 --
Subject: i18n: unpack-trees: avoid substituting only a verb in sentences

Instead of reusing the same set of message templates for checkout
and other actions and substituting the verb with "%s", prepare
separate message templates for each known action.  That would make
it easier for translation into languages where the same verb may
conjugate differently depending on the message we are giving.

---

diff --git a/unpack-trees.c b/unpack-trees.c
index 4bc6b4f..edb1ee5 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -58,27 +58,61 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
int i;
const char **msgs = opts->msgs;
const char *msg;
-   const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
 
-   if (advice_commit_before_merge)
-   msg = _("Your local changes to the following files would be 
overwritten by %s:\n%%s"
-   "Please, commit your changes or stash them before you 
can %s.");
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by checkout:\n%%s"
+ "Please, commit your changes or stash them before you 
can switch branches.")
+ : _("Your local changes to the following files would be 
overwritten by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by merge:\n%%s"
+ "Please, commit your changes or stash them before you 
can merge.")
+ : _("Your local changes to the following files would be 
overwritten by merge:\n%%s");
else
-   msg = _("Your local changes to the following files would be 
overwritten by %s:\n%%s");
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by %s:\n%%s"
+ "Please, commit your changes or stash them before you 
can %s.")
+ : _("Your local changes to the following files would be 
overwritten by %s:\n%%s");
msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] =
-   xstrfmt(msg, cmd, cmd2);
+   xstrfmt(msg, cmd, cmd);
 
msgs[ERROR_NOT_UPTODATE_DIR] =
_("Updating the following directories would lose untracked 
files in it:\n%s");
 
-   if (advice_commit_before_merge)
-   msg = _("The following untracked working tree files would be %s 
by %s:\n%%s"
-   "Please move or remove them before you can %s.");
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by checkout:\n%%s"
+ "Please move or remove them before you can switch 
branches.")
+ : _("The following untracked working tree files would be 
removed by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by merge:\n%%s"
+ "Please move or remove them before you can merge.")
+ : _("The following untracked working tree files would be 
removed by merge:\n%%s");
else
-   msg = _("The following untracked working tree files would be %s 
by %s:\n%%s");
-
-   msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, "removed", cmd, 
cmd2);
-   msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = xstrfmt(msg, 
"overwritten", cmd, cmd2);
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by %s:\n%%s"
+ "Please move or remove them before you can %s.")
+ : _("The following untracked working tree files would be 
removed by %s:\n%%s");
+   msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, cmd, cmd);
+
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
overwritten by checkout:\n%%s"
+ "Please move or remove them before you can switch 
branches.")
+ : _("The following untracked working tree files would be 
overwritten by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
overwritten by 

Re: [PATCH v6 2/3] bisect: rewrite `check_term_format` shell function in C

[Junio C Hamano]

Pranit Bauva  writes:

> + /*
> +  * In theory, nothing prevents swapping completely good and bad,
> +  * but this situation could be confusing and hasn't been tested
> +  * enough. Forbid it for now.
> +  */
> +
> + if ((strcmp(orig_term, "bad") && one_of(term, "bad", "new", NULL)) ||
> +  (strcmp(orig_term, "good") && one_of(term, "good", "old", 
> NULL)))
> + return error(_("can't change the meaning of the term '%s'"), 
> term);

The above comes from below

> - bad|new)
> - if test "$2" != bad
> - then
> - die "$(eval_gettext "can't change the meaning ...

So it is not your fault, but it is quite hard to understand.

The original says "You are trying to use 'bad' (or 'new') for
something that is not 'bad'--which is confusing; do not do it".

I _think_ the reason I find C version harder to read is the use of
strcmp(orig_term, "bad") to say "orig_term is not BAD".  The shell
version visually tells with "!=" that the meaning of the new term is
*NOT* "bad", and does not ahve such a problem.

Perhaps if we rewrote it to

if ((!strcmp(orig_term, "good") &&
 one_of(term, "bad", "new", NULL)) ||
 (!strcmp(orig_term, "bad") &&
 one_of(term, "good", "old", NULL)))

i.e. "If you are using "bad" or "new" to mean "good", that is not a
good idea", as a follow-up change after the dust settles, the result
might be easier to read.

But this is just commenting for future reference, not suggesting to
update this patch at all.  If we were to do the change, that must
come as a separate step.

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: [Bug?] log -p -W showing the whole file for a patch that adds to the end?

[René Scharfe]

Am 11.05.2016 um 00:51 schrieb Junio C Hamano:

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.


It's a more conservative fallback in this case, but it regresses e.g. if 
you have a long list of includes at the top of a C file -- it won't show 
the full list anymore with -W.


The problem is that we are trying to access lines in the preimage that 
are only actually in the postimage.  Simply switching to useing the 
postimage unconditionally is not enough -- we'd have the same problem 
with the complementary case of lines at the end being removed.


I wonder if using the postimage as a fallback suffices.  Need to look at 
this more closely.


René
--
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] Documentation: correct typo in example

[Junio C Hamano]

Stefan Beller  writes:

> The example want's to explain how 'To see how attributes "crlf" and
> "indent" are set for different paths.'
>
> Spell the selected attribute correctly.
>
> Signed-off-by: Stefan Beller 
> ---
>  Documentation/technical/api-gitattributes.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/api-gitattributes.txt 
> b/Documentation/technical/api-gitattributes.txt
> index 2602668..0d7e98c 100644
> --- a/Documentation/technical/api-gitattributes.txt
> +++ b/Documentation/technical/api-gitattributes.txt
> @@ -74,7 +74,7 @@ static void setup_check(void)
>   if (check[0].attr)
>   return; /* already done */
>   check[0].attr = git_attr("crlf");
> - check[1].attr = git_attr("ident");
> + check[1].attr = git_attr("indent");
>  }
>  

Technically the text and code after this patch is more "correct".

Back when I wrote the original, 530e741c (Start preparing the API
documents., 2007-11-24), however, I did mean to use "ident", iow,
the introductory text has typo, and the code sample is correct.
There wasn't any "indent" attribute that is natively recognised by
Git back then for git_attr("indent") would have made sense, either.

So perhaps update the introductory text instead?
--
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

git svn clone cannot go beyond a specific rev on codeplex

[Hin-Tak Leung]

I tried bin-wrappers/ from current git HEAD.

$ git describe
v2.8.2-396-g5fe494c

bin-wrappers/git svn clone https://ironpython.svn.codeplex.com/svn 
ironpython-old-codeplex

always fails at this rev:

M   Src/Tests/test_re.py
r7605 = e581bc66eda2b86bf46681191034844c4ba7d7a5 (refs/remotes/git-svn)
Connection reset by peer: Error running context: Connection reset by peer at 
/home/Nobak-Hin-Tak/tmp-git/git/perl/blib/lib/Git/SVN/Ra.pm line 312.

I am sure there are later revs, as the web front end says so, but git svn 
clone/fetch does not seem to be able to get at it.

(vger.kernel did not seem to like my posts a while ago - I hope situation has 
improved and this can go through...)
--
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] Documentation: correct typo in example

[Stefan Beller]

The example want's to explain how 'To see how attributes "crlf" and
"indent" are set for different paths.'

Spell the selected attribute correctly.

Signed-off-by: Stefan Beller 
---
 Documentation/technical/api-gitattributes.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/technical/api-gitattributes.txt 
b/Documentation/technical/api-gitattributes.txt
index 2602668..0d7e98c 100644
--- a/Documentation/technical/api-gitattributes.txt
+++ b/Documentation/technical/api-gitattributes.txt
@@ -74,7 +74,7 @@ static void setup_check(void)
if (check[0].attr)
return; /* already done */
check[0].attr = git_attr("crlf");
-   check[1].attr = git_attr("ident");
+   check[1].attr = git_attr("indent");
 }
 
 
-- 
2.8.2.397.g23dc3dd.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: [PATCH v2 22/94] builtin/apply: move 'threeway' global into 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

> Ok, I will try to group knobs like that, but the comments tend to
> break the groups.

By keeping the comment on a single field short, and reserve comment
occupying its own line(s) to comment on group, you can do

/* These control what gets looked at and modified */
int apply;  /* this is not a dry-run */
int check_index; /* preimage must match the indexed version */
int cached; /* apply to the index only */

/* These control cosmetic aspect of the output */
int diffstat; /* show diffstat */
int numstat; /* numerical stat */
int summary; /* report creation, deletion, 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: proposal for extending smudge/clean filters with raw file access

[Joey Hess]

Junio C Hamano wrote:
> > Secondly, and harder to get around, the filename passed to the clean
> > filter is not necessarily a path to the actual existing file that is
> > being cleaned.
> 
> Either one of us is confused.  I was talking about updating the
> current "clean" implementation without changing its interface,
> i.e. gets fed via its standard input, expected to respond to its
> standard output.  There is no filename involved.

I'm talking about the %f that can be passed to the clean filter.
The context that I left out is that my clean filter could avoid reading
all of stdin, and quickly produce the cleaned result, but only if it
can examine the file that's being cleaned. Which is not currently
entirely safe to use the %f for.

There may be a way to make a clean filter that can do something useful
without reading all of stdin, and without examining the file that's
being cleaned. Maybe. Hard to see how. I don't feel such a hypothetical
clean filter is worth changing the current EPIPE behavior to support.

So I think it's better to add a separate clean-from-fs and keep the
current clean filter interface as it stands.

-- 
see shy jo


signature.asc
Description: PGP signature

Re: [PATCH v10 03/20] pkt-line: add gentle version of packet_write

[Junio C Hamano]

David Turner  writes:

> packet_write calls write_or_die, which dies with a sigpipe even if
> calling code has explicitly blocked that signal.
>
> Add packet_write_gently and packet_flush_gently, which don't.  Soon,
> we will use this for communication with git index-helper, which, being
> merely an optimization, should be permitted to die without disrupting
> clients.
>
> Signed-off-by: David Turner 
> ---

Looks quite sensible.  Thanks.

>  pkt-line.c | 18 ++
>  pkt-line.h |  2 ++
>  2 files changed, 20 insertions(+)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 62fdb37..f964446 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -91,6 +91,12 @@ void packet_flush(int fd)
>   write_or_die(fd, "", 4);
>  }
>  
> +int packet_flush_gently(int fd)
> +{
> + packet_trace("", 4, 1);
> + return write_in_full(fd, "", 4) != 4;
> +}
> +
>  void packet_buf_flush(struct strbuf *buf)
>  {
>   packet_trace("", 4, 1);
> @@ -130,6 +136,18 @@ void packet_write(int fd, const char *fmt, ...)
>   write_or_die(fd, buf.buf, buf.len);
>  }
>  
> +int packet_write_gently(int fd, const char *fmt, ...)
> +{
> + static struct strbuf buf = STRBUF_INIT;
> + va_list args;
> +
> + strbuf_reset();
> + va_start(args, fmt);
> + format_packet(, fmt, args);
> + va_end(args);
> + return write_in_full(fd, buf.buf, buf.len) != buf.len;
> +}
> +
>  void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
>  {
>   va_list args;
> diff --git a/pkt-line.h b/pkt-line.h
> index 3cb9d91..deffcb5 100644
> --- a/pkt-line.h
> +++ b/pkt-line.h
> @@ -20,7 +20,9 @@
>   * side can't, we stay with pure read/write interfaces.
>   */
>  void packet_flush(int fd);
> +int packet_flush_gently(int fd);
>  void packet_write(int fd, const char *fmt, ...) __attribute__((format 
> (printf, 2, 3)));
> +int packet_write_gently(int fd, const char *fmt, ...) __attribute__((format 
> (printf, 2, 3)));
>  void packet_buf_flush(struct strbuf *buf);
>  void packet_buf_write(struct strbuf *buf, const char *fmt, ...) 
> __attribute__((format (printf, 2, 3)));
--
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 0/7] i18n miscellaneous updates

[Junio C Hamano]

Vasco Almeida  writes:

> This re-roll upadates patch
>   i18n: unpack-trees: mark strings for translation

Sorry, unfortunately you cannot take them back anymore.  These from
the previous round are already in 'next'.

daf9f64 i18n: builtin/pull.c: split strings marked for translation
8a0de58 i18n: builtin/pull.c: mark placeholders for translation
045fac5 i18n: git-parse-remote.sh: mark strings for translation
60ea78b i18n: branch: move comment for translators
2010aab i18n: branch: unmark string for translation
8ae51c4 i18n: builtin/rm.c: remove a comma ',' from string
ed47fdf i18n: unpack-trees: mark strings for translation
ab86885 i18n: builtin/branch.c: mark option for translation
71d99b8 i18n: index-pack: use plural string instead of normal one

I think the update to unpack-trees.c (v4 2/7) makes more sense than
what was in ed47fdf, so perhaps you can send it in as an incremental
update (and others changes made since the commits listed above)?

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 6/7] i18n: builtin/rm.c: remove a comma ',' from string

[Junio C Hamano]

Vasco Almeida  writes:

> Remove a comma from string marked for translation. Make the string match the
> one in builtin/mv.c. Now translators have do handle this string only once.
>
> Signed-off-by: Vasco Almeida 
> ---

Looks good.  BTW, I think you just added two more "Please," in
"i18n: unpack-trees" patch by repeating a set of very similar
messages.

>  builtin/rm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/rm.c b/builtin/rm.c
> index 8829b09..be83c43 100644
> --- a/builtin/rm.c
> +++ b/builtin/rm.c
> @@ -314,7 +314,7 @@ int cmd_rm(int argc, const char **argv, const char 
> *prefix)
>   list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
>   if (list.entry[list.nr++].is_submodule &&
>   !is_staging_gitmodules_ok())
> - die (_("Please, stage your changes to .gitmodules or 
> stash them to proceed"));
> + die (_("Please stage your changes to .gitmodules or 
> stash them to proceed"));
>   }
>  
>   if (pathspec.nr) {
--
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: proposal for extending smudge/clean filters with raw file access

[Joey Hess]

Junio C Hamano wrote:
> The smudge happens to be the last to run, so it is quite true that
> it can say "Hey Git, I've written it out already".
> 
> I didn't check all codepaths to ensure that we won't need the
> smudged result in core at all, but I am guessing you did so before
> making this proposal, so in that case I would say this sounds fine.

Well, the idea is to only use smudge-to-file when the smudged content is
going to be written out to a file. Any other code paths that need to
smudge some content would use the smudge filter.

So, try_create_file would use it. Maybe some other places I have not
found yet could as well.

-- 
see shy jo


signature.asc
Description: PGP signature

Re: proposal for extending smudge/clean filters with raw file access

[Junio C Hamano]

Joey Hess  writes:

> Junio C Hamano wrote:
>> This side, I do not think we even need a new variant.  We can just
>> update the code to interact with "clean" so that it the writer to
>> the pipe ignores SIGPIPE, detects EPIPE on write(2), says "ah, the
>> other end does not need the full input to produce its output".  The
>> reader from the pipe consumes its output without failing AS LONG AS
>> the "clean" filter exits with zero (we do check its exit status,
>> right?)
>
> There are two problems with doing that. First, any clean filter that
> relied on that would not work with older versions of git.

That's a fair point.

> Secondly, and harder to get around, the filename passed to the clean
> filter is not necessarily a path to the actual existing file that is
> being cleaned.

Either one of us is confused.  I was talking about updating the
current "clean" implementation without changing its interface,
i.e. gets fed via its standard input, expected to respond to its
standard output.  There is no filename involved.

And yes, "clean-from-fs" that is spawned by us with the name of the
file in the filesystem that has the contents as its argument, must
be prepared to see a filename that does not have any relation to the
filename in the working tree.
--
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: proposal for extending smudge/clean filters with raw file access

[Joey Hess]

Junio C Hamano wrote:
> This side, I do not think we even need a new variant.  We can just
> update the code to interact with "clean" so that it the writer to
> the pipe ignores SIGPIPE, detects EPIPE on write(2), says "ah, the
> other end does not need the full input to produce its output".  The
> reader from the pipe consumes its output without failing AS LONG AS
> the "clean" filter exits with zero (we do check its exit status,
> right?)

There are two problems with doing that. First, any clean filter that
relied on that would not work with older versions of git.

Secondly, and harder to get around, the filename passed to the clean
filter is not necessarily a path to the actual existing file that is
being cleaned. For example, git hash-object --stdin --path=whatever.
So the current clean filter can't really safely rely on accessing the
file to short-circuit its cleaning. (Although it seems to mostly work..
currently..)

> We cannot do a similar "we can just unconditionally update" like I
> said on the "clean" side to "smudge", so it would need a new
> variant.  I do not want to call it anything "raw", as there is
> nothing "raw" about it.  "smudge-to-fs" would be a better name.

"raw" was just a placeholder. "clean-from-fs" and "smudge-to-fs" are
pretty descriptive.

-- 
see shy jo


signature.asc
Description: PGP signature

Re: [PATCH v2 48/94] builtin/apply: rename 'prefix_' parameter to 'prefix'

[Junio C Hamano]

Junio C Hamano  writes:

> Up to this point, the conversion looks quite sensible, even though I
> think the organization of fields in apply_state do not look logical.

I'd stop here for now, as everything before this step looks
uncontroversial.  Anybody whose tasked to move the global state for
these variables into a structure would reach the samestate after
applying these 48 patches, modulo minor differences in the way the
comments would say things, how the patches are split and how the
fields in apply_state() are organized.

One thing that is missing is a counterpart of init_apply_state().
In the early part of patches where it added only "const char *"
borrowed from the caller and fields of intregral type, the lack of
clear_apply_state() did not mattter, but with a few fields with
"string_list" type, anybody who want to make repeated call into the
apply machinery would want a way to release the resource the
structure holds.

Because 49/94 is a step to add an unfreeable resource, this is a
good place to stop and then add the clean_apply_state() before that
happens, I would think.  After that, I think both the contributor
and the reviewers would benefit if these early patches are merged
early without waiting for the review of the remainder.

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

[PATCH v10 16/20] index-helper: don't run if already running

[David Turner]

Signed-off-by: David Turner 
---
 index-helper.c  | 7 +++
 t/t7900-index-helper.sh | 9 +
 2 files changed, 16 insertions(+)

diff --git a/index-helper.c b/index-helper.c
index a1b33e4..7b893a0 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -435,6 +435,13 @@ int main(int argc, char **argv)
return 0;
}
 
+   /* check that no other copy is running */
+   fd = unix_stream_connect(git_path("index-helper.sock"));
+   if (fd > 0)
+   die(_("Already running"));
+   if (errno != ECONNREFUSED && errno != ENOENT)
+   die_errno(_("Unexpected error checking socket"));
+
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
 
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index e71b5af..7159971 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -29,4 +29,13 @@ test_expect_success 'index-helper creates usable path file 
and can be killed' '
test_path_is_missing .git/index-helper.sock
 '
 
+test_expect_success 'index-helper will not start if already running' '
+   test_when_finished "git index-helper --kill" &&
+   git index-helper --detach &&
+   test -S .git/index-helper.sock &&
+   test_must_fail git index-helper 2>err &&
+   test -S .git/index-helper.sock &&
+   grep "Already running" err
+'
+
 test_done
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 04/20] index-helper: new daemon for caching index and related stuff

[David Turner]

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

Instead of reading the index from disk and worrying about disk
corruption, the index is cached in memory (memory bit-flips happen
too, but hopefully less often). The result is faster read. Read time
is reduced by 70%.

The biggest gain is not having to verify the trailing SHA-1, which
takes lots of time especially on large index files. But this also
opens doors for further optimizations:

 - we could create an in-memory format that's essentially the memory
   dump of the index to eliminate most of parsing/allocation
   overhead. The mmap'd memory can be used straight away. Experiment
   [1] shows we could reduce read time by 88%.

 - we could cache non-index info such as name hash

Shared memory is done by storing files in a per-repository temporary
directory.  This is more portable than shm (which requires
posix-realtime and has various quirks on OS X).  It might even work on
Windows, although this has not been tested. The shared memory file's
name follows the template "shm--" where  is the
trailing SHA-1 of the index file.  is "index" for cached index
files (and might later be "name-hash" for name-hash cache). If such
shared memory exists, it contains the same index content as on
disk. The content is already validated by the daemon and git won't
validate it again (except comparing the trailing SHA-1s).

We also add some bits to the index (to_shm and from_shm) to track
when an index came from shared memory or is going to shared memory.

We keep this daemon's logic as thin as possible. The "brain" stays in
git. So the daemon can read and validate stuff, but that's all it's
allowed to do. It does not add/create new information. It doesn't even
accept direct updates from git.

Git can poke the daemon via unix domain sockets to tell it to refresh
the index cache, or to keep it alive some more minutes. It can't give
any real index data directly to the daemon. Real data goes to disk
first, then the daemon reads and verifies it from there. The daemon
only handles $GIT_DIR/index, not temporary index files; it only gets
poked for the former.

$GIT_DIR/index-helper.sock is the socket for the daemon process. The
daemon reads from the socket and executes commands.

Named pipes were considered for portability reasons, but then commands
that need replies from the daemon would have to open their own pipes,
since a named pipe should only have one reader.  Unix domain sockets
don't have this problem.

On webkit.git with index format v2, duplicating 8 times to 1.5m
entries and 236MB in size:

(vanilla)  0.50 s: read_index_from .git/index
(index-helper) 0.18 s: read_index_from .git/index

Interestingly with index v4, we get less out of index-helper. It makes
sense as v4 requires more processing after loading the index:

(vanilla)  0.37 s: read_index_from .git/index
(index-helper) 0.22 s: read_index_from .git/index

[1] http://thread.gmane.org/gmane.comp.version-control.git/247268/focus=248771

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
Signed-off-by: Ramsay Jones 
---
 .gitignore |   1 +
 Documentation/git-index-helper.txt |  50 ++
 Makefile   |   5 +
 cache.h|  11 ++
 contrib/completion/git-completion.bash |   1 +
 git-compat-util.h  |   1 +
 index-helper.c | 277 +
 read-cache.c   | 125 +--
 t/t7900-index-helper.sh|  23 +++
 9 files changed, 485 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/git-index-helper.txt
 create mode 100644 index-helper.c
 create mode 100755 t/t7900-index-helper.sh

diff --git a/.gitignore b/.gitignore
index 5087ce1..b92f122 100644
--- a/.gitignore
+++ b/.gitignore
@@ -71,6 +71,7 @@
 /git-http-fetch
 /git-http-push
 /git-imap-send
+/git-index-helper
 /git-index-pack
 /git-init
 /git-init-db
diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
new file mode 100644
index 000..f892184
--- /dev/null
+++ b/Documentation/git-index-helper.txt
@@ -0,0 +1,50 @@
+git-index-helper(1)
+===
+
+NAME
+
+git-index-helper - A simple cache daemon for speeding up index file access
+
+SYNOPSIS
+
+[verse]
+'git index-helper' [options]
+
+DESCRIPTION
+---
+Keep the index file in memory for faster access. This daemon is per
+repository and per working tree.  So if you have two working trees
+each with a submodule, you might need four index-helpers.  (In practice,
+this is only worthwhile for large indexes, so only use it if you notice
+that git status is slow).
+
+OPTIONS
+---
+
+--exit-after=::
+   Exit if the cached index is not accessed for ``
+   seconds. Specify 0 to wait forever. Default is 600.
+
+NOTES
+-
+
+$GIT_DIR/index-helper.sock a 

[PATCH v10 12/20] update-index: enable/disable watchman support

[David Turner]

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 Documentation/git-index-helper.txt |  3 +++
 Documentation/git-update-index.txt |  6 ++
 builtin/update-index.c | 16 
 3 files changed, 25 insertions(+)

diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
index cce00cb..55a8a5a 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -18,6 +18,9 @@ each with a submodule, you might need four index-helpers.  
(In practice,
 this is only worthwhile for large indexes, so only use it if you notice
 that git status is slow).
 
+If you want the index-helper to accelerate untracked file checking,
+run git update-index --watchman before using it.
+
 OPTIONS
 ---
 
diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index c6cbed1..6736487 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -19,6 +19,7 @@ SYNOPSIS
 [--ignore-submodules]
 [--[no-]split-index]
 [--[no-|test-|force-]untracked-cache]
+[--[no-]watchman]
 [--really-refresh] [--unresolve] [--again | -g]
 [--info-only] [--index-info]
 [-z] [--stdin] [--index-version ]
@@ -176,6 +177,11 @@ may not support it yet.
 --no-untracked-cache::
Enable or disable untracked cache feature. Please use
`--test-untracked-cache` before enabling it.
+
+--watchman::
+--no-watchman::
+   Enable or disable watchman support. This is, at present,
+   only useful with git index-helper.
 +
 These options take effect whatever the value of the `core.untrackedCache`
 configuration variable (see linkgit:git-config[1]). But a warning is
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 1c94ca5..a3b4b5d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -914,6 +914,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
 {
int newfd, entries, has_errors = 0, nul_term_line = 0;
enum uc_mode untracked_cache = UC_UNSPECIFIED;
+   int use_watchman = -1;
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
@@ -1012,6 +1013,8 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
N_("test if the filesystem supports untracked 
cache"), UC_TEST),
OPT_SET_INT(0, "force-untracked-cache", _cache,
N_("enable untracked cache without testing the 
filesystem"), UC_FORCE),
+   OPT_BOOL(0, "watchman", _watchman,
+   N_("use or not use watchman to reduce refresh cost")),
OPT_END()
};
 
@@ -1149,6 +1152,19 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
die("Bug: bad untracked_cache value: %d", untracked_cache);
}
 
+   if (use_watchman > 0) {
+   the_index.last_update= xstrdup("");
+   the_index.cache_changed |= WATCHMAN_CHANGED;
+#ifndef USE_WATCHMAN
+   warning("git was built without watchman support -- I'm "
+   "adding the extension here, but it probably won't "
+   "do you any good.");
+#endif
+   } else if (!use_watchman) {
+   the_index.last_update= NULL;
+   the_index.cache_changed |= WATCHMAN_CHANGED;
+   }
+
if (active_cache_changed) {
if (newfd < 0) {
if (refresh_args.flags & REFRESH_QUIET)
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 01/20] read-cache.c: fix constness of verify_hdr()

[David Turner]

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 read-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index d9fb78b..16cc487 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1345,7 +1345,7 @@ struct ondisk_cache_entry_extended {
ondisk_cache_entry_extended_size(ce_namelen(ce)) : \
ondisk_cache_entry_size(ce_namelen(ce)))
 
-static int verify_hdr(struct cache_header *hdr, unsigned long size)
+static int verify_hdr(const struct cache_header *hdr, unsigned long size)
 {
git_SHA_CTX c;
unsigned char sha1[20];
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 08/20] index-helper: log warnings

[David Turner]

Instead of writing warnings to stderr, write them to a log.  Later, we'll
probably be daemonized, so writing to stderr will be pointless.

Signed-off-by: David Turner 
---
 Documentation/git-index-helper.txt |  3 +++
 index-helper.c | 12 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
index f853960..e144752 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -57,6 +57,9 @@ command. The following commands are used to control the 
daemon:
 
 All commands and replies are terminated by a NUL byte.
 
+In the event of an error, messages may be written to
+$GIT_DIR/index-helper.log.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/index-helper.c b/index-helper.c
index 65abfd0..a3a9b00 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -95,7 +95,8 @@ static void share_index(struct index_state *istate, struct 
shm *is)
if (shared_mmap_create(istate->mmap_size, _mmap,
   git_path("shm-index-%s",
sha1_to_hex(istate->sha1))) < 0) {
-   die("Failed to create shm-index file");
+   warning("Failed to create shm-index file");
+   exit(1);
}
 
 
@@ -321,8 +322,17 @@ int main(int argc, char **argv)
if (fd < 0)
die_errno(_("could not set up index-helper socket"));
 
+   if (detach) {
+   FILE *fp = fopen(git_path("index-helper.log"), "a");
+   if (!fp)
+   die("failed to open %s for writing",
+   git_path("index-helper.log"));
+   set_error_handle(fp);
+   }
+
if (detach && daemonize())
die_errno(_("unable to detach"));
+
loop(fd, idle_in_seconds);
 
close(fd);
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 00/20] index-helper/watchman

[David Turner]

packet_write was causing the sigpipes (by calling write_or_die, which
intentionally overrides the caller's preferences about signal handling).

This version fixes that.  I didn't test on a virtual machine, but I did
test by adding a sleep().

David Turner (9):
  pkt-line: add gentle version of packet_write
  index-helper: log warnings
  unpack-trees: preserve index extensions
  watchman: add a config option to enable the extension
  index-helper: kill mode
  index-helper: don't run if already running
  index-helper: autorun mode
  index-helper: optionally automatically run
  untracked-cache: config option

Nguyễn Thái Ngọc Duy (11):
  read-cache.c: fix constness of verify_hdr()
  read-cache: allow to keep mmap'd memory after reading
  index-helper: new daemon for caching index and related stuff
  index-helper: add --strict
  daemonize(): set a flag before exiting the main process
  index-helper: add --detach
  read-cache: add watchman 'WAMA' extension
  watchman: support watchman to reduce index refresh cost
  index-helper: use watchman to avoid refreshing index with lstat()
  update-index: enable/disable watchman support
  trace: measure where the time is spent in the index-heavy operations

 .gitignore   |   2 +
 Documentation/config.txt |  12 +
 Documentation/git-index-helper.txt   |  81 +
 Documentation/git-update-index.txt   |   6 +
 Documentation/technical/index-format.txt |  22 ++
 Makefile |  18 ++
 builtin/gc.c |   2 +-
 builtin/update-index.c   |  16 +
 cache.h  |  25 +-
 config.c |   5 +
 configure.ac |   8 +
 contrib/completion/git-completion.bash   |   1 +
 daemon.c |   2 +-
 diff-lib.c   |   4 +
 dir.c|  25 +-
 dir.h|   6 +
 environment.c|   3 +
 git-compat-util.h|   1 +
 index-helper.c   | 486 
 name-hash.c  |   2 +
 pkt-line.c   |  18 ++
 pkt-line.h   |   2 +
 preload-index.c  |   2 +
 read-cache.c | 536 ++-
 refs/files-backend.c |   2 +
 setup.c  |   4 +-
 t/t1701-watchman-extension.sh|  37 +++
 t/t7063-status-untracked-cache.sh|  22 ++
 t/t7900-index-helper.sh  |  69 
 t/test-lib-functions.sh  |   4 +
 test-dump-watchman.c |  16 +
 unpack-trees.c   |   1 +
 watchman-support.c   | 135 
 watchman-support.h   |   7 +
 34 files changed, 1561 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/git-index-helper.txt
 create mode 100644 index-helper.c
 create mode 100755 t/t1701-watchman-extension.sh
 create mode 100755 t/t7900-index-helper.sh
 create mode 100644 test-dump-watchman.c
 create mode 100644 watchman-support.c
 create mode 100644 watchman-support.h

-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 17/20] index-helper: autorun mode

[David Turner]

Soon, we'll want to automatically start index-helper, so we need
a mode that silently exits if it can't start up (either because
it's not in a git dir, or because another one is already running).

Signed-off-by: David Turner 
---
 Documentation/git-index-helper.txt |  4 
 index-helper.c | 29 +++--
 t/t7900-index-helper.sh|  8 
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
index addf694..0466296 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -43,6 +43,10 @@ OPTIONS
 --kill::
Kill any running index-helper and clean up the socket
 
+--autorun::
+   If index-helper is not already running, start it.  Else, do
+   nothing.
+
 NOTES
 -
 
diff --git a/index-helper.c b/index-helper.c
index 7b893a0..4b31da2 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -404,8 +404,9 @@ static void request_kill(void)
 int main(int argc, char **argv)
 {
const char *prefix;
-   int idle_in_seconds = 600, detach = 0, kill = 0;
+   int idle_in_seconds = 600, detach = 0, kill = 0, autorun = 0;
int fd;
+   int nongit;
struct strbuf socket_path = STRBUF_INIT;
struct option options[] = {
OPT_INTEGER(0, "exit-after", _in_seconds,
@@ -414,6 +415,7 @@ int main(int argc, char **argv)
 "verify shared memory after creating"),
OPT_BOOL(0, "detach", , "detach the process"),
OPT_BOOL(0, "kill", , "request that existing index helper 
processes exit"),
+   OPT_BOOL(0, "autorun", , "this is an automatic run of 
git index-helper, so certain errors can be solved by silently exiting"),
OPT_END()
};
 
@@ -423,7 +425,14 @@ int main(int argc, char **argv)
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(usage_text, options);
 
-   prefix = setup_git_directory();
+   prefix = setup_git_directory_gently();
+   if (nongit) {
+   if (autorun)
+   exit(0);
+   else
+   die(_("not a git repository"));
+   }
+
if (parse_options(argc, (const char **)argv, prefix,
  options, usage_text, 0))
die(_("too many arguments"));
@@ -437,10 +446,18 @@ int main(int argc, char **argv)
 
/* check that no other copy is running */
fd = unix_stream_connect(git_path("index-helper.sock"));
-   if (fd > 0)
-   die(_("Already running"));
-   if (errno != ECONNREFUSED && errno != ENOENT)
-   die_errno(_("Unexpected error checking socket"));
+   if (fd > 0) {
+   if (autorun)
+   exit(0);
+   else
+   die(_("Already running"));
+   }
+   if (errno != ECONNREFUSED && errno != ENOENT) {
+   if (autorun)
+   return 0;
+   else
+   die_errno(_("Unexpected error checking socket"));
+   }
 
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index 7159971..aa6e5fc 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -38,4 +38,12 @@ test_expect_success 'index-helper will not start if already 
running' '
grep "Already running" err
 '
 
+test_expect_success 'index-helper is quiet with --autorun' '
+   test_when_finished "git index-helper --kill" &&
+   git index-helper --kill &&
+   git index-helper --detach &&
+   test -S .git/index-helper.sock &&
+   git index-helper --autorun
+'
+
 test_done
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 13/20] unpack-trees: preserve index extensions

[David Turner]

Make git checkout (and other unpack_tree operations) preserve the
untracked cache and watchman status. This is valuable for two reasons:

1. Often, an unpack_tree operation will not touch large parts of the
working tree, and thus most of the untracked cache will continue to be
valid.

2. Even if the untracked cache were entirely invalidated by such an
operation, the user has signaled their intention to have such a cache,
and we don't want to throw it away.

The same logic applies to the watchman state.

Signed-off-by: David Turner 
---
 cache.h   |  1 +
 read-cache.c  |  8 
 t/t7063-status-untracked-cache.sh | 22 ++
 t/test-lib-functions.sh   |  4 
 unpack-trees.c|  1 +
 5 files changed, 36 insertions(+)

diff --git a/cache.h b/cache.h
index 633e1dd..1b372ed 100644
--- a/cache.h
+++ b/cache.h
@@ -580,6 +580,7 @@ extern void write_watchman_ext(struct strbuf *sb, struct 
index_state *istate);
 #define CLOSE_LOCK (1 << 1)
 extern int write_locked_index(struct index_state *, struct lock_file *lock, 
unsigned flags);
 extern int discard_index(struct index_state *);
+extern void move_index_extensions(struct index_state *dst, struct index_state 
*src);
 extern int unmerged_index(const struct index_state *);
 extern int verify_path(const char *path);
 extern int index_dir_exists(struct index_state *istate, const char *name, int 
namelen);
diff --git a/read-cache.c b/read-cache.c
index 8f6b956..75a1b05 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2771,3 +2771,11 @@ void stat_validity_update(struct stat_validity *sv, int 
fd)
fill_stat_data(sv->sd, );
}
 }
+
+void move_index_extensions(struct index_state *dst, struct index_state *src)
+{
+   dst->untracked = src->untracked;
+   src->untracked = NULL;
+   dst->last_update = src->last_update;
+   src->last_update = NULL;
+}
diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index a971884..083516d 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -646,4 +646,26 @@ test_expect_success 'test ident field is working' '
test_cmp ../expect ../err
 '
 
+test_expect_success 'untracked cache survives a checkout' '
+   git commit --allow-empty -m empty &&
+   test-dump-untracked-cache >../before &&
+   test_when_finished  "git checkout master" &&
+   git checkout -b other_branch &&
+   test-dump-untracked-cache >../after &&
+   test_cmp ../before ../after &&
+   test_commit test &&
+   test-dump-untracked-cache >../before &&
+   git checkout master &&
+   test-dump-untracked-cache >../after &&
+   test_cmp ../before ../after
+'
+
+test_expect_success 'untracked cache survives a commit' '
+   test-dump-untracked-cache >../before &&
+   git add done/two &&
+   git commit -m commit &&
+   test-dump-untracked-cache >../after &&
+   test_cmp ../before ../after
+'
+
 test_done
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d99eb3..e974b5b 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -186,6 +186,10 @@ test_commit () {
test_tick
fi &&
git commit $signoff -m "$1" &&
+   if [ "$(git config core.bare)" = false ]
+   then
+   git update-index --force-untracked-cache
+   fi
git tag "${4:-$1}"
 }
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 9f55cc2..fc90eb3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1215,6 +1215,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, 
struct unpack_trees_options
  WRITE_TREE_SILENT |
  WRITE_TREE_REPAIR);
}
+   move_index_extensions(>result, o->dst_index);
discard_index(o->dst_index);
*o->dst_index = o->result;
} else {
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 19/20] trace: measure where the time is spent in the index-heavy operations

[David Turner]

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

All the known heavy code blocks are measured (except object database
access). This should help identify if an optimization is effective or
not. An unoptimized git-status would give something like below (92% of
time is accounted). To sum up the effort of making git scale better:

 - read cache line is being addressed by index-helper
 - preload/refresh index by watchman
 - read packed refs by lmdb backend
 - diff-index by rebuilding cache-tree
 - read directory by untracked cache and watchman
 - write index by split index
 - name hash potientially by index-helper

read-cache.c:2075 performance: 0.004058570 s: read cache .../index
preload-index.c:104   performance: 0.004419864 s: preload index
read-cache.c:1265 performance: 0.000185224 s: refresh index
refs/files-backend.c:1100 performance: 0.001935788 s: read packed refs
diff-lib.c:240performance: 0.000144132 s: diff-files
diff-lib.c:506performance: 0.013592000 s: diff-index
name-hash.c:128   performance: 0.000614177 s: initialize name hash
dir.c:2030performance: 0.015814103 s: read directory
read-cache.c:2565 performance: 0.004052343 s: write index, changed mask 
= 2
trace.c:420   performance: 0.048365509 s: git command: './git' 
'status'

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 diff-lib.c   |  4 
 dir.c|  2 ++
 name-hash.c  |  2 ++
 preload-index.c  |  2 ++
 read-cache.c | 11 +++
 refs/files-backend.c |  2 ++
 6 files changed, 23 insertions(+)

diff --git a/diff-lib.c b/diff-lib.c
index bc49c70..7af7f9a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -90,6 +90,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
int diff_unmerged_stage = revs->max_count;
unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
  ? CE_MATCH_RACY_IS_DIRTY : 0);
+   uint64_t start = getnanotime();
 
diff_set_mnemonic_prefix(>diffopt, "i/", "w/");
 
@@ -236,6 +237,7 @@ int run_diff_files(struct rev_info *revs, unsigned int 
option)
}
diffcore_std(>diffopt);
diff_flush(>diffopt);
+   trace_performance_since(start, "diff-files");
return 0;
 }
 
@@ -491,6 +493,7 @@ static int diff_cache(struct rev_info *revs,
 int run_diff_index(struct rev_info *revs, int cached)
 {
struct object_array_entry *ent;
+   uint64_t start = getnanotime();
 
ent = revs->pending.objects;
if (diff_cache(revs, ent->item->oid.hash, ent->name, cached))
@@ -500,6 +503,7 @@ int run_diff_index(struct rev_info *revs, int cached)
diffcore_fix_diff_index(>diffopt);
diffcore_std(>diffopt);
diff_flush(>diffopt);
+   trace_performance_since(start, "diff-index");
return 0;
 }
 
diff --git a/dir.c b/dir.c
index 5058b29..c56a8b9 100644
--- a/dir.c
+++ b/dir.c
@@ -2183,6 +2183,7 @@ int read_directory(struct dir_struct *dir, const char 
*path, int len, const stru
 {
struct path_simplify *simplify;
struct untracked_cache_dir *untracked;
+   uint64_t start = getnanotime();
 
/*
 * Check out create_simplify()
@@ -2224,6 +2225,7 @@ int read_directory(struct dir_struct *dir, const char 
*path, int len, const stru
free_simplify(simplify);
qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), 
cmp_name);
+   trace_performance_since(start, "read directory %.*s", len, path);
if (dir->untracked) {
static struct trace_key trace_untracked_stats = 
TRACE_KEY_INIT(UNTRACKED_STATS);
trace_printf_key(_untracked_stats,
diff --git a/name-hash.c b/name-hash.c
index 6d9f23e..b3966d8 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -115,6 +115,7 @@ static int cache_entry_cmp(const struct cache_entry *ce1,
 static void lazy_init_name_hash(struct index_state *istate)
 {
int nr;
+   uint64_t start = getnanotime();
 
if (istate->name_hash_initialized)
return;
@@ -124,6 +125,7 @@ static void lazy_init_name_hash(struct index_state *istate)
for (nr = 0; nr < istate->cache_nr; nr++)
hash_index_entry(istate, istate->cache[nr]);
istate->name_hash_initialized = 1;
+   trace_performance_since(start, "initialize name hash");
 }
 
 void add_name_hash(struct index_state *istate, struct cache_entry *ce)
diff --git a/preload-index.c b/preload-index.c
index c1fe3a3..7bb4809 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -72,6 +72,7 @@ static void preload_index(struct index_state *index,
 {
int threads, i, work, offset;
struct thread_data data[MAX_PARALLEL];
+   uint64_t start = getnanotime();
 
if (!core_preload_index)
return;
@@ -100,6 

[PATCH v10 14/20] watchman: add a config option to enable the extension

[David Turner]

For installations that have centrally-managed configuration, it's
easier to set a config once than to run update-index on every
repository.

Signed-off-by: David Turner 
---
 .gitignore|  1 +
 Documentation/config.txt  |  4 
 Makefile  |  1 +
 read-cache.c  |  6 ++
 t/t1701-watchman-extension.sh | 37 +
 test-dump-watchman.c  | 16 
 6 files changed, 65 insertions(+)
 create mode 100755 t/t1701-watchman-extension.sh
 create mode 100644 test-dump-watchman.c

diff --git a/.gitignore b/.gitignore
index b92f122..e6a5b2c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -188,6 +188,7 @@
 /test-dump-cache-tree
 /test-dump-split-index
 /test-dump-untracked-cache
+/test-dump-watchman
 /test-fake-ssh
 /test-scrap-cache-tree
 /test-genrandom
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2cd6bdd..15001ce 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1848,6 +1848,10 @@ imap::
The configuration variables in the 'imap' section are described
in linkgit:git-imap-send[1].
 
+index.addwatchmanextension::
+   Automatically add the watchman extension to the index whenever
+   it is written.
+
 index.version::
Specify the version with which new index files should be
initialized.  This does not affect existing repositories.
diff --git a/Makefile b/Makefile
index 65ab0f4..5f0a954 100644
--- a/Makefile
+++ b/Makefile
@@ -599,6 +599,7 @@ TEST_PROGRAMS_NEED_X += test-delta
 TEST_PROGRAMS_NEED_X += test-dump-cache-tree
 TEST_PROGRAMS_NEED_X += test-dump-split-index
 TEST_PROGRAMS_NEED_X += test-dump-untracked-cache
+TEST_PROGRAMS_NEED_X += test-dump-watchman
 TEST_PROGRAMS_NEED_X += test-fake-ssh
 TEST_PROGRAMS_NEED_X += test-genrandom
 TEST_PROGRAMS_NEED_X += test-hashmap
diff --git a/read-cache.c b/read-cache.c
index 75a1b05..a2107f0 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2430,6 +2430,7 @@ static int do_write_index(struct index_state *istate, int 
newfd,
int entries = istate->cache_nr;
struct stat st;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
+   int watchman = 0;
 
for (i = removed = extended = 0; i < entries; i++) {
if (cache[i]->ce_flags & CE_REMOVE)
@@ -2453,6 +2454,11 @@ static int do_write_index(struct index_state *istate, 
int newfd,
if (istate->version == 3 || istate->version == 2)
istate->version = extended ? 3 : 2;
 
+   if (!git_config_get_bool("index.addwatchmanextension", ) &&
+   watchman &&
+   !the_index.last_update)
+   the_index.last_update = xstrdup("");
+
hdr_version = istate->version;
 
hdr.hdr_signature = htonl(CACHE_SIGNATURE);
diff --git a/t/t1701-watchman-extension.sh b/t/t1701-watchman-extension.sh
new file mode 100755
index 000..71f1d46
--- /dev/null
+++ b/t/t1701-watchman-extension.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='watchman extension smoke tests'
+
+# These don't actually test watchman interaction -- just the
+# index extension
+
+. ./test-lib.sh
+
+test_expect_success 'enable watchman' '
+   test_commit a &&
+   test-dump-watchman .git/index >actual &&
+   echo "last_update: (null)" >expect &&
+   test_cmp expect actual &&
+   git update-index --watchman &&
+   test-dump-watchman .git/index >actual &&
+   echo "last_update: " >expect &&
+   test_cmp expect actual
+'
+
+test_expect_success 'disable watchman' '
+   git update-index --no-watchman &&
+   test-dump-watchman .git/index >actual &&
+   echo "last_update: (null)" >expect &&
+   test_cmp expect actual
+'
+
+test_expect_success 'auto-enable watchman' '
+   test_config index.addwatchmanextension true &&
+   test_commit c &&
+   test-dump-watchman .git/index >actual &&
+   echo "last_update: " >expect &&
+   test_cmp expect actual
+'
+
+
+test_done
diff --git a/test-dump-watchman.c b/test-dump-watchman.c
new file mode 100644
index 000..0314fa5
--- /dev/null
+++ b/test-dump-watchman.c
@@ -0,0 +1,16 @@
+#include "cache.h"
+#include "ewah/ewok.h"
+
+int main(int argc, char **argv)
+{
+   do_read_index(_index, argv[1], 1);
+   printf("last_update: %s\n", the_index.last_update ?
+  the_index.last_update : "(null)");
+
+   /*
+* For now, we just dump last_update, since it is not reasonable
+* to populate the extension itself in tests.
+*/
+
+   return 0;
+}
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 06/20] daemonize(): set a flag before exiting the main process

[David Turner]

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

This allows signal handlers and atexit functions to realize this
situation and not clean up.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 builtin/gc.c | 2 +-
 cache.h  | 2 +-
 daemon.c | 2 +-
 setup.c  | 4 +++-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c583aad..37180de 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -385,7 +385,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 * failure to daemonize is ok, we'll continue
 * in foreground
 */
-   daemonized = !daemonize();
+   daemonized = !daemonize(NULL);
}
} else
add_repack_all_option();
diff --git a/cache.h b/cache.h
index 6cb0d02..4c1529a 100644
--- a/cache.h
+++ b/cache.h
@@ -539,7 +539,7 @@ extern int set_git_dir_init(const char *git_dir, const char 
*real_git_dir, int);
 extern int init_db(const char *template_dir, unsigned int flags);
 
 extern void sanitize_stdfds(void);
-extern int daemonize(void);
+extern int daemonize(int *);
 
 #define alloc_nr(x) (((x)+16)*3/2)
 
diff --git a/daemon.c b/daemon.c
index 8d45c33..a5cf954 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1365,7 +1365,7 @@ int main(int argc, char **argv)
return execute();
 
if (detach) {
-   if (daemonize())
+   if (daemonize(NULL))
die("--detach not supported on this platform");
} else
sanitize_stdfds();
diff --git a/setup.c b/setup.c
index de1a2a7..9adf13f 100644
--- a/setup.c
+++ b/setup.c
@@ -1017,7 +1017,7 @@ void sanitize_stdfds(void)
close(fd);
 }
 
-int daemonize(void)
+int daemonize(int *daemonized)
 {
 #ifdef NO_POSIX_GOODIES
errno = ENOSYS;
@@ -1029,6 +1029,8 @@ int daemonize(void)
case -1:
die_errno("fork failed");
default:
+   if (daemonized)
+   *daemonized = 1;
exit(0);
}
if (setsid() == -1)
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 07/20] index-helper: add --detach

[David Turner]

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

We detach after creating and opening the socket, because otherwise
we might return control to the shell before index-helper is ready to
accept commands.  This might lead to flaky tests.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 Documentation/git-index-helper.txt | 3 +++
 index-helper.c | 9 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
index 1f92c89..f853960 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -34,6 +34,9 @@ OPTIONS
for reading an index, but because it will happen in the
background, it's not noticable. `--strict` is enabled by default.
 
+--detach::
+   Detach from the shell.
+
 NOTES
 -
 
diff --git a/index-helper.c b/index-helper.c
index bc7e110..65abfd0 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -17,7 +17,7 @@ struct shm {
 
 static struct shm shm_index;
 static struct shm shm_base_index;
-static int to_verify = 1;
+static int daemonized, to_verify = 1;
 
 static void release_index_shm(struct shm *is)
 {
@@ -36,6 +36,8 @@ static void cleanup_shm(void)
 
 static void cleanup(void)
 {
+   if (daemonized)
+   return;
unlink(git_path("index-helper.sock"));
cleanup_shm();
 }
@@ -287,7 +289,7 @@ static const char * const usage_text[] = {
 int main(int argc, char **argv)
 {
const char *prefix;
-   int idle_in_seconds = 600;
+   int idle_in_seconds = 600, detach = 0;
int fd;
struct strbuf socket_path = STRBUF_INIT;
struct option options[] = {
@@ -295,6 +297,7 @@ int main(int argc, char **argv)
N_("exit if not used after some seconds")),
OPT_BOOL(0, "strict", _verify,
 "verify shared memory after creating"),
+   OPT_BOOL(0, "detach", , "detach the process"),
OPT_END()
};
 
@@ -318,6 +321,8 @@ int main(int argc, char **argv)
if (fd < 0)
die_errno(_("could not set up index-helper socket"));
 
+   if (detach && daemonize())
+   die_errno(_("unable to detach"));
loop(fd, idle_in_seconds);
 
close(fd);
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 20/20] untracked-cache: config option

[David Turner]

Add a config option to populate the untracked cache.

For installations that have centrally-managed configuration, it's
easier to set a config once than to run update-index on every
repository.

Signed-off-by: David Turner 
---
 Documentation/config.txt | 4 
 read-cache.c | 7 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 385ea66..c7b76ef 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1848,6 +1848,10 @@ imap::
The configuration variables in the 'imap' section are described
in linkgit:git-imap-send[1].
 
+index.adduntrackedcache::
+   Automatically populate the untracked cache whenever the index
+   is written.
+
 index.addwatchmanextension::
Automatically add the watchman extension to the index whenever
it is written.
diff --git a/read-cache.c b/read-cache.c
index 22c64d0..4a1cccf 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2472,7 +2472,7 @@ static int do_write_index(struct index_state *istate, int 
newfd,
int entries = istate->cache_nr;
struct stat st;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
-   int watchman = 0;
+   int watchman = 0, untracked = 0;
uint64_t start = getnanotime();
 
for (i = removed = extended = 0; i < entries; i++) {
@@ -2502,6 +2502,11 @@ static int do_write_index(struct index_state *istate, 
int newfd,
!the_index.last_update)
the_index.last_update = xstrdup("");
 
+   if (!git_config_get_bool("index.adduntrackedcache", ) &&
+   untracked &&
+   !istate->untracked)
+   add_untracked_cache(_index);
+
hdr_version = istate->version;
 
hdr.hdr_signature = htonl(CACHE_SIGNATURE);
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 05/20] index-helper: add --strict

[David Turner]

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

There are "holes" in the index-helper approach because the shared
memory is not verified again by git. If $USER is compromised, shared
memory could be modified. But anyone who could do this could already
modify $GIT_DIR/index. A more realistic risk is some bugs in
index-helper that produce corrupt shared memory. --strict is added to
avoid that.

Strictly speaking there's still a very small gap where corrupt shared
memory could still be read by git: after we write the trailing SHA-1 in
the shared memory (thus signaling "this shm is ready") and before
verify_shm() detects an error.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 Documentation/git-index-helper.txt |  9 +++
 cache.h|  1 +
 index-helper.c | 48 ++
 read-cache.c   |  9 ---
 4 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
index f892184..1f92c89 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -25,6 +25,15 @@ OPTIONS
Exit if the cached index is not accessed for ``
seconds. Specify 0 to wait forever. Default is 600.
 
+--strict::
+--no-strict::
+   Strict mode makes index-helper verify the shared memory after
+   it's created. If the result does not match what's read from
+   $GIT_DIR/index, the shared memory is destroyed. This makes
+   index-helper take more than double the amount of time required
+   for reading an index, but because it will happen in the
+   background, it's not noticable. `--strict` is enabled by default.
+
 NOTES
 -
 
diff --git a/cache.h b/cache.h
index 2d7af6f..6cb0d02 100644
--- a/cache.h
+++ b/cache.h
@@ -345,6 +345,7 @@ struct index_state {
  * on it.
  */
 to_shm : 1,
+always_verify_trailing_sha1 : 1,
 initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
diff --git a/index-helper.c b/index-helper.c
index b9443f4..bc7e110 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -17,6 +17,7 @@ struct shm {
 
 static struct shm shm_index;
 static struct shm shm_base_index;
+static int to_verify = 1;
 
 static void release_index_shm(struct shm *is)
 {
@@ -114,11 +115,56 @@ static void share_index(struct index_state *istate, 
struct shm *is)
hashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);
 }
 
+static int verify_shm(void)
+{
+   int i;
+   struct index_state istate;
+   memset(, 0, sizeof(istate));
+   istate.always_verify_trailing_sha1 = 1;
+   istate.to_shm = 1;
+   i = read_index();
+   if (i != the_index.cache_nr)
+   goto done;
+   for (; i < the_index.cache_nr; i++) {
+   struct cache_entry *base, *ce;
+   /* namelen is checked separately */
+   const unsigned int ondisk_flags =
+   CE_STAGEMASK | CE_VALID | CE_EXTENDED_FLAGS;
+   unsigned int ce_flags, base_flags, ret;
+   base = the_index.cache[i];
+   ce = istate.cache[i];
+   if (ce->ce_namelen != base->ce_namelen ||
+   strcmp(ce->name, base->name)) {
+   warning("mismatch at entry %d", i);
+   break;
+   }
+   ce_flags = ce->ce_flags;
+   base_flags = base->ce_flags;
+   /* only on-disk flags matter */
+   ce->ce_flags   &= ondisk_flags;
+   base->ce_flags &= ondisk_flags;
+   ret = memcmp(>ce_stat_data, >ce_stat_data,
+offsetof(struct cache_entry, name) -
+offsetof(struct cache_entry, ce_stat_data));
+   ce->ce_flags = ce_flags;
+   base->ce_flags = base_flags;
+   if (ret) {
+   warning("mismatch at entry %d", i);
+   break;
+   }
+   }
+done:
+   discard_index();
+   return i == the_index.cache_nr;
+}
+
 static void share_the_index(void)
 {
if (the_index.split_index && the_index.split_index->base)
share_index(the_index.split_index->base, _base_index);
share_index(_index, _index);
+   if (to_verify && !verify_shm())
+   cleanup_shm();
discard_index(_index);
 }
 
@@ -247,6 +293,8 @@ int main(int argc, char **argv)
struct option options[] = {
OPT_INTEGER(0, "exit-after", _in_seconds,
N_("exit if not used after some seconds")),
+   OPT_BOOL(0, "strict", _verify,
+"verify shared memory after creating"),
OPT_END()
};
 
diff --git 

[PATCH v10 18/20] index-helper: optionally automatically run

[David Turner]

Introduce a new config option, indexhelper.autorun, to automatically
run git index-helper before starting up a builtin git command.  This
enables users to keep index-helper running without manual
intervention.

Signed-off-by: David Turner 
---
 Documentation/config.txt |  4 
 read-cache.c | 37 +++--
 t/t7900-index-helper.sh  | 20 
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 15001ce..385ea66 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1856,6 +1856,10 @@ index.version::
Specify the version with which new index files should be
initialized.  This does not affect existing repositories.
 
+indexhelper.autorun::
+   Automatically run git index-helper when any builtin git
+   command is run inside a repository.
+
 init.templateDir::
Specify the directory from which templates will be copied.
(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
diff --git a/read-cache.c b/read-cache.c
index a2107f0..120263c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -22,6 +22,7 @@
 #include "pkt-line.h"
 #include "sigchain.h"
 #include "ewah/ewok.h"
+#include "run-command.h"
 
 static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
   unsigned int options);
@@ -1723,6 +1724,33 @@ static void post_read_index_from(struct index_state 
*istate)
tweak_untracked_cache(istate);
 }
 
+static int want_auto_index_helper(void)
+{
+   int want = -1;
+   const char *value = NULL;
+   const char *key = "indexhelper.autorun";
+
+   if (git_config_key_is_valid(key) &&
+   !git_config_get_value(key, )) {
+   int b = git_config_maybe_bool(key, value);
+   want = b >= 0 ? b : 0;
+   }
+   return want;
+}
+
+static void autorun_index_helper(void)
+{
+   const char *argv[] = {"git-index-helper", "--detach", "--autorun", 
NULL};
+   if (want_auto_index_helper() <= 0)
+   return;
+
+   trace_argv_printf(argv, "trace: auto index-helper:");
+
+   if (run_command_v_opt(argv,
+ RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT))
+   warning(_("You specified indexhelper.autorun, but running 
git-index-helper failed."));
+}
+
 /* in ms */
 #define WATCHMAN_TIMEOUT 1000
 
@@ -1798,6 +1826,7 @@ static int poke_daemon(struct index_state *istate,
if (fd < 0) {
warning("Failed to connect to index-helper socket");
unlink(git_path("index-helper.sock"));
+   autorun_index_helper();
return -1;
}
sigchain_push(SIGPIPE, SIG_IGN);
@@ -1837,9 +1866,13 @@ static int try_shm(struct index_state *istate)
int fd = -1;
 
if (!is_main_index(istate) ||
-   old_size <= 20 ||
-   stat(git_path("index-helper.sock"), ))
+   old_size <= 20)
return -1;
+
+   if (stat(git_path("index-helper.sock"), )) {
+   autorun_index_helper();
+   return -1;
+   }
if (poke_daemon(istate, , 0))
return -1;
sha1 = (unsigned char *)istate->mmap + old_size - 20;
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index aa6e5fc..3cfdf63 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -16,6 +16,9 @@ test -n "$NO_MMAP" && {
 }
 
 test_expect_success 'index-helper smoke test' '
+   # We need an existing commit so that the index exists (otherwise,
+   # the index-helper will not be autostarted)
+   test_commit x &&
git index-helper --exit-after 1 &&
test_path_is_missing .git/index-helper.sock
 '
@@ -46,4 +49,21 @@ test_expect_success 'index-helper is quiet with --autorun' '
git index-helper --autorun
 '
 
+test_expect_success '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
+'
+
 test_done
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 11/20] index-helper: use watchman to avoid refreshing index with lstat()

[David Turner]

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

Watchman is hidden behind index-helper. Before git tries to read the
index from shm, it notifies index-helper through the socket and waits
for index-helper to prepare a file for sharing memory (with
MAP_SHARED). index-helper then contacts watchman, updates 'WAMA'
extension and put it in a separate file and wakes git up with a reply
to git's socket.

Git uses this extension to not lstat unchanged entries. Git only
trusts the 'WAMA' extension when it's received from the separate file,
not from disk. Unmarked entries are "clean". Marked entries are dirty
from watchman point of view. If it finds out some entries are
'watchman-dirty', but are really unchanged (e.g. the file was changed,
then reverted back), then Git will clear the marking in 'WAMA' before
writing it down.

Hiding watchman behind index-helper means you need both daemons. You
can't run watchman alone. Not so good. But on the other hand, 'git'
binary is not linked to watchman/json libraries, which is good for
packaging. Core git package will run fine without watchman-related
packages. If they need watchman, they can install git-index-helper and
dependencies.

This also lets us trust anything in the untracked cache that we haven't
marked invalid, saving those stat() calls.

Another reason for tying watchman to index-helper is, when used with
untracked cache, we need to keep track of $GIT_WORK_TREE file
listing. That kind of list can be kept in index-helper.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 Documentation/git-index-helper.txt |   6 +
 cache.h|   2 +
 dir.c  |  23 +++-
 dir.h  |   3 +
 index-helper.c | 107 ++--
 read-cache.c   | 245 ++---
 6 files changed, 359 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
index e144752..cce00cb 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -55,6 +55,12 @@ command. The following commands are used to control the 
daemon:
Let the daemon know the index is to be read. It keeps the
daemon alive longer, unless `--exit-after=0` is used.
 
+"poke ":
+   Like "poke", but replies with "OK".  If the index has the
+   watchman extension, index-helper queries watchman, then
+   prepares a shared memory object with the watchman index
+   extension before replying.
+
 All commands and replies are terminated by a NUL byte.
 
 In the event of an error, messages may be written to
diff --git a/cache.h b/cache.h
index 452aea2..633e1dd 100644
--- a/cache.h
+++ b/cache.h
@@ -567,6 +567,7 @@ extern int daemonize(int *);
 
 /* Initialize and use the cache information */
 struct lock_file;
+extern int verify_index(const struct index_state *);
 extern int read_index(struct index_state *);
 extern int read_index_preload(struct index_state *, const struct pathspec 
*pathspec);
 extern int do_read_index(struct index_state *istate, const char *path,
@@ -574,6 +575,7 @@ extern int do_read_index(struct index_state *istate, const 
char *path,
 extern int read_index_from(struct index_state *, const char *path);
 extern int is_index_unborn(struct index_state *);
 extern int read_index_unmerged(struct index_state *);
+extern void write_watchman_ext(struct strbuf *sb, struct index_state *istate);
 #define COMMIT_LOCK(1 << 0)
 #define CLOSE_LOCK (1 << 1)
 extern int write_locked_index(struct index_state *, struct lock_file *lock, 
unsigned flags);
diff --git a/dir.c b/dir.c
index 69e0be6..5058b29 100644
--- a/dir.c
+++ b/dir.c
@@ -597,9 +597,9 @@ static void trim_trailing_spaces(char *buf)
  *
  * If "name" has the trailing slash, it'll be excluded in the search.
  */
-static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
-   struct untracked_cache_dir 
*dir,
-   const char *name, int len)
+struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
+struct untracked_cache_dir *dir,
+const char *name, int len)
 {
int first, last;
struct untracked_cache_dir *d;
@@ -1726,6 +1726,17 @@ static int valid_cached_dir(struct dir_struct *dir,
if (!untracked)
return 0;
 
+   if (dir->untracked->use_watchman) {
+   /*
+* With watchman, we can trust the untracked cache's
+* valid field.
+*/
+   if (untracked->valid)
+   goto skip_stat;
+   else
+   invalidate_directory(dir->untracked, untracked);
+   }
+
if 

[PATCH v10 03/20] pkt-line: add gentle version of packet_write

[David Turner]

packet_write calls write_or_die, which dies with a sigpipe even if
calling code has explicitly blocked that signal.

Add packet_write_gently and packet_flush_gently, which don't.  Soon,
we will use this for communication with git index-helper, which, being
merely an optimization, should be permitted to die without disrupting
clients.

Signed-off-by: David Turner 
---
 pkt-line.c | 18 ++
 pkt-line.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/pkt-line.c b/pkt-line.c
index 62fdb37..f964446 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -91,6 +91,12 @@ void packet_flush(int fd)
write_or_die(fd, "", 4);
 }
 
+int packet_flush_gently(int fd)
+{
+   packet_trace("", 4, 1);
+   return write_in_full(fd, "", 4) != 4;
+}
+
 void packet_buf_flush(struct strbuf *buf)
 {
packet_trace("", 4, 1);
@@ -130,6 +136,18 @@ void packet_write(int fd, const char *fmt, ...)
write_or_die(fd, buf.buf, buf.len);
 }
 
+int packet_write_gently(int fd, const char *fmt, ...)
+{
+   static struct strbuf buf = STRBUF_INIT;
+   va_list args;
+
+   strbuf_reset();
+   va_start(args, fmt);
+   format_packet(, fmt, args);
+   va_end(args);
+   return write_in_full(fd, buf.buf, buf.len) != buf.len;
+}
+
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
 {
va_list args;
diff --git a/pkt-line.h b/pkt-line.h
index 3cb9d91..deffcb5 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -20,7 +20,9 @@
  * side can't, we stay with pure read/write interfaces.
  */
 void packet_flush(int fd);
+int packet_flush_gently(int fd);
 void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 
2, 3)));
+int packet_write_gently(int fd, const char *fmt, ...) __attribute__((format 
(printf, 2, 3)));
 void packet_buf_flush(struct strbuf *buf);
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
 
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 15/20] index-helper: kill mode

[David Turner]

Add a new command (and command-line arg) to allow index-helpers to
exit cleanly.

This is mainly useful for tests.

Signed-off-by: David Turner 
---
 Documentation/git-index-helper.txt |  3 +++
 index-helper.c | 31 ++-
 t/t7900-index-helper.sh|  9 +
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-index-helper.txt 
b/Documentation/git-index-helper.txt
index 55a8a5a..addf694 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -40,6 +40,9 @@ OPTIONS
 --detach::
Detach from the shell.
 
+--kill::
+   Kill any running index-helper and clean up the socket
+
 NOTES
 -
 
diff --git a/index-helper.c b/index-helper.c
index 6026c74..a1b33e4 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -350,6 +350,8 @@ static void loop(int fd, int idle_in_seconds)
 * alive, nothing to do.
 */
}
+   } else if (!strcmp(buf, "die")) {
+   break;
} else {
warning("BUG: Bogus command %s", buf);
}
@@ -380,10 +382,29 @@ static const char * const usage_text[] = {
NULL
 };
 
+static void request_kill(void)
+{
+   int fd = unix_stream_connect(git_path("index-helper.sock"));
+
+   if (fd >= 0) {
+   write_in_full(fd, "die", 4);
+   close(fd);
+   }
+
+   /*
+* The child will try to do this anyway, but we want to be
+* ready to launch a new daemon immediately after this command
+* returns.
+*/
+
+   unlink(git_path("index-helper.sock"));
+   return;
+}
+
 int main(int argc, char **argv)
 {
const char *prefix;
-   int idle_in_seconds = 600, detach = 0;
+   int idle_in_seconds = 600, detach = 0, kill = 0;
int fd;
struct strbuf socket_path = STRBUF_INIT;
struct option options[] = {
@@ -392,6 +413,7 @@ int main(int argc, char **argv)
OPT_BOOL(0, "strict", _verify,
 "verify shared memory after creating"),
OPT_BOOL(0, "detach", , "detach the process"),
+   OPT_BOOL(0, "kill", , "request that existing index helper 
processes exit"),
OPT_END()
};
 
@@ -406,6 +428,13 @@ int main(int argc, char **argv)
  options, usage_text, 0))
die(_("too many arguments"));
 
+   if (kill) {
+   if (detach)
+   die(_("--kill doesn't want any other options"));
+   request_kill();
+   return 0;
+   }
+
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
 
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index 114c112..e71b5af 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -20,4 +20,13 @@ test_expect_success 'index-helper smoke test' '
test_path_is_missing .git/index-helper.sock
 '
 
+test_expect_success 'index-helper creates usable path file and can be killed' '
+   test_when_finished "git index-helper --kill" &&
+   test_path_is_missing .git/index-helper.sock &&
+   git index-helper --detach &&
+   test -S .git/index-helper.sock &&
+   git index-helper --kill &&
+   test_path_is_missing .git/index-helper.sock
+'
+
 test_done
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 v10 10/20] watchman: support watchman to reduce index refresh cost

[David Turner]

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

The previous patch has the logic to clear bits in 'WAMA' bitmap. This
patch has logic to set bits as told by watchman. The missing bit,
_using_ these bits, are not here yet.

A lot of this code is written by David Turner originally, mostly from
[1]. I'm just copying and polishing it a bit.

[1] http://article.gmane.org/gmane.comp.version-control.git/248006

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 Makefile   |  12 +
 cache.h|   1 +
 config.c   |   5 ++
 configure.ac   |   8 
 environment.c  |   3 ++
 watchman-support.c | 135 +
 watchman-support.h |   7 +++
 7 files changed, 171 insertions(+)
 create mode 100644 watchman-support.c
 create mode 100644 watchman-support.h

diff --git a/Makefile b/Makefile
index c8be0e7..65ab0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -451,6 +451,7 @@ MSGFMT = msgfmt
 CURL_CONFIG = curl-config
 PTHREAD_LIBS = -lpthread
 PTHREAD_CFLAGS =
+WATCHMAN_LIBS =
 GCOV = gcov
 
 export TCL_PATH TCLTK_PATH
@@ -1416,6 +1417,13 @@ else
LIB_OBJS += thread-utils.o
 endif
 
+ifdef USE_WATCHMAN
+   LIB_H += watchman-support.h
+   LIB_OBJS += watchman-support.o
+   WATCHMAN_LIBS = -lwatchman
+   BASIC_CFLAGS += -DUSE_WATCHMAN
+endif
+
 ifdef HAVE_PATHS_H
BASIC_CFLAGS += -DHAVE_PATHS_H
 endif
@@ -2025,6 +2033,9 @@ git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS 
$(GITLIBS) $(VCSSVN_LIB)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
$(LIBS) \
$(VCSSVN_LIB)
 
+git-index-helper$X: index-helper.o GIT-LDFLAGS $(GITLIBS)
+   $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
$(LIBS) $(WATCHMAN_LIBS)
+
 $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
$(QUIET_LNCP)$(RM) $@ && \
ln $< $@ 2>/dev/null || \
@@ -2164,6 +2175,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst 
','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
@echo NO_MMAP=\''$(subst ','\'',$(subst ','\'',$(NO_MMAP)))'\' >>$@+
+   @echo USE_WATCHMAN=\''$(subst ','\'',$(subst 
','\'',$(USE_WATCHMAN)))'\' >>$@+
 ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst 
','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+
 endif
diff --git a/cache.h b/cache.h
index f10992d..452aea2 100644
--- a/cache.h
+++ b/cache.h
@@ -696,6 +696,7 @@ extern char *git_replace_ref_base;
 
 extern int fsync_object_files;
 extern int core_preload_index;
+extern int core_watchman_sync_timeout;
 extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
 extern int protect_hfs;
diff --git a/config.c b/config.c
index 9ba40bc..e6dc141 100644
--- a/config.c
+++ b/config.c
@@ -882,6 +882,11 @@ static int git_default_core_config(const char *var, const 
char *value)
return 0;
}
 
+   if (!strcmp(var, "core.watchmansynctimeout")) {
+   core_watchman_sync_timeout = git_config_int(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "core.createobject")) {
if (!strcmp(value, "rename"))
object_creation_mode = OBJECT_CREATION_USES_RENAMES;
diff --git a/configure.ac b/configure.ac
index 0cd9f46..334d63b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1099,6 +1099,14 @@ AC_COMPILE_IFELSE([BSD_SYSCTL_SRC],
HAVE_BSD_SYSCTL=])
 GIT_CONF_SUBST([HAVE_BSD_SYSCTL])
 
+#
+# Check for watchman client library
+
+AC_CHECK_LIB([watchman], [watchman_connect],
+   [USE_WATCHMAN=YesPlease],
+   [USE_WATCHMAN=])
+GIT_CONF_SUBST([USE_WATCHMAN])
+
 ## Other checks.
 # Define USE_PIC if you need the main git objects to be built with -fPIC
 # in order to build and link perl/Git.so.  x86-64 seems to need this.
diff --git a/environment.c b/environment.c
index 6dec9d0..35e03c7 100644
--- a/environment.c
+++ b/environment.c
@@ -94,6 +94,9 @@ int core_preload_index = 1;
  */
 int ignore_untracked_cache_config;
 
+int core_watchman_sync_timeout = 300;
+
+
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
 static char *work_tree;
diff --git a/watchman-support.c b/watchman-support.c
new file mode 100644
index 000..dc8cd51
--- /dev/null
+++ b/watchman-support.c
@@ -0,0 +1,135 @@
+#include "cache.h"
+#include "watchman-support.h"
+#include "strbuf.h"
+#include "dir.h"
+#include 
+
+static struct watchman_query *make_query(const char *last_update)
+{
+   struct watchman_query *query = watchman_query();
+   watchman_query_set_fields(query, WATCHMAN_FIELD_NAME |
+WATCHMAN_FIELD_EXISTS |
+WATCHMAN_FIELD_NEWER);
+   watchman_query_set_empty_on_fresh(query, 1);
+   

[PATCH v10 09/20] read-cache: add watchman 'WAMA' extension

[David Turner]

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

The extension contains a bitmap, one bit for each entry in the
index. If the n-th bit is zero, the n-th entry is considered
unchanged, we can ce_mark_uptodate() it without refreshing. If the bit
is non-zero and we found out the corresponding file is clean after
refresh, we can clear the bit.

In addition, there's a list of directories in the untracked-cache
to invalidate (because they have new or modified entries).

The 'skipping refresh' bit is not in this patch yet as we would need
watchman. More details in later patches.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 Documentation/technical/index-format.txt |  22 ++
 cache.h  |   4 ++
 dir.h|   3 +
 read-cache.c | 117 ++-
 4 files changed, 144 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/index-format.txt 
b/Documentation/technical/index-format.txt
index ade0b0c..86ed3a6 100644
--- a/Documentation/technical/index-format.txt
+++ b/Documentation/technical/index-format.txt
@@ -295,3 +295,25 @@ The remaining data of each directory block is grouped by 
type:
 in the previous ewah bitmap.
 
   - One NUL.
+
+== Watchman cache
+
+  The watchman cache tracks files for which watchman has told us about
+  changes.  The signature for this extension is { 'W', 'A', 'M', 'A' }.
+
+  The extension starts with
+
+  - A NUL-terminated string: the watchman vector clock at the last
+time we heard from watchman.
+
+  - 32-bit bitmap size: the size of the CE_WATCHMAN_DIRTY bitmap
+
+  - 32-bit untracked cache entry count: the number of dirty untracked
+cache entries
+
+  - An ewah bitmap, the n-th bit indicates whether the n-th index entry
+is CE_WATCHMAN_DIRTY.
+
+  - a list of N NUL-terminated strings.  Each is a directory that should
+be marked dirty in the untracked cache because watchman has told us
+about an update to a file in it.
diff --git a/cache.h b/cache.h
index 4c1529a..f10992d 100644
--- a/cache.h
+++ b/cache.h
@@ -182,6 +182,8 @@ struct cache_entry {
 #define CE_VALID (0x8000)
 #define CE_STAGESHIFT 12
 
+#define CE_WATCHMAN_DIRTY  (0x0001)
+
 /*
  * Range 0x0FFF in ce_flags is divided into
  * two parts: in-memory flags and on-disk ones.
@@ -320,6 +322,7 @@ static inline unsigned int canon_mode(unsigned int mode)
 #define CACHE_TREE_CHANGED (1 << 5)
 #define SPLIT_INDEX_ORDERED(1 << 6)
 #define UNTRACKED_CHANGED  (1 << 7)
+#define WATCHMAN_CHANGED   (1 << 8)
 
 struct split_index;
 struct untracked_cache;
@@ -353,6 +356,7 @@ struct index_state {
struct untracked_cache *untracked;
void *mmap;
size_t mmap_size;
+   char *last_update;
 };
 
 extern struct index_state the_index;
diff --git a/dir.h b/dir.h
index 3ec3fb0..3d540de 100644
--- a/dir.h
+++ b/dir.h
@@ -142,6 +142,9 @@ struct untracked_cache {
int gitignore_invalidated;
int dir_invalidated;
int dir_opened;
+   /* watchman invalidation data */
+   unsigned int use_watchman : 1;
+   struct string_list invalid_untracked;
 };
 
 struct dir_struct {
diff --git a/read-cache.c b/read-cache.c
index 41647ea..1719f5a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -21,6 +21,7 @@
 #include "unix-socket.h"
 #include "pkt-line.h"
 #include "sigchain.h"
+#include "ewah/ewok.h"
 
 static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
   unsigned int options);
@@ -43,11 +44,13 @@ static struct cache_entry *refresh_cache_entry(struct 
cache_entry *ce,
 #define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* "REUC" */
 #define CACHE_EXT_LINK 0x6c696e6b/* "link" */
 #define CACHE_EXT_UNTRACKED 0x554E5452   /* "UNTR" */
+#define CACHE_EXT_WATCHMAN 0x57414D41/* "WAMA" */
 
 /* changes that can be kept in $GIT_DIR/index (basically all extensions) */
 #define EXTMASK (RESOLVE_UNDO_CHANGED | CACHE_TREE_CHANGED | \
 CE_ENTRY_ADDED | CE_ENTRY_REMOVED | CE_ENTRY_CHANGED | \
-SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED)
+SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED | \
+WATCHMAN_CHANGED)
 
 struct index_state the_index;
 static const char *alternate_index_output;
@@ -1222,8 +1225,13 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
continue;
 
new = refresh_cache_ent(istate, ce, options, _errno, 
);
-   if (new == ce)
+   if (new == ce) {
+   if (ce->ce_flags & CE_WATCHMAN_DIRTY) {
+   ce->ce_flags  &= ~CE_WATCHMAN_DIRTY;
+   istate->cache_changed |= WATCHMAN_CHANGED;
+   }
continue;
+   }
if (!new) {
   

[PATCH v10 02/20] read-cache: allow to keep mmap'd memory after reading

[David Turner]

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

Later, we will introduce git index-helper to share this memory with
other git processes.

We only unmap it when we discard the index (although the kernel may of
course choose to page it out).

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: David Turner 
---
 cache.h  |  3 +++
 read-cache.c | 13 -
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index b829410..4180e2b 100644
--- a/cache.h
+++ b/cache.h
@@ -333,11 +333,14 @@ struct index_state {
struct split_index *split_index;
struct cache_time timestamp;
unsigned name_hash_initialized : 1,
+keep_mmap : 1,
 initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
unsigned char sha1[20];
struct untracked_cache *untracked;
+   void *mmap;
+   size_t mmap_size;
 };
 
 extern struct index_state the_index;
diff --git a/read-cache.c b/read-cache.c
index 16cc487..3cb0ec3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1574,6 +1574,10 @@ int do_read_index(struct index_state *istate, const char 
*path, int must_exist)
mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
if (mmap == MAP_FAILED)
die_errno("unable to map index file");
+   if (istate->keep_mmap) {
+   istate->mmap = mmap;
+   istate->mmap_size = mmap_size;
+   }
close(fd);
 
hdr = mmap;
@@ -1626,11 +1630,13 @@ int do_read_index(struct index_state *istate, const 
char *path, int must_exist)
src_offset += 8;
src_offset += extsize;
}
-   munmap(mmap, mmap_size);
+   if (!istate->keep_mmap)
+   munmap(mmap, mmap_size);
return istate->cache_nr;
 
 unmap:
munmap(mmap, mmap_size);
+   istate->mmap = NULL;
die("index file corrupt");
 }
 
@@ -1655,6 +1661,7 @@ int read_index_from(struct index_state *istate, const 
char *path)
discard_index(split_index->base);
else
split_index->base = xcalloc(1, sizeof(*split_index->base));
+   split_index->base->keep_mmap = istate->keep_mmap;
ret = do_read_index(split_index->base,
git_path("sharedindex.%s",
 sha1_to_hex(split_index->base_sha1)), 1);
@@ -1698,6 +1705,10 @@ int discard_index(struct index_state *istate)
free(istate->cache);
istate->cache = NULL;
istate->cache_alloc = 0;
+   if (istate->keep_mmap && istate->mmap) {
+   munmap(istate->mmap, istate->mmap_size);
+   istate->mmap = NULL;
+   }
discard_split_index(istate);
free_untracked_cache(istate->untracked);
istate->untracked = NULL;
-- 
2.4.2.767.g62658d5-twtrsrc

--
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 22/94] builtin/apply: move 'threeway' global into 'struct apply_state'

[Christian Couder]

On Thu, May 12, 2016 at 9:41 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> To libify the apply functionality the 'threeway' variable should
>> not be static and global to the file. Let's move it into
>> 'struct apply_state'.
>>
>> Reviewed-by: Stefan Beller 
>> Signed-off-by: Christian Couder 
>> ---
>>  builtin/apply.c | 14 +++---
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index 6216723..3650922 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -40,6 +40,7 @@ struct apply_state {
>>   int numstat;
>>
>>   int summary;
>> + int threeway;
>
> This makes threeway look as if it is one of the cosmetic options
> like stat/numstat/summary, doesn't it?  If anything, the blank
> between numstat and summary should be moved below summary.

There is a blank because there is a comment on the line before
numstat. The result looks like this:

/* --cached updates only the cache without ever touching the
working tree. */
int cached;

/* --stat does just a diffstat, and doesn't actually apply */
int diffstat;

/* --numstat does numeric diffstat, and doesn't actually apply */
int numstat;

int summary;
int threeway;
int no_add;


>  I'd
> group knobs that affect "what is affected (check, check_index,
> cached, etc.)", "how the application is done (allow-overlap,
> threeway, in-reverse, etc.)" and "cosmetics" and place the ones in
> the same group next to each other.

Ok, I will try to group knobs like that, but the comments tend to
break the groups.
--
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 10/94] builtin/apply: move 'unidiff_zero' global into 'struct apply_state'

[Christian Couder]

On Thu, May 12, 2016 at 9:28 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> To libify the apply functionality the 'unidiff_zero' variable should
>> not be static and global to the file. Let's move it into
>> 'struct apply_state'.
>>
>> Reviewed-by: Stefan Beller 
>> Signed-off-by: Christian Couder 
>> ---
>
> Looks correct; I would have expected from the flow of thought in the
> series that p_value and linenr that were already mentioned would be
> the first two to be put in this, but that would have to wait until
> more functions are taught to pass the state around, so this ordering
> would probably be fine.

Yeah, if the patches that put p_value and linenr into 'struct
apply_state' were earlier in the series, they would have to pass the
state around to a lot more functions, so they would have been very
big.
--
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: proposal for extending smudge/clean filters with raw file access

[Junio C Hamano]

Joey Hess  writes:

> The clean filter has to consume the whole file content on stdin;
> not reading it all will make git think the clean filter failed.
> But, git-annex often doesn't need to read the whole content of a
> work-tree file in order to clean it.

This side, I do not think we even need a new variant.  We can just
update the code to interact with "clean" so that it the writer to
the pipe ignores SIGPIPE, detects EPIPE on write(2), says "ah, the
other end does not need the full input to produce its output".  The
reader from the pipe consumes its output without failing AS LONG AS
the "clean" filter exits with zero (we do check its exit status,
right?)

And I think we should do the same for any codepath that spawns
custom script and feeds it via a pipe from us (I am talking about
hooks here).  

What may require a new variant is when your clean filter may not
even need earlier contents of the file, in which case we are better
off not opening the file ourselves and slurping it into core, only
to pipe it and earlier part discarded by the filter.  "clean-from-fs"
filter that gets a path on the working tree and feeds us via pipe
would be appropriate to deal with such a requirement.

> The smudge filter has to output the whole file content to stdout.

We cannot do a similar "we can just unconditionally update" like I
said on the "clean" side to "smudge", so it would need a new
variant.  I do not want to call it anything "raw", as there is
nothing "raw" about it.  "smudge-to-fs" would 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 v2 00/94] libify apply and use lib in am

[Christian Couder]

On Thu, May 12, 2016 at 9:04 PM, Junio C Hamano  wrote:
>
> As Christian said in 00/94, this probably needs to go in steps, as I
> do not think anybody wants to review fouteen rounds of 90+ patch
> series.  I thought the early 40+ patches in the series were at least
> cursory reviewed already?

Yeah, Stefan said he was ok to give his Reviewed-by to v1 patches 01
to 47 and they haven't changed much from v1 to v2.

Here is the diff for those patches:

> git diff 57be628 66c994d
diff --git a/builtin/apply.c b/builtin/apply.c
index 787426f..67c64a5 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -67,13 +67,15 @@ struct apply_state {
/* --numstat does numeric diffstat, and doesn't actually apply */
int numstat;

-   const char *fake_ancestor;
-
int summary;
-
int threeway;
-
int no_add;
+   const char *fake_ancestor;
+   const char *patch_input_file;
+   struct string_list limit_by_name;
+   int has_include;
+   struct strbuf root;
+   struct string_list symlink_changes;

/*
 *  --check turns on checking that the working tree matches the
@@ -85,11 +87,8 @@ struct apply_state {
int check_index;

int unidiff_zero;
-
int update_index;
-
int unsafe_paths;
-
int line_termination;

/*
@@ -113,19 +112,10 @@ struct apply_state {
 */
struct string_list fn_table;

-   struct string_list symlink_changes;
-
int p_value;
int p_value_known;
unsigned int p_context;

-   const char *patch_input_file;
-
-   struct string_list limit_by_name;
-   int has_include;
-
-   struct strbuf root;
-
const char *whitespace_option;
int whitespace_error;
int squelch_whitespace_errors;
@@ -1626,7 +1616,7 @@ static void record_ws_error(struct apply_state *state,
unsigned result,
const char *line,
int len,
-   int l_nr)
+   int linenr)
 {
char *err;

@@ -1640,7 +1630,7 @@ static void record_ws_error(struct apply_state *state,

err = whitespace_error_string(result);
fprintf(stderr, "%s:%d: %s.\n%.*s\n",
-   state->patch_input_file, l_nr, err, len, line);
+   state->patch_input_file, linenr, err, len, line);
free(err);
 }

@@ -2445,7 +2435,7 @@ static int match_fragment(struct apply_state *state,
  int match_beginning, int match_end)
 {
int i;
-   char *fixed_buf, *orig, *target;
+   char *fixed_buf, *buf, *orig, *target;
struct strbuf fixed;
size_t fixed_len, postlen;
int preimage_limit;
@@ -2506,7 +2496,6 @@ static int match_fragment(struct apply_state *state,
 * There must be one non-blank context line that match
 * a line before the end of img.
 */
-   char *buf;
char *buf_end;

buf = preimage->buf;
@@ -4670,10 +4659,10 @@ static int option_parse_directory(const struct
option *opt,
return 0;
 }

-static void init_apply_state(struct apply_state *state, const char *prefix_)
+static void init_apply_state(struct apply_state *state, const char *prefix)
 {
memset(state, 0, sizeof(*state));
-   state->prefix = prefix_;
+   state->prefix = prefix;
state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
state->apply = 1;
state->line_termination = '\n';
--
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 v4 6/7] i18n: builtin/rm.c: remove a comma ',' from string

[Vasco Almeida]

Remove a comma from string marked for translation. Make the string match the
one in builtin/mv.c. Now translators have do handle this string only once.

Signed-off-by: Vasco Almeida 
---
 builtin/rm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 8829b09..be83c43 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -314,7 +314,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
if (list.entry[list.nr++].is_submodule &&
!is_staging_gitmodules_ok())
-   die (_("Please, stage your changes to .gitmodules or 
stash them to proceed"));
+   die (_("Please stage your changes to .gitmodules or 
stash them to proceed"));
}
 
if (pathspec.nr) {
-- 
2.7.3

--
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 v4 4/7] i18n: builtin/pull.c: mark placeholders for translation

[Vasco Almeida]

Some translations might also translate "" and "".

Signed-off-by: Vasco Almeida 
---
 builtin/pull.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 596b92f..96b98ea 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -478,13 +478,13 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
fprintf_ln(stderr, _("Please specify which branch you 
want to merge with."));
fprintf_ln(stderr, _("See git-pull(1) for details."));
fprintf(stderr, "\n");
-   fprintf_ln(stderr, "git pull  ");
+   fprintf_ln(stderr, "git pull %s %s", _(""), 
_(""));
fprintf(stderr, "\n");
} else if (!curr_branch->merge_nr) {
const char *remote_name = NULL;
 
if (for_each_remote(get_only_remote, _name) || 
!remote_name)
-   remote_name = "";
+   remote_name = _("");
 
fprintf_ln(stderr, _("There is no tracking information for the 
current branch."));
if (opt_rebase)
@@ -493,7 +493,7 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
fprintf_ln(stderr, _("Please specify which branch you 
want to merge with."));
fprintf_ln(stderr, _("See git-pull(1) for details."));
fprintf(stderr, "\n");
-   fprintf_ln(stderr, "git pull  ");
+   fprintf_ln(stderr, "git pull %s %s", _(""), 
_(""));
fprintf(stderr, "\n");
fprintf_ln(stderr, _("If you wish to set tracking information 
for this branch you can do so with:\n"
"\n"
-- 
2.7.3

--
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 v4 5/7] i18n: builtin/pull.c: split strings marked for translation

[Vasco Almeida]

Split string "If you wish to set tracking information
for this branch you can do so with:\n" to match occurring string in
git-parse-remote.sh. In this case, the translator handles it only once.

On the other hand, the translations of the string that were already made
are mark as fuzzy and the translator needs to correct it herself.

Signed-off-by: Vasco Almeida 
---
 builtin/pull.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 96b98ea..1d7333c 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -495,10 +495,10 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
fprintf(stderr, "\n");
fprintf_ln(stderr, "git pull %s %s", _(""), 
_(""));
fprintf(stderr, "\n");
-   fprintf_ln(stderr, _("If you wish to set tracking information 
for this branch you can do so with:\n"
-   "\n"
-   "git branch --set-upstream-to=%s/ 
%s\n"),
-   remote_name, curr_branch->name);
+   fprintf_ln(stderr, _("If you wish to set tracking information 
for this branch you can do so with:"));
+   fprintf(stderr, "\n");
+   fprintf_ln(stderr, "git branch --set-upstream-to=%s/%s 
%s\n",
+   remote_name, _(""), curr_branch->name);
} else
fprintf_ln(stderr, _("Your configuration specifies to merge 
with the ref '%s'\n"
"from the remote, but no such ref was fetched."),
-- 
2.7.3

--
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 v4 7/7] i18n: builtin/branch.c: mark option for translation

[Vasco Almeida]

Mark description and parameter for option "set-upstream-to" for translation.

Signed-off-by: Vasco Almeida 
---
 builtin/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0adba62..b7d906d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -630,7 +630,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
BRANCH_TRACK_EXPLICIT),
OPT_SET_INT( 0, "set-upstream",  , N_("change upstream 
info"),
BRANCH_TRACK_OVERRIDE),
-   OPT_STRING('u', "set-upstream-to", _upstream, "upstream", 
"change the upstream info"),
+   OPT_STRING('u', "set-upstream-to", _upstream, 
N_("upstream"), N_("change the upstream info")),
OPT_BOOL(0, "unset-upstream", _upstream, "Unset the 
upstream info"),
OPT__COLOR(_use_color, N_("use colored output")),
OPT_SET_INT('r', "remotes", , N_("act on 
remote-tracking branches"),
-- 
2.7.3

--
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 v4 1/7] i18n: index-pack: use plural string instead of normal one

[Vasco Almeida]

Git could output "completed with 1 local objects", but in this case
using "object" instead of "objects" is the correct form.
Use Q_() instead of _().

Signed-off-by: Vasco Almeida 
---
 builtin/index-pack.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2d1eb8b..e8c71fc 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1250,7 +1250,9 @@ static void conclude_pack(int fix_thin_pack, const char 
*curr_pack, unsigned cha
   nr_unresolved * sizeof(*objects));
f = sha1fd(output_fd, curr_pack);
fix_unresolved_deltas(f);
-   strbuf_addf(, _("completed with %d local objects"),
+   strbuf_addf(, Q_("completed with %d local object",
+"completed with %d local objects",
+nr_objects - nr_objects_initial),
nr_objects - nr_objects_initial);
stop_progress_msg(, msg.buf);
strbuf_release();
-- 
2.7.3

--
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 v4 2/7] i18n: unpack-trees: mark strings for translation

[Vasco Almeida]

Mark strings seen by the user inside setup_unpack_trees_porcelain() and
display_error_msgs() functions for translation.

Signed-off-by: Vasco Almeida 
---
 unpack-trees.c | 74 ++
 1 file changed, 54 insertions(+), 20 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 11308e9..673a04e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -58,40 +58,74 @@ void setup_unpack_trees_porcelain(struct 
unpack_trees_options *opts,
int i;
const char **msgs = opts->msgs;
const char *msg;
-   const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
 
-   if (advice_commit_before_merge)
-   msg = "Your local changes to the following files would be 
overwritten by %s:\n%%s"
-   "Please, commit your changes or stash them before you 
can %s.";
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by checkout:\n%%s"
+ "Please, commit your changes or stash them before you 
can switch branches.")
+ : _("Your local changes to the following files would be 
overwritten by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by merge:\n%%s"
+ "Please, commit your changes or stash them before you 
can merge.")
+ : _("Your local changes to the following files would be 
overwritten by merge:\n%%s");
else
-   msg = "Your local changes to the following files would be 
overwritten by %s:\n%%s";
+   msg = advice_commit_before_merge
+ ? _("Your local changes to the following files would be 
overwritten by %s:\n%%s"
+ "Please, commit your changes or stash them before you 
can %s.")
+ : _("Your local changes to the following files would be 
overwritten by %s:\n%%s");
msgs[ERROR_WOULD_OVERWRITE] = msgs[ERROR_NOT_UPTODATE_FILE] =
-   xstrfmt(msg, cmd, cmd2);
+   xstrfmt(msg, cmd, cmd);
 
msgs[ERROR_NOT_UPTODATE_DIR] =
-   "Updating the following directories would lose untracked files 
in it:\n%s";
-
-   if (advice_commit_before_merge)
-   msg = "The following untracked working tree files would be %s 
by %s:\n%%s"
-   "Please move or remove them before you can %s.";
+   _("Updating the following directories would lose untracked 
files in it:\n%s");
+
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by checkout:\n%%s"
+ "Please move or remove them before you can switch 
branches.")
+ : _("The following untracked working tree files would be 
removed by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by merge:\n%%s"
+ "Please move or remove them before you can merge.")
+ : _("The following untracked working tree files would be 
removed by merge:\n%%s");
else
-   msg = "The following untracked working tree files would be %s 
by %s:\n%%s";
-
-   msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, "removed", cmd, 
cmd2);
-   msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = xstrfmt(msg, 
"overwritten", cmd, cmd2);
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
removed by %s:\n%%s"
+ "Please move or remove them before you can %s.")
+ : _("The following untracked working tree files would be 
removed by %s:\n%%s");
+   msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = xstrfmt(msg, cmd, cmd);
+
+   if (!strcmp(cmd, "checkout"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
overwritten by checkout:\n%%s"
+ "Please move or remove them before you can switch 
branches.")
+ : _("The following untracked working tree files would be 
overwritten by checkout:\n%%s");
+   else if (!strcmp(cmd, "merge"))
+   msg = advice_commit_before_merge
+ ? _("The following untracked working tree files would be 
overwritten by merge:\n%%s"
+ "Please move or remove them before you can merge.")
+ : _("The following untracked working tree files would be 

[PATCH v4 3/7] i18n: git-parse-remote.sh: mark strings for translation

[Vasco Almeida]

Change Makefile to include git-parse-remote.sh in LOCALIZED_SH.

TODO: remove 3rd argument of error_on_missing_default_upstream function
that is no longer required.

Signed-off-by: Vasco Almeida 
---
 Makefile|  2 +-
 git-parse-remote.sh | 46 +-
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index 3f03366..bc3d41e 100644
--- a/Makefile
+++ b/Makefile
@@ -2062,7 +2062,7 @@ XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
--keyword=gettextln --keyword=eval_gettextln
 XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl
 LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
-LOCALIZED_SH = $(SCRIPT_SH)
+LOCALIZED_SH = $(SCRIPT_SH) git-parse-remote.sh
 LOCALIZED_PERL = $(SCRIPT_PERL)
 
 ifdef XGETTEXT_INCLUDE_TESTS
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index 55fe8d5..d3c3998 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -56,11 +56,13 @@ get_remote_merge_branch () {
 error_on_missing_default_upstream () {
cmd="$1"
op_type="$2"
-   op_prep="$3"
+   op_prep="$3" # FIXME: op_prep is no longer used
example="$4"
branch_name=$(git symbolic-ref -q HEAD)
+   display_branch_name="${branch_name#refs/heads/}"
# If there's only one remote, use that in the suggestion
-   remote=""
+   remote="$(gettext "")"
+   branch="$(gettext "")"
if test $(git remote | wc -l) = 1
then
remote=$(git remote)
@@ -68,22 +70,32 @@ error_on_missing_default_upstream () {
 
if test -z "$branch_name"
then
-   echo "You are not currently on a branch. Please specify which
-branch you want to $op_type $op_prep. See git-${cmd}(1) for details.
-
-$example
-"
+   gettextln "You are not currently on a branch."
else
-   echo "There is no tracking information for the current branch.
-Please specify which branch you want to $op_type $op_prep.
-See git-${cmd}(1) for details
-
-$example
-
-If you wish to set tracking information for this branch you can do so with:
-
-git branch --set-upstream-to=$remote/ ${branch_name#refs/heads/}
-"
+   gettextln "There is no tracking information for the current 
branch."
+   fi
+   case "$op_type" in
+   rebase)
+   gettextln "Please specify which branch you want to rebase 
against."
+   ;;
+   merge)
+   gettextln "Please specify which branch you want to merge with."
+   ;;
+   *)
+   echo >&2 "BUG: unknown operation type: $op_type"
+   exit 1
+   ;;
+   esac
+   eval_gettextln "See git-\${cmd}(1) for details."
+   echo
+   echo "$example"
+   echo
+   if test -n "$branch_name"
+   then
+   gettextln "If you wish to set tracking information for this 
branch you can do so with:"
+   echo
+   echo "git branch --set-upstream-to=$remote/$branch 
$display_branch_name"
+   echo
fi
exit 1
 }
-- 
2.7.3

--
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 v4 0/7] i18n miscellaneous updates

[Vasco Almeida]

This re-roll upadates patch
  i18n: unpack-trees: mark strings for translation

I have decoupled/untangled some strings to mark entire sentences instead of
assemble them using placeholders "%s". This makes the translation work
easier and more reliable.

Vasco Almeida (7):
  i18n: index-pack: use plural string instead of normal one
  i18n: unpack-trees: mark strings for translation
  i18n: git-parse-remote.sh: mark strings for translation
  i18n: builtin/pull.c: mark placeholders for translation
  i18n: builtin/pull.c: split strings marked for translation
  i18n: builtin/rm.c: remove a comma ',' from string
  i18n: builtin/branch.c: mark option for translation

 Makefile |  2 +-
 builtin/branch.c |  2 +-
 builtin/index-pack.c |  4 ++-
 builtin/pull.c   | 14 +-
 builtin/rm.c |  2 +-
 git-parse-remote.sh  | 46 
 unpack-trees.c   | 74 ++--
 7 files changed, 96 insertions(+), 48 deletions(-)

-- 
2.7.3

--
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 48/94] builtin/apply: rename 'prefix_' parameter to 'prefix'

[Junio C Hamano]

Christian Couder  writes:

> This is just a small cleanup.

... which may have been better happened at 09/94.

Up to this point, the conversion looks quite sensible, even though I
think the organization of fields in apply_state do not look logical.

--
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 40/94] builtin/apply: move 'ws_error_action' into 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

>  struct apply_state {
>   const char *prefix;
>   int prefix_length;
> @@ -71,6 +78,8 @@ struct apply_state {
>   int whitespace_error;
>   int squelch_whitespace_errors;
>   int applied_after_fixing_ws;
> +
> + enum ws_error_action ws_error_action;

It is very good that these whitespace-error related things are
sitting next to each other.  I do not think the extra blank line is
warranted, though.

If anything, I'd say error-action should come at the beginning (as
it is an end-user input that is set before the processing happens),
and other three are states (that get new values after processing is
done) that should be listed after, to preserve the rough
chronological order the variables are used (which translates to the
order the readers need to become aware of and understand these
variables).
--
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 33/94] builtin/apply: move 'p_value_known' global into 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

> To libify the apply functionality the 'p_value_known' variable should
> not be static and global to the file. Let's move it into
> 'struct apply_state'.

This and p_value belong together, I would think, so this can be
squashed with 32/94 if the series is to be rerolled (this alone is
not a reason to reroll the series, though).

>
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 4e476d5..30eea9c 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -63,13 +63,12 @@ struct apply_state {
>   int line_termination;
>  
>   int p_value;
> + int p_value_known;
>   unsigned int p_context;
>  };
>  
>  static int newfd = -1;
>  
> -static int p_value_known;
> -
>  static const char * const apply_usage[] = {
>   N_("git apply [] [...]"),
>   NULL
> @@ -882,14 +881,14 @@ static void parse_traditional_patch(struct apply_state 
> *state,
>  
>   first += 4; /* skip "--- " */
>   second += 4;/* skip "+++ " */
> - if (!p_value_known) {
> + if (!state->p_value_known) {
>   int p, q;
>   p = guess_p_value(state, first);
>   q = guess_p_value(state, second);
>   if (p < 0) p = q;
>   if (0 <= p && p == q) {
>   state->p_value = p;
> - p_value_known = 1;
> + state->p_value_known = 1;
>   }
>   }
>   if (is_dev_null(first)) {
> @@ -4595,7 +4594,7 @@ static int option_parse_p(const struct option *opt,
>  {
>   struct apply_state *state = opt->value;
>   state->p_value = atoi(arg);
> - p_value_known = 1;
> + state->p_value_known = 1;
>   return 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

Re: [PATCH v2 22/94] builtin/apply: move 'threeway' global into 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

> To libify the apply functionality the 'threeway' variable should
> not be static and global to the file. Let's move it into
> 'struct apply_state'.
>
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 6216723..3650922 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -40,6 +40,7 @@ struct apply_state {
>   int numstat;
>  
>   int summary;
> + int threeway;

This makes threeway look as if it is one of the cosmetic options
like stat/numstat/summary, doesn't it?  If anything, the blank
between numstat and summary should be moved below summary.  I'd
group knobs that affect "what is affected (check, check_index,
cached, etc.)", "how the application is done (allow-overlap,
threeway, in-reverse, etc.)" and "cosmetics" and place the ones in
the same group next to each other.

--
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 18/94] builtin/apply: move 'cached' global into 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

> To libify the apply functionality the 'cached' variable should
> not be static and global to the file. Let's move it into
> 'struct apply_state'.
>
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 37 +
>  1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 8791b28..09af5dc 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -30,6 +30,9 @@ struct apply_state {
>   int apply_with_reject;
>   int apply_verbosely;
>  
> + /* --cached updates only the cache without ever touching the working 
> tree. */
> + int cached;
> +

Again, this should sit right next to update_index and check_index.

Other than that, this step looks correct.
--
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 10/94] builtin/apply: move 'unidiff_zero' global into 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

> To libify the apply functionality the 'unidiff_zero' variable should
> not be static and global to the file. Let's move it into
> 'struct apply_state'.
>
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---

Looks correct; I would have expected from the flow of thought in the
series that p_value and linenr that were already mentioned would be
the first two to be put in this, but that would have to wait until
more functions are taught to pass the state around, so this ordering
would probably be fine.

>  builtin/apply.c | 42 --
>  1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index e133033..44ae95d 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -24,6 +24,8 @@
>  struct apply_state {
>   const char *prefix;
>   int prefix_length;
> +
> + int unidiff_zero;
>  };
>  
>  /*
> @@ -37,7 +39,6 @@ struct apply_state {
>   */
>  static int newfd = -1;
>  
> -static int unidiff_zero;
>  static int state_p_value = 1;
>  static int p_value_known;
>  static int check_index;
> @@ -2694,7 +2695,8 @@ static void update_image(struct image *img,
>   * postimage) for the hunk.  Find lines that match "preimage" in "img" and
>   * replace the part of "img" with "postimage" text.
>   */
> -static int apply_one_fragment(struct image *img, struct fragment *frag,
> +static int apply_one_fragment(struct apply_state *state,
> +   struct image *img, struct fragment *frag,
> int inaccurate_eof, unsigned ws_rule,
> int nth_fragment)
>  {
> @@ -2836,7 +2838,7 @@ static int apply_one_fragment(struct image *img, struct 
> fragment *frag,
>* without leading context must match at the beginning.
>*/
>   match_beginning = (!frag->oldpos ||
> -(frag->oldpos == 1 && !unidiff_zero));
> +(frag->oldpos == 1 && !state->unidiff_zero));
>  
>   /*
>* A hunk without trailing lines must match at the end.
> @@ -2844,7 +2846,7 @@ static int apply_one_fragment(struct image *img, struct 
> fragment *frag,
>* from the lack of trailing lines if the patch was generated
>* with unidiff without any context.
>*/
> - match_end = !unidiff_zero && !trailing;
> + match_end = !state->unidiff_zero && !trailing;
>  
>   pos = frag->newpos ? (frag->newpos - 1) : 0;
>   preimage.buf = oldlines;
> @@ -3067,7 +3069,7 @@ static int apply_binary(struct image *img, struct patch 
> *patch)
>   return 0;
>  }
>  
> -static int apply_fragments(struct image *img, struct patch *patch)
> +static int apply_fragments(struct apply_state *state, struct image *img, 
> struct patch *patch)
>  {
>   struct fragment *frag = patch->fragments;
>   const char *name = patch->old_name ? patch->old_name : patch->new_name;
> @@ -3080,7 +3082,7 @@ static int apply_fragments(struct image *img, struct 
> patch *patch)
>  
>   while (frag) {
>   nth++;
> - if (apply_one_fragment(img, frag, inaccurate_eof, ws_rule, 
> nth)) {
> + if (apply_one_fragment(state, img, frag, inaccurate_eof, 
> ws_rule, nth)) {
>   error(_("patch failed: %s:%ld"), name, frag->oldpos);
>   if (!apply_with_reject)
>   return -1;
> @@ -3388,8 +3390,11 @@ static int load_current(struct image *image, struct 
> patch *patch)
>   return 0;
>  }
>  
> -static int try_threeway(struct image *image, struct patch *patch,
> - struct stat *st, const struct cache_entry *ce)
> +static int try_threeway(struct apply_state *state,
> + struct image *image,
> + struct patch *patch,
> + struct stat *st,
> + const struct cache_entry *ce)
>  {
>   unsigned char pre_sha1[20], post_sha1[20], our_sha1[20];
>   struct strbuf buf = STRBUF_INIT;
> @@ -3415,7 +3420,7 @@ static int try_threeway(struct image *image, struct 
> patch *patch,
>   img = strbuf_detach(, );
>   prepare_image(_image, img, len, 1);
>   /* Apply the patch to get the post image */
> - if (apply_fragments(_image, patch) < 0) {
> + if (apply_fragments(state, _image, patch) < 0) {
>   clear_image(_image);
>   return -1;
>   }
> @@ -3459,7 +3464,8 @@ static int try_threeway(struct image *image, struct 
> patch *patch,
>   return 0;
>  }
>  
> -static int apply_data(struct patch *patch, struct stat *st, const struct 
> cache_entry *ce)
> +static int apply_data(struct apply_state *state, struct patch *patch,
> +   struct stat *st, const struct cache_entry *ce)
>  {
>   struct image image;
>  
> @@ -3467,9 +3473,9 @@ static int apply_data(struct patch 

Re: [PATCH v2 16/94] builtin/apply: move 'update_index' global into 'struct apply_state'

[Junio C Hamano]

Christian Couder  writes:

> To libify the apply functionality the 'update_index' variable should
> not be static and global to the file. Let's move it into
> 'struct apply_state'.
>
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 45 ++---
>  1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 97af6ea..635a9ff 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -39,6 +39,7 @@ struct apply_state {
>   int check_index;
>  
>   int unidiff_zero;
> + int update_index;

This should sit right next to check_index, I would think.

Otherwise looks correct.
--
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 09/94] builtin/apply: move 'state' init into init_apply_state()

[Junio C Hamano]

Christian Couder  writes:

> When the apply functionality will be libified, the 'struct apply_state'
> will be used by different pieces of code.
>
> To properly initialize a 'struct apply_state', let's provide a nice
> and easy to use init_apply_state() function.

This probably should be done at 08/94, but I'll let it pass for now.
I am hoping 'prefix' is just one of a very few parameters this
function needs to take to initialize this (I do not think we want to
feed many different parameters to this function to initialize).

Will queue.

> Helped-by: Eric Sunshine 
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index ae068e7..e133033 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -4522,6 +4522,19 @@ static int option_parse_directory(const struct option 
> *opt,
>   return 0;
>  }
>  
> +static void init_apply_state(struct apply_state *state, const char *prefix)
> +{
> + memset(state, 0, sizeof(*state));
> + state->prefix = prefix;
> + state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
> +
> + git_apply_config();
> + if (apply_default_whitespace)
> + parse_whitespace_option(apply_default_whitespace);
> + if (apply_default_ignorewhitespace)
> + parse_ignorewhitespace_option(apply_default_ignorewhitespace);
> +}
> +
>  int cmd_apply(int argc, const char **argv, const char *prefix_)
>  {
>   int i;
> @@ -4603,15 +4616,7 @@ int cmd_apply(int argc, const char **argv, const char 
> *prefix_)
>   OPT_END()
>   };
>  
> - memset(, 0, sizeof(state));
> - state.prefix = prefix_;
> - state.prefix_length = state.prefix ? strlen(state.prefix) : 0;
> -
> - git_apply_config();
> - if (apply_default_whitespace)
> - parse_whitespace_option(apply_default_whitespace);
> - if (apply_default_ignorewhitespace)
> - parse_ignorewhitespace_option(apply_default_ignorewhitespace);
> + init_apply_state(, prefix_);
>  
>   argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
>   apply_usage, 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

Re: [PATCH v2 05/94] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

[Junio C Hamano]

Christian Couder  writes:

> The match_fragment() function is very big and contains a big special case
> algorithm that does line by line fuzzy matching. So let's extract this
> algorithm in a separate line_by_line_fuzzy_match() function.
>
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---

Looks good.

I am puzzled by the "git blame -w -M" output after applying this
patch, though.  The body of the new function came more or less
verbatim from the origina with reindentation.
--
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 03/94] builtin/apply: avoid parameter shadowing 'linenr' global

[Junio C Hamano]

Christian Couder  writes:

> Let's just rename the global 'state_linenr' as it will become
> 'state->linenr' in a following patch.
>
> This also avoid errors when compiling with -Wshadow and makes
> it safer to later move global variables into a "state" struct.

Looks correctly done (I looked at remaining instances of "linenr").

Will queue.  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 v2 02/94] builtin/apply: avoid parameter shadowing 'p_value' global

[Junio C Hamano]

Christian Couder  writes:

> Let's just rename the global 'state_p_value' as it will become
> 'state->p_value' in a following patch.
>
> This also avoid errors when compiling with -Wshadow and makes
> it safer to later move global variables into a "state" struct.

Looks correctly done (I looked at remaining instances of "p_value").

Will queue.  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 v2 01/94] builtin/apply: make gitdiff_verify_name() return void

[Junio C Hamano]

Christian Couder  writes:

> As the value returned by gitdiff_verify_name() is put into the
> same variable that is passed as a parameter to this function,
> it is simpler to pass the address of the variable and have
> gitdiff_verify_name() change the variable itself.
>
> This also makes it possible to later have this function return
> -1 instead of die()ing in case of error.
>
> Reviewed-by: Stefan Beller 
> Signed-off-by: Christian Couder 
> ---

While I do not agree with "it is simpler" at all (it makes it harder
for callers if they ever want to use different variables--they would
need to invent a temporary variable just for that, and it makes it
no difference for existing callers that happen to use the same
variable), I like the way the real reason why we want to do this is
mentioned in the proposed log message.

Will queue.

>  builtin/apply.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 8e4da2e..fe5aebd 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -925,43 +925,43 @@ static int gitdiff_hdrend(const char *line, struct 
> patch *patch)
>  #define DIFF_OLD_NAME 0
>  #define DIFF_NEW_NAME 1
>  
> -static char *gitdiff_verify_name(const char *line, int isnull, char 
> *orig_name, int side)
> +static void gitdiff_verify_name(const char *line, int isnull, char **name, 
> int side)
>  {
> - if (!orig_name && !isnull)
> - return find_name(line, NULL, p_value, TERM_TAB);
> + if (!*name && !isnull) {
> + *name = find_name(line, NULL, p_value, TERM_TAB);
> + return;
> + }
>  
> - if (orig_name) {
> - int len = strlen(orig_name);
> + if (*name) {
> + int len = strlen(*name);
>   char *another;
>   if (isnull)
>   die(_("git apply: bad git-diff - expected /dev/null, 
> got %s on line %d"),
> - orig_name, linenr);
> + *name, linenr);
>   another = find_name(line, NULL, p_value, TERM_TAB);
> - if (!another || memcmp(another, orig_name, len + 1))
> + if (!another || memcmp(another, *name, len + 1))
>   die((side == DIFF_NEW_NAME) ?
>   _("git apply: bad git-diff - inconsistent new 
> filename on line %d") :
>   _("git apply: bad git-diff - inconsistent old 
> filename on line %d"), linenr);
>   free(another);
> - return orig_name;
>   } else {
>   /* expect "/dev/null" */
>   if (memcmp("/dev/null", line, 9) || line[9] != '\n')
>   die(_("git apply: bad git-diff - expected /dev/null on 
> line %d"), linenr);
> - return NULL;
>   }
>  }
>  
>  static int gitdiff_oldname(const char *line, struct patch *patch)
>  {
> - patch->old_name = gitdiff_verify_name(line, patch->is_new, 
> patch->old_name,
> -   DIFF_OLD_NAME);
> + gitdiff_verify_name(line, patch->is_new, >old_name,
> + DIFF_OLD_NAME);
>   return 0;
>  }
>  
>  static int gitdiff_newname(const char *line, struct patch *patch)
>  {
> - patch->new_name = gitdiff_verify_name(line, patch->is_delete, 
> patch->new_name,
> -   DIFF_NEW_NAME);
> + gitdiff_verify_name(line, patch->is_delete, >new_name,
> + DIFF_NEW_NAME);
>   return 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

Re: [PATCH v2 00/94] libify apply and use lib in am

[Junio C Hamano]

Johannes Sixt  writes:

> I'll also use it in production for a while, although I am not a git-am
> consumer nor do I use git-rebase without -i, hence, my tests will
> probably only show that there is no bad fall-out.

It will probably only show that you do not use the part that was
touched by the series ;-)

As Christian said in 00/94, this probably needs to go in steps, as I
do not think anybody wants to review fouteen rounds of 90+ patch
series.  I thought the early 40+ patches in the series were at least
cursory reviewed already?
--
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: proposal for extending smudge/clean filters with raw file access

[Junio C Hamano]

Joey Hess  writes:

> I'm using smudge/clean filters in git-annex now, and it's not been an
> entirely smooth fit between the interface and what git-annex wants
> to do.
>
> The clean filter has to consume the whole file content on stdin;
> not reading it all will make git think the clean filter failed.
> But, git-annex often doesn't need to read the whole content of a
> work-tree file in order to clean it.
>
> The smudge filter has to output the whole file content to stdout. But
> git-annex often has the file's content on disk already, and could just
> move it into place in the working tree. This would save CPU and IO and
> often disk space too. But the smudge interface doesn't let git-annex use
> the efficient approach.

The smudge happens to be the last to run, so it is quite true that
it can say "Hey Git, I've written it out already".

I didn't check all codepaths to ensure that we won't need the
smudged result in core at all, but I am guessing you did so before
making this proposal, so in that case I would say this sounds fine.
--
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

proposal for extending smudge/clean filters with raw file access

[Joey Hess]

I'm using smudge/clean filters in git-annex now, and it's not been an
entirely smooth fit between the interface and what git-annex wants
to do.

The clean filter has to consume the whole file content on stdin;
not reading it all will make git think the clean filter failed.
But, git-annex often doesn't need to read the whole content of a
work-tree file in order to clean it.

The smudge filter has to output the whole file content to stdout. But
git-annex often has the file's content on disk already, and could just
move it into place in the working tree. This would save CPU and IO and
often disk space too. But the smudge interface doesn't let git-annex use
the efficient approach.

So I propose extending the filter driver with two more optional
commands. Call them raw-clean and raw-smudge for now.

raw-clean would be like clean, but rather than being fed the whole
content of a large file on stdin, it would be passed the filename, and
can access the file itself. Like the clean filter, it outputs the
cleaned version on stdout.

raw-smudge would be like smudge, but rather than needing to output the
whole content of a large file on stdout, it would be passed a filename,
and can create that file itself.

To keep this backwards compatible, and to handle the cases where the
object being filtered is not a file on disk, the smudge and clean
filters would be required to be configured too, in order for raw-clean
and raw-smudge to be used.

It seems fairly easy to implement raw-clean. In sha1_file.c, index_path
would use raw-clean when available, while index_fd etc keep on using
the clean filter. I have not investigated what would be needed to implement
raw-smudge yet.

-- 
see shy jo
--
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: t0008 test fails with ksh

[Jeff King]

On Thu, May 12, 2016 at 07:53:28PM +0200, Armin Kunaschik wrote:

> The reason seems that the snippet
> cat <<-EOF >expected-all
> .gitignore:1:one../one
> ::  ../not-ignored
> .gitignore:1:oneone
> ::  not-ignored
> a/b/.gitignore:8:!on*   b/on
> a/b/.gitignore:8:!on*   b/one
> a/b/.gitignore:8:!on*   b/one one
> a/b/.gitignore:8:!on*   b/one two
> a/b/.gitignore:8:!on*   "b/one\"three"
> a/b/.gitignore:9:!two   b/two
> ::  b/not-ignored
> a/.gitignore:1:two* b/twooo
> $global_excludes:2:!globaltwo   ../globaltwo
> $global_excludes:2:!globaltwo   globaltwo
> $global_excludes:2:!globaltwo   b/globaltwo
> $global_excludes:2:!globaltwo   ../b/globaltwo
> ::  c/not-ignored
> EOF
> 
> behaves differently in bash and in ksh.
> a/b/.gitignore:8:!on*   "b/one\"three" comes out unmodified
> with bash but with ksh it becomes
> a/b/.gitignore:8:!on*   "b/one"three"
> I'm not sure what shell is "wrong" here, but when I modify the line to
> a/b/.gitignore:8:!on*   "b/one\\"three"
> both shells generate the "right" output:
> a/b/.gitignore:8:!on*   "b/one\"three"
> 
> From what I've learned I'd expect the double backslash to be the
> "right" way to escape one
> backslash. Do you agree or am I wrong?
> 
> This snippet appears twice in this test, generates expected-all and
> expected-verbose.

I think either is reasonable (there is no need to backslash-escape a
double-quote inside a here-doc, but one assumes that backslash would
generally have its usual behavior). I'm not quite sure how to interpret
POSIX here (see below), but it seems clear that spelling it with two
backslashes as you suggest is the best bet.

For the curious, here's what POSIX says (in the section on
here-documents; "word" here refers to the EOF word):

  If no characters in word are quoted, all lines of the here-document
  shall be expanded for parameter expansion, command substitution, and
  arithmetic expansion. In this case, the backslash in the input behaves
  as the backslash inside double-quotes (see Double-Quotes). However,
  the double-quote character ( '"' ) shall not be treated specially
  within a here-document, except when the double-quote appears within
  "$()", "``", or "${}".

So OK, that sounds like ksh is doing the right thing. But what's that
"specially" in the last sentence? Do they just mean it doesn't start a
quoted section as it would elsewhere? Or do they mean in the section on
Double-Quotes, when they say:

  \
  The backslash shall retain its special meaning as an escape
  character (see Escape Character (Backslash)) only when followed by
  one of the following characters when considered special:

  $   `   "   \   

Since the double-quote isn't special here, does that mean that backslash
shouldn't retain its special meaning in this case?  That would make bash
right.

Anyway, it doesn't really matter what the standard says. We can spell
this in a less ambiguous way, and it does not hurt too much to do so.

-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 v2 00/94] libify apply and use lib in am

[Christian Couder]

On Thu, May 12, 2016 at 7:06 PM, Johannes Sixt  wrote:
> Am 11.05.2016 um 15:16 schrieb Christian Couder:
>>
>> This is a patch series about libifying `git apply` functionality, and
>> using this libified functionality in `git am`, so that no 'git apply'
>> process is spawn anymore. This makes `git am` significantly faster, so
>> `git rebase`, when it uses the am backend, is also significantly
>> faster.
>
>
> I'm including this in my build on Windows. It passes the test suite.

Great! Thanks for the report!

> I'll also use it in production for a while, although I am not a git-am
> consumer nor do I use git-rebase without -i, hence, my tests will probably
> only show that there is no bad fall-out.

Thanks anyway,
Christian.
--
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

t0008 test fails with ksh

[Armin Kunaschik]

Hello,

in t0008 I see tests fails with
not ok 374 - --stdin -v
#
#   expect_from_stdin expected-all
.gitignore:1:one../one
::  ../not-ignored
.gitignore:1:oneone
::  not-ignored
a/b/.gitignore:8:!on*   b/on
a/b/.gitignore:8:!on*   b/one
a/b/.gitignore:8:!on*   b/one one
a/b/.gitignore:8:!on*   b/one two
a/b/.gitignore:8:!on*   "b/one\"three"
a/b/.gitignore:9:!two   b/two
::  b/not-ignored
a/.gitignore:1:two* b/twooo
$global_excludes:2:!globaltwo   ../globaltwo
$global_excludes:2:!globaltwo   globaltwo
$global_excludes:2:!globaltwo   b/globaltwo
$global_excludes:2:!globaltwo   ../b/globaltwo
::  c/not-ignored
EOF

behaves differently in bash and in ksh.
a/b/.gitignore:8:!on*   "b/one\"three" comes out unmodified
with bash but with ksh it becomes
a/b/.gitignore:8:!on*   "b/one"three"
I'm not sure what shell is "wrong" here, but when I modify the line to
a/b/.gitignore:8:!on*   "b/one\\"three"
both shells generate the "right" output:
a/b/.gitignore:8:!on*   "b/one\"three"

>From what I've learned I'd expect the double backslash to be the
"right" way to escape one
backslash. Do you agree or am I wrong?

This snippet appears twice in this test, generates expected-all and
expected-verbose.

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] Documentation: clarify signature verification v2

[Pranit Bauva]

On Thu, May 12, 2016 at 10:08 PM, Junio C Hamano  wrote:
> Pranit Bauva  writes:
>
>> Seems like Junio was waiting for someone to point this out[2].
>
> Thanks. I think you covered most of them correctly; I only have one
> thing to add.
>
>>  * Comments are put after ---. So your paragraph
>>   "Clarify which commits need to be signed.
>>
>> Uniformise the vocabulary used wrt. key/signature validity with 
>> OpenPGP
>>  - a signature is valid if made by a key with a valid uid;
>>  - in the default trust-model, a uid is valid if signed by a
>> trusted key;
>>  - a key is trusted if the (local) user set a trust level for it.
>>
>>Thanks to Junio C Hamano  for reviewing
>>the first attempt at this patch."
>>
>> is actually treated as a comment.
>
> This is half-true, I think. The message you are responding to had
> only two dashes, not three.

My bad. Didn't carefully notice it.

>
> The usual way to do what the original wanted to do is like this:
>
> ... e-mail headers like From:, Subject:, ...
>
> Hi,
>
> Here is a second attempt.
>
> -- >8 --
> Subject: Documentation: clarify --verify signature
>
> Clarify that only the signature of the commit at the tip of
> the branch being merged is checked.  Also align the
> vocabulary to describe key & signature validity with those
> used by OpenPGP, namely:
>
>  - a signature is valid if ...
>  ...
>  - a key is trusted if ...
>
> Signed-off-by: A U Thor 
> ---
>  Documentation/merge-options.txt | ... diffstat comes here
>
> Notice the "-- >8 --" (cut here) line.  "am" will notice it, discard
> what it has read so far and restart from there.

Not having used this personally, I forgot to mention this. Thanks for
mentioning it!
Looks like you have written the commit message for him. :)
--
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/94] libify apply and use lib in am

[Johannes Sixt]

Am 11.05.2016 um 15:16 schrieb Christian Couder:

This is a patch series about libifying `git apply` functionality, and
using this libified functionality in `git am`, so that no 'git apply'
process is spawn anymore. This makes `git am` significantly faster, so
`git rebase`, when it uses the am backend, is also significantly
faster.


I'm including this in my build on Windows. It passes the test suite.

I'll also use it in production for a while, although I am not a git-am 
consumer nor do I use git-rebase without -i, hence, my tests will 
probably only show that there is no bad fall-out.


-- Hannes

--
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:

> We can still keep the submodule.defaultGroup. (In the WIP I renamed
> it to updateGroup as its only feature is to have it set during clone
> and remebered for `git submodule update`)

Yes, my idle speculation between "[submodule"x"] label=A" stored
in .gitmodules and "path/to/submodule  group=A" in .gitattributes
is completely orthogonal to the need for submodule.defaultGroup.

The only difference it brings in is how we evaluate what
submodule.defaultGroup names (i.e. via "submodule label" vs
"path label").

> When we allow labels to be generic path labels instead of submodule
> labels, the user might be tempted to ask, why the submodules can
> be specified but not the individual paths, i.e.
>
> git clone --init-submodule="(:group=docs)" ...
>
> may strongly hint at:
>
> git clone --narrow="(:group=docs)" ...
>
> would only get parts of the repository.

If we are to have a (proper) narrow clone, I do not think there is
any reason why the latter should not work as expected.

> For the submodule case, this may add confusion as the user would
> need to configure some properties in the .gitmodules file and some
> in the .gitattributes file.

I do not quite understand this comment.

A new reader who knows nothing about "grouping submodules" would not
have any trouble, I suspect, if all and the only grouping she learns
is "grouping paths" from the beginning.

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] Documentation: clarify signature verification v2

[Junio C Hamano]

Pranit Bauva  writes:

> Seems like Junio was waiting for someone to point this out[2].

Thanks. I think you covered most of them correctly; I only have one
thing to add.

>  * Comments are put after ---. So your paragraph
>   "Clarify which commits need to be signed.
>
> Uniformise the vocabulary used wrt. key/signature validity with 
> OpenPGP
>  - a signature is valid if made by a key with a valid uid;
>  - in the default trust-model, a uid is valid if signed by a
> trusted key;
>  - a key is trusted if the (local) user set a trust level for it.
>
>Thanks to Junio C Hamano  for reviewing
>the first attempt at this patch."
>
> is actually treated as a comment.

This is half-true, I think. The message you are responding to had
only two dashes, not three.

The usual way to do what the original wanted to do is like this:

... e-mail headers like From:, Subject:, ...

Hi,

Here is a second attempt.

-- >8 --
Subject: Documentation: clarify --verify signature

Clarify that only the signature of the commit at the tip of
the branch being merged is checked.  Also align the
vocabulary to describe key & signature validity with those
used by OpenPGP, namely:

 - a signature is valid if ...
 ...
 - a key is trusted if ...

Signed-off-by: A U Thor 
---
 Documentation/merge-options.txt | ... diffstat comes here

Notice the "-- >8 --" (cut here) line.  "am" will notice it, discard
what it has read so far and restart from there.
--
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

[Stefan Beller]

On Thu, May 12, 2016 at 8:58 AM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> The reason why I suspect that this may not work well with submodule
>> labels is because submodule labels (or any attribute we give via
>> .gitmodules to a submodule) are keyed by a submodule name, which is
>> the primary unchanging key (so that people can "mv" a submodule in
>> the context of the toplevel superproject without losing track of
>> submodule identity), not by paths to submodules, while the "group"
>> thing I want is merely a short-hand for pathspec elements and wants
>> to be keyed by paths.
>>
>> But there may be somebody more clever than I who can come up with a
>> way to unify these two similar concepts without confusing end users.
>
> Thinking about this even more, if there is no requirement that
> labels must be tied to submodule names, we just can scrap the idea
> of "submodule labels" to group things and instead just use "path
> labels", i.e. write the full path to the submodule and assign it a
> label in .gitattributes and use it in place of where we used *label
> in the patch series.  After all, an easy way to choose some among
> many submodules logically is a subset of an easy way to choose some
> among many paths.
>
> The only reason why we added the submodule label to .gitmodules is
> because we viewed it as submodule-specific thing and the "keyed by
> name, not path" came as a consequence, not because any real "we must
> key it by name because..." reason, I would think.
>
> I know this is a huge departure from the design presented both at
> the conceptual level and also at the implementation level, and that
> is one of the reasons why I do not particularly want to like it, but
> on the other hand, I am not bold enough to say that I will have a
> good answer when later somebody asks "Why can we group only
> submodules with labels, but not random group of paths (e.g. 'these
> directories are about documentation')?"  And then, if we add path
> labels to allow expressing groups of paths later, the follow-up
> question would be "When should I use submodule labels and when
> should I use path labels?  I can use path labels to group submodules
> and say 'git submodule update -- :(group=docs)' can't I?".
>
> And that makes me pause and step back from the "submodule labels"
> idea.
>

It sounds better at first (and I haven't thought further).
So if we were to go with this idea:

Label paths (or even pathspecs?) in the .gitattributes file.

I think it is important to keep the property of defining the labeling
in the tree, so you can follow upstream easier.

I tried coming up with an example for labels for generic paths,
but it looks like most of the time it is a substitution for a pathspec,
I did not find a convincing example which makes it easier to use.

`:(group=docs)` in the non submodule case could be expressed
as `Documentation/**`. Well maybe we also want to include README
and some other files which need to stay outside the Documentation
directory, so I can see how it may be useful.

We do not need a special labeling command. We do not
ship with a command which writes the .gitattributes or .gitignore
for you, and labels don't require this. So I could drop the patch
for "submodule add --label".

We can still keep the submodule.defaultGroup. (In the WIP I renamed
it to updateGroup as its only feature is to have it set during clone
and remebered for `git submodule update`)

When we allow labels to be generic path labels instead of submodule
labels, the user might be tempted to ask, why the submodules can
be specified but not the individual paths, i.e.

git clone --init-submodule="(:group=docs)" ...

may strongly hint at:

git clone --narrow="(:group=docs)" ...

would only get parts of the repository.

For the submodule case, this may add confusion as the user would
need to configure some properties in the .gitmodules file and some
in the .gitattributes file.

I think I'll try implementing a mock and see how much code it is for
a more fundamental pathspec extension.

Thanks,
Stefan
--
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] Work on t3404 in preparation for rebase--helper

[Junio C Hamano]

I took these separately already, and plan to fast-track them as they
are both "trivially correct"; I double checked that what I have
match these two, too.

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 v2 00/33] Yet more preparation for reference backends

[Junio C Hamano]

Jeff King  writes:

> My trick for checking the before/after of a patch is:
>
>   1. Compile git without the patch.
>
>   2. Apply the patch, then run the test (via ./t1234-*, which does not
>  want to re-build git), confirm that it fails.
>
>   3. Re-compile and re-run the test, confirming that it passes.
>
> That also works well with "rebase -i" where you stop at the patch before
> to compile.
>
> I like it because it's simple and doesn't affect git's view (so you
> can't accidentally commit the in-work-tree revert, for example). But
> since there's nothing telling you what state the compiled git is in, it
> can be easy to get confused.

True.  It also would not work well with debuggers, but it is usually
rare to need a debugger to verify the claim of a "fix" patch, so I
think the above is easier to use in practice.

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 2/2] am: plug FILE * leak in split_mail_conv()

[Junio C Hamano]

Jeff King  writes:

> Presumably `fclose` doesn't ever overwrite errno in practice, but I
> guess it could in theory.

Yeah, these two patches share the same 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

[PATCH v2 2/2] test-lib: set BASH_XTRACEFD automatically

[Johannes Schindelin]

From: Jeff King 

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 simple one-liner, but note the caveats
in the accompanying comments.

Automatic tests for our "-x" option may be a bit too meta
(and a pain, because they are bash-specific), but I did
confirm that it works correctly both with regular "-x" and
with "--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 
Signed-off-by: Johannes Schindelin 
---
 t/README  |  6 +++---
 t/test-lib.sh | 13 +
 2 files changed, 16 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..0055ebb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -322,6 +322,19 @@ else
exec 4>/dev/null 3>/dev/null
 fi
 
+# Send any "-x" output directly to stderr to avoid polluting tests
+# which capture stderr. We can do this unconditionally since it
+# has no effect if tracing isn't turned on.
+#
+# Note that this sets up the trace fd as soon as we assign the variable, so it
+# must come after the creation of descriptor 4 above. Likewise, we must never
+# unset this, as it has the side effect of closing descriptor 4, which we
+# use to show verbose tests to the user.
+#
+# Note also that we don't need or want to export it. The tracing is local to
+# this shell, and we would not want to influence any shells we exec.
+BASH_XTRACEFD=4
+
 test_failure=0
 test_count=0
 test_fixed=0
-- 
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 v2 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.


Jeff King (1):
  test-lib: set BASH_XTRACEFD automatically

Johannes Schindelin (1):
  t3404: fix typo

 t/README  |  6 +++---
 t/t3404-rebase-interactive.sh |  2 +-
 t/test-lib.sh | 13 +
 3 files changed, 17 insertions(+), 4 deletions(-)

Published-As: https://github.com/dscho/git/releases/tag/t3404-fixes-v2
Interdiff vs v1:

 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/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
 index 25f1118..66348f1 100755
 --- a/t/t3404-rebase-interactive.sh
 +++ b/t/t3404-rebase-interactive.sh
 @@ -882,8 +882,7 @@ 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 "/option .exec. requires a value/d" -e '/^+/d' \
 -  tmp >actual &&
 +  sed -e "1d" tmp >actual &&
test_must_fail git rebase -h >expected &&
test_cmp expected actual &&
git checkout master
 @@ -1150,6 +1149,10 @@ test_expect_success 'drop' '
test A = $(git cat-file commit HEAD^^ | sed -ne \$p)
  '
  
 +cat >expect expect expect expect expect /dev/null 3>/dev/null
  fi
  
 +# Send any "-x" output directly to stderr to avoid polluting tests
 +# which capture stderr. We can do this unconditionally since it
 +# has no effect if tracing isn't turned on.
 +#
 +# Note that this sets up the trace fd as soon as we assign the variable, so it
 +# must come after the creation of descriptor 4 above. Likewise, we must never
 +# unset this, as it has the side effect of closing descriptor 4, which we
 +# use to show verbose tests to the user.
 +#
 +# Note also that we don't need or want to export it. The tracing is local to
 +# this shell, and we would not want to influence any shells we exec.
 +BASH_XTRACEFD=4
 +
  test_failure=0
  test_count=0
  test_fixed=0

-- 
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 v2 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.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 0/7] submodule groups

[Junio C Hamano]

Junio C Hamano  writes:

> The reason why I suspect that this may not work well with submodule
> labels is because submodule labels (or any attribute we give via
> .gitmodules to a submodule) are keyed by a submodule name, which is
> the primary unchanging key (so that people can "mv" a submodule in
> the context of the toplevel superproject without losing track of
> submodule identity), not by paths to submodules, while the "group"
> thing I want is merely a short-hand for pathspec elements and wants
> to be keyed by paths.
>
> But there may be somebody more clever than I who can come up with a
> way to unify these two similar concepts without confusing end users.

Thinking about this even more, if there is no requirement that
labels must be tied to submodule names, we just can scrap the idea
of "submodule labels" to group things and instead just use "path
labels", i.e. write the full path to the submodule and assign it a
label in .gitattributes and use it in place of where we used *label
in the patch series.  After all, an easy way to choose some among
many submodules logically is a subset of an easy way to choose some
among many paths.

The only reason why we added the submodule label to .gitmodules is
because we viewed it as submodule-specific thing and the "keyed by
name, not path" came as a consequence, not because any real "we must
key it by name because..." reason, I would think.

I know this is a huge departure from the design presented both at
the conceptual level and also at the implementation level, and that
is one of the reasons why I do not particularly want to like it, but
on the other hand, I am not bold enough to say that I will have a
good answer when later somebody asks "Why can we group only
submodules with labels, but not random group of paths (e.g. 'these
directories are about documentation')?"  And then, if we add path
labels to allow expressing groups of paths later, the follow-up
question would be "When should I use submodule labels and when
should I use path labels?  I can use path labels to group submodules
and say 'git submodule update -- :(group=docs)' can't I?".

And that makes me pause and step back from the "submodule labels"
idea.



--
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] test-lib: set BASH_XTRACEFD automatically

[Johannes Schindelin]

Hi Peff,

On Wed, 11 May 2016, Jeff King wrote:

> Subject: test-lib: set BASH_XTRACEFD automatically

I confirm that this simple weird trick obsoletes my painstakingly
developed patch ;-)

To make it easy for everybody involved, I'll just go ahead and send out
the next iteration with your patch.

Thanks,
Dscho
--
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] diff: run arguments through precompose_argv

[Junio C Hamano]

Alexander Rinass  writes:

> I will create a v2 patch until the weekend and send it to the mailing list.

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 0/7] submodule groups

[Stefan Beller]

On Wed, May 11, 2016 at 10:50 PM, Junio C Hamano  wrote:
>
> git cmd -- \*.perl \*.pl \*.pm
>
> I've often wondered if it would be a good idea to let attributes
> file to specify "these paths form the group Perl" with something
> like:
>
> *.pmgroup=perl
> *.plgroup=perl
> *.perl  group=perl
> *.h group=c
> *.c group=c
>
> and say
>
> git cmd -- ':(group=perl)'
>
> instead.

How is that different to the file size example I gave earlier?
You could have a change which includes:

-*.plgroup=prolog
+*.pl   group=perl

What happens in the diff/log case then (just as a file can pass an
arbitrary file size within that change) ? These file attributes are as
arbitrary as sizes as well as submodule labels or names.
--
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

URGENT RESPONSE NEEDED, PLEASE REPLY....

[Mr. Ragner Henderson]

Dear Friend,

Pardon me for not having the pleasure of knowing your mindset before making you 
this offer and it is utterly confidential and genuine by virtue of its nature I 
write to solicit your assistance in a funds transfer deal involving £15.2M.This 
fund has been stashed out of the excess profit made last 2years by my branch 
office of  the Co-operative Bank Plc here in United Kingdom which I am the 
manager.

I have already submitted an approved end-of-the-year report for the year 2015 
to my head office here in Manchester UK. and they will never know of this 
excess.

I have since then, placed this amount on a Non-Investment Account without a 
beneficiary. Upon your response, I will configure your name on our database as 
holder of the Non-Investment Account. I will then guide you on how to apply to 
my head office for the Account Closure/bank-to-bank remittance of the funds to 
your designated bank account.
If you concur with this proposal, I intend for you to retain 30% of the funds 
while 70% shall be for me. Kindly forward your response to me immediately to my 
private mail box: (mr.ragnerhender...@yahoo.com) thank you.

With Regards,

Mr. Ragner Henderson.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

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