[PATCH 06/11] completion: zsh: trivial cleanups

[Felipe Contreras]

We don't need to override IFS, zsh has a native way of splitting by new
lines: the expansion flag (f).

Also, we don't need to split files by ':' or '='; that's only for words.

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.zsh | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index e10df7d..317b8eb 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -65,26 +65,22 @@ __gitcomp_nl ()
 {
emulate -L zsh
 
-   local IFS=$'\n'
compset -P '*[=:]'
-   compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+   compadd -Q -S "${4- }" -p "${2-}" -- ${(f)1} && _ret=0
 }
 
 __gitcomp_nl_append ()
 {
emulate -L zsh
 
-   local IFS=$'\n'
-   compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+   compadd -Q -S "${4- }" -p "${2-}" -- ${(f)1} && _ret=0
 }
 
 __gitcomp_file ()
 {
emulate -L zsh
 
-   local IFS=$'\n'
-   compset -P '*[=:]'
-   compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+   compadd -Q -p "${2-}" -f -- ${(f)1} && _ret=0
 }
 
 __git_zsh_bash_func ()
-- 
2.8.0+fc1

--
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 04/11] completion: zsh: don't hide ourselves

[Felipe Contreras]

There's no need to hide the fact that we are on zsh any more.

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index e255413..475705a 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -39,7 +39,7 @@ if [ -z "$script" ]; then
test -f $e && script="$e" && break
done
 fi
-ZSH_VERSION='' . "$script"
+. "$script"
 
 __gitcomp ()
 {
-- 
2.8.0+fc1

--
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 09/11] completion: zsh: fix for directories with spaces

[Felipe Contreras]

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 1f786cc..28eaaed 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -24,7 +24,7 @@ if [ -z "$script" ]; then
local -a locations
local e
locations=(
-   $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
+   "$(dirname ${funcsourcetrace[1]%:*})"/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu, 
new debian
'/usr/share/bash-completion/git' # gentoo
-- 
2.8.0+fc1

--
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 05/11] completion: remove zsh hack

[Felipe Contreras]

We don't want to override the 'complete()' function in zsh, which can be
used by bashcomp.

Reported-by: Mark Lodato 
Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.bash | 1 +
 contrib/completion/git-completion.zsh  | 6 --
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 9cbae6f..6c338ae 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2684,6 +2684,7 @@ __git_func_wrap ()
 # This is NOT a public function; use at your own risk.
 __git_complete ()
 {
+   test -n "$ZSH_VERSION" && return
local wrapper="__git_wrap${2}"
eval "$wrapper () { __git_func_wrap $2 ; }"
complete -o bashdefault -o default -o nospace -F $wrapper $1 
2>/dev/null \
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 475705a..e10df7d 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -16,12 +16,6 @@
 #
 #  fpath=(~/.zsh $fpath)
 
-complete ()
-{
-   # do nothing
-   return 0
-}
-
 zstyle -T ':completion:*:*:git:*' tag-order && \
zstyle ':completion:*:*:git:*' tag-order 'common-commands'
 
-- 
2.8.0+fc1

--
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 07/11] completion: bash: cleanup cygwin check

[Felipe Contreras]

Avoid Yoda conditions, use test, and cleaner statement.

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.bash | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 6c338ae..398f3a7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2698,6 +2698,5 @@ __git_complete gitk __gitk_main
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
-if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
+test "$(uname -o 2>/dev/null)" = "Cygwin" &&
 __git_complete git.exe __git_main
-fi
-- 
2.8.0+fc1

--
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 03/11] completion: bash: remove zsh wrapper

[Felipe Contreras]

It has been deprecated for more than three years. It's time to move on.

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.bash | 64 --
 1 file changed, 64 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 5e2e590..9cbae6f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2672,70 +2672,6 @@ __gitk_main ()
__git_complete_revlist
 }
 
-if [[ -n ${ZSH_VERSION-} ]]; then
-   echo "WARNING: this script is deprecated, please see 
git-completion.zsh" 1>&2
-
-   autoload -U +X compinit && compinit
-
-   __gitcomp ()
-   {
-   emulate -L zsh
-
-   local cur_="${3-$cur}"
-
-   case "$cur_" in
-   --*=)
-   ;;
-   *)
-   local c IFS=$' \t\n'
-   local -a array
-   for c in ${=1}; do
-   c="$c${4-}"
-   case $c in
-   --*=*|*.) ;;
-   *) c="$c " ;;
-   esac
-   array[${#array[@]}+1]="$c"
-   done
-   compset -P '*[=:]'
-   compadd -Q -S '' -p "${2-}" -a -- array && _ret=0
-   ;;
-   esac
-   }
-
-   __gitcomp_nl ()
-   {
-   emulate -L zsh
-
-   local IFS=$'\n'
-   compset -P '*[=:]'
-   compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
-   }
-
-   __gitcomp_file ()
-   {
-   emulate -L zsh
-
-   local IFS=$'\n'
-   compset -P '*[=:]'
-   compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
-   }
-
-   _git ()
-   {
-   local _ret=1 cur cword prev
-   cur=${words[CURRENT]}
-   prev=${words[CURRENT-1]}
-   let cword=CURRENT-1
-   emulate ksh -c __${service}_main
-   let _ret && _default && _ret=0
-   return _ret
-   }
-
-   compdef _git git gitk
-   return
-fi
-
 __git_func_wrap ()
 {
local cur words cword prev
-- 
2.8.0+fc1

--
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 11/11] Revert "Update documentation occurrences of filename .sh"

[Felipe Contreras]

The original code was correct: the example location ~/.git-completion.sh
is correct, because it's not only used by Bash. And zstyle command in
Zsh should use that same location; the Bash script.

This reverts commit 0e5ed7cca3c51c821c2bb0465617e75d994f432f.

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.bash | 4 ++--
 contrib/completion/git-completion.zsh  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 398f3a7..3224ae1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -17,9 +17,9 @@
 #
 # To use these routines:
 #
-#1) Copy this file to somewhere (e.g. ~/.git-completion.bash).
+#1) Copy this file to somewhere (e.g. ~/.git-completion.sh).
 #2) Add the following line to your .bashrc/.zshrc:
-#source ~/.git-completion.bash
+#source ~/.git-completion.sh
 #3) Consider changing your PS1 to also show the current branch,
 #   see git-prompt.sh for details.
 #
diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 28eaaed..6075111 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -9,7 +9,7 @@
 #
 # If your script is somewhere else, you can configure it on your ~/.zshrc:
 #
-#  zstyle ':completion:*:*:git:*' script ~/.git-completion.zsh
+#  zstyle ':completion:*:*:git:*' script ~/.git-completion.sh
 #
 # The recommended way to install this script is to copy to '~/.zsh/_git', and
 # then add the following to your ~/.zshrc file:
-- 
2.8.0+fc1

--
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 10/11] completion: prompt: fix for Zsh

[Felipe Contreras]

We can add colour in Zsh without the need of pcmode.

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-prompt.sh | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 64219e6..0da14ee 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -502,9 +502,11 @@ __git_ps1 ()
 
local z="${GIT_PS1_STATESEPARATOR-" "}"
 
-   # NO color option unless in PROMPT_COMMAND mode
-   if [ $pcmode = yes ] && [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
-   __git_ps1_colorize_gitstring
+   # NO color option unless in PROMPT_COMMAND mode or it's Zsh
+   if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
+   if [ $pcmode = yes ] || [ -n "${ZSH_VERSION-}" ]; then
+   __git_ps1_colorize_gitstring
+   fi
fi
 
b=${b##refs/heads/}
-- 
2.8.0+fc1

--
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 08/11] completion: zsh: improve main function selection

[Felipe Contreras]

Sometimes we want to use the function directly (e.g. _git_checkout), for
example when zsh has the option 'complete_aliases', this way, we can do
something like:

  compdef _git gco=git_checkout

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.zsh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh 
b/contrib/completion/git-completion.zsh
index 317b8eb..1f786cc 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -204,8 +204,10 @@ _git ()
 
if (( $+functions[__${service}_zsh_main] )); then
__${service}_zsh_main
-   else
+   elif (( $+functions[__${service}_main] )); then
emulate ksh -c __${service}_main
+   elif (( $+functions[_${service}] )); then
+   emulate ksh -c _${service}
fi
 
let _ret && _default && _ret=0
-- 
2.8.0+fc1

--
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 01/11] completion: add missing fetch options

[Felipe Contreras]

Signed-off-by: Felipe Contreras 
---
 contrib/completion/git-completion.bash | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index e3918c8..ecdf742 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1225,6 +1225,8 @@ __git_fetch_recurse_submodules="yes on-demand no"
 __git_fetch_options="
--quiet --verbose --append --upload-pack --force --keep --depth=
--tags --no-tags --all --prune --dry-run --recurse-submodules=
+   --no-recurse-submodules --unshallow --update-shallow --multiple
+   --submodule-prefix= --update-head-ok --progress
 "
 
 _git_fetch ()
-- 
2.8.0+fc1

--
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 00/11] Completion fixes and improvements

[Felipe Contreras]

Hi,

Here's a bunch of patches I've been using for a long time. I don't recall if
I've sent some of these before.

Here they are in case anybody is interested.

Cheers.


Felipe Contreras (11):
  completion: add missing fetch options
  completion: bash: remove old wrappers
  completion: bash: remove zsh wrapper
  completion: zsh: don't hide ourselves
  completion: remove zsh hack
  completion: zsh: trivial cleanups
  completion: bash: cleanup cygwin check
  completion: zsh: improve main function selection
  completion: zsh: fix for directories with spaces
  completion: prompt: fix for Zsh
  Revert "Update documentation occurrences of filename .sh"

 contrib/completion/git-completion.bash | 86 +++---
 contrib/completion/git-completion.zsh  | 26 --
 contrib/completion/git-prompt.sh   |  8 ++--
 3 files changed, 20 insertions(+), 100 deletions(-)

-- 
2.8.0+fc1

--
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 19/21] t9003: become resilient to GETTEXT_POISON

[Eric Sunshine]

[cc:+junio]

On Thu, May 19, 2016 at 5:31 PM, Vasco Almeida  wrote:
> Às 18:34 de 19-05-2016, Eric Sunshine escreveu:
>> On Wed, May 18, 2016 at 11:27 AM, Vasco Almeida  
>> wrote:
>>> -   sed -e "1,/^Did you mean this/d" actual | grep lgf &&
>>> +   sed -e "1,/^Did you mean this/d" actual |
>>> +   sed -e "/GETTEXT POISON/d" actual |
>>> +   grep distimdistim
>>
>> Why not do so with a single sed invocation?
>>
>>sed -e "..." -e "..." |
>
> I tried but it seems not to work.
>
> Actually I did this wrong, I haven't thought this through.
>
> Under gettext poison:
> sed -e "1,/^Did you mean this/d" removes all lines, gives no output.
> And then the one second, sed -e "/GETTEXT POISON/d", outputs "lgf" as
> expected.
>
> But running normally (without gettext poison):
> 1st sed outputs "lgf" as expected
> And then second one output the entire 'actual' file, like if it were
> cat, undoing the first sed.
>
> I think the sed here is superfluous in the first place.
> Should we remove it? If it weren't the case of gettext poison it was ok
> to have sed there, but it makes the test fail under gettext poison.

Indeed, the sed seems superfluous. The output of the test command is:

git: 'lfg' is not a git command. See 'git --help'.

Did you mean this?
lgf

And, the grep'd string, "lgf" only appears once, so grep alone should
be sufficient to verify expected behavior. Likewise for the other case
of misspelled "distimdist" and grep'd "distimdistim" which appears
only once.

I agree that the simplest fix to make GETTEXT_POISON work is to drop
the sed invocation.

Anyhow, I've cc:'d the author of 9d1d2b7 (git: remove an early return
from save_env_before_alias(), 2016-01-26) which introduced it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Odd Difference Between Windows Git and Standard Git

[Jon Forrest]

I'm running Git version 2.8.2 built from source on Ubuntu 16.04.
I'm using a repository that's stored on Dropbox. I'm the only person
accessing this repo. Everything works great.

For reasons unrelated to Git, I decided to try Git for Windows,
so I installed "git version 2.8.2.windows.1" on Windows 10.
When I run 'git status' on Ubuntu the list I see is exactly what
I expect. However, when I run 'git status' on the
same Dropbox repo on Windows, I see what I expect plus I'm told
that every .pdf file and some .png files are modified.

I'm guessing that this is caused by some mishandling of
binary files. Is this behavior to be expected? If so, is there
something I can do to have the output on Windows be the
same as on Ubuntu? I'm aware of 'git update-index --assume-unchanged'
but this seems harsh.

I copied the repo to a non-Dropbox location, just in case
it was Dropbox that was causing the problem, but this didn't
make any difference.

(If you want to try this yourself, try it on the ProGit2
book source on Github).

Thanks,
Jon Forrest

--
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: [PATCHv9 4/4] pathspec: allow querying for attributes

[Junio C Hamano]

Stefan Beller  writes:

> +attr;;
> +After `attr:` comes a space separated list of "attribute
> +...
> ++

The text looks OK, but does it format well?

> + attr_len = strcspn(attr, "=");

Scanning for '=' here retains the same bug from the previous
iteration where you take !VAR=VAL and silently ignore =VAL part
without diagnosing the error, doesn't it?

Perhaps strlen(attr) here, and...

