Re: [PATCH v2 3/3] worktree: teach "add" to check out existing branches

[Duy Nguyen]

On Sun, Feb 04, 2018 at 10:13:05PM +, Thomas Gummerer wrote:
> - if (opts->new_branch)
> + if (opts->checkout_existing_branch)
> + fprintf(stderr, _(", checking out existing branch '%s'"),
> + refname);
> + else if (opts->new_branch)
>   fprintf(stderr, _(", creating new branch '%s'"), 
> opts->new_branch);

I wonder if "creating branch" and "checkout out branch" are enough.

> @@ -423,14 +427,25 @@ static int add(int ac, const char **av, const char 
> *prefix)
>   if (ac < 2 && !opts.new_branch && !opts.detach) {
>   int n;
>   const char *s = worktree_basename(path, );
> - opts.new_branch = xstrndup(s, n);
> - if (guess_remote) {
> - struct object_id oid;
> - const char *remote =
> - unique_tracking_name(opts.new_branch, );
> - if (remote)
> - branch = remote;
> + const char *branchname = xstrndup(s, n);
> + struct strbuf ref = STRBUF_INIT;
> +
> + if (!strbuf_check_branch_ref(, branchname) &&
> + ref_exists(ref.buf)) {
> + branch = branchname;
> + opts.checkout_existing_branch = 1;
> + UNLEAK(branch);
> + } else {
> + opts.new_branch = branchname;
> + if (guess_remote) {
> + struct object_id oid;
> + const char *remote =
> + unique_tracking_name(opts.new_branch, 
> );

Deep indentation may be a sign that it's time to move all this code to
a separate function, maybe dwim_branch() or something.

> + if (remote)
> + branch = remote;
> + }
>   }
> + strbuf_release();
>   }
>  
>   if (ac == 2 && !opts.new_branch && !opts.detach) {

Re: [PATCH v2 1/3] worktree: improve message when creating a new worktree

[Duy Nguyen]

On Sun, Feb 04, 2018 at 10:13:03PM +, Thomas Gummerer wrote:
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 7cef5b120b..d1549e441d 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -303,7 +303,7 @@ static int add_worktree(const char *path, const char 
> *refname,
>   strbuf_addf(, "%s/commondir", sb_repo.buf);
>   write_file(sb.buf, "../..");
>  
> - fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
> + fprintf(stderr, _("Preparing %s (identifier %s)"), path, name);
>  
>   argv_array_pushf(_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
>   argv_array_pushf(_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
> @@ -320,10 +320,19 @@ static int add_worktree(const char *path, const char 
> *refname,
>   if (ret)
>   goto done;
>  
> + fprintf(stderr, _(", setting HEAD to %s"),

As a former translator, I'm not thrilled to see a sentence broken into
two pieces like this. I'm not a Japanese translator, but I think this
sentence is translated differently when the translator sees the whole
line "Preparing ..., setting ...".

Since the code between the first fprintf and this one should not take
long to execute, perhaps we can just delete the first printf and print
a whole [*] sentence here?

I think the purpose of "Preparing..." in the first place is to show
something when git is busy checkout out the worktree. As long as we
print it before git-reset, we should be good.

> + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
> +
> + strbuf_reset();
> + pp_commit_easy(CMIT_FMT_ONELINE, commit, );
> + if (sb.len > 0)
> + fprintf(stderr, " %s", sb.buf);

[*] Yes I know it's not "whole" because of this piece. But this one is
more or less a separate sentence already so it's probably ok.

Is it a bit too long to print everything in one line though?
CMIT_FMT_ONELINE could already fill 70 columns easily.

> + fputc('\n', stderr);
> +
>   if (opts->checkout) {
>   cp.argv = NULL;
>   argv_array_clear();
> - argv_array_pushl(, "reset", "--hard", NULL);
> + argv_array_pushl(, "reset", "--hard", "--quiet", NULL);
>   cp.env = child_env.argv;
>   ret = run_command();
>   if (ret)
> -- 
> 2.16.1.101.gde0f0111ea
> 

[PATCH v2 1/3] worktree: improve message when creating a new worktree

[Thomas Gummerer]

Currently 'git worktree add' produces output like the following, when
'--no-checkout' is not given:

Preparing foo (identifier foo)
HEAD is now at 26da330922 

where the first line is written to stderr, and the second line coming
from 'git reset --hard' is written to stdout, even though both lines are
supposed to tell the user what has happened.  In addition to someone not
familiar with 'git worktree', this might seem as if the current HEAD was
modified, not the HEAD in the new working tree.

If the '--no-checkout' flag is given, the output of 'git worktree add'
is just:

Preparing foo (identifier foo)

even though the HEAD is set to a commit, which is just not checked out.

Fix these inconsistencies by making the 'git reset --hard' call quiet,
and printing the message ourselves instead.

Signed-off-by: Thomas Gummerer 
---

We might want to do something similar for the 'git branch' command in
the 'add()' function, which currently prints some output if a branch
is set up to track a remote.  I couldn't find a good way to convey
that information in the output here, without making it too verbose,
and it's probably not great to loose that output either.

If anyone has any suggestions for that, I'd be glad to hear them :)

 builtin/worktree.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7cef5b120b..d1549e441d 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -303,7 +303,7 @@ static int add_worktree(const char *path, const char 
*refname,
strbuf_addf(, "%s/commondir", sb_repo.buf);
write_file(sb.buf, "../..");
 
-   fprintf_ln(stderr, _("Preparing %s (identifier %s)"), path, name);
+   fprintf(stderr, _("Preparing %s (identifier %s)"), path, name);
 
argv_array_pushf(_env, "%s=%s", GIT_DIR_ENVIRONMENT, sb_git.buf);
argv_array_pushf(_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
@@ -320,10 +320,19 @@ static int add_worktree(const char *path, const char 
*refname,
if (ret)
goto done;
 
+   fprintf(stderr, _(", setting HEAD to %s"),
+   find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
+
+   strbuf_reset();
+   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
+   if (sb.len > 0)
+   fprintf(stderr, " %s", sb.buf);
+   fputc('\n', stderr);
+
if (opts->checkout) {
cp.argv = NULL;
argv_array_clear();
-   argv_array_pushl(, "reset", "--hard", NULL);
+   argv_array_pushl(, "reset", "--hard", "--quiet", NULL);
cp.env = child_env.argv;
ret = run_command();
if (ret)
-- 
2.16.1.101.gde0f0111ea

[PATCH v2 2/3] worktree: be clearer when "add" dwim-ery kicks in

[Thomas Gummerer]

Currently there is no indication in the "git worktree add" output that
a new branch was created.  This would be especially useful information
in the case where the dwim of "git worktree add " kicks in, as the
user didn't explicitly ask for a new branch, but we create one from
them.

Print some additional output showing that a branch was created and the
branch name to help the user.

This will also be useful in the next commit, which introduces a new kind
of dwim-ery of checking out the branch if it exists instead of refusing
to create a new worktree in that case, and where it's nice to tell the
user which kind of dwim-ery kicked in.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index d1549e441d..74a853c2a3 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -320,6 +320,9 @@ static int add_worktree(const char *path, const char 
*refname,
if (ret)
goto done;
 
+   if (opts->new_branch)
+   fprintf(stderr, _(", creating new branch '%s'"), 
opts->new_branch);
+
fprintf(stderr, _(", setting HEAD to %s"),
find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV));
 
-- 
2.16.1.101.gde0f0111ea

[PATCH v2 0/3] worktree: teach "add" to check out existing branches

[Thomas Gummerer]

The previous round was at <20180121120208.12760-1-t.gumme...@gmail.com>.
Thanks Duy for the comments on the previous round.

In addition to the additional functionality, this series now includes
improvements to the output of the "git worktree add" command.  It
doesn't include any new magic to guess the branchname, as was
suggested, as that would be a bigger change in the behaviour of git
worktree, and is not a particular itch I have right now, so I'd prefer
to keep it separate.

Thomas Gummerer (3):
  worktree: improve message when creating a new worktree
  worktree: be clearer when "add" dwim-ery kicks in
  worktree: teach "add" to check out existing branches

 Documentation/git-worktree.txt |  9 +++--
 builtin/worktree.c | 45 +-
 t/t2025-worktree-add.sh| 15 +++---
 3 files changed, 55 insertions(+), 14 deletions(-)

-- 
2.16.1.101.gde0f0111ea

[PATCH v2 3/3] worktree: teach "add" to check out existing branches

[Thomas Gummerer]

Currently 'git worktree add ' creates a new branch named after the
basename of the path by default.  If a branch with that name already
exists, the command refuses to do anything, unless the '--force' option
is given.

However we can do a little better than that, and check the branch out if
it is not checked out anywhere else.  This will help users who just want
to check an existing branch out into a new worktree, and save a few
keystrokes.

As the current behaviour is to simply 'die()' when a branch with the name
of the basename of the path already exists, there are no backwards
compatibility worries here.

We will still 'die()' if the branch is checked out in another worktree,
unless the --force flag is passed.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt |  9 +++--
 builtin/worktree.c | 31 +++
 t/t2025-worktree-add.sh| 15 ---
 3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 41585f535d..98731b71a7 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -61,8 +61,13 @@ $ git worktree add --track -b   
/
 
 +
 If `` is omitted and neither `-b` nor `-B` nor `--detach` used,
-then, as a convenience, a new branch based at HEAD is created automatically,
-as if `-b $(basename )` was specified.
+then, as a convenience, a worktree with a branch named after
+`$(basename )` (call it ``) is created.  If ``
+doesn't exist, a new branch based on HEAD is automatically created as
+if `-b ` was given.  If `` exists in the repository,
+it will be checked out in the new worktree, if it's not checked out
+anywhere else, otherwise the command will refuse to create the
+worktree.
 
 list::
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 74a853c2a3..ea420bb90b 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -29,6 +29,7 @@ struct add_opts {
int keep_locked;
const char *new_branch;
int force_new_branch;
+   int checkout_existing_branch;
 };
 
 static int show_only;
@@ -320,7 +321,10 @@ static int add_worktree(const char *path, const char 
*refname,
if (ret)
goto done;
 
-   if (opts->new_branch)
+   if (opts->checkout_existing_branch)
+   fprintf(stderr, _(", checking out existing branch '%s'"),
+   refname);
+   else if (opts->new_branch)
fprintf(stderr, _(", creating new branch '%s'"), 
opts->new_branch);
 
fprintf(stderr, _(", setting HEAD to %s"),
@@ -423,14 +427,25 @@ static int add(int ac, const char **av, const char 
*prefix)
if (ac < 2 && !opts.new_branch && !opts.detach) {
int n;
const char *s = worktree_basename(path, );
-   opts.new_branch = xstrndup(s, n);
-   if (guess_remote) {
-   struct object_id oid;
-   const char *remote =
-   unique_tracking_name(opts.new_branch, );
-   if (remote)
-   branch = remote;
+   const char *branchname = xstrndup(s, n);
+   struct strbuf ref = STRBUF_INIT;
+
+   if (!strbuf_check_branch_ref(, branchname) &&
+   ref_exists(ref.buf)) {
+   branch = branchname;
+   opts.checkout_existing_branch = 1;
+   UNLEAK(branch);
+   } else {
+   opts.new_branch = branchname;
+   if (guess_remote) {
+   struct object_id oid;
+   const char *remote =
+   unique_tracking_name(opts.new_branch, 
);
+   if (remote)
+   branch = remote;
+   }
}
+   strbuf_release();
}
 
if (ac == 2 && !opts.new_branch && !opts.detach) {
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 2b95944973..721b0e4c26 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -198,13 +198,22 @@ test_expect_success '"add" with  omitted' '
test_cmp_rev HEAD bat
 '
 
-test_expect_success '"add" auto-vivify does not clobber existing branch' '
+test_expect_success '"add" auto-vivify checks out existing branch' '
test_commit c1 &&
test_commit c2 &&
git branch precious HEAD~1 &&
-   test_must_fail git worktree add precious &&
+   git worktree add precious &&
test_cmp_rev HEAD~1 precious &&
-   test_path_is_missing precious
+   (
+   cd precious &&
+   test_cmp_rev precious HEAD
+   )
+'
+
+test_expect_success '"add" auto-vivify fails with checked out branch' '
+   git checkout -b test-branch 

[PATCH v2] rebase: add --allow-empty-message option

[Genki Sky]

This option allows commits with empty commit messages to be rebased,
matching the same option in git-commit and git-cherry-pick. While empty
log messages are frowned upon, sometimes one finds them in older
repositories (e.g. translated from another VCS [0]), or have other
reasons for desiring them. The option is available in git-commit and
git-cherry-pick, so it is natural to make other git tools play nicely
with them. Adding this as an option allows the default to be "give the
user a chance to fix", while not interrupting the user's workflow
otherwise [1].

  [0]: https://stackoverflow.com/q/8542304
  [1]: https://public-inbox.org/git/7vd33afqjh@alter.siamese.dyndns.org/

To implement this, add a new --allow-empty-message flag. Then propagate
it to all calls of 'git commit', 'git cherry-pick', and 'git rebase--helper'
within the rebase scripts.

Signed-off-by: Genki Sky 
---

Notes:

  Thanks for the initial feedback, here are the changes from [v1]:
  - Made my commit message include the main motivations inline.
  - Moved new tests to t/t3405-rebase-malformed.sh, which has the
relevant test description: "rebase should handle arbitrary git
message".
  - Accordingly re-used existing test setup.
  - Minimized tests to just one for git-rebase--interactive.sh and one
for git-rebase--merge.sh. First, one test per file keeps things
focused while getting most benefit (other code within same file is
likely to be noticed by modifiers). And, while git-rebase--am.sh
does have one cherry-pick, it is only for a special case with
--keep-empty. So git-rebase--am.sh otherwise doesn't have this
empty-message issue.

  In general, there was a concern of over-testing, and over-checking
  implementation details. So, this time, I erred on the conservative
  side.

  [v1]: 
https://public-inbox.org/git/9d0414b869f21f38b81f92ee0619fd76410cbcfc.1517552392.git@genki.is/t/

 Documentation/git-rebase.txt |  5 +
 builtin/rebase--helper.c |  2 ++
 git-rebase--am.sh|  1 +
 git-rebase--interactive.sh   | 25 +++--
 git-rebase--merge.sh |  3 ++-
 git-rebase.sh|  5 +
 t/t3405-rebase-malformed.sh  | 23 +++
 7 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0..d713951b8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -244,6 +244,11 @@ leave out at most one of A and B, in which case it 
defaults to HEAD.
Keep the commits that do not change anything from its
parents in the result.

+--allow-empty-message::
+   By default, rebasing commits with an empty message will fail.
+   This option overrides that behavior, allowing commits with empty
+   messages to be rebased.
+
 --skip::
Restart the rebasing process by skipping the current patch.

diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index 7daee544b..2090114e9 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -22,6 +22,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
OPT_BOOL(0, "keep-empty", _empty, N_("keep empty 
commits")),
+   OPT_BOOL(0, "allow-empty-message", _empty_message,
+   N_("allow commits with empty messages")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e..0f7805179 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -46,6 +46,7 @@ then
# makes this easy
git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} --allow-empty \
$allow_rerere_autoupdate --right-only "$revisions" \
+   $allow_empty_message \
${restrict_revision+^$restrict_revision}
ret=$?
 else
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd2959..81c5b4287 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -281,7 +281,7 @@ pick_one () {

test -d "$rewritten" &&
pick_one_preserving_merges "$@" && return
-   output eval git cherry-pick $allow_rerere_autoupdate \
+   output eval git cherry-pick $allow_rerere_autoupdate 
$allow_empty_message \
${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")} \
"$strategy_args" $empty_args $ff "$@"

@@ -406,6 +406,7 @@ pick_one_preserving_merges () {
;;
*)
output eval git cherry-pick $allow_rerere_autoupdate \
+   $allow_empty_message \

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Eric Sunshine]

On Sun, Feb 4, 2018 at 1:55 PM, Ævar Arnfjörð Bjarmason
 wrote:
> On Sun, Feb 04 2018, Lucas Werkmeister jotted:
>>[--inetd |
>> [--listen=] [--port=]
>> [--user= [--group=]]]
>> +  [--log-destination=(stderr|syslog|none)]
>
> I micronit, but maybe worthwhile to have a preceeding commit to fix up
> that indentation of --listen and --user.

The '--listen' and '--user' lines are in the "alternate" ('|') branch
of '--inetd' so, as Lucas points out, this indentation appears
intentional, thus seems okay as-is.

>> +--log-destination=::
>> + Send log messages to the specified destination.
>> + Note that this option does not imply --verbose,
>
> Should `` quote --verbose, although I see similar to the WS change I
> noted above there's plenty of existing stuff in that doc doing it wrong.

As you mention, there are plenty of existing offenders already in this
file, so probably not worth a re-roll (perhaps Junio can fix this new
instance locally), but certainly good fodder for a follow-up patch.

>> + } else
>> + die("unknown log destination '%s'", v);
>
> Should be die(_("unknown..., i.e. use the _() macro.

No message in this source file use _(...) yet so probably not worth a
re-roll, but definitely something for a follow-up patch (by someone).

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Eric Sunshine]

On Sun, Feb 4, 2018 at 1:30 PM, Lucas Werkmeister
 wrote:
> This new option can be used to override the implicit --syslog of
> --inetd, or to disable all logging. (While --detach also implies
> --syslog, --log-destination=stderr with --detach is useless since
> --detach disassociates the process from the original stderr.) --syslog
> is retained as an alias for --log-destination=syslog.
> [...]
> Signed-off-by: Lucas Werkmeister 
> ---
> Notes:
> Fixes log_destination not being initialized correctly and some minor
> style issues.

Thanks. With the 'log_destination' initialization bug fixed, this
version looks good; I didn't find anything else worth commenting upon.
Ævar's micronits[1] could be addressed by a follow-up patch (if
desirable), but probably needn't hold up this patch.

[1]: https://public-inbox.org/git/871si0mvo0@evledraar.gmail.com/

Re: [GSoC][PATCH] commit: add a commit.signOff config variable

[Ævar Arnfjörð Bjarmason]

On Sun, Feb 04 2018, Eric Sunshine jotted:

> --- >8 ---
> for cfg in true false
> do
> for opt in '' --signoff --no-signoff
> do
> case "$opt:$cfg" in
> --signoff:*|:true) expect= ;;
> --no-signoff:*|:false) expect=! ;;
> esac
> test_expect_success "commit.signoff=$cfg & ${opt:---signoff omitted}" 
> '
> git -c commit.signoff=$cfg commit --allow-empty -m x $opt &&
> eval "$expect git log -1 --format=%B | grep ^Signed-off-by:"
> '
> done
> done
> --- >8 ---
>
> A final consideration is that tests run slowly on Windows, and although
> it's nice to be thorough by testing all six combinations, you can
> probably exercise the new code sufficiently by instead testing just two
> combinations. For instance, instead of all six combinations, test just
> these two:
>
> --- >8 ---
> test_expect_success 'commit.signoff=true & --signoff omitted' '
> git -c commit.signoff=true commit --allow-empty -m x &&
> git log -1 --format=%B | grep ^Signed-off-by:
> '
>
> test_expect_success 'commit.signoff=true & --no-signoff' '
> git -c commit.signoff=true commit --allow-empty -m x --no-signoff &&
> ! git log -1 --format=%B | grep ^Signed-off-by:
> '
> --- >8 ---

I just skimmed this, but just to this question. I don't think we need to
worry about 2 v.s. 6 tests having an impact on Windows performance, it's
just massive amounts of tests like my in-flight wildmatch test series
that really matter.

But if we are worring about 2 v.s. 6 there's always my in-flight
EXPENSIVE_ON_WINDOWS prereq :)

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Lucas Werkmeister]

On 04.02.2018 19:55, Ævar Arnfjörð Bjarmason wrote:
> 
> On Sun, Feb 04 2018, Lucas Werkmeister jotted:
> 
>>   [--inetd |
>>[--listen=] [--port=]
>>[--user= [--group=]]]
>> + [--log-destination=(stderr|syslog|none)]
> 
> I micronit, but maybe worthwhile to have a preceeding commit to fix up
> that indentation of --listen and --user.

I thought the indentation here is intentional, since we’re still inside
the [] pair (either --inetd or --listen, --port, …).

> 
>> +--log-destination=::
>> +Send log messages to the specified destination.
>> +Note that this option does not imply --verbose,
> 
> Should `` quote --verbose, although I see similar to the WS change I
> noted above there's plenty of existing stuff in that doc doing it wrong.

I could send a follow-up to consistently ``-quote all options in
git-daemon.txt… or would that be rejected as unnecessarily cluttering
the history or the `git blame`?

>> +} else
>> +die("unknown log destination '%s'", v);
> 
> Should be die(_("unknown..., i.e. use the _() macro.
> 
> Anyway, this looks fine to be with our without these proposed
> bikeshedding changes above. Thanks.
>



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Ævar Arnfjörð Bjarmason]

On Sun, Feb 04 2018, Lucas Werkmeister jotted:

>[--inetd |
> [--listen=] [--port=]
> [--user= [--group=]]]
> +  [--log-destination=(stderr|syslog|none)]

I micronit, but maybe worthwhile to have a preceeding commit to fix up
that indentation of --listen and --user.

> +--log-destination=::
> + Send log messages to the specified destination.
> + Note that this option does not imply --verbose,

Should `` quote --verbose, although I see similar to the WS change I
noted above there's plenty of existing stuff in that doc doing it wrong.
> + } else
> + die("unknown log destination '%s'", v);

Should be die(_("unknown..., i.e. use the _() macro.

Anyway, this looks fine to be with our without these proposed
bikeshedding changes above. Thanks.

[PATCH v4] daemon: add --log-destination=(stderr|syslog|none)

[Lucas Werkmeister]

This new option can be used to override the implicit --syslog of
--inetd, or to disable all logging. (While --detach also implies
--syslog, --log-destination=stderr with --detach is useless since
--detach disassociates the process from the original stderr.) --syslog
is retained as an alias for --log-destination=syslog.

--log-destination always overrides implicit --syslog regardless of
option order. This is different than the “last one wins” logic that
applies to some implicit options elsewhere in Git, but should hopefully
be less confusing. (I also don’t know if *all* implicit options in Git
follow “last one wins”.)

The combination of --inetd with --log-destination=stderr is useful, for
instance, when running `git daemon` as an instanced systemd service
(with associated socket unit). In this case, log messages sent via
syslog are received by the journal daemon, but run the risk of being
processed at a time when the `git daemon` process has already exited
(especially if the process was very short-lived, e.g. due to client
error), so that the journal daemon can no longer read its cgroup and
attach the message to the correct systemd unit (see systemd/systemd#2913
[1]). Logging to stderr instead can solve this problem, because systemd
can connect stderr directly to the journal daemon, which then already
knows which unit is associated with this stream.

[1]: https://github.com/systemd/systemd/issues/2913

Helped-by: Ævar Arnfjörð Bjarmason 
Helped-by: Junio C Hamano 
Helped-by: Eric Sunshine 
Signed-off-by: Lucas Werkmeister 
---

Notes:
Fixes log_destination not being initialized correctly and some minor
style issues.

 Documentation/git-daemon.txt | 28 ---
 daemon.c | 46 +---
 2 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 3c91db7be..56d54a489 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -20,6 +20,7 @@ SYNOPSIS
 [--inetd |
  [--listen=] [--port=]
  [--user= [--group=]]]
+[--log-destination=(stderr|syslog|none)]
 [...]
 
 DESCRIPTION
@@ -80,7 +81,8 @@ OPTIONS
do not have the 'git-daemon-export-ok' file.
 
 --inetd::
-   Have the server run as an inetd service. Implies --syslog.
+   Have the server run as an inetd service. Implies --syslog (may be
+   overridden with `--log-destination=`).
Incompatible with --detach, --port, --listen, --user and --group
options.
 
@@ -110,8 +112,28 @@ OPTIONS
zero for no limit.
 
 --syslog::
-   Log to syslog instead of stderr. Note that this option does not imply
-   --verbose, thus by default only error conditions will be logged.
+   Short for `--log-destination=syslog`.
+
+--log-destination=::
+   Send log messages to the specified destination.
+   Note that this option does not imply --verbose,
+   thus by default only error conditions will be logged.
+   The  must be one of:
++
+--
+stderr::
+   Write to standard error.
+   Note that if `--detach` is specified,
+   the process disconnects from the real standard error,
+   making this destination effectively equivalent to `none`.
+syslog::
+   Write to syslog, using the `git-daemon` identifier.
+none::
+   Disable all logging.
+--
++
+The default destination is `syslog` if `--inetd` or `--detach` is specified,
+otherwise `stderr`.
 
 --user-path::
 --user-path=::
diff --git a/daemon.c b/daemon.c
index e37e343d0..fb538e367 100644
--- a/daemon.c
+++ b/daemon.c
@@ -9,7 +9,12 @@
 #define initgroups(x, y) (0) /* nothing */
 #endif
 
-static int log_syslog;
+static enum log_destination {
+   LOG_DESTINATION_UNSET = -1,
+   LOG_DESTINATION_NONE = 0,
+   LOG_DESTINATION_STDERR = 1,
+   LOG_DESTINATION_SYSLOG = 2,
+} log_destination = LOG_DESTINATION_UNSET;
 static int verbose;
 static int reuseaddr;
 static int informative_errors;
@@ -25,6 +30,7 @@ static const char daemon_usage[] =
 "   [--access-hook=]\n"
 "   [--inetd | [--listen=] [--port=]\n"
 "  [--detach] [--user= [--group=]]\n"
+"   [--log-destination=(stderr|syslog|none)]\n"
 "   [...]";
 
 /* List of acceptable pathname prefixes */
@@ -74,11 +80,14 @@ static const char *get_ip_address(struct hostinfo *hi)
 
 static void logreport(int priority, const char *err, va_list params)
 {
-   if (log_syslog) {
+   switch (log_destination) {
+   case LOG_DESTINATION_SYSLOG: {
char buf[1024];
vsnprintf(buf, sizeof(buf), err, params);
syslog(priority, "%s", buf);
-   } else {
+   break;
+   }
+   case LOG_DESTINATION_STDERR:
/*
 * Since stderr is set to 

Re: [PATCH v3] daemon: add --log-destination=(stderr|syslog|none)

[Lucas Werkmeister]

On 04.02.2018 07:36, Eric Sunshine wrote:
> On Sat, Feb 3, 2018 at 6:08 PM, Lucas Werkmeister
>  wrote:
>> This new option can be used to override the implicit --syslog of
>> --inetd, or to disable all logging. (While --detach also implies
>> --syslog, --log-destination=stderr with --detach is useless since
>> --detach disassociates the process from the original stderr.) --syslog
>> is retained as an alias for --log-destination=syslog.
>> [...]
>> Signed-off-by: Lucas Werkmeister 
> 
> Thanks for the re-roll. There are a few comments below. Except for one
> apparent bug, I'm not sure the others are worth a re-roll...
> 
>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
>> @@ -110,8 +112,26 @@ OPTIONS
>> +--log-destination=::
>> +   Send log messages to the specified destination.
>> +   Note that this option does not imply --verbose,
>> +   thus by default only error conditions will be logged.
>> +   The  defaults to `stderr`, and must be one of:
> 
> I wonder if this should say instead:
> 
> The default destination is `stderr` unless `syslog`
> is implied by `--inetd` or `--detach`, ...
> 
>> diff --git a/daemon.c b/daemon.c
>> @@ -9,7 +9,12 @@
>> -static int log_syslog;
>> +static enum log_destination {
>> +   LOG_DESTINATION_UNSET = -1,
>> +   LOG_DESTINATION_NONE = 0,
>> +   LOG_DESTINATION_STDERR = 1,
>> +   LOG_DESTINATION_SYSLOG = 2,
>> +} log_destination;
> 
> Doesn't log_destination need to be initialized to
> LOG_DESTINATION_UNSET (see [1])? As it stands, being static, it's
> initialized automatically to 0 (LOG_DESTINATION_NONE), which borks the
> logic below.

Thanks, I knew I’d forgotten something :)

> 
>> @@ -74,11 +80,14 @@ static const char *get_ip_address(struct hostinfo *hi)
>>  static void logreport(int priority, const char *err, va_list params)
>>  {
>> +   switch (log_destination) {
>> +   case LOG_DESTINATION_SYSLOG: {
>> char buf[1024];
>> vsnprintf(buf, sizeof(buf), err, params);
>> syslog(priority, "%s", buf);
>> +   break;
>> +   }
>> +   case LOG_DESTINATION_STDERR:
>> /*
>>  * Since stderr is set to buffered mode, the
>>  * logging of different processes will not overlap
>> @@ -88,6 +97,10 @@ static void logreport(int priority, const char *err, 
>> va_list params)
>> vfprintf(stderr, err, params);
>> fputc('\n', stderr);
>> fflush(stderr);
>> +   break;
>> +   case LOG_DESTINATION_NONE:
>> +   case LOG_DESTINATION_UNSET:
>> +   break;
> 
> Since LOG_DESTINATION_UNSET should never occur, perhaps this should be
> written as:
> 
> case LOG_DESTINATION_NONE:
> break;
> case LOG_DESTINATION_UNSET:
> BUG("impossible destination: %d", log_destination);

Good point, I didn’t know about the BUG() macro. But putting the
destination in the message seems unnecessary if it can only be a single
value – or would you make this a default: case?

> 
>> @@ -1297,9 +1309,22 @@ int cmd_main(int argc, const char **argv)
>> +   if (skip_prefix(arg, "--log-destination=", )) {
>> +   if (!strcmp(v, "syslog")) {
>> +   log_destination = LOG_DESTINATION_SYSLOG;
>> +   continue;
>> +   } else if (!strcmp(v, "stderr")) {
>> +   log_destination = LOG_DESTINATION_STDERR;
>> +   continue;
>> +   } else if (!strcmp(v, "none")) {
>> +   log_destination = LOG_DESTINATION_NONE;
>> +   continue;
>> +   } else
>> +   die("Unknown log destination %s", v);
> 
> Mentioned previously[1], this probably ought to start with lowercase.
> It also would be more readable to set off the unknown value with a
> colon or quotes:
> 
> die("unknown log destination '%s', v);
> 
>> @@ -1402,7 +1426,14 @@ int cmd_main(int argc, const char **argv)
>> -   if (log_syslog) {
>> +   if (log_destination == LOG_DESTINATION_UNSET) {
>> +   if (inetd_mode || detach)
>> +   log_destination = LOG_DESTINATION_SYSLOG;
>> +   else
>> +   log_destination = LOG_DESTINATION_STDERR;
>> +   }
>> +
>> +   if (log_destination == LOG_DESTINATION_SYSLOG) {
>> openlog("git-daemon", LOG_PID, LOG_DAEMON);
>> set_die_routine(daemon_die);
> 
> [1]: 
> https://public-inbox.org/git/capig+ctetjq9lsh68fe5otcj9twq9gsbgzdrjzhohtavfvr...@mail.gmail.com/
> 

I’ll send a new version shortly, also addressing your other comments
which I didn’t reply to here.

Cheers,
Lucas



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [PATCHv2] tag: add --edit option

[Nicolas Morey-Chaisemartin]

Le 02/02/2018 à 20:16, Eric Sunshine a écrit :
> On Fri, Feb 2, 2018 at 11:48 AM, Nicolas Morey-Chaisemartin
>  wrote:
>> What message do you suggest ?  As I said in a previous mail, a
>> simple "Editor failure, cancelling {commit, tag}" should be enough
>> as launch_editor already outputs error messages describing what
>> issue the editor had.
>>
>> I don't think suggesting moving to --no-edit || -m || -F is that
>> helpful.  It's basically saying your setup is broken, but you can
>> workaround by setting those options (and not saying that you're
>> going to have some more issues later one).
> If it's the case the launch_editor() indeed outputs an appropriate
> error message, then the existing error message from tag.c is already
> appropriate when --edit is not specified.

I don't fully agree with the current message. The right thing to do is to fix 
the editor, not to hide the issue.
A better message would be "Editor failed. Fix it, or supply the message using 
either..."
At least we suggest the right way to do it first.

>  It's only the --edit case
> that the tag.c's additional message is somewhat weird. And, in fact,
> suppressing tag.c's message might be the correct thing to do in the
> --edit case:
>
> static void create_tag(...) {
> ...
> if (launch_editor(...)) {
>if (!opt->use_editor)
>fprintf(stderr, _("... use either -m or -F ..."));
> exit(1);
> }
>
> I don't feel strongly about it either way and am fine with just
> punting on the issue until someone actually complains about it.

The test should be opt->message_given && opt->use_editor.
If just --edit is provided but no -m/-F, --edit does not have any effect and it 
should be the same error message as when no option is given.

Nicolas

[PATCH] gitk: Add option to not save window geometry

[Rodney Lorrimar]

Saving and restoring the pane widths is not helpful when running gitk
under a tiling window manager. It often results in the second and
third panes being pushed to the far right of the window at startup.

This change adds a preference to disable the saving of window geometry
-- i.e. window position, size, and pane widths.

Signed-off-by: Rodney Lorrimar 
---
 gitk-git/gitk | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index a14d7a16b..c3ef0e824 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2857,6 +2857,7 @@ proc savestuff {w} {
 upvar #0 viewperm current_viewperm
 upvar #0 nextviewnum current_nextviewnum
 upvar #0 use_ttk current_use_ttk
+upvar #0 geometry current_geometry
 
 if {$stuffsaved} return
 if {![winfo viewable .]} return
@@ -2886,19 +2887,23 @@ proc savestuff {w} {
}
}
 
-   puts $f "set geometry(main) [wm geometry .]"
-   puts $f "set geometry(state) [wm state .]"
-   puts $f "set geometry(topwidth) [winfo width .tf]"
-   puts $f "set geometry(topheight) [winfo height .tf]"
-   if {$current_use_ttk} {
-   puts $f "set geometry(pwsash0) \"[.tf.histframe.pwclist sashpos 0] 
1\""
-   puts $f "set geometry(pwsash1) \"[.tf.histframe.pwclist sashpos 1] 
1\""
-   } else {
-   puts $f "set geometry(pwsash0) \"[.tf.histframe.pwclist sash coord 
0]\""
-   puts $f "set geometry(pwsash1) \"[.tf.histframe.pwclist sash coord 
1]\""
+   set savegeometry [expr {![info exists current_geometry(save)] || 
$current_geometry(save)}]
+   puts $f "set geometry(save) $savegeometry"
+   if {$savegeometry} {
+   puts $f "set geometry(main) [wm geometry .]"
+   puts $f "set geometry(state) [wm state .]"
+   puts $f "set geometry(topwidth) [winfo width .tf]"
+   puts $f "set geometry(topheight) [winfo height .tf]"
+   if {$current_use_ttk} {
+   puts $f "set geometry(pwsash0) \"[.tf.histframe.pwclist sashpos 
0] 1\""
+   puts $f "set geometry(pwsash1) \"[.tf.histframe.pwclist sashpos 
1] 1\""
+   } else {
+   puts $f "set geometry(pwsash0) \"[.tf.histframe.pwclist sash 
coord 0]\""
+   puts $f "set geometry(pwsash1) \"[.tf.histframe.pwclist sash 
coord 1]\""
+   }
+   puts $f "set geometry(botwidth) [winfo width .bleft]"
+   puts $f "set geometry(botheight) [winfo height .bleft]"
}
-   puts $f "set geometry(botwidth) [winfo width .bleft]"
-   puts $f "set geometry(botheight) [winfo height .bleft]"
 
array set view_save {}
array set views {}
@@ -11488,7 +11493,7 @@ proc create_prefs_page {w} {
 proc prefspage_general {notebook} {
 global NS maxwidth maxgraphpct showneartags showlocalchanges
 global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
-global hideremotes want_ttk have_ttk maxrefs
+global hideremotes want_ttk have_ttk maxrefs geometry
 
 set page [create_prefs_page $notebook.general]
 
@@ -11549,6 +11554,11 @@ proc prefspage_general {notebook} {
${NS}::label $page.ttk_note -text [mc "(currently unavailable)"]
 }
 grid x $page.want_ttk $page.ttk_note -sticky w
+${NS}::checkbutton $page.save_geometry -variable geometry(save) \
+   -text [mc "Save/restore window geometry"]
+${NS}::label $page.save_geometry_note -text [mc "(disable for tiling WMs)"]
+grid x $page.save_geometry $page.save_geometry_note -sticky w
+
 return $page
 }
 
-- 
2.16.1

F.LLI PISTOLESI Snc

[. F.LLI PISTOLESI Snc]

Hello ,

I am looking for a reliable supplier /manufacturer of products for sell in 
Europe.

I came across your listing and wanted to get some information regarding minimum 
Order Quantities, FOB pricing and also the possibility of packaging including 
payments terms.

So could you please get back to be with the above informations as soon as 
possible .

My email is :tm6428...@gmail.com

Many thanks and i looking forward to hearing from you and hopefully placing an 
order with you company.

Best Regards
Lorenzo Delleani.

F.LLI PISTOLESI Snc Company P.O. box 205
2740 AE Waddinxveen
The Netherlands

Re: contrib/completion/git-completion.bash: declare -g is not portable

[Lucas Werkmeister]

On 04.02.2018 10:57, Eric Sunshine wrote:
> On Sun, Feb 4, 2018 at 4:45 AM, Duy Nguyen  wrote:
>> On Sun, Feb 4, 2018 at 12:20 AM, Torsten Bögershausen  wrote:
>>> After running t9902-completion.sh on Mac OS I got a failure
>>> in this style:
>>
>> Sorry I was new with this bash thingy. Jeff already answered this (and
>> I will fix it in the re-roll) but just for my own information, what
>> bash version is shipped with Mac OS?
> 
> The MacOS bash is very old; note the copyright date:
> 
> % /bin/bash --version
> GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin16)
> Copyright (C) 2007 Free Software Foundation, Inc.
> 
> A recent bash installed manually (not from Apple):
> 
> % /usr/local/bin/bash --version
> GNU bash, version 4.4.18(1)-release (x86_64-apple-darwin16.7.0)
> Copyright (C) 2016 Free Software Foundation, Inc.
> 

Specifically, Apple ships the latest version of Bash 3.x, which is the
last version published under the GPLv2+ – Bash 4.x switched to GPLv3+.
Users can install newer Bash versions themselves, e. g. using Homebrew,
but that doesn’t help us here.



smime.p7s
Description: S/MIME Cryptographic Signature

Re: [GSoC][PATCH] commit: add a commit.signOff config variable

[Eric Sunshine]

On Sun, Feb 04, 2018 at 10:03:18AM +0800, Chen Jingpiao wrote:
> Add the commit.signOff configuration variable to use the -s or --signoff
> option of git commit by default.
> 
> Signed-off-by: Chen Jingpiao 
> ---
> 
> Though we can configure signoff using format.signOff variable. Someone like to
> add Signed-off-by line by the committer.

This commentary explains why this feature is desirable, therefore it
would be a good idea to include this in the commit message itself.

> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
> @@ -505,6 +505,75 @@ Myfooter: x" &&
> +test_expect_success "commit.signoff=true and --signoff omitted" '
> + echo 7 >positive &&
> + git add positive &&
> + git -c commit.signoff=true commit -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + (
> + echo thank you
> + echo
> + git var GIT_COMMITTER_IDENT |
> + sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> + ) >expected &&
> + test_cmp expected actual
> +'

The bodies of these test are quite noisy, doing a lot of work that isn't
really necessary, which makes it difficult to figure out what is really
being tested. Other tests in this script already check that the commit
message is properly formatted when Signed-off-by: is inserted so you
don't need to repeat all that boilerplate here.

Instead, you are interested only in whether or not Signed-off-by: has
been added to the message. For that purpose, you can use a simple 'grep'
expression.

The amount of copy/paste code in these six tests is also unfortunate.
Rather than merely repeating the same code over and over, you could
instead parameterize the test. For instance, you could run all six tests
via a simple for-loop:

--- >8 ---
for cfg in true false
do
for opt in '' --signoff --no-signoff
do
case "$opt:$cfg" in
--signoff:*|:true) expect= ;;
--no-signoff:*|:false) expect=! ;;
esac
test_expect_success "commit.signoff=$cfg & ${opt:---signoff omitted}" '
git -c commit.signoff=$cfg commit --allow-empty -m x $opt &&
eval "$expect git log -1 --format=%B | grep ^Signed-off-by:"
'
done
done
--- >8 ---

A final consideration is that tests run slowly on Windows, and although
it's nice to be thorough by testing all six combinations, you can
probably exercise the new code sufficiently by instead testing just two
combinations. For instance, instead of all six combinations, test just
these two:

--- >8 ---
test_expect_success 'commit.signoff=true & --signoff omitted' '
git -c commit.signoff=true commit --allow-empty -m x &&
git log -1 --format=%B | grep ^Signed-off-by:
'

test_expect_success 'commit.signoff=true & --no-signoff' '
git -c commit.signoff=true commit --allow-empty -m x --no-signoff &&
! git log -1 --format=%B | grep ^Signed-off-by:
'
--- >8 ---

> +test_expect_success "commit.signoff=true and --signoff" '
> + echo 8 >positive &&
> + git add positive &&
> + git -c commit.signoff=true commit --signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + (
> + echo thank you
> + echo
> + git var GIT_COMMITTER_IDENT |
> + sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> + ) >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=true and --no-signoff" '
> + echo 9 >positive &&
> + git add positive &&
> + git -c commit.signoff=true commit --no-signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + echo thank you >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --signoff omitted" '
> + echo 10 >positive &&
> + git add positive &&
> + git -c commit.signoff=false commit -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + echo thank you >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --signoff" '
> + echo 11 >positive &&
> + git add positive &&
> + git -c commit.signoff=false commit --signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + (
> + echo thank you
> + echo
> + git var GIT_COMMITTER_IDENT |
> + sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
> + ) >expected &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success "commit.signoff=false and --no-signoff" '
> + echo 12 >positive &&
> + git add positive &&
> + git -c commit.signoff=false commit --no-signoff -m "thank you" &&
> + git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> + echo thank you >expected &&
> + test_cmp expected actual
> +'

Re: contrib/completion/git-completion.bash: declare -g is not portable

[Eric Sunshine]

On Sun, Feb 4, 2018 at 4:45 AM, Duy Nguyen  wrote:
> On Sun, Feb 4, 2018 at 12:20 AM, Torsten Bögershausen  wrote:
>> After running t9902-completion.sh on Mac OS I got a failure
>> in this style:
>
> Sorry I was new with this bash thingy. Jeff already answered this (and
> I will fix it in the re-roll) but just for my own information, what
> bash version is shipped with Mac OS?

The MacOS bash is very old; note the copyright date:

% /bin/bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin16)
Copyright (C) 2007 Free Software Foundation, Inc.

A recent bash installed manually (not from Apple):

% /usr/local/bin/bash --version
GNU bash, version 4.4.18(1)-release (x86_64-apple-darwin16.7.0)
Copyright (C) 2016 Free Software Foundation, Inc.

Re: contrib/completion/git-completion.bash: declare -g is not portable

[Duy Nguyen]

On Sun, Feb 4, 2018 at 12:20 AM, Torsten Bögershausen  wrote:
> Hej Duy,
> After running t9902-completion.sh on Mac OS I got a failure
> in this style:

Sorry I was new with this bash thingy. Jeff already answered this (and
I will fix it in the re-roll) but just for my own information, what
bash version is shipped with Mac OS?
-- 
Duy

[PATCH] dir.c: ignore paths containing .git when invalidating untracked cache

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

read_directory() code ignores all paths named ".git" even if it's not
a valid git repository. See treat_path() for details. Since ".git" is
basically invisible to read_directory(), when we are asked to
invalidate a path that contains ".git", we can safely ignore it
because the slow path would not consider it anyway.

This helps when fsmonitor is used and we have a real ".git" repo at
worktree top. Occasionally .git/index will be updated and if the
fsmonitor hook does not filter it, untracked cache is asked to
invalidate the path ".git/index".

Without this patch, we invalidate the root directory unncessarily,
which:

- makes read_directory() fall back to slow path for root directory
  (slower)

- makes the index dirty (because UNTR extension is updated). Depending
  on the index size, writing it down could also be slow.

Noticed-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Sorry for the resend, I forgot git@vger.

 dir.c | 13 -
 git-compat-util.h |  2 ++
 wrapper.c | 12 
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 7c4b45e30e..f8b4cabba9 100644
--- a/dir.c
+++ b/dir.c
@@ -1773,7 +1773,7 @@ static enum path_treatment treat_path(struct dir_struct 
*dir,
if (!de)
return treat_path_fast(dir, untracked, cdir, istate, path,
   baselen, pathspec);
-   if (is_dot_or_dotdot(de->d_name) || !strcmp(de->d_name, ".git"))
+   if (is_dot_or_dotdot(de->d_name) || !fspathcmp(de->d_name, ".git"))
return path_none;
strbuf_setlen(path, baselen);
strbuf_addstr(path, de->d_name);
@@ -2970,8 +2970,19 @@ static int invalidate_one_component(struct 
untracked_cache *uc,
 void untracked_cache_invalidate_path(struct index_state *istate,
 const char *path)
 {
+   const char *end;
+   int skipped;
+
if (!istate->untracked || !istate->untracked->root)
return;
+   if (!fspathcmp(path, ".git"))
+   return;
+   if (ignore_case)
+   skipped = skip_caseprefix(path, "/.git", );
+   else
+   skipped = skip_prefix(path, "/.git", );
+   if (skipped && (*end == '\0' || *end == '/'))
+   return;
invalidate_one_component(istate->untracked, istate->untracked->root,
 path, strlen(path));
 }
diff --git a/git-compat-util.h b/git-compat-util.h
index 68b2ad531e..27e0b761a3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -484,6 +484,8 @@ static inline int skip_prefix(const char *str, const char 
*prefix,
return 0;
 }
 
+int skip_caseprefix(const char *str, const char *prefix, const char **out);
+
 /*
  * If the string "str" is the same as the string in "prefix", then the "arg"
  * parameter is set to the "def" parameter and 1 is returned.
diff --git a/wrapper.c b/wrapper.c
index d20356a776..bb888d9401 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -690,3 +690,15 @@ int xgethostname(char *buf, size_t len)
buf[len - 1] = 0;
return ret;
 }
+
+int skip_caseprefix(const char *str, const char *prefix, const char **out)
+{
+   do {
+   if (!*prefix) {
+   *out = str;
+   return 1;
+   }
+   } while (tolower(*str++) == tolower(*prefix++));
+
+   return 0;
+}
-- 
2.16.1.207.gedba492059

Re: [PATCH v7 17/31] merge-recursive: add a new hashmap for storing directory renames

[Johannes Sixt]

Am 03.02.2018 um 22:34 schrieb Elijah Newren:
>  If anyone can find an
> example of a real world open source repository (linux, webkit, git,
> etc.) with a merge where n is greater than about 10, I'll be
> surprised.

git rev-list --parents --merges master |
 grep " .* .* .* .* .* .* .* .* .* .* "

returns quite a few hits in the Linux repository. Most notable is
fa623d1b0222adbe8f822e53c08003b9679a410c; spoiler: it has 30 parents.

-- Hannes