> + switch (*attr) {
> + case '!':
> + am->match_mode = MATCH_UNSPECIFIED;
> + attr++;
> + attr_len--;
> + break;
> + case '-':
> + am->match_mode = MATCH_UNSET;
> + attr++;
> + attr_len--;
> + break;
> + default:
> + if (attr[attr_len] != '=')
> + am->match_mode = MATCH_SET;
> + else {
> + am->match_mode = MATCH_VALUE;
> + am->value = xstrdup([attr_len + 1]);
> + if (strchr(am->value, '\\'))
> + die(_("attr spec values must not 
> contain backslashes"));
> + }
> + break;
> + }

... doing strcspn() only in default: part would be a quick fix.
--
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

Pulling git tags from multiple sources incorrectly reports stale tags

[Christian Goetze]

See https://gist.github.com/cg-soft/62ac3529cf9ad6f6586e07866de43bc4
and discussion here:
http://stackoverflow.com/questions/37330041/merging-git-tags-from-multiple-reference-locations

Essentially, using this git config to pull tags from multiple remote
refs works fine:

[remote "origin"]
url = /Users/christian.goetze/git/tag-merge-demo/origin
fetch = +refs/heads/*:refs/remotes/origin/*
fetch = +refs/private/tags/*:refs/tags/*
fetch = +refs/tags/*:refs/tags/*

But this, leaving out the last line,  doesn't:

[remote "origin"]
url = /Users/christian.goetze/git/tag-merge-demo/origin
fetch = +refs/heads/*:refs/remotes/origin/*
fetch = +refs/private/tags/*:refs/tags/*

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

[PATCHv9 4/4] pathspec: allow querying for attributes

[Stefan Beller]

The pathspec mechanism is extended via the new
":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
requires paths to not just match the given pattern but also have the
specified attrs attached for them to be chosen.

Signed-off-by: Stefan Beller 
---
 Documentation/glossary-content.txt |  20 +
 dir.c  |  35 
 pathspec.c | 101 ++-
 pathspec.h |  16 
 t/t6134-pathspec-with-labels.sh| 163 +
 5 files changed, 331 insertions(+), 4 deletions(-)
 create mode 100755 t/t6134-pathspec-with-labels.sh

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index cafc284..e06520b 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -384,6 +384,26 @@ full pathname may have special meaning:
 +
 Glob magic is incompatible with literal magic.
 
+attr;;
+After `attr:` comes a space separated list of "attribute
+requirements", all of which must be met in order for the
+path to be considered a match; this is in addition to the
+usual non-magic pathspec pattern matching.
+
+Each of the attribute requirements for the path takes one of
+these forms:
+
+- "`ATTR`" requires that the attribute `ATTR` must be set.
+
+- "`-ATTR`" requires that the attribute `ATTR` must be unset.
+
+- "`ATTR=VALUE`" requires that the attribute `ATTR` must be
+  set to the string `VALUE`.
+
+- "`!ATTR`" requires that the attribute `ATTR` must be
+  unspecified.
++
+
 exclude;;
After a path matches any non-exclude pathspec, it will be run
through all exclude pathspec (magic signature: `!`). If it
diff --git a/dir.c b/dir.c
index 996653b..fc071af 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"
@@ -215,6 +216,37 @@ int within_depth(const char *name, int namelen,
return 1;
 }
 
+static int match_attrs(const char *name, int namelen,
+  const struct pathspec_item *item)
+{
+   int i;
+
+   git_check_attr_counted(name, namelen, item->attr_check);
+   for (i = 0; i < item->attr_match_nr; i++) {
+   const char *value;
+   int matched;
+   enum attr_match_mode match_mode;
+
+   value = item->attr_check->check[i].value;
+   match_mode = item->attr_match[i].match_mode;
+
+   if (ATTR_TRUE(value))
+   matched = (match_mode == MATCH_SET);
+   else if (ATTR_FALSE(value))
+   matched = (match_mode == MATCH_UNSET);
+   else if (ATTR_UNSET(value))
+   matched = (match_mode == MATCH_UNSPECIFIED);
+   else
+   matched = (match_mode == MATCH_VALUE &&
+  !strcmp(item->attr_match[i].value, value));
+
+   if (!matched)
+   return 0;
+   }
+
+   return 1;
+}
+
 #define DO_MATCH_EXCLUDE   1
 #define DO_MATCH_DIRECTORY 2
 
@@ -270,6 +302,9 @@ static int match_pathspec_item(const struct pathspec_item 
*item, int prefix,
strncmp(item->match, name - prefix, item->prefix))
return 0;
 
+   if (item->attr_match_nr && !match_attrs(name, namelen, item))
+   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..693a5e7 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "dir.h"
 #include "pathspec.h"
+#include "attr.h"
 
 /*
  * Finds which of the given pathspecs match items in the index.
@@ -88,12 +89,78 @@ static void prefix_short_magic(struct strbuf *sb, int 
prefixlen,
strbuf_addf(sb, ",prefix:%d)", prefixlen);
 }
 
+static void parse_pathspec_attr_match(struct pathspec_item *item, const char 
*value)
+{
+   struct string_list_item *si;
+   struct string_list list = STRING_LIST_INIT_DUP;
+
+
+   if (!value || !strlen(value))
+   die(_("attr spec must not be empty"));
+
+   string_list_split(, value, ' ', -1);
+   string_list_remove_empty_items(, 0);
+
+   if (!item->attr_check)
+   item->attr_check = git_attr_check_alloc();
+   else
+   die(_("Only one 'attr:' specification is allowed."));
+
+   ALLOC_GROW(item->attr_match, item->attr_match_nr + list.nr, 
item->attr_match_alloc);
+
+   for_each_string_list_item(si, ) {
+   size_t attr_len;
+
+   int j = item->attr_match_nr++;
+   const char *attr = si->string;
+   struct attr_match *am = >attr_match[j];
+
+   attr_len = strcspn(attr, "=");
+   switch (*attr) {
+   case '!':

[PATCHv9 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.123.gb4ad9b6.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

[PATCHv9 0/4] pathspec magic extension to search for attributes

[Stefan Beller]

This goes on top of origin/jc/attr, (396bf756f95, attr: expose validity check
for attribute names)

Patches 1 is a small fix, which could go independently as well.
I dropped the patch "string list: improve comment"
Patches 3 and 4 are refactoring pathspec.c a little.
These did not change since v7

Patch 5 contains all of Junios suggestions.

Thanks,
Stefan

diff to v8:
diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index aa9f220..e06520b 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -385,20 +385,23 @@ full pathname may have special meaning:
 Glob magic is incompatible with literal magic.
 
 attr;;
-   Additionally to matching the pathspec, the path must have the
-   attribute as specified. The syntax for specifying the required
-   attributes is "`attr: [mode]  [=value]`"
-+
-Attributes can have 4 states (Set, Unset, Set to a value, unspecified) and
-you can query each attribute for certain states. The "`[mode]`" is a special
-character to indicate which attribute states are looked for. The following
-modes are available:
-
- - an empty "`[mode]`" matches if the attribute is set
- - "`-`" the attribute must be unset
- - "`!`" the attribute must be unspecified
- - an empty "`[mode]`" combined with "`[=value]`" matches if the attribute has
-   the given value.
+After `attr:` comes a space separated list of "attribute
+requirements", all of which must be met in order for the
+path to be considered a match; this is in addition to the
+usual non-magic pathspec pattern matching.
+
+Each of the attribute requirements for the path takes one of
+these forms:
+
+- "`ATTR`" requires that the attribute `ATTR` must be set.
+
+- "`-ATTR`" requires that the attribute `ATTR` must be unset.
+
+- "`ATTR=VALUE`" requires that the attribute `ATTR` must be
+  set to the string `VALUE`.
+
+- "`!ATTR`" requires that the attribute `ATTR` must be
+  unspecified.
 +
 
 exclude;;
diff --git a/dir.c b/dir.c
index f60a503..fc071af 100644
--- a/dir.c
+++ b/dir.c
@@ -231,11 +231,11 @@ static int match_attrs(const char *name, int namelen,
match_mode = item->attr_match[i].match_mode;
 
if (ATTR_TRUE(value))
-   matched = match_mode == MATCH_SET;
+   matched = (match_mode == MATCH_SET);
else if (ATTR_FALSE(value))
-   matched = match_mode == MATCH_UNSET;
+   matched = (match_mode == MATCH_UNSET);
else if (ATTR_UNSET(value))
-   matched = match_mode == MATCH_UNSPECIFIED;
+   matched = (match_mode == MATCH_UNSPECIFIED);
else
matched = (match_mode == MATCH_VALUE &&
   !strcmp(item->attr_match[i].value, value));
diff --git a/pathspec.c b/pathspec.c
index b795a9c..693a5e7 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -115,34 +115,38 @@ static void parse_pathspec_attr_match(struct 
pathspec_item *item, const char *va
const char *attr = si->string;
struct attr_match *am = >attr_match[j];
 
-   if (attr[0] == '!')
+   attr_len = strcspn(attr, "=");
+   switch (*attr) {
+   case '!':
am->match_mode = MATCH_UNSPECIFIED;
-   else if (attr[0] == '-')
+   attr++;
+   attr_len--;
+   break;
+   case '-':
am->match_mode = MATCH_UNSET;
-   else
-   am->match_mode = MATCH_SET;
-
-   if (am->match_mode != MATCH_SET)
-   /* skip first character */
attr++;
+   attr_len--;
+   break;
+   default:
+   if (attr[attr_len] != '=')
+   am->match_mode = MATCH_SET;
+   else {
+   am->match_mode = MATCH_VALUE;
+   am->value = xstrdup([attr_len + 1]);
+   if (strchr(am->value, '\\'))
+   die(_("attr spec values must not 
contain backslashes"));
+   }
+   break;
+   }
 
-   attr_len = strcspn(attr, "=");
-   if (attr[attr_len] == '=') {
-   am->match_mode = MATCH_VALUE;
-   am->value = xstrdup([attr_len + 1]);
-   if (strchr(am->value, '\\'))
-   die(_("attr spec values must not contain 
backslashes"));
-   } else
-   am->value = NULL;
-
-   if (!attr_name_valid(attr, attr_len)) {
+   am->attr = git_attr_counted(attr, attr_len);
+   if (!am->attr) {
   

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

[PATCHv9 1/4] 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.123.gb4ad9b6.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 v5 0/2] CRLF: ce_compare_data() checks for a sha1 of a path

[Junio C Hamano]

tbo...@web.de writes:

> From: Torsten Bögershausen 
>
> Break up the old 10/10 series about CLRF handling into smaller
> series. This is a small bugfix, when merge.renormalize is used
> with core.autocrlf (and no attributes are set).

Is it worth protecting the fix with a new test?  Or does this flip
an existing "expect-failure" to "expect-success"?

> Prepare the refactoring to use the streaming interface.

> Changes since v4:
>  - Rename #define in cache.h into HASH_USE_SHA_NOT_PATH
>  - convert.c: Rename has_cr_in_index into blob_has_cr()
>Better logic when sha1 != NULL,
>Adjusted the commit message
>
> Torsten Bögershausen (2):
>   read-cache: factor out get_sha1_from_index() helper
>   convert: ce_compare_data() checks for a sha1 of a path
>
>  cache.h  |  4 
>  convert.c| 34 ++
>  convert.h| 23 +++
>  read-cache.c | 33 +
>  sha1_file.c  | 17 +
>  5 files changed, 79 insertions(+), 32 deletions(-)
--
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 v5 2/2] convert: ce_compare_data() checks for a sha1 of a path

[Junio C Hamano]

tbo...@web.de writes:

> +int convert_to_git_ce_sha1(const char *path, const unsigned char *sha1,
> +const char *src, size_t len,
> +struct strbuf *dst, enum safe_crlf checksafe)

That's a strange name for the function, as "ce" and "sha1" gives no
hints what they are about.

If I understand correctly:

 - convert_to_git() is about converting  into , and
   "path" is not for reading the contents to be converted.  It is
   used to tell crlf_to_git() the index entry to pay attention to
   when defeating the usual conversion rules due to strange contents
   in the index (it also is used to report errors for safe-crlf
   check).

 - This one adds "sha1", and that is not about the contents to be
   converted, either.  Like "path", "sha1" is used to tell what blob
   to check when disabling the usual conversion rules.

Does the above description help in coming up with a better
description for the ce/sha1 thing?  The comment near the code that
uses them reads like so:

/*
 * If the file in the index has any CR in it, do not convert.
 * This is the new safer autocrlf handling.
 */

What is a good name to call the input to that logic?  "This
function, in addition to convert_to_git(), takes an extra parameter,
that tells it an extra piece of information 'X'"; what is X?

At the same time, the parameter "sha1" needs to be renamed to
clarify what object it is and what purpose it serves.  "sha1" alone
is an overly generic name, and it does not hint that it may not even
be given to the function, and that it doesn't have anything to do
with the contents  points at.

Note. Perhaps you wanted _ce_sha1 suffix to tell the readers
that it takes "an object name of the cache entry" that
further affects the conversion?  If so the sha1 parameter
should be renamed to match (and make it clear to readers
that is what you meant).

The "sha1" is pretending to be the more important input for the
primary function of this function by being in early part of the
parameter list.  This may need to be rethought; we probably should
have done so as part of your previous refactoring of this file.

convert_to_git() takes the data for  in  and gives
result in , so these four should be its first parameters.  The
detail of the way the conversion works may be affected by additional
parameters, e.g.  controls if extra warnings are given.

The  is to influence the conversion logic further to disable
the crlf-to-git conversion by inspecting a blob, and it tells the
function that the blob comes from an unusual place (as opposed to
the index entry at ).  So it should sit next to checksafe as
an auxiliary input parameter.

The logic implemented by the patch looks correct, but I'd have to
say that the result is an unmaintainable mess.  Right now, I can see
what is going on in the new code.  I am sure that in 6 months, with
poorly named helper functions and parameters, I will have hard time
recalling how the new code works.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v12 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 6d3fe71..76aecca 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
 
@@ -1794,6 +1822,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);
@@ -1833,9 +1862,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 v12 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 v12 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 82d4446..6d3fe71 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2426,6 +2426,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)
@@ -2449,6 +2450,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 v12 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 6c8108f..e99109a 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);
}
 
 
@@ -324,8 +325,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 v12 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 | 280 +
 read-cache.c   | 125 +--
 t/t7900-index-helper.sh|  23 +++
 9 files changed, 488 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 v12 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 ddc641a..2d97c77 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -407,8 +407,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,
@@ -417,6 +418,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()
};
 
@@ -426,7 +428,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"));
@@ -440,10 +449,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 v12 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 v12 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 v12 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 v12 20/20] index-helper: indexhelper.exitafter config

[David Turner]

Add a configuration variable, indexhelper.exitafter, which provides a
default time to keep the index-helper alive.  This is useful with
indexhelper.autorun; some users will want to keep the
automatically-run index-helper alive across their lunch break and will
thus set indexhelper.exitafter to a high value.

Signed-off-by: David Turner 
---
 Documentation/config.txt | 4 
 index-helper.c   | 2 ++
 t/t7900-index-helper.sh  | 8 
 3 files changed, 14 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 385ea66..336d5a2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1860,6 +1860,10 @@ indexhelper.autorun::
Automatically run git index-helper when any builtin git
command is run inside a repository.
 
+indexhelper.exitafter::
+   When no exit-after argument is given, git index-helper defaults
+   to this number of seconds. Specify 0 to wait forever. Default is 600.
+
 init.templateDir::
Specify the directory from which templates will be copied.
(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
diff --git a/index-helper.c b/index-helper.c
index 2d97c77..a639de8 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -425,6 +425,8 @@ int main(int argc, char **argv)
git_extract_argv0_path(argv[0]);
git_setup_gettext();
 
+   git_config_get_int("indexhelper.exitafter", _in_seconds);
+
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(usage_text, options);
 
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index 3cfdf63..6c9b4dd 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -66,4 +66,12 @@ test_expect_success 'index-helper autorun works' '
test_path_is_missing .git/index-helper.sock
 '
 
+test_expect_success 'indexhelper.exitafter config works' '
+   test_when_finished "git index-helper --kill" &&
+   test_config indexhelper.exitafter 1 &&
+   git index-helper --detach &&
+   sleep 3 &&
+   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 v12 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.

Helped-by: Ramsay Jones 
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   | 241 ++---
 6 files changed, 355 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
+   

[PATCH v12 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 4a171e6..ddc641a 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -438,6 +438,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 v12 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 d3a1f39..4a171e6 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -353,6 +353,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);
}
@@ -383,10 +385,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[] = {
@@ -395,6 +416,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()
};
 
@@ -409,6 +431,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 v12 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 260ef4a..a7f8a42 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);
 }
 
@@ -250,6 +296,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 v12 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 8ec4be3..82d4446 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2767,3 +2767,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 v12 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 v12 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 v12 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

[PATCH v12 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 a7f8a42..6c8108f 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();
 }
@@ -290,7 +292,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[] = {
@@ -298,6 +300,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()
};
 
@@ -321,6 +324,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 v12 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 v12 00/20] index-helper/watchman

[David Turner]

Of course, as soon as I pinged on the previous version, I noticed an issue.

In that version, git index-helper --exit-after 0 was documented to
never exit, but it would in fact exit immediately.  This changes patch
04/20.

In addition, I noticed that there was no way to control the timeout on
automatically-run index-helpers, so I fixed that (with a new patch at
the end of the series).

Everything else is the same as the updated version of v11.

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
  index-helper: indexhelper.exitafter config

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   | 491 
 name-hash.c  |   2 +
 pkt-line.c   |  18 ++
 pkt-line.h   |   2 +
 preload-index.c  |   2 +
 read-cache.c | 527 ++-
 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  |  77 +
 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, 1565 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

Re: [PATCH 19/21] t9003: become resilient to GETTEXT_POISON

[Vasco Almeida]

Às 18:34 de 19-05-2016, Eric Sunshine escreveu:
> On Wed, May 18, 2016 at 11:27 AM, Vasco Almeida  wrote:
>> The test t9003-help-autocorrect.sh fails when run under GETTEXT_POISON,
>> because it's expecting to filter out the original output. Accommodate
>> gettext poison case by also filtering out the default simulated output.
>>
>> Signed-off-by: Vasco Almeida 
>> ---
>> diff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh
>> @@ -31,10 +31,14 @@ test_expect_success 'autocorrect showing candidates' '
>> git config help.autocorrect 0 &&
>>
>> test_must_fail git lfg 2>actual &&
>> -   sed -e "1,/^Did you mean this/d" actual | grep lgf &&
>> +   sed -e "1,/^Did you mean this/d" actual |
>> +   sed -e "/GETTEXT POISON/d" actual |
> 
> Why not do so with a single sed invocation?
> 
>sed -e "..." -e "..." |
> 
>> +   grep lgf &&
>>
>> test_must_fail git distimdist 2>actual &&
>> -   sed -e "1,/^Did you mean this/d" actual | grep distimdistim
>> +   sed -e "1,/^Did you mean this/d" actual |
>> +   sed -e "/GETTEXT POISON/d" actual |
> 
> Ditto.
> 
>> +   grep distimdistim
>>  '

I tried but it seems not to work.

Actually I did this wrong, I haven't thought this through.

Under gettext poison:
sed -e "1,/^Did you mean this/d" removes all lines, gives no output.
And then the one second, sed -e "/GETTEXT POISON/d", outputs "lgf" as
expected.

But running normally (without gettext poison):
1st sed outputs "lgf" as expected
And then second one output the entire 'actual' file, like if it were
cat, undoing the first sed.

I think the sed here is superfluous in the first place.
Should we remove it? If it weren't the case of gettext poison it was ok
to have sed there, but it makes the test fail under gettext poison.
--
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: [PATCHv8 0/5] pathspec magic extension to search for attributes

[Stefan Beller]

On Thu, May 19, 2016 at 2:05 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> (B) requires some thought though. Here is my vision:
>>
>> 1) Allow pathspecs for sparse checkout.
>>
>>   I wonder if we just add support for that in .git/info-sparse-checkout
>>   or if we add a new file that is for pathspecs only, or we have a config
>>   option whether sparse-checkout follows pathspecs or gitignore patterns
>>
>> 2) Teach `git clone` a new option `--sparse-checkout `
>>   When that option is set the pathspec is written into the new file from
>>   (1) and core.sparsecheckout is set to true
>>
>> 3) Advertise to do a `git clone --sparse-checkout
>> :(attr:default_submodules)`
>>
>> Going this way we would help making submodules not different but integrate 
>> more
>> into other concepts of Git. As a downside this would require touching
>> sparse checkout which may be more time consuming than just adding a
>> `git clone --init-submodules-by-label` which stores the labels and only 
>> upddates
>> those submodules.
>>
>> Or are there other ideas how to go from here?
>
> My take is to pretend sparse checkout does not exist at all and then
> go from there ;-)  It is a poorly designed and implemented "concept"
> that we do not want to spread around.
>
> You were going to add defaultGroup and teach 'clone' (and other
> commands) about it, and have clone find submodules in that Group,
> right?

Right. But upon finding the new name for clone, I wondered why
this has to be submodule specific. The attr pathspecs are also working
with any other files. So if you don't use submodules, I think it would be
pretty cool to have a

git clone --sparse-checkout=Documentation/ ...

> Isn't the pathspec magic just another way to introduce
> how you express the "defaultGroup", i.e. not with the "label" thing
> in .gitmodules, but with pathspec elements with attribute magic?

Right. So instead I could do a

git clone --recursive --restrict-submodules-to= --save-restriction

and then later on

git submodule update --use-restriction-saved-by-clone

I think we'd not want more than that for some first real tests
by some engineers.

However after thinking about this for a while I think it's too
submodule centric? The more special sauce we add for submodules
the harder they will be to maintain/support as they grow into their own
thing down the road.

Using these pathspec attrs are a good example for why we would
want to make it more generic. Imagine a future, where more attributes
can be codified into pathspecs and this is one of the possibilities:

git clone --sparse=":(exclude,size>5MB,binary)

which would not checkout files that are large binary files. We could
also extend the protocol (v2 with the capabilities in client speaks first)
to transmit such a requirement to the server.

Why is sparseness considered bad?
(I did find only limited resources on the net, those bogs I found are
advertising it as the last remainder Git needed to be superior to svn in
any discipline; I did not find a lot of negative feedback on it. So I guess
usability and confusion?)

If I wanted to just do the submodule only thing, this would be a smaller
series, I would guess.

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: [PATCHv8 0/5] pathspec magic extension to search for attributes

[Junio C Hamano]

Stefan Beller  writes:

> (B) requires some thought though. Here is my vision:
>
> 1) Allow pathspecs for sparse checkout.
>
>   I wonder if we just add support for that in .git/info-sparse-checkout
>   or if we add a new file that is for pathspecs only, or we have a config
>   option whether sparse-checkout follows pathspecs or gitignore patterns
>
> 2) Teach `git clone` a new option `--sparse-checkout `
>   When that option is set the pathspec is written into the new file from
>   (1) and core.sparsecheckout is set to true
>
> 3) Advertise to do a `git clone --sparse-checkout
> :(attr:default_submodules)`
>
> Going this way we would help making submodules not different but integrate 
> more
> into other concepts of Git. As a downside this would require touching
> sparse checkout which may be more time consuming than just adding a
> `git clone --init-submodules-by-label` which stores the labels and only 
> upddates
> those submodules.
>
> Or are there other ideas how to go from here?

My take is to pretend sparse checkout does not exist at all and then
go from there ;-)  It is a poorly designed and implemented "concept"
that we do not want to spread around.

You were going to add defaultGroup and teach 'clone' (and other
commands) about it, and have clone find submodules in that Group,
right?  Isn't the pathspec magic just another way to introduce
how you express the "defaultGroup", i.e. not with the "label" thing
in .gitmodules, but with pathspec elements with attribute magic?
--
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: [PATCHv8 5/5] pathspec: allow querying for attributes

[Junio C Hamano]

Stefan Beller  writes:

> $ grep -r "cat" |grep "<<-"|wc -l
> 915
> $ grep -r "cat" |grep "<<"|grep -v "<<-"| wc -l
> 1329
>
> I was undecided what the prevailing style is, some did indent,
> others did not.

FWIW, newer ones tend to use "<<-"; just FYI.
--
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: [PATCHv8 0/5] pathspec magic extension to search for attributes

[Stefan Beller]

On Thu, May 19, 2016 at 11:55 AM, Junio C Hamano  wrote:
> I think this round is 99% there.  The next step would be to answer
> "does the feature set we have here meet your needs that you wanted
> to fill with the submodule labels originally?" and I am hoping it is
> "yes".

But it's no. (...not quite / not yet)

I thought about this question since sending out the
series and I think we would want to improve submodules more.

In the original patch series "submodule groups" I tried to achieve two goals:
 (A) a grouping scheme for submodules
 (B) some sort of persistent configuration for such a group

(A) will be mostly solved now by the pathspec attrs. It may be a bit
confusing for users, as any other submodule related configuration is done
in .gitmodules. Also when moving a submodule (changing its path on the file
system, not the name in the config), any configuration still applies
except for the grouping scheme, which may not match any more.

(B) requires some thought though. Here is my vision:

1) Allow pathspecs for sparse checkout.

  I wonder if we just add support for that in .git/info-sparse-checkout
  or if we add a new file that is for pathspecs only, or we have a config
  option whether sparse-checkout follows pathspecs or gitignore patterns

2) Teach `git clone` a new option `--sparse-checkout `
  When that option is set the pathspec is written into the new file from
  (1) and core.sparsecheckout is set to true

3) Advertise to do a `git clone --sparse-checkout
:(attr:default_submodules)`

Going this way we would help making submodules not different but integrate more
into other concepts of Git. As a downside this would require touching
sparse checkout which may be more time consuming than just adding a
`git clone --init-submodules-by-label` which stores the labels and only upddates
those submodules.

Or are there other ideas how to go from here?

Thanks,
Stefan



>
> 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: [PATCHv8 5/5] pathspec: allow querying for attributes

[Stefan Beller]

On Thu, May 19, 2016 at 11:53 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> diff --git a/Documentation/glossary-content.txt 
>> b/Documentation/glossary-content.txt
>> index cafc284..aa9f220 100644
>> --- a/Documentation/glossary-content.txt
>> +++ b/Documentation/glossary-content.txt
>> @@ -384,6 +384,23 @@ full pathname may have special meaning:
>>  +
>>  Glob magic is incompatible with literal magic.
>>
>> +attr;;
>> + Additionally to matching the pathspec, the path must have the
>> + attribute as specified. The syntax for specifying the required
>> + attributes is "`attr: [mode]  [=value]`"
>> ++
>> +Attributes can have 4 states (Set, Unset, Set to a value, unspecified) and
>> +you can query each attribute for certain states. The "`[mode]`" is a special
>> +character to indicate which attribute states are looked for. The following
>> +modes are available:
>> +
>> + - an empty "`[mode]`" matches if the attribute is set
>> + - "`-`" the attribute must be unset
>> + - "`!`" the attribute must be unspecified
>> + - an empty "`[mode]`" combined with "`[=value]`" matches if the attribute 
>> has
>> +   the given value.
>> ++
>
> As an initial design, I find this much more agreeable than the
> previous rounds.  I however find the phrasing of the above harder
> than necessary to understand, for a few reasons.
>
>  * Mixed use of "X matches if ..." and "... must be Y" makes it
>unclear if they are talking about different kind of things, or
>the same kind of things in merely different ways.
>
>  * It does not make it clear "=value" is only meaningful when [mode]
>is empty.
>
> Perhaps dropping the '[mode]' thing altogether and instead saying
>
> After `attr:` comes a space separated list of "attribute
> requirements", all of which must be met in order for the
> path to be considered a match; this is in addition to the
> usual non-magic pathspec pattern matching.
>
> Each of the attribute requirements for the path takes one of
> these forms:
>
> - "`ATTR`" requires that the attribute `ATTR` must be set.
>
> - "`-ATTR`" requires that the attribute `ATTR` must be unset.
>
> - "`ATTR=VALUE`" requires that the attribute `ATTR` must be
>   set to the string `VALUE`.
>
> - "`!ATTR`" requires that the attribute `ATTR` must be
>   unspecified.
>
> would make the resulting text easier to read?

That is way better!

>
>> +static int match_attrs(const char *name, int namelen,
>> +const struct pathspec_item *item)
>> +{
>> + int i;
>> +
>> + git_check_attr_counted(name, namelen, item->attr_check);
>> + for (i = 0; i < item->attr_match_nr; i++) {
>> + const char *value;
>> + int matched;
>> + enum attr_match_mode match_mode;
>> +
>> + value = item->attr_check->check[i].value;
>> + match_mode = item->attr_match[i].match_mode;
>> +
>> + if (ATTR_TRUE(value))
>> + matched = match_mode == MATCH_SET;
>> + else if (ATTR_FALSE(value))
>> + matched = match_mode == MATCH_UNSET;
>> + else if (ATTR_UNSET(value))
>> + matched = match_mode == MATCH_UNSPECIFIED;
>
> readability nit:
>
> matched = (match_mode == MATCH_WHATEVER);
>
> would be easier to view

ok.

>
>> + else
>> + matched = (match_mode == MATCH_VALUE &&
>> +!strcmp(item->attr_match[i].value, value));
>
> and would match the last case above better.
>
>> +static void parse_pathspec_attr_match(struct pathspec_item *item, const 
>> char *value)
>> +{
>> + struct string_list_item *si;
>> + struct string_list list = STRING_LIST_INIT_DUP;
>> +
>> +
>> + if (!value || !strlen(value))
>> + die(_("attr spec must not be empty"));
>> +
>> + string_list_split(, value, ' ', -1);
>> + string_list_remove_empty_items(, 0);
>> +
>> + if (!item->attr_check)
>> + item->attr_check = git_attr_check_alloc();
>> + else
>> + die(_("Only one 'attr:' specification is allowed."));
>> +
>> + ALLOC_GROW(item->attr_match, item->attr_match_nr + list.nr, 
>> item->attr_match_alloc);
>> +
>> + for_each_string_list_item(si, ) {
>> + size_t attr_len;
>> +
>> + int j = item->attr_match_nr++;
>> + const char *attr = si->string;
>> + struct attr_match *am = >attr_match[j];
>> +
>> + if (attr[0] == '!')
>> + am->match_mode = MATCH_UNSPECIFIED;
>> + else if (attr[0] == '-')
>> + am->match_mode = MATCH_UNSET;
>> + else
>> + am->match_mode = MATCH_SET;
>> +
>> + if (am->match_mode != MATCH_SET)
>> + /* skip first character */
>> + 

Re: [PATCH v10 00/20] index-helper/watchman

[David Turner]

On Thu, 2016-05-19 at 13:11 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > Do folks have any more comments on this version?
> 
> Not from me at the moment.
> 
> > Do I need to re-roll
> > to replace 11/20 as I proposed and drop 20/20?  
> 
> FYI, I think I have the one taken from
> 
> Message-Id:
> <1463174182-20200-1-git-send-email-dtur...@twopensource.com>
> 
> aka http://thread.gmane.org/gmane.comp.version-control.git/294470/foc
> us=294585
> 
> queued at 11/20.

LGTM!

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

[Junio C Hamano]

David Turner  writes:

> Do folks have any more comments on this version?

Not from me at the moment.

> Do I need to re-roll
> to replace 11/20 as I proposed and drop 20/20?  

FYI, I think I have the one taken from

Message-Id:
<1463174182-20200-1-git-send-email-dtur...@twopensource.com>

aka http://thread.gmane.org/gmane.comp.version-control.git/294470/focus=294585

queued at 11/20.
--
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] rerere: plug memory leaks upon "rerere forget" failure

[Junio C Hamano]

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: since/after not working properly

[Hawk, Jeff]

Thanks for your help and sorry for my confused regarding ad and cd.

From: Junio C Hamano 
Sent: Thursday, May 19, 2016 2:03:54 PM
To: Hawk, Jeff
Cc: Git Mailing List
Subject: Re: since/after not working properly

"Hawk, Jeff"  writes:

> They suggest this:
> git log --all --numstat --date=short --pretty=format:'--%h--%ad--%aN' 
> --no-renames
>
> Seems like the %ad should be %cd.

I have no opinion on that.  If somebody wants to show author dates,
that's her choice.
--
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] rerere: plug memory leaks upon "rerere forget" failure

[Johannes Sixt]

Am 12.05.2016 um 01:32 schrieb Junio C Hamano:

+   if (unlink(filename)) {
+   if (errno == ENOENT)
+   error("no remembered resolution for %s", path);
+   else
+   error("cannot unlink %s: %s", filename, 
strerror(errno));
+   goto fail_exit;
+   };


If you haven't merged this topic yet, you might want to remove this 
superfluous empty statement.


-- 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: [PATCHv8 5/5] pathspec: allow querying for attributes

[Junio C Hamano]

Stefan Beller  writes:

> +test_expect_success 'setup a tree' '
> + mkdir sub &&
> + for p in fileA fileB fileC fileAB fileAC fileBC fileNoLabel 
> fileUnsetLabel fileSetLabel fileValue fileWrongLabel; do
> + : >$p &&
> + git add $p &&
> + : >sub/$p
> + git add sub/$p
> + done &&
> + git commit -m $p &&

What does this $p refer to?

> + git ls-files >actual &&
> + cat  +fileA
> +fileAB
> +fileAC
> +fileB
> +fileBC
> +fileC
> +fileNoLabel
> +fileSetLabel
> +fileUnsetLabel
> +fileValue
> +fileWrongLabel
> +sub/fileA
> +sub/fileAB
> +sub/fileAC
> +sub/fileB
> +sub/fileBC
> +sub/fileC
> +sub/fileNoLabel
> +sub/fileSetLabel
> +sub/fileUnsetLabel
> +sub/fileValue
> +sub/fileWrongLabel
> +EOF
> + test_cmp expect actual
> +'

If I were doing this, I'd prepare the list of paths (i.e. expect)
first and then create these paths using that list, i.e.

test_expect_success 'setup a tree' '
cat <<-\EOF >expect &&
fileA
fileAB
...
sub/fileWrongLabel
EOF
mkdir sub &&
while read path
do
: >$path &&
git add $path || return 1
done actual &&
test_cmp expect actual
'

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

[David Turner]

Do folks have any more comments on this version? Do I need to re-roll
to replace 11/20 as I proposed and drop 20/20?  

Thanks.

On Thu, 2016-05-12 at 16:19 -0400, David Turner wrote:
> 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
> 
--
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: since/after not working properly

[Junio C Hamano]

"Hawk, Jeff"  writes:

> They suggest this:
> git log --all --numstat --date=short --pretty=format:'--%h--%ad--%aN' 
> --no-renames
>
> Seems like the %ad should be %cd.

I have no opinion on that.  If somebody wants to show author dates,
that's her choice.
--
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: [PATCHv8 0/5] pathspec magic extension to search for attributes

[Junio C Hamano]

I think this round is 99% there.  The next step would be to answer
"does the feature set we have here meet your needs that you wanted
to fill with the submodule labels originally?" and I am hoping it is
"yes".

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: [PATCHv8 5/5] pathspec: allow querying for attributes

[Junio C Hamano]

Stefan Beller  writes:

> diff --git a/Documentation/glossary-content.txt 
> b/Documentation/glossary-content.txt
> index cafc284..aa9f220 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -384,6 +384,23 @@ full pathname may have special meaning:
>  +
>  Glob magic is incompatible with literal magic.
>  
> +attr;;
> + Additionally to matching the pathspec, the path must have the
> + attribute as specified. The syntax for specifying the required
> + attributes is "`attr: [mode]  [=value]`"
> ++
> +Attributes can have 4 states (Set, Unset, Set to a value, unspecified) and
> +you can query each attribute for certain states. The "`[mode]`" is a special
> +character to indicate which attribute states are looked for. The following
> +modes are available:
> +
> + - an empty "`[mode]`" matches if the attribute is set
> + - "`-`" the attribute must be unset
> + - "`!`" the attribute must be unspecified
> + - an empty "`[mode]`" combined with "`[=value]`" matches if the attribute 
> has
> +   the given value.
> ++

As an initial design, I find this much more agreeable than the
previous rounds.  I however find the phrasing of the above harder
than necessary to understand, for a few reasons.

 * Mixed use of "X matches if ..." and "... must be Y" makes it
   unclear if they are talking about different kind of things, or
   the same kind of things in merely different ways.

 * It does not make it clear "=value" is only meaningful when [mode]
   is empty.

Perhaps dropping the '[mode]' thing altogether and instead saying

After `attr:` comes a space separated list of "attribute
requirements", all of which must be met in order for the
path to be considered a match; this is in addition to the
usual non-magic pathspec pattern matching.

Each of the attribute requirements for the path takes one of
these forms:

- "`ATTR`" requires that the attribute `ATTR` must be set.

- "`-ATTR`" requires that the attribute `ATTR` must be unset.

- "`ATTR=VALUE`" requires that the attribute `ATTR` must be
  set to the string `VALUE`.

- "`!ATTR`" requires that the attribute `ATTR` must be
  unspecified.

would make the resulting text easier to read?

> +static int match_attrs(const char *name, int namelen,
> +const struct pathspec_item *item)
> +{
> + int i;
> +
> + git_check_attr_counted(name, namelen, item->attr_check);
> + for (i = 0; i < item->attr_match_nr; i++) {
> + const char *value;
> + int matched;
> + enum attr_match_mode match_mode;
> +
> + value = item->attr_check->check[i].value;
> + match_mode = item->attr_match[i].match_mode;
> +
> + if (ATTR_TRUE(value))
> + matched = match_mode == MATCH_SET;
> + else if (ATTR_FALSE(value))
> + matched = match_mode == MATCH_UNSET;
> + else if (ATTR_UNSET(value))
> + matched = match_mode == MATCH_UNSPECIFIED;

readability nit:

matched = (match_mode == MATCH_WHATEVER);

would be easier to view

> + else
> + matched = (match_mode == MATCH_VALUE &&
> +!strcmp(item->attr_match[i].value, value));

and would match the last case above better.

> +static void parse_pathspec_attr_match(struct pathspec_item *item, const char 
> *value)
> +{
> + struct string_list_item *si;
> + struct string_list list = STRING_LIST_INIT_DUP;
> +
> +
> + if (!value || !strlen(value))
> + die(_("attr spec must not be empty"));
> +
> + string_list_split(, value, ' ', -1);
> + string_list_remove_empty_items(, 0);
> +
> + if (!item->attr_check)
> + item->attr_check = git_attr_check_alloc();
> + else
> + die(_("Only one 'attr:' specification is allowed."));
> +
> + ALLOC_GROW(item->attr_match, item->attr_match_nr + list.nr, 
> item->attr_match_alloc);
> +
> + for_each_string_list_item(si, ) {
> + size_t attr_len;
> +
> + int j = item->attr_match_nr++;
> + const char *attr = si->string;
> + struct attr_match *am = >attr_match[j];
> +
> + if (attr[0] == '!')
> + am->match_mode = MATCH_UNSPECIFIED;
> + else if (attr[0] == '-')
> + am->match_mode = MATCH_UNSET;
> + else
> + am->match_mode = MATCH_SET;
> +
> + if (am->match_mode != MATCH_SET)
> + /* skip first character */
> + attr++;
> + attr_len = strcspn(attr, "=");
> + if (attr[attr_len] == '=') {
> + am->match_mode = MATCH_VALUE;
> + am->value = xstrdup([attr_len + 1]);
> + if (strchr(am->value, 

Re: [PATCH 19/21] t9003: become resilient to GETTEXT_POISON

[Eric Sunshine]

On Wed, May 18, 2016 at 11:27 AM, Vasco Almeida  wrote:
> The test t9003-help-autocorrect.sh fails when run under GETTEXT_POISON,
> because it's expecting to filter out the original output. Accommodate
> gettext poison case by also filtering out the default simulated output.
>
> Signed-off-by: Vasco Almeida 
> ---
> diff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh
> @@ -31,10 +31,14 @@ test_expect_success 'autocorrect showing candidates' '
> git config help.autocorrect 0 &&
>
> test_must_fail git lfg 2>actual &&
> -   sed -e "1,/^Did you mean this/d" actual | grep lgf &&
> +   sed -e "1,/^Did you mean this/d" actual |
> +   sed -e "/GETTEXT POISON/d" actual |

Why not do so with a single sed invocation?

   sed -e "..." -e "..." |

> +   grep lgf &&
>
> test_must_fail git distimdist 2>actual &&
> -   sed -e "1,/^Did you mean this/d" actual | grep distimdistim
> +   sed -e "1,/^Did you mean this/d" actual |
> +   sed -e "/GETTEXT POISON/d" actual |

Ditto.

> +   grep distimdistim
>  '
--
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/RFC] Add userdiff built-in pattern for CSS code

[William Duclot]

> On Thu, May 19, 2016 at 10:45 AM, Matthieu Moy
>  wrote:
> >> Add the info in documentation that CSS is now built-in.
> >
> > This doesn't add much to the patch (we can already see that from the patch
> > itself). I'd remove it.
> 
> I think you meant to say this "doesn't add much to the *commit
> message*", and that the sentence should be removed from the commit
> message, while retaining the actual documentation update in the patch.
> 
That's of course how I understood it, but thanks for pointing it out !

Beside that, all of Matthieu Moy comments seem correct to me and will be taken 
into account.
--
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: [PATCHv8 1/5] string list: improve comment

[Stefan Beller]

On Thu, May 19, 2016 at 11:08 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Signed-off-by: Stefan Beller 
>> ---
>>  string-list.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/string-list.h b/string-list.h
>> index d3809a1..465a1f0 100644
>> --- a/string-list.h
>> +++ b/string-list.h
>> @@ -106,7 +106,7 @@ void unsorted_string_list_delete_item(struct string_list 
>> *list, int i, int free_
>>   * list->strdup_strings must be set, as new memory needs to be
>>   * allocated to hold the substrings.  If maxsplit is non-negative,
>>   * then split at most maxsplit times.  Return the number of substrings
>> - * appended to list.
>> + * appended to list. The list may be non-empty already.
>
> I personally find that the original comment is clear enough, though.
>
> When somebody says "resulting elements of the split are appended to
> the list" without saying either:
>
>   a. "the list MUST be empty in the beginning", or
>   b. "the list will be emptied first before the split result are appended",
>
> wouldn't it be natural to take it as "you can append them to any
> list, be it empty or not, and they are _appended_, not replaced"?

That is true. I missed that though when reading the documentation and
read the source code to be clear.

>
> So while this is not incorrect per-se, I am not sure if it adds much
> value.  If somebody needs this additional clarification, because the
> original did not say a. above, she would certainly need more
> clarification because the original did not say b. above, either.
>
> "The list may be non-empty already", but she would keep wondering if
> the existing contents would be discarded before the result gets
> appended to it.
>
> You may say "No, there won't be such a confusion, because we say
> 'append'; empty and then append is 'replace'".  But then the same
> logic would say "There cannot be a requirement for the list to be
> empty in the first place, because we say 'append'".
>
> So...

So, please drop?

I do not feel strongly about this patch. I basically wrote to for myself
after I consulted the source.
--
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: [PATCHv8 1/5] string list: improve comment

[Junio C Hamano]

Stefan Beller  writes:

> Signed-off-by: Stefan Beller 
> ---
>  string-list.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/string-list.h b/string-list.h
> index d3809a1..465a1f0 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -106,7 +106,7 @@ void unsorted_string_list_delete_item(struct string_list 
> *list, int i, int free_
>   * list->strdup_strings must be set, as new memory needs to be
>   * allocated to hold the substrings.  If maxsplit is non-negative,
>   * then split at most maxsplit times.  Return the number of substrings
> - * appended to list.
> + * appended to list. The list may be non-empty already.

I personally find that the original comment is clear enough, though.

When somebody says "resulting elements of the split are appended to
the list" without saying either:

  a. "the list MUST be empty in the beginning", or
  b. "the list will be emptied first before the split result are appended",

wouldn't it be natural to take it as "you can append them to any
list, be it empty or not, and they are _appended_, not replaced"?

So while this is not incorrect per-se, I am not sure if it adds much
value.  If somebody needs this additional clarification, because the
original did not say a. above, she would certainly need more
clarification because the original did not say b. above, either.

"The list may be non-empty already", but she would keep wondering if
the existing contents would be discarded before the result gets
appended to it.

You may say "No, there won't be such a confusion, because we say
'append'; empty and then append is 'replace'".  But then the same
logic would say "There cannot be a requirement for the list to be
empty in the first place, because we say 'append'".

So...


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

Re: since/after not working properly

[Hawk, Jeff]

Thanks Junio,

Do you think that code-maat has the wrong format documented on their github 
page?

At: https://github.com/adamtornhill/code-maat

They suggest this:
git log --all --numstat --date=short --pretty=format:'--%h--%ad--%aN' 
--no-renames

Seems like the %ad should be %cd.

Regards,
Jeff H


From: jch2...@gmail.com  on behalf of Junio C Hamano 

Sent: Thursday, May 19, 2016 12:27:17 PM
To: Hawk, Jeff
Cc: Git Mailing List
Subject: Re: since/after not working properly

On Thu, May 19, 2016 at 8:41 AM, Jeff Hawk  wrote:
> [jeff:~/src/git] master* 2s ± git log --pretty=format:"%ad" --date=short

Perhaps try it with %cd instead of %ad?
--
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 14/21] i18n: rebase-interactive: mark strings for translation

[Eric Sunshine]

On Wed, May 18, 2016 at 11:27 AM, Vasco Almeida  wrote:
> Mark strings in git-rebase--interactive.sh for translation. There is no
> need to source git-sh-i18n since git-rebase.sh already does so.
>
> Add git-rebase--interactive.sh to LOCALIZED_SH in Makefile in order to
> enabled extracting strings marked for translation by xgettext.

s/enabled/enable/

> Signed-off-by: Vasco Almeida 
--
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

A note from the maintainer

[Junio C Hamano]

Welcome to the Git development community.

This message is written by the maintainer and talks about how Git
project is managed, and how you can work with it.

* Mailing list and the community

The development is primarily done on the Git mailing list. Help
requests, feature proposals, bug reports and patches should be sent to
the list address .  You don't have to be
subscribed to send messages.  The convention on the list is to keep
everybody involved on Cc:, so it is unnecessary to say "Please Cc: me,
I am not subscribed".

Before sending patches, please read Documentation/SubmittingPatches
and Documentation/CodingGuidelines to familiarize yourself with the
project convention.

If you sent a patch and you did not hear any response from anybody for
several days, it could be that your patch was totally uninteresting,
but it also is possible that it was simply lost in the noise.  Please
do not hesitate to send a reminder message in such a case.  Messages
getting lost in the noise may be a sign that those who can evaluate
your patch don't have enough mental/time bandwidth to process them
right at the moment, and it often helps to wait until the list traffic
becomes calmer before sending such a reminder.

The list archive is available at a few public sites:

http://news.gmane.org/gmane.comp.version-control.git/
http://marc.info/?l=git
http://www.spinics.net/lists/git/

For those who prefer to read it over NNTP:

nntp://news.gmane.org/gmane.comp.version-control.git

When you point at a message in a mailing list archive, using
gmane is often the easiest to follow by readers, like this:

http://thread.gmane.org/gmane.comp.version-control.git/27/focus=217

as it also allows people who subscribe to the mailing list as gmane
newsgroup to "jump to" the article.

Some members of the development community can sometimes be found on
the #git and #git-devel IRC channels on Freenode.  Their logs are
available at:

http://colabti.org/irclogger/irclogger_log/git
http://colabti.org/irclogger/irclogger_log/git-devel

There is a volunteer-run newsletter to serve our community ("Git Rev
News" http://git.github.io/rev_news/rev_news.html).

Git is a member project of software freedom conservancy, a non-profit
organization (https://sfconservancy.org/).  To reach a committee of
liaisons to the conservancy, contact them at .


* Reporting bugs

When you think git does not behave as you expect, please do not stop
your bug report with just "git does not work".  "I used git in this
way, but it did not work" is not much better, neither is "I used git
in this way, and X happend, which is broken".  It often is that git is
correct to cause X happen in such a case, and it is your expectation
that is broken. People would not know what other result Y you expected
to see instead of X, if you left it unsaid.

Please remember to always state

 - what you wanted to achieve;

 - what you did (the version of git and the command sequence to reproduce
   the behavior);

 - what you saw happen (X above);

 - what you expected to see (Y above); and

 - how the last two are different.

See http://www.chiark.greenend.org.uk/~sgtatham/bugs.html for further
hints.

If you think you found a security-sensitive issue and want to disclose
it to us without announcing it to wider public, please contact us at
our security mailing list .  This is
a closed list that is limited to people who need to know early about
vulnerabilities, including:

  - people triaging and fixing reported vulnerabilities
  - people operating major git hosting sites with many users
  - people packaging and distributing git to large numbers of people

where these issues are discussed without risk of the information
leaking out before we're ready to make public announcements.


* Repositories and documentation.

My public git.git repositories are at:

  git://git.kernel.org/pub/scm/git/git.git/
  https://kernel.googlesource.com/pub/scm/git/git
  git://repo.or.cz/alt-git.git/
  https://github.com/git/git/
  git://git.sourceforge.jp/gitroot/git-core/git.git/
  git://git-core.git.sourceforge.net/gitroot/git-core/git-core/

A few web interfaces are found at:

  http://git.kernel.org/cgit/git/git.git
  https://kernel.googlesource.com/pub/scm/git/git
  http://repo.or.cz/w/alt-git.git

Preformatted documentation from the tip of the "master" branch can be
found in:

  git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
  git://repo.or.cz/git-{htmldocs,manpages}.git/
  https://github.com/gitster/git-{htmldocs,manpages}.git/

Also GitHub shows the manual pages formatted in HTML (with a
formatting backend different from the one that is used to create the
above) at:

  http://git-scm.com/docs/git


* How various branches are used.

There are four branches in git.git repository that track the source tree
of git: "master", "maint", "next", and "pu".

The "master" 

[ANNOUNCE] Git v2.8.3

[Junio C Hamano]

The latest maintenance release Git v2.8.3 is now available at
the usual places.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.8.3'
tag and the 'maint' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git



Git v2.8.3 Release Notes


Fixes since v2.8.2
--

 * "git send-email" now uses a more readable timestamps when
   formulating a message ID.

 * The repository set-up sequence has been streamlined (the biggest
   change is that there is no longer git_config_early()), so that we
   do not attempt to look into refs/* when we know we do not have a
   Git repository.

 * When "git worktree" feature is in use, "git branch -d" allowed
   deletion of a branch that is checked out in another worktree

 * When "git worktree" feature is in use, "git branch -m" renamed a
   branch that is checked out in another worktree without adjusting
   the HEAD symbolic ref for the worktree.

 * "git format-patch --help" showed `-s` and `--no-patch` as if these
   are valid options to the command.  We already hide `--patch` option
   from the documentation, because format-patch is about showing the
   diff, and the documentation now hides these options as well.

 * A change back in version 2.7 to "git branch" broke display of a
   symbolic ref in a non-standard place in the refs/ hierarchy (we
   expect symbolic refs to appear in refs/remotes/*/HEAD to point at
   the primary branch the remote has, and as .git/HEAD to point at the
   branch we locally checked out).

 * A partial rewrite of "git submodule" in the 2.7 timeframe changed
   the way the gitdir: pointer in the submodules point at the real
   repository location to use absolute paths by accident.  This has
   been corrected.

 * "git commit" misbehaved in a few minor ways when an empty message
   is given via -m '', all of which has been corrected.

 * Support for CRAM-MD5 authentication method in "git imap-send" did
   not work well.

 * The socks5:// proxy support added back in 2.6.4 days was not aware
   that socks5h:// proxies behave differently.

 * "git config" had a codepath that tried to pass a NULL to
   printf("%s"), which nobody seems to have noticed.

 * On Cygwin, object creation uses the "create a temporary and then
   rename it to the final name" pattern, not "create a temporary,
   hardlink it to the final name and then unlink the temporary"
   pattern.

   This is necessary to use Git on Windows shared directories, and is
   already enabled for the MinGW and plain Windows builds.  It also
   has been used in Cygwin packaged versions of Git for quite a while.
   See http://thread.gmane.org/gmane.comp.version-control.git/291853
   and http://thread.gmane.org/gmane.comp.version-control.git/275680.

 * "git replace -e" did not honour "core.editor" configuration.

 * Upcoming OpenSSL 1.1.0 will break compilation b updating a few APIs
   we use in imap-send, which has been adjusted for the change.

 * "git submodule" reports the paths of submodules the command
   recurses into, but this was incorrect when the command was not run
   from the root level of the superproject.

 * The test scripts for "git p4" (but not "git p4" implementation
   itself) has been updated so that they would work even on a system
   where the installed version of Python is python 3.

 * The "user.useConfigOnly" configuration variable makes it an error
   if users do not explicitly set user.name and user.email.  However,
   its check was not done early enough and allowed another error to
   trigger, reporting that the default value we guessed from the
   system setting was unusable.  This was a suboptimal end-user
   experience as we want the users to set user.name/user.email without
   relying on the auto-detection at all.

 * "git mv old new" did not adjust the path for a submodule that lives
   as a subdirectory inside old/ directory correctly.

 * "git push" from a corrupt repository that attempts to push a large
   number of refs deadlocked; the thread to relay rejection notices
   for these ref updates blocked on writing them to the main thread,
   after the main thread at the receiving end notices that the push
   failed and decides not to read these notices and return a failure.

 * A question by "git send-email" to ask the identity of the sender
   has been updated.

 * Recent update to Git LFS broke "git p4" by changing the output from
   its "lfs pointer" subcommand.

 * Some multi-byte encoding can have a backslash byte as a later part
   of one letter, which would confuse "highlight" filter used in
   gitweb.

Also contains minor 

Re: [PATCH/RFC] Add userdiff built-in pattern for CSS code

[Eric Sunshine]

On Thu, May 19, 2016 at 10:45 AM, Matthieu Moy
 wrote:
>> Subject: [PATCH/RFC] Add userdiff built-in pattern for CSS code
>> [...snip...]
>> Add the info in documentation that CSS is now built-in.
>
> This doesn't add much to the patch (we can already see that from the patch
> itself). I'd remove it.

I think you meant to say this "doesn't add much to the *commit
message*", and that the sentence should be removed from the commit
message, while retaining the actual documentation update in the patch.
--
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] t9801 and t9803 broken on next

[Lars Schneider]

> On 19 May 2016, at 19:03, Junio C Hamano  wrote:
> 
> Lars Schneider  writes:
> 
>> From my point of view little packs are no problem. I run fast-import on
>> a dedicated migration machine. After fast-import completion I run repack [1] 
>> before I upload the repo to its final location.
> 
> How do you determine that many little packs are not a problem to
> you, though?  Until they get repacked, they are where the
> fast-import will get existing objects from.  The more little packs
> you accumulate as you go, the more slow fast-import's access to them
> has to become.
True. But my particular use case is a one time operation for a given
repository. Therefore I don't care how long it takes. The only important
aspect is that the result is correct.

That being said, Peff's proposed fix looks correct to me but since I 
am no fast-import expert my opinion doesn't count too much.

- Lars

--
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: since/after not working properly

[Junio C Hamano]

On Thu, May 19, 2016 at 8:41 AM, Jeff Hawk  wrote:
> [jeff:~/src/git] master* 2s ± git log --pretty=format:"%ad" --date=short

Perhaps try it with %cd instead of %ad?
--
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

since/after not working properly

[Jeff Hawk]

[jeff:~/src/git] master* 2s ± git log --pretty=format:"%ad" --date=short 
--after=2016-05-01 | sort -u

2016-04-14
2016-04-27
2016-04-29
2016-05-01
2016-05-02
2016-05-03
2016-05-04
2016-05-05
2016-05-06
2016-05-07
2016-05-08
2016-05-09
2016-05-10
2016-05-11
2016-05-12
2016-05-13
2016-05-17
2016-05-18
[jeff:~/src/git] master* ± git --version
git version 2.8.2.537.gb153d2a

[jeff:~/src/git] master* ± git log --pretty=format:"%ad" --date=short 
--after=2016-04-01 | sort -u

2016-02-27
2016-03-08
2016-03-11
2016-03-14
2016-03-16
2016-03-18
2016-03-25
2016-03-27
2016-03-28
2016-03-30
2016-03-31
2016-04-01
2016-04-02
2016-04-03
2016-04-04
2016-04-05
2016-04-06
2016-04-07
2016-04-08
2016-04-09
2016-04-10
2016-04-12
2016-04-13
2016-04-14
2016-04-15
2016-04-17
2016-04-18
2016-04-19
2016-04-20
2016-04-21
2016-04-22
2016-04-24
2016-04-25
2016-04-26
2016-04-27
2016-04-28
2016-04-29
2016-05-01
2016-05-02
2016-05-03
2016-05-04
2016-05-05
2016-05-06
2016-05-07
2016-05-08
2016-05-09
2016-05-10
2016-05-11
2016-05-12
2016-05-13
2016-05-17
2016-05-18

--
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 v1 2/2] travis-ci: enable sequential test execution for t9113 and 9126

[Junio C Hamano]

Eric Wong  writes:

> Anyways, how about making the tests run on separate ports and
> not worry about serializing them at all?

Yeah, that does sound like a more sensible approach.
--
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] t9801 and t9803 broken on next

[Junio C Hamano]

Lars Schneider  writes:

> From my point of view little packs are no problem. I run fast-import on
> a dedicated migration machine. After fast-import completion I run repack [1] 
> before I upload the repo to its final location.

How do you determine that many little packs are not a problem to
you, though?  Until they get repacked, they are where the
fast-import will get existing objects from.  The more little packs
you accumulate as you go, the more slow fast-import's access to them
has to become.


--
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] upload-pack.c: use of parse-options API

[Junio C Hamano]

Matthieu Moy  writes:

>> +if (argc != 1)
>> +usage_with_options(upload_pack_usage, options);
>>  
>> -setup_path();
>> +if (timeout)
>> +daemon_mode = 1;
>>  
>> -dir = argv[i];
>> +setup_path();
>>  
>> +dir = argv[0];
>
> Not a problem with your code, but the patch shows "setup_path()"
> as moved while it is not really. Maybe using "send-email
> --patience" or some other diff option could make the patch nicer.

Encouraging use of "send-email" with "--patience" is a losing
approach if your goal is to present more reviewable diff, isn't it?
Using "send-email" as if it is a front-end of "format-patch" means
you lose the opportunity for the final proof-reading (not just
finding typoes in the message, but the shape of diff, like you are
pointing out).

Using "format-patch --patience" or some other diff option, and pick
the best one to give to "send-email" would indeed be a way to do so.

> Not really important as it does not change the final state.

I wondered if this is an example of real-world fallout from
0018da1^2~1 (xdiff: implement empty line chunk heuristic,
2016-04-19), but it does not seem to be so.

What is happening is that Antoine's patch (which is slightly
different from what you quoted above) has trailing whitespace after
"setup_path();", so it indeed is the original setup_path(); is
removed, a few lines were inserted, argv[i] reference is removed
and then a totally different "setup_path(); " was added there.

With that whitespace-breakage corrected, the resulting patch ends
more like this:

+   if (argc != 1)
+   usage_with_options(upload_pack_usage, options);
 
-   if (i != argc-1)
-   usage(upload_pack_usage);
+   if (timeout)
+   daemon_mode = 1;
 
setup_path();
 
-   dir = argv[i];
-
+   dir = argv[0];
if (!enter_repo(dir, strict))
die("'%s' does not appear to be a git repository", dir);
 
which is more reasonable.

So in the end, this was not "Not a problem with your code" ;-) It
was.
--
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] upload-pack.c: use of parse-options API

[Junio C Hamano]

Matthieu Moy  writes:

> antoine.qu...@ensimag.grenoble-inp.fr wrote:
>> Option parsing now uses the parser API instead of a local parser.
>> Code is now more compact.
>> Description for -stateless-rpc and --advertise-refs
>> come from the commit (gmane/131517)
>
> Please, use a real commit id instead of a Gmane link.
>
> We don't know how long Gmane will remain up, but a self
> reference from Git's history to itself will always remain valid.
>
> The following alias is handy for this:
>
> [alias]
> whatis = show -s --pretty='tformat:%h (%s, %ad)' --date=short
>
> In your case it would allow writing: 
>
> Description for --stateless-rpc and --advertise-refs is taken
> from commit 42526b4 (Add stateless RPC options to upload-pack,
> receive-pack, 2009-10-30).

Good suggestion; a real question may be how you went from
$gmane/131517 to 42526b4 (I vaguely recall that you have and publish
a database of sort; this would be a good place to advertise it ;-).

>
>> diff v1 v2:
>
> We usually say "diff" to refer to an actual diff. I'd write "changes since 
> v1" here.
>
>> +OPT_BOOL(0, "stateless-rpc", _rpc,
>> + N_("may perform only a single read-write cycle with 
>> stdin and stdout")),
>
> It's weird to define what an option does with "may". It's a
> property of --stateless-rpc, but does not really define it.

If this "may" were to express "The program might or might not do
this, and we do not know what it would do until we try", then it
indeed would be weird.  But the way the word "may" was used in
42526b4 is "the program is ALLOWED to do only a single exchange".

I agree that the phrasing is bad, in the sense that it can be
misread as a non-definition; perhaps

quit after a single request/response exchange

or something?

--
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] upload-pack.c: use of parse-options API

[Matthieu Moy]

antoine.qu...@ensimag.grenoble-inp.fr wrote:
> Option parsing now uses the parser API instead of a local parser.
> Code is now more compact.
> Description for -stateless-rpc and --advertise-refs
> come from the commit (gmane/131517)

Please, use a real commit id instead of a Gmane link.

We don't know how long Gmane will remain up, but a self
reference from Git's history to itself will always remain valid.

The following alias is handy for this:

[alias]
whatis = show -s --pretty='tformat:%h (%s, %ad)' --date=short

In your case it would allow writing: 

Description for --stateless-rpc and --advertise-refs is taken
from commit 42526b4 (Add stateless RPC options to upload-pack,
receive-pack, 2009-10-30).

> diff v1 v2:

We usually say "diff" to refer to an actual diff. I'd write "changes since v1" 
here.

> + OPT_BOOL(0, "stateless-rpc", _rpc,
> +  N_("may perform only a single read-write cycle with 
> stdin and stdout")),

It's weird to define what an option does with "may". It's a
property of --stateless-rpc, but does not really define it.

> + if (argc != 1)
> + usage_with_options(upload_pack_usage, options);
>  
> - setup_path();
> + if (timeout)
> + daemon_mode = 1;
>  
> - dir = argv[i];
> + setup_path();
>  
> + dir = argv[0];

Not a problem with your code, but the patch shows "setup_path()"
as moved while it is not really. Maybe using "send-email
--patience" or some other diff option could make the patch nicer.
Not really important as it does not change the final state.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] pull: warn on --verify-signatures with --rebase

[Junio C Hamano]

Alexander 'z33ky' Hirsch <1ze...@gmail.com> writes:

> Would "ignoring --verify-signatures for rebase" be sufficient? It does
> not describe why it is ignored though.

Yeah, I agree that that would be sufficient.
--
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] git-log prints wrong unixtime with --date=format:%s

[Michael Heerdegen]

Jeff King  writes:

> Oh, I agree that unix times are handy. I just think that "use %at in the
> pretty-format, instead of %ad and then %s in the date-format" is not
> such a bad workaround.

I had missed %at (and %ct).  Yes, works perfectly - thanks for the hint.


Regards,

Michael.
--
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] upload-pack.c: use of parse-options API

[Antoine Queru]

Option parsing now uses the parser API instead of a local parser.
Code is now more compact.
Description for -stateless-rpc and --advertise-refs
come from the commit (gmane/131517) where there were implemented.

Signed-off-by: Antoine Queru 
Signed-off-by: Matthieu Moy 
---

diff v1 v2:
Usage display "[options]" instead of "[--strict] [--timeout=]".
"argv" is now "const char **".
"-stateless-rpc and --advertise-refs" are no more hidden.
Description has been added for every option.
Usage is displayed if there is not exactly one non option argument.

If we agree on not having hidden option anymore, I will update the doc. 
 upload-pack.c | 62 +--
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/upload-pack.c b/upload-pack.c
index dc802a0..a8617ac 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -14,8 +14,12 @@
 #include "sigchain.h"
 #include "version.h"
 #include "string-list.h"
+#include "parse-options.h"
 
-static const char upload_pack_usage[] = "git upload-pack [--strict] 
[--timeout=] ";
+static const char * const upload_pack_usage[] = {
+   N_("git upload-pack [options] "),
+   NULL
+};
 
 /* Remember to update object flag allocation in object.h */
 #define THEY_HAVE  (1u << 11)
@@ -817,11 +821,21 @@ static int upload_pack_config(const char *var, const char 
*value, void *unused)
return parse_hide_refs_config(var, value, "uploadpack");
 }
 
-int main(int argc, char **argv)
+int main(int argc, const char **argv)
 {
-   char *dir;
-   int i;
+   const char *dir;
int strict = 0;
+   struct option options[] = {
+   OPT_BOOL(0, "stateless-rpc", _rpc,
+N_("may perform only a single read-write cycle with 
stdin and stdout")),
+   OPT_BOOL(0, "advertise-refs", _refs,
+N_("only the initial ref advertisement is output, 
program exits immediately")),
+   OPT_BOOL(0, "strict", ,
+N_("do not try /.git/ if  is no 
Git directory")),
+   OPT_INTEGER(0, "timeout", ,
+   N_("interrupt transfer after  seconds of 
inactivity")),
+   OPT_END()
+   };
 
git_setup_gettext();
 
@@ -829,41 +843,17 @@ int main(int argc, char **argv)
git_extract_argv0_path(argv[0]);
check_replace_refs = 0;
 
-   for (i = 1; i < argc; i++) {
-   char *arg = argv[i];
-
-   if (arg[0] != '-')
-   break;
-   if (!strcmp(arg, "--advertise-refs")) {
-   advertise_refs = 1;
-   continue;
-   }
-   if (!strcmp(arg, "--stateless-rpc")) {
-   stateless_rpc = 1;
-   continue;
-   }
-   if (!strcmp(arg, "--strict")) {
-   strict = 1;
-   continue;
-   }
-   if (starts_with(arg, "--timeout=")) {
-   timeout = atoi(arg+10);
-   daemon_mode = 1;
-   continue;
-   }
-   if (!strcmp(arg, "--")) {
-   i++;
-   break;
-   }
-   }
-
-   if (i != argc-1)
-   usage(upload_pack_usage);
+   argc = parse_options(argc, argv, NULL, options, upload_pack_usage, 0);
+   
+   if (argc != 1)
+   usage_with_options(upload_pack_usage, options);
 
-   setup_path();
+   if (timeout)
+   daemon_mode = 1;
 
-   dir = argv[i];
+   setup_path();   
 
+   dir = argv[0];
if (!enter_repo(dir, strict))
die("'%s' does not appear to be a git repository", dir);
 
-- 
2.8.2.403.gf2352ca

--
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] t6044: replace seq by test_seq

[Junio C Hamano]

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: Confusing --graph --all output with detached branches

[Junio C Hamano]

Bjørnar Snoksrud  writes:

> .. which indicates that `foo` is contained within `bar`. Maybe
>
> *   ff4265f  (HEAD -> master) Merge branch 'bar'
> |\
> | * 0bbc311  (bar) 5
> | * b1c9c49  4
> |
> | * ce053f9  (foo) 3
> |/
> * 8b62de9  2
> * cb7e7e2  1
>
> .. would be better?
>

Yes, it would be.
--
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: Confusing --graph --all output with detached branches

[Junio C Hamano]

--
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 6/6] upload-pack: provide a hook for running pack-objects

[Ævar Arnfjörð Bjarmason]

On Thu, May 19, 2016 at 2:08 PM, Jeff King  wrote:
> On Thu, May 19, 2016 at 12:12:43PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> But as you point out this makes the hook interface a bit unusual.
>> Wouldn't this give us the same security and normalize the hook
>> interface:
>>
>>  * Don't do the uploadpack.packObjectsHook variable, just have a
>> normal "pack-objects" hook that works like any other git hook
>>  * By default we don't run this hook unless core.runDangerousHooks (or
>> whatever we call it) is true.
>>  * The core.runDangerousHooks variable cannot be set on a per-repo
>> basis using your new config facility.
>>  * If there's a pack-objects hook and core.runDangerousHooks isn't
>> true we warn "not executing potentially unsafe hook $path_to_hook" and
>> carry on
>
> This is the "could we just set a bool" option I discussed in the commit
> message. The problem is that it doesn't let the admin say "I don't trust
> these repositories, but I _do_ want to run just this one hook when
> serving them, and not any other hooks".

Indeed. I wonder if there's really any overlap in practice between
systems administrators on a central Git server that are going to want
this relatively obscure feature *but* have potentially malicious users
/ repos, or enough to warrant this unusual edge case in how this
specific hook is configured, as opposed to reducing the special case
in how the hook is run with something like core.runDangerousHooks.

I'm definitely not saying that these patches should be blocked by
this, but it occurs to me that both your uploadpack.packObjectsHook
implementation and my proposed core.runDangerousHooks which normalizes
it a bit in some ways, but leaves it as a special case in others, are
both stumbling their way toward hacks that we might also solve with
some generally configurable restrictions system that takes advantage
of your earlier patches, e.g.:

$ cat /etc/gitconfig
# Not "repository" so hooksPath can't be set per-repo
[core]
configRestriction = "core.hooksPath: system, global"
hooksPath= "/etc/git/hooks"
disableHook.pack-objects = false # "true" by default
$ ls /etc/git/hooks/
# pre-receive, update etc. would just wrap the repository hook,
# but pack-objects is global
pre-receive update pack-objects, [...]

Of course those are some rather large hoops to jump through just to
accomplish this particular thing, but it would be more generally
composable and you could e.g. say users can't disable gc.auto or
whatever on their repos if they're hosted on your server. Which of
course assumes that you control the git binary and they can't run
their own.

>> This would allow use-cases that are a bit inconvenient with your patch
>> (again, if I'm understanding it correctly):
>>
>>  * I can set core.runDangerousHooks=true in /etc/gitconfig on my git
>> server because I also control all the repos, and I want to experiment
>> with trying this on a per-repo basis for users that are cloning from
>> me.
>>  * I can similarly play with this locally knowing I'm only cloning
>> repos I trust by setting core.runDangerousHooks=true in ~/.gitconfig
>
> Yes, those use cases are not well served by the git config alone. But
> you can do them (and much more) once your trusted hook is running (by
> checking $GIT_DIR, or looking in a database, or whatever you want).

Yeah, the reason I'm prodding you about this is because I want to test
this out at some point, and a *really* nice thing about the Git
configuration facility is that you can test all these sorts of things
on a per-repo basis now due to how all the git-config variables work
now.

With uploadpack.packObjectsHook you *can* do that by defining a global
pass-through hook, but it makes it more of a hassle to test changes
that straddle the divide between testing & production.

I.e. I might test this on one repo on our main git server, or on one
active repo, i.e. I don't have to deal with the case where I make some
silly syntax/logic error in the uploadpack.packObjectsHook dispatch
code (which is only needed for the security consideration) and that
impacts all repositories on the machine.
--
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] upload-pack.c: use of parse-options API

[Matthieu Moy]

Jeff King wrote:
> On Thu, May 19, 2016 at 12:10:31PM +0200, Antoine Queru wrote:
> 
> > > I'm not sure whether it is worth hiding the first two options. We
> > > typically hide "internal" options like this for user-facing programs, so
> > > as not to clutter the "-h" output. But upload-pack isn't a user-facing
> > > program. Anybody who is calling it directly with "-h" may be interested
> > > in even its more esoteric options.
> > 
> > In fact, to do this, I looked at builtin/receive-pack.c, where the parser
> > API
> > was already implemented, and these first two options were hidden. There
> > were
> > also no description for any options, so I thought it was not needed. Maybe
> > we
> > could update this file too ?
> 
> Yeah, I don't think it's that bad to hide them, and perhaps consistency
> with receive-pack is better.

IIRC, part of the reason receive-pack has hidden options is that it was a
GSoC microproject, and writing an accurate description is much harder than
what we expect from a microproject.

IOW, I'm all for un-hiding these options, but that shouldn't be a requirement
for a beginner's project.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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/RFC] Add userdiff built-in pattern for CSS code

[Matthieu Moy]

> Subject: [PATCH/RFC] Add userdiff built-in pattern for CSS code

We normally write subject lines as ":  CSS is widely used, motivating it being included as a built-in pattern.
> It must be noted that the word_regex for CSS (i.e. the regex defining what is
> a word in the language) does not consider '.' and '#' characters (in CSS
> selectors) to be part of the word. This behavior is documented by the test
> t/t4018/css-rule.

This text wasn't properly wrapped. Please wrap around 72 columns (M-q on Emacs,
or google "text wrap $youreditor" for others).

Also, saying _what_ your patch does is not the most important question here.
Focus on the _why_ in the commit message.

(We had a real-life off-list discussion about this and I agree that it's a
sensible behavior, but others may disagree)

> Add the info in documentation that CSS is now built-in.

This doesn't add much to the patch (we can already see that from the patch
itself). I'd remove it.

Missing sign-off (yours, and you can add mine to mark the fact that I allow
you to contribute to Git as part of your student project).

> +PATTERNS("css",
> +  "^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
> +  /* -- */
> +  /* This regex comes from W3C CSS specs. Should theorically also allow 
> ISO

s/theorically/theoretically/

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] rev-parse: fix --git-common-dir when executed from subpath of main tree

[Mike Rappazzo]

On Thu, May 19, 2016 at 3:49 AM, Mike Hommey  wrote:
> On Fri, Apr 08, 2016 at 08:35:51AM -0400, Mike Rappazzo wrote:
>> On Fri, Apr 8, 2016 at 7:47 AM, Duy Nguyen  wrote:
>> > On Mon, Apr 4, 2016 at 8:42 AM, Michael Rappazzo  
>> > wrote:
>> >> Executing `git-rev-parse --git-common-dir` from the root of the main
>> >> worktree results in '.git', which is the relative path to the git dir.
>> >> When executed from a subpath of the main tree it returned somthing like:
>> >> 'sub/path/.git'.  Change this to return the proper relative path to the
>> >> git directory (similar to `--show-cdup`).
>> >
>> > I faced a similar problem just a couple days ago, I expected "git
>> > rev-parse --git-path" to return a path relative to cwd too, but it
>> > returned relative to git dir. The same solution (or Eric's, which is
>> > cleaner in my opinion) applies. --shared-index-path also does
>> > puts(git_path(... and has the same problem. Do you want to fix them
>> > too?
>>
>> Sure, I can do that.  I will try to get it up soon.
>
> If I'm not mistaken, it looks like this fell off your radar. (I haven't
> seen an updated patch, and it doesn't look like the fix made it to any
> git branch). Would you mind updating?
>
> Cheers,
>
> Mike

There is a newer version submitted on May 6th[1].  Eric Sunshine has
submitted a patch [2] which fixes
up t1500.  It looks like that is in a stable form, so I will rebase my
v2 onto those changes, and resubmit
in the near future.

[1] http://thread.gmane.org/gmane.comp.version-control.git/293778
[2] http://thread.gmane.org/gmane.comp.version-control.git/294999
--
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 v5 2/2] convert: ce_compare_data() checks for a sha1 of a path

[tboegi]

From: Torsten Bögershausen 

To compare a file in working tree with the index, convert_to_git() is used,
the the result is hashed and the hash value compared with ce->sha1.

Deep down would_convert_crlf_at_commit() is invoked, to check if CRLF
are converted or not: When a CRLF had been in the index before, CRLF in
the working tree are not converted.

While in a merge, a file name in the working tree has different blobs
in the index with different hash values.
Forwarding ce->sha1 from ce_compare_data() into crlf_to_git() makes sure
the would_convert_crlf_at_commit() looks at the appropriate blob.

While at it, rename has_cr_in_index() into blob_has_cr()

Signed-off-by: Torsten Bögershausen 
---
 cache.h  |  1 +
 convert.c| 34 ++
 convert.h| 23 +++
 read-cache.c |  4 +++-
 sha1_file.c  | 17 +
 5 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/cache.h b/cache.h
index 15a2a10..868599e 100644
--- a/cache.h
+++ b/cache.h
@@ -605,6 +605,7 @@ extern int ie_modified(const struct index_state *, const 
struct cache_entry *, s
 
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
+#define HASH_USE_SHA_NOT_PATH 4
 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum 
object_type type, const char *path, unsigned flags);
 extern int index_path(unsigned char *sha1, const char *path, struct stat *st, 
unsigned flags);
 
diff --git a/convert.c b/convert.c
index f524b8d..92ddfb1 100644
--- a/convert.c
+++ b/convert.c
@@ -217,21 +217,26 @@ static void check_safe_crlf(const char *path, enum 
crlf_action crlf_action,
}
 }
 
-static int has_cr_in_index(const char *path)
+static int blob_has_cr(const unsigned char *sha1)
 {
unsigned long sz;
void *data;
-   int has_cr;
-
-   data = read_blob_data_from_cache(path, );
+   int has_cr = 0;
+   enum object_type type;
+   if (!sha1)
+   return 0;
+   data = read_sha1_file(sha1, , );
if (!data)
return 0;
-   has_cr = memchr(data, '\r', sz) != NULL;
+   if (type == OBJ_BLOB)
+   has_cr = memchr(data, '\r', sz) != NULL;
+
free(data);
return has_cr;
 }
 
-static int crlf_to_git(const char *path, const char *src, size_t len,
+static int crlf_to_git(const char *path, const unsigned char *sha1,
+  const char *src, size_t len,
   struct strbuf *buf,
   enum crlf_action crlf_action, enum safe_crlf checksafe)
 {
@@ -260,7 +265,9 @@ static int crlf_to_git(const char *path, const char *src, 
size_t len,
 * If the file in the index has any CR in it, do not 
convert.
 * This is the new safer autocrlf handling.
 */
-   if (has_cr_in_index(path))
+   if (!sha1)
+   sha1 = get_sha1_from_cache(path);
+   if (blob_has_cr(sha1))
return 0;
}
}
@@ -852,8 +859,9 @@ const char *get_convert_attr_ascii(const char *path)
return "";
 }
 
-int convert_to_git(const char *path, const char *src, size_t len,
-   struct strbuf *dst, enum safe_crlf checksafe)
+int convert_to_git_ce_sha1(const char *path, const unsigned char *sha1,
+  const char *src, size_t len,
+  struct strbuf *dst, enum safe_crlf checksafe)
 {
int ret = 0;
const char *filter = NULL;
@@ -874,7 +882,7 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
src = dst->buf;
len = dst->len;
}
-   ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
+   ret |= crlf_to_git(path, sha1, src, len, dst, ca.crlf_action, 
checksafe);
if (ret && dst) {
src = dst->buf;
len = dst->len;
@@ -882,7 +890,9 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
return ret | ident_to_git(path, src, len, dst, ca.ident);
 }
 
-void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,
+void convert_to_git_filter_fd(const char *path,
+ const unsigned char *sha1,
+ int fd, struct strbuf *dst,
  enum safe_crlf checksafe)
 {
struct conv_attrs ca;
@@ -894,7 +904,7 @@ void convert_to_git_filter_fd(const char *path, int fd, 
struct strbuf *dst,
if (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))
die("%s: clean filter '%s' failed", path, ca.drv->name);
 
-   crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
+   crlf_to_git(path, sha1, dst->buf, dst->len, dst, ca.crlf_action, 
checksafe);
ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
 }
 

[PATCH v5 1/2] read-cache: factor out get_sha1_from_index() helper

[tboegi]

From: Torsten Bögershausen 

Factor out the retrieval of the sha1 for a given path in
read_blob_data_from_index() into the function get_sha1_from_index().

This will be used in the next commit, when convert.c can do the
analyze for "text=auto" without slurping the whole blob into memory
at once.

Add a wrapper definition get_sha1_from_cache().

Signed-off-by: Torsten Bögershausen 
---
 cache.h  |  3 +++
 read-cache.c | 29 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index 160f8e3..15a2a10 100644
--- a/cache.h
+++ b/cache.h
@@ -379,6 +379,7 @@ extern void free_name_hash(struct index_state *istate);
 #define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
 #define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
 #define read_blob_data_from_cache(path, sz) 
read_blob_data_from_index(_index, (path), (sz))
+#define get_sha1_from_cache(path)  get_sha1_from_index (_index, (path))
 #endif
 
 enum object_type {
@@ -1038,6 +1039,8 @@ static inline void *read_sha1_file(const unsigned char 
*sha1, enum object_type *
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
 }
 
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path);
+
 /*
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..a3ef967 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2263,13 +2263,27 @@ int index_name_is_other(const struct index_state 
*istate, const char *name,
 
 void *read_blob_data_from_index(struct index_state *istate, const char *path, 
unsigned long *size)
 {
-   int pos, len;
+   const unsigned char *sha1;
unsigned long sz;
enum object_type type;
void *data;
 
-   len = strlen(path);
-   pos = index_name_pos(istate, path, len);
+   sha1 = get_sha1_from_index(istate, path);
+   if (!sha1)
+   return NULL;
+   data = read_sha1_file(sha1, , );
+   if (!data || type != OBJ_BLOB) {
+   free(data);
+   return NULL;
+   }
+   if (size)
+   *size = sz;
+   return data;
+}
+
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path)
+{
+   int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0) {
/*
 * We might be in the middle of a merge, in which
@@ -2285,14 +2299,7 @@ void *read_blob_data_from_index(struct index_state 
*istate, const char *path, un
}
if (pos < 0)
return NULL;
-   data = read_sha1_file(istate->cache[pos]->sha1, , );
-   if (!data || type != OBJ_BLOB) {
-   free(data);
-   return NULL;
-   }
-   if (size)
-   *size = sz;
-   return data;
+   return (istate->cache[pos]->sha1);
 }
 
 void stat_validity_clear(struct stat_validity *sv)
-- 
2.0.0.rc1.6318.g0c2c796

--
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 v5 0/2] CRLF: ce_compare_data() checks for a sha1 of a path

[tboegi]

From: Torsten Bögershausen 

Break up the old 10/10 series about CLRF handling into smaller
series. This is a small bugfix, when merge.renormalize is used
with core.autocrlf (and no attributes are set).
Prepare the refactoring to use the streaming interface.
Changes since v4:
 - Rename #define in cache.h into HASH_USE_SHA_NOT_PATH
 - convert.c: Rename has_cr_in_index into blob_has_cr()
   Better logic when sha1 != NULL,
   Adjusted the commit message

Torsten Bögershausen (2):
  read-cache: factor out get_sha1_from_index() helper
  convert: ce_compare_data() checks for a sha1 of a path

 cache.h  |  4 
 convert.c| 34 ++
 convert.h| 23 +++
 read-cache.c | 33 +
 sha1_file.c  | 17 +
 5 files changed, 79 insertions(+), 32 deletions(-)

-- 
2.0.0.rc1.6318.g0c2c796

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

Confusing --graph --all output with detached branches

[Bjørnar Snoksrud]

If you end up with a history containing commit A with parent B, where
a detached branch is merged into A while an unrelated branch branches
off of B, you will get the following output:

$ git log --graph --all
*   commit ff4265fcbfe94a2abe93c97d86e0d9f0e0a136cb
|\  Merge: 8b62de9 0bbc311
| | Author: XXX
| | Date:   Thu May 19 15:31:46 2016 +0200
| |
| | Merge branch 'bar'
| |
| * commit 0bbc3115caa089d8578eb52ba6c12c1b43153dad
| | Author: XXX
| | Date:   Thu May 19 15:31:40 2016 +0200
| |
| | 5
| |
| * commit b1c9c491a05d9ffeca2e1d7b5cbd392cd90eef82
|   Author: XXX
|   Date:   Thu May 19 15:31:39 2016 +0200
|
|   4
|
| * commit ce053f92a9290f5472aac3319ddadbaf5bf62371
|/  Author: XXX
|   Date:   Thu May 19 15:31:31 2016 +0200
|
|   3
|
* commit 8b62de9f421c0be46300a3e68f85c6e7608c24f6
| Author: XXX
| Date:   Thu May 19 15:31:02 2016 +0200
|
| 2
|
* commit cb7e7e2662f1477f030a889cab135ed5a19ba43e
  Author: XXX
  Date:   Thu May 19 15:31:00 2016 +0200

  1

-

Which is pretty informative  - after '2' we branched out for commit 3,
and the branch containing 4 and 5 was merged into master.

However, if you use the pretty common `one line` git log alias, you get this:

*   ff4265f  (HEAD -> master) Merge branch 'bar'
|\
| * 0bbc311  (bar) 5
| * b1c9c49  4
| * ce053f9  (foo) 3
|/
* 8b62de9  2
* cb7e7e2  1

.. which indicates that `foo` is contained within `bar`. Maybe

*   ff4265f  (HEAD -> master) Merge branch 'bar'
|\
| * 0bbc311  (bar) 5
| * b1c9c49  4
|
| * ce053f9  (foo) 3
|/
* 8b62de9  2
* cb7e7e2  1

.. would be better?

Reproduction steps:

git init
git commit --allow-empty -m 1
git commit --allow-empty -m 2
git checkout --branch foo
git checkout -b foo
git commit --allow-empty -m 3
git checkout --orphan bar
git commit --allow-empty -m 4
git commit --allow-empty -m 5
git checkout master
git merge bar -m merge
git log --graph --all  --pretty=format:'%Cred%h %C(yellow)%-d%Creset %s '


-- 
bjor...@snoksrud.no
--
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 6/6] upload-pack: provide a hook for running pack-objects

[Jeff King]

On Thu, May 19, 2016 at 12:12:43PM +0200, Ævar Arnfjörð Bjarmason wrote:

> On Thu, May 19, 2016 at 12:45 AM, Jeff King  wrote:
> >   3. You may want to insert a caching layer around
> >  pack-objects; it is the most CPU- and memory-intensive
> >  part of serving a fetch, and its output is a pure
> >  function[1] of its input, making it an ideal place to
> >  consolidate identical requests.
> 
> Cool to see this on the list after we talked briefly about this at Git
> Merge. Being able to cache this so simply is a great optimization.
> 
> As I recall you guys at GitHub ended up writing your own utility to
> cache output depending on stdin/argv because none existed already.

Yeah, we do have such a tool internally. It's possible we may one day
open-source that, but there aren't plans to do so right now.

I don't know whether this kind of caching would be useful to most sites
or not. It's good if you have lots of clients asking you for the same
thing at roughly the same time (say, somebody using "git pull" as a
deploy mechanism from their AWS cluster), but otherwise not.

> So do I understand correctly that you're trying to guard against the
> case where you e.g.:
> 
> rsync untrusted.example.com:/tmp/poison.git /tmp/
> git clone /tmp/poison.git /tmp/safe.git
> 
> Not hosing your system if the poison.git/config has a
> uploadpack.packObjectsHook that's "sudo rm -rf /".

I'm not that worried about this case, as it's just not that common.  I
think we're more concerned with two cases:

  1. multi-user servers where you ssh as yourself, but then access
 repositories owned by somebody else. This is basically the ssh case
 you described later.

  2. hosting sites that run git-daemon as the "daemon" user, but serve
 repositories owned by random untrusted users (where you would not
 want those users to run arbitrary code as "daemon").

> We've already accepted that "push" hooks like the pre-receive or
> update hook can do something malicious like this, so on one hand maybe
> we should say if you scp raw *.git repositories with hooks this sort
> of thing might happen, or if you ssh to a remote box and run their
> per-repo hooks it's really their problem to make sure their users
> don't run malicious hooks on your behalf.

Yeah, we make no promises for repositories that you push to. It's _only_
for the fetching side. It's kind of a funny distinction, but it's one we
have maintained since the beginning of git, and I do think there are
real sites that depend on it (see, e.g., the history of the
post-upload-pack hook added in the v1.6.x time frame).

Rsyncing a repository is generally of questionable safety. It's OK to
fetch from the result, but certainly not to run "git log" (which can run
arbitrary commands via external diff, etc).

> But as you point out this makes the hook interface a bit unusual.
> Wouldn't this give us the same security and normalize the hook
> interface:
> 
>  * Don't do the uploadpack.packObjectsHook variable, just have a
> normal "pack-objects" hook that works like any other git hook
>  * By default we don't run this hook unless core.runDangerousHooks (or
> whatever we call it) is true.
>  * The core.runDangerousHooks variable cannot be set on a per-repo
> basis using your new config facility.
>  * If there's a pack-objects hook and core.runDangerousHooks isn't
> true we warn "not executing potentially unsafe hook $path_to_hook" and
> carry on

This is the "could we just set a bool" option I discussed in the commit
message. The problem is that it doesn't let the admin say "I don't trust
these repositories, but I _do_ want to run just this one hook when
serving them, and not any other hooks".

> This would allow use-cases that are a bit inconvenient with your patch
> (again, if I'm understanding it correctly):
> 
>  * I can set core.runDangerousHooks=true in /etc/gitconfig on my git
> server because I also control all the repos, and I want to experiment
> with trying this on a per-repo basis for users that are cloning from
> me.
>  * I can similarly play with this locally knowing I'm only cloning
> repos I trust by setting core.runDangerousHooks=true in ~/.gitconfig

Yes, those use cases are not well served by the git config alone. But
you can do them (and much more) once your trusted hook is running (by
checking $GIT_DIR, or looking in a database, or whatever you want).

-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] upload-pack.c: use of parse-options API

[Jeff King]

On Thu, May 19, 2016 at 12:10:31PM +0200, Antoine Queru wrote:

> > I'm not sure whether it is worth hiding the first two options. We
> > typically hide "internal" options like this for user-facing programs, so
> > as not to clutter the "-h" output. But upload-pack isn't a user-facing
> > program. Anybody who is calling it directly with "-h" may be interested
> > in even its more esoteric options.
> 
> In fact, to do this, I looked at builtin/receive-pack.c, where the parser API
> was already implemented, and these first two options were hidden. There were 
> also no description for any options, so I thought it was not needed. Maybe we 
> could update this file too ?

Yeah, I don't think it's that bad to hide them, and perhaps consistency
with receive-pack is better. AFAICT, descriptions for hidden options are
pointless; they are never shown (in fact, it seems like OPT_HIDDEN_BOOL
probably shouldn't even take such an option?).

But the non-hidden options _do_ need non-NULL descriptions.

-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 6/6] upload-pack: provide a hook for running pack-objects

[Ævar Arnfjörð Bjarmason]

On Thu, May 19, 2016 at 12:45 AM, Jeff King  wrote:
>   3. You may want to insert a caching layer around
>  pack-objects; it is the most CPU- and memory-intensive
>  part of serving a fetch, and its output is a pure
>  function[1] of its input, making it an ideal place to
>  consolidate identical requests.

Cool to see this on the list after we talked briefly about this at Git
Merge. Being able to cache this so simply is a great optimization.

As I recall you guys at GitHub ended up writing your own utility to
cache output depending on stdin/argv because none existed already.

If anyone on-list knows about a generic command-line utility like that
(because apparently Peff couldn't think of any, and neither can I)
that would be useful to know.

> This hook is unlike the normal hook scripts found in the
> "hooks/" directory of a repository. Because we promise that
> upload-pack is safe to run in an untrusted repository, we
> cannot execute arbitrary code or commands found in the
> repository (neither in hooks/, nor in the config). So
> instead, this hook is triggered from a config variable that
> is explicitly ignored in the per-repo config.

So do I understand correctly that you're trying to guard against the
case where you e.g.:

rsync untrusted.example.com:/tmp/poison.git /tmp/
git clone /tmp/poison.git /tmp/safe.git

Not hosing your system if the poison.git/config has a
uploadpack.packObjectsHook that's "sudo rm -rf /".

And similarly having this run the hook on the remote:

# foo.example.com has a /etc/gitconfig with
uploadpack.packObjectsHook "sudo rm -rf /";
echo -n | ssh foo.example.com "git upload-pack /tmp/poison.git

But not this:

# bar.example.com has a /tmp/poison.git/config with
uploadpack.packObjectsHook "sudo rm -rf /";
echo -n | ssh foo.example.com "git upload-pack /tmp/poison.git

We've already accepted that "push" hooks like the pre-receive or
update hook can do something malicious like this, so on one hand maybe
we should say if you scp raw *.git repositories with hooks this sort
of thing might happen, or if you ssh to a remote box and run their
per-repo hooks it's really their problem to make sure their users
don't run malicious hooks on your behalf.

But I agree with you (if I've understand what this actually does) that
saying that it's always safe to "git clone" a repository is more
valuable and worth jumping through some hoops for.

But as you point out this makes the hook interface a bit unusual.
Wouldn't this give us the same security and normalize the hook
interface:

 * Don't do the uploadpack.packObjectsHook variable, just have a
normal "pack-objects" hook that works like any other git hook
 * By default we don't run this hook unless core.runDangerousHooks (or
whatever we call it) is true.
 * The core.runDangerousHooks variable cannot be set on a per-repo
basis using your new config facility.
 * If there's a pack-objects hook and core.runDangerousHooks isn't
true we warn "not executing potentially unsafe hook $path_to_hook" and
carry on

This would allow use-cases that are a bit inconvenient with your patch
(again, if I'm understanding it correctly):

 * I can set core.runDangerousHooks=true in /etc/gitconfig on my git
server because I also control all the repos, and I want to experiment
with trying this on a per-repo basis for users that are cloning from
me.
 * I can similarly play with this locally knowing I'm only cloning
repos I trust by setting core.runDangerousHooks=true in ~/.gitconfig
--
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/RFC] Add userdiff built-in pattern for CSS code

[William Duclot]

CSS is widely used, motivating it being included as a built-in pattern.
It must be noted that the word_regex for CSS (i.e. the regex defining what is a 
word in the language) does not consider '.' and '#' characters (in CSS 
selectors) to be part of the word. This behavior is documented by the test 
t/t4018/css-rule.

Add the info in documentation that CSS is now built-in.
---
 Documentation/gitattributes.txt |  2 ++
 t/t4018-diff-funcname.sh|  1 +
 t/t4018/css-rule|  4 
 t/t4034-diff-words.sh   |  1 +
 t/t4034/css/expect  | 16 
 t/t4034/css/post|  9 +
 t/t4034/css/pre |  9 +
 userdiff.c  |  8 
 8 files changed, 50 insertions(+)
 create mode 100644 t/t4018/css-rule
 create mode 100644 t/t4034/css/expect
 create mode 100644 t/t4034/css/post
 create mode 100644 t/t4034/css/pre

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e3b1de8..81f60ad 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -525,6 +525,8 @@ patterns are available:
 
 - `csharp` suitable for source code in the C# language.
 
+- `css` suitable for source code in the CSS language.
+
 - `fortran` suitable for source code in the Fortran language.
 
 - `fountain` suitable for Fountain documents.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 67373dc..1795ffc 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -30,6 +30,7 @@ diffpatterns="
bibtex
cpp
csharp
+   css
fortran
fountain
html
diff --git a/t/t4018/css-rule b/t/t4018/css-rule
new file mode 100644
index 000..84ed754
--- /dev/null
+++ b/t/t4018/css-rule
@@ -0,0 +1,4 @@
+RIGHT label.control-label {
+margin-top: 10px!important;
+border : 10px ChangeMe #C6C6C6;
+}
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index f2f55fc..912df91 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -302,6 +302,7 @@ test_language_driver ada
 test_language_driver bibtex
 test_language_driver cpp
 test_language_driver csharp
+test_language_driver css
 test_language_driver fortran
 test_language_driver html
 test_language_driver java
diff --git a/t/t4034/css/expect b/t/t4034/css/expect
new file mode 100644
index 000..b025d88
--- /dev/null
+++ b/t/t4034/css/expect
@@ -0,0 +1,16 @@
+diff --git a/pre b/post
+index 735f301..bdf6a90 100644
+--- a/pre
 b/post
+@@ -1,9 +1,9 @@
+.class-formother-form label.control-label {
+margin-top: 1015px!important;
+border : 10px dasheddotted #C6C6C6;
+}
+#CC
+padding-bottom#CB
+margin-left
+150pxem
+10px
+!important
+divli.class#id
diff --git a/t/t4034/css/post b/t/t4034/css/post
new file mode 100644
index 000..bdf6a90
--- /dev/null
+++ b/t/t4034/css/post
@@ -0,0 +1,9 @@
+.other-form label.control-label {
+margin-top: 15px!important;
+border : 10px dotted #C6C6C6;
+}
+#CB
+margin-left
+150em
+10px
+li.class#id
diff --git a/t/t4034/css/pre b/t/t4034/css/pre
new file mode 100644
index 000..735f301
--- /dev/null
+++ b/t/t4034/css/pre
@@ -0,0 +1,9 @@
+.class-form label.control-label {
+margin-top: 10px!important;
+border : 10px dashed #C6C6C6;
+}
+#CC
+padding-bottom
+150px
+10px!important
+div.class#id
diff --git a/userdiff.c b/userdiff.c
index 6bf2505..715a1fd 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -148,6 +148,14 @@ PATTERNS("csharp",
 "[a-zA-Z_][a-zA-Z0-9_]*"
 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"),
+PATTERNS("css",
+"^([^,{}]+)((,[^}]*\\{)|([ \t]*\\{))$",
+/* -- */
+/* This regex comes from W3C CSS specs. Should theorically also allow 
ISO 10646 characters U+00A0 and higher,
+ * this not handled in this regex. */
+"-?[_a-zA-F][-_a-zA-F0-9]*" /* identifiers */
+"|-?[0-9]+|\\#[0-9a-fA-F]+" /* numbers */
+),
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS
-- 
2.8.2.403.gdc9c9d0.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 v1 2/2] travis-ci: enable sequential test execution for t9113 and 9126

[Eric Wong]

larsxschnei...@gmail.com wrote:
> Enable t9113 and 9126 by defining the SVNSERVER_PORT. Since both tests
> open the same port during execution, they cannot run in parallel. Add
> a ".seq.sh" suffix to the test files and teach "prove" to run them
> sequentially.

Interesting, I guess I forgot the problem because had some
rules in config.mak to serialize them for many years, now :x

Anyways, how about making the tests run on separate ports and
not worry about serializing them at all?  Maybe there was a
reason we didn't do this years ago, but I forget...

But probably the best (but I guess more difficult) option is to
get svnserve+apache to do socket activation off a random port
bound by a parent process at startup.
--
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] upload-pack.c: use of parse-options API

[Antoine Queru]

Thanks for your input.

> > -static const char upload_pack_usage[] = "git upload-pack [--strict]
> > [--timeout=] ";
> > +static const char * const upload_pack_usage[] = {
> > +   N_("git upload-pack [--strict] [--timeout=] "),
> > +   NULL
> > +};
> 
> Do we need to enumerate the options here now? The usage message should
> list the options from "struct options", which make these redundant.
> 
> Something like:
> 
>   git -upload-pack [options] 
> 
> probably makes more sense.
> 

Yes, you are right.

> Of course, it's hard to read the usage message because...
> 
> > +   struct option options[] = {
> > +   OPT_HIDDEN_BOOL(0, "stateless-rpc", _rpc, NULL),
> > +   OPT_HIDDEN_BOOL(0, "advertise-refs", _refs, NULL),
> > +   OPT_BOOL(0, "strict", , NULL),
> > +   OPT_INTEGER(0, "timeout", , NULL),
> > +   OPT_END()
> > +   };
> 
> You've left the description text for each of these options as NULL, so
> running "git-upload-pack -h" segfaults.
> 
> I'm not sure whether it is worth hiding the first two options. We
> typically hide "internal" options like this for user-facing programs, so
> as not to clutter the "-h" output. But upload-pack isn't a user-facing
> program. Anybody who is calling it directly with "-h" may be interested
> in even its more esoteric options.
> 

In fact, to do this, I looked at builtin/receive-pack.c, where the parser API
was already implemented, and these first two options were hidden. There were 
also no description for any options, so I thought it was not needed. Maybe we 
could update this file too ?

> As a style nit, we usually spell comparison-with-zero as just:
> 
>   if (timeout)
>   daemon_mode = 1;

Because timeout is an int, I personnally think it is more understable to 
treat it as it. But I'll update itfor consistency. 

> > +   argc = parse_options(argc, (const char **)argv, NULL, options,
> > upload_pack_usage, 0);
> 
> Perhaps this is a good opportunity to use "const" in the declaration of
> main(), as most other git programs do. Then you can drop this cast.
> 

Ok.

> > setup_path();
> >  
> > -   dir = argv[i];
> > +   dir = argv[0];
> >  
> > if (!enter_repo(dir, strict))
> > die("'%s' does not appear to be a git repository", dir);
> 
> Prior to your patch, we used to check:
> 
>   -   if (i != argc-1)
>   -   usage(upload_pack_usage);
> 
> which ensured that "dir" was non-NULL. But with your patch, we may pass
> NULL to enter_repo. It fortunately catches this, but then we pass the
> NULL to die, which can segfault (though on glibc systems, stdio is kind
> enough to replace it with the "(null)").
> 
> Related, we silently accept extra arguments after your patch (whereas
> before we showed the usage message). You probably want to check "argc ==
> 1", and otherwise show the usage message.

Ok.

-Antoine
--
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] pull: warn on --verify-signatures with --rebase

[Alexander 'z33ky' Hirsch]

On Wed, May 18, 2016 at 09:04:24AM -0700, Junio C Hamano wrote:
> > Previously git-pull silently ignored the --verify-signatures option for
> > --rebase.
> 
> Missing pieces information that would have made the patch more
> complete are answers to these questions:
> 
>  - Is that a bad thing?  Why?
> 
>  - Assuming it is a bad thing, what is the solution this patch
>presents us?  Teach rebase about the option?  Error out the
>request?  What is the reason why "warn" was chosen as the best
>way forward?
> 

Is the warning a solution "for now" and might this become an error
should a valid usecase not be found after a while?

> >  builtin/pull.c  |  2 ++
> >  t/t5520-pull.sh | 16 
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/builtin/pull.c b/builtin/pull.c
> > index 1d7333c..0eafae7 100644
> > --- a/builtin/pull.c
> > +++ b/builtin/pull.c
> > @@ -815,6 +815,8 @@ static int run_rebase(const unsigned char *curr_head,
> > argv_array_push(, "--no-autostash");
> > else if (opt_autostash == 1)
> > argv_array_push(, "--autostash");
> > +   if (opt_verify_signatures && strcmp(opt_verify_signatures, 
> > "--verify-signatures") == 0)
> 
> The logic looks OK.  I would have written that long line as two
> lines, e.g.
> 
>   if (opt_verify_signatures &&
> !strcmp(opt_verify_signatures, "--verify-signatures")
> 
> though.
> 

I shall format it as per your suggestion in the next submission.

> > +   warning(_("git-rebase does not support --verify-signatures"));
> 
> Is this a good warning message?
> 
> As a casual reader, my reaction to this warning would be "Does not
> support?  Then what did it do instead?  Did it refuse to integrate
> my changes on top of what happened on the remote?"
> 

Indeed.

> Something like
> 
> warning(_("ignored --verify-signatures as it is meaningless in rebase"));
> 
> may convey what is going on better, in that it makes it clear that
> we are not failing "rebase" and instead we are ignoring "verify".
> 
> It is way too long for the final version, though.  A more concise
> way to say the same thing needs to be found.
> 

Would "ignoring --verify-signatures for rebase" be sufficient? It does
not describe why it is ignored though.


With Regards,
Alexander Hirsch
--
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 v1 1/2] travis-ci: enable Git SVN tests t91xx on Linux

[larsxschneider]

From: Lars Schneider 

Install the "git-svn" package to make the Perl SVN libraries available
to the Git SVN tests on Travis-CI Linux build machines.

Signed-off-by: Lars Schneider 
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index adab5b8..c20ec54 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -18,6 +18,7 @@ addons:
   apt:
 packages:
 - language-pack-is
+- git-svn
 
 env:
   global:
-- 
2.5.1

--
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 v1 2/2] travis-ci: enable sequential test execution for t9113 and 9126

[larsxschneider]

From: Lars Schneider 

Enable t9113 and 9126 by defining the SVNSERVER_PORT. Since both tests
open the same port during execution, they cannot run in parallel. Add
a ".seq.sh" suffix to the test files and teach "prove" to run them
sequentially.

Signed-off-by: Lars Schneider 
---
 .travis.yml| 3 ++-
 ...t-svn-dcommit-new-file.sh => t9113-git-svn-dcommit-new-file.seq.sh} | 0
 ...ectory.sh => t9126-git-svn-follow-deleted-readded-directory.seq.sh} | 0
 3 files changed, 2 insertions(+), 1 deletion(-)
 rename t/{t9113-git-svn-dcommit-new-file.sh => 
t9113-git-svn-dcommit-new-file.seq.sh} (100%)
 rename t/{t9126-git-svn-follow-deleted-readded-directory.sh => 
t9126-git-svn-follow-deleted-readded-directory.seq.sh} (100%)

diff --git a/.travis.yml b/.travis.yml
index c20ec54..605ced1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -29,9 +29,10 @@ env:
 - LINUX_P4_VERSION="16.1"
 - LINUX_GIT_LFS_VERSION="1.2.0"
 - DEFAULT_TEST_TARGET=prove
-- GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
+- GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save 
--rules='seq=*.seq.*' --rules='par=**'"
 - GIT_TEST_OPTS="--verbose --tee"
 - GIT_TEST_CLONE_2GB=YesPlease
+- SVNSERVE_PORT=3690
 # t9810 occasionally fails on Travis CI OS X
 # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI 
OS X
 - GIT_SKIP_TESTS="t9810 t9816"
diff --git a/t/t9113-git-svn-dcommit-new-file.sh 
b/t/t9113-git-svn-dcommit-new-file.seq.sh
similarity index 100%
rename from t/t9113-git-svn-dcommit-new-file.sh
rename to t/t9113-git-svn-dcommit-new-file.seq.sh
diff --git a/t/t9126-git-svn-follow-deleted-readded-directory.sh 
b/t/t9126-git-svn-follow-deleted-readded-directory.seq.sh
similarity index 100%
rename from t/t9126-git-svn-follow-deleted-readded-directory.sh
rename to t/t9126-git-svn-follow-deleted-readded-directory.seq.sh
-- 
2.5.1

--
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 v1 0/2] travis-ci: enable Git SVN tests

[larsxschneider]

From: Lars Schneider 

Hi,

this mini series enables SVN tests on Linux. Installing the Perl SVN libraries
was not that straight forward on OSX and therefore I skipped it (plus the OS X
tests take quite some time already).

The most notable change is the rename of two SVN test cases. I did that to
identify tests that need to run sequentially using prove [1]. Is this an
acceptable pattern? If yes, then I will document it in t/README.

Thanks,
Lars

[1] https://github.com/Perl-Toolchain-Gang/Test-Harness/pull/5

Lars Schneider (2):
  travis-ci: enable Git SVN tests t91xx on Linux
  travis-ci: enable sequential test execution for t9113 and 9126

 .travis.yml   | 4 +++-
 ...-svn-dcommit-new-file.sh => t9113-git-svn-dcommit-new-file.seq.sh} | 0
 ...ctory.sh => t9126-git-svn-follow-deleted-readded-directory.seq.sh} | 0
 3 files changed, 3 insertions(+), 1 deletion(-)
 rename t/{t9113-git-svn-dcommit-new-file.sh => 
t9113-git-svn-dcommit-new-file.seq.sh} (100%)
 rename t/{t9126-git-svn-follow-deleted-readded-directory.sh => 
t9126-git-svn-follow-deleted-readded-directory.seq.sh} (100%)

--
2.5.1

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