Re: [PATCH] git-send-email: fix get_maintainer.pl regression

[Eric Sunshine]

On Thu, Nov 16, 2017 at 10:48 AM, Alex Bennée  wrote:
> Getting rid of Mail::Address regressed behaviour with common
> get_maintainer scripts such as the Linux kernel. Fix the missed corner
> case and add a test for it.

It is not at all clear, based upon this text, what this is fixing.
When you re-roll, please provide a description of the regression in
sufficient detail for readers to easily understand the problem and the
solution.

More below...

> Signed-off-by: Alex Bennée 
> ---
> diff --git a/t/t9000/test.pl b/t/t9000/test.pl
> @@ -35,7 +35,9 @@ my @success_list = (q[Jane],
> q['Jane 'Doe' ],
> q[Jane@:;\.,()<>Doe ],
> q[Jane  Doe],
> -   q[ Jane Doe]);
> +   q[ Jane Doe],
> +   q[j...@example.com (open list:for thing (foo/bar))],
> +);

Style nit: The dangling ");" introduced by this change differs from
the other list(s) in this file which have the closing ");" on the same
line as the last item in the list.

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> +test_expect_success $PREREQ 'setup get_mainter script for cc trailer' "
> +cat >expected-cc-script.sh <<-EOF && chmod +x expected-cc-script.sh
> +#!/bin/sh
> +echo 'One Person  (supporter:THIS (FOO/bar))'
> +echo 'Two Person  (maintainer:THIS THING)'
> +echo 'Third List  (moderated list:THIS THING (FOO/bar))'
> +echo ' (moderated list:FOR THING)'
> +echo 'f...@example.com (open list:FOR THING (FOO/bar))'
> +echo 's...@example.com (open list)'
> +EOF
> +"

Use write_script() to create the script:

--- 8< ---
write_script expected-cc-script.sh <<-\EOF &&
echo '...'
...
EOF
--- 8< ---

No need for #!/bin/sh or chmod, both of which are handled by
write_script(). In fact, you could fold this into the following test
(since there doesn't seem a good reason for it to be separate).

> +test_expect_success $PREREQ 'cc trailer with get_maintainer output' '
> +   test_commit cc-trailer &&
> +   clean_fake_sendmail &&
> +   git send-email -1 --to=recipi...@example.com \
> +   --cc-cmd="$(pwd)/expected-cc-script.sh" \
> +   --smtp-server="$(pwd)/fake.sendmail" &&
> +   test_cmp expected-cc commandline1
> +'
> OK I'm afraid I don't fully understand the test harness as this breaks a
> bunch of other tests. If anyone can offer some pointers on how to fix
> I'd be grateful.

There are several problems:

* test_commit bombs because there is already a tag named "cc-trailer"
created by an earlier test. Fix this by choosing a different name for
the commit. Even better would be to avoid making the commit in the
first place since it doesn't appear to be relevant to the test, and
the test works fine without it.

* The directory in which the expected-cc-script.sh is created contains
a space; this is intentional to catch bugs in tests and Git itself. In
this case, your test is exposing what might be considered a bug in
git-send-email itself, in which it invokes the --cc-cmd as "/path/with
space/expected-cc-script.sh", which is interpreted as trying to invoke
program "/path/with" with argument "space/expected-cc-script.sh". One
fix (which you could submit as a preparatory patch, making this a
2-patch series) would be this:

--- 8< ---
diff --git a/git-send-email.perl b/git-send-email.perl
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1724,7 +1724,7 @@ sub recipients_cmd {
my ($prefix, $what, $cmd, $file) = @_;

 my @addresses = ();
-open my $fh, "-|", "$cmd \Q$file\E"
+   open my $fh, "-|", "\Q$cmd\E \Q$file\E"
 or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
 while (my $address = <$fh>) {
  $address =~ s/^\s*//g;
--- 8< ---

However, it's possible that might break existing users who rely on
--cc-cmd="myscript --option arg" working. It's not clear which
behavior is correct.

* Subsequent tests fail because you've added a commit which they are
not expecting. If you look at tests earlier in the file, you will see
that they deal with this by removing the added commit (via "git reset
--hard HEAD^") by the time the test finishes. However, as noted above,
this new test doesn't actually need to make this commit in the first
place.

So, with fixes, your test might look like this:

--- 8< ---
test_expect_success $PREREQ 'cc trailer with get_maintainer output' '
test_commit cc-trailer-get-maintainer &&
test_when_finished "git reset --hard HEAD^" &&
clean_fake_sendmail &&
git send-email -1 --to=recipi...@example.com \
--cc-cmd="$(pwd)/expected-cc-script.sh" \
--smtp-server="$(pwd)/fake.sendmail" &&
test_cmp expected-cc commandline1
'
--- 8< ---

Or, if you drop the unnecessary test_commit():

--- 8< ---
test_expect_success $PREREQ 'cc trailer with get_maintainer output' '

Re: [PATCH 1/2] Simplify tracing code by removing trace key normalization concept

[Junio C Hamano]

gennady.kup...@gmail.com writes:

> Subject: Re: [PATCH 1/2] Simplify tracing code by removing trace key 
> normalization concept

The usual style comment on the subject applies here.

> From: Gennady Kupava 
>
> - to implement efficient traces with normalization, normalization
>   implementation should be moved to header. as it seems better to not
>   overload header file with this normalization logic, suggestion is
>   just to remove it
> - different macro exist specifically to handle traces with default key
> - there is no use of normalization in current code
> - it could be reintroduced if necessary

I cannot quite tell what it is trying to achive to make it a
bulleted list.  It's not like four things at the same conceptual
level is enumerated; instead it just has four sentences that talk
about random things.

More importantly, I am not sure I understand what these sentences
are trying to say.  "Should be moved to header"---so?  Does that
move something from the source to the header?  It seems to me that
the patch removes a helper function from trace.c but does not add
anything to the header.

Or am I wasting everybody's time by commenting on a stale comment
that used to describe an ancient iteration of this code?

Puzzled.

Re: [PATCH] git-rebase: clean up dashed-usages in messages

[Junio C Hamano]

>  git rebase [-i] [options] [--exec ] [--onto ] [] 
> []
>  git rebase [-i] [options] [--exec ] [--onto ] --root []
> -git-rebase --continue | --abort | --skip | --edit-todo
> +git rebase --continue | --abort | --skip | --edit-todo

A good change.

>  test -f "$apply_dir"/applying &&
> - die "$(gettext "It looks like git-am is in progress. Cannot rebase.")"
> + die "$(gettext "It looks like you are in the middle of an am session. 
> Cannot rebase.")"

Probably not, as 'am' alone would be confusing.

"It looks like 'git am' is in progress. Cannot rebase."

may be a more sensible improvement.

v2.15.0: commits become falsely joined when rebasing (interactively)

[Steffen Nurpmeso]

Hello again,

i see an error with v2.15.0 that happened already back in early
October (AlpineLinux [edge] pretty much up-to-date with newest git
but please don't ask exact version).  I failed to reproduce it
back then, but now again, here is how.

- It seems related to having a hook (pre-commit), and the "reword"
  action.
- Doing a rebase interactively to move (two) commits back from
  HEAD downwards in a linear hierarchy, as in
* 306b5c7e (HEAD -> refs/heads/notpushed) [-] COMMIT 1
* 7e34d5fa [-] COMMIT 2
  the above two to be moved down
* 65d216c3 (refs/remotes/origin/notpushed)...
...
* 5ff5ef05 (refs/remotes/origin/next, refs/heads/next) [-]..
  to end up stacked upon and as new [next]

- COMMIT 2 is to be picked, but COMMIT 1 shall be "r"eworded.

Now what happens is that COMMIT 2 is rebased ok, but instead of
simply opening the editor to allow rewording of the commit message
of COMMIT 2 the pre-commit hook runs, and it has to complain in
this case (lines too long), but i "commit -n" that once
i committed first.  Anyway git says

  You can amend the commit now, with

git commit --amend

  Once you are satisfied with your changes, run

git rebase --continue

Which i have not asked for!  More:

  ?1[steffen@essex nail.git]$ git status
  interactive rebase in progress; onto 6d437ab6
  Last commands done (12 commands done):
 pick 7e34d5fa [-] COMMIT 2
 r 306b5c7e [-] COMMIT 1
(see more in file .git/rebase-merge/done)
  Next commands to do (27 remaining commands):
  ...
(use "git rebase --edit-todo" to view and edit)
  You are currently rebasing branch 'notpushed' on '6d437ab6'.
(all conflicts fixed: run "git rebase --continue")

  Changes to be committed:
(use "git reset HEAD ..." to unstage)

  modified:   gen-okeys.h

  Untracked files not listed (use -u option to show untracked files)

So far so good, but now:

  ?0[steffen@essex nail.git]$ git rec
alias: rebase --continue
  [detached HEAD ca77a94a] COMMIT 1 with adjusted commit message
   Date: Sun Nov 19 02:19:30 2017 +0100
   9 files changed, 357 insertions(+), 339 deletions(-)
  Successfully rebased and updated refs/heads/notpushed.

Uh!  It joined COMMIT 1 with COMMIT 2!
Thanks for git!

Ciao,

--steffen
|
|Der Kragenbaer,The moon bear,
|der holt sich munter   he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)

Re: [PATCH 1/6] t4051: add test for comments preceding function lines

[Eric Sunshine]

On Sat, Nov 18, 2017 at 1:04 PM, René Scharfe  wrote:
> When showing function context it would be helpful to show comments
> immediately before declarations, as they are most likely relevant.  Add
> a test for that.
>
> Signed-off-by: Rene Scharfe 
> ---
> diff --git a/t/t4051-diff-function-context.sh 
> b/t/t4051-diff-function-context.sh
> @@ -85,6 +85,10 @@ test_expect_success 'setup' '
>> +test_expect_failure ' context includes comment' '
> +   grep "^ .*Hello comment" changed_hello.diff
> +'

This shows only that the the "* Hello comment." line is in the context
but says nothing about the entire comment block being included (which
patch 3/6 implements). Perhaps the test could be more thorough?

> diff --git a/t/t4051/hello.c b/t/t4051/hello.c
> index 63b1a1e4ef..73e767e178 100644
> --- a/t/t4051/hello.c
> +++ b/t/t4051/hello.c
> @@ -1,4 +1,7 @@
>
> +/*
> + * Hello comment.
> + */
>  static void hello(void)// Begin of hello
>  {

Re: [PATCH] launch_editor(): indicate that Git waits for user input

[Junio C Hamano]

Kaartic Sivaraam  writes:

> $ GIT_EDITOR=gedit ./git commit --allow-empty
> Launched your editor, waiting...error: gedit died of signal 15
> error: There was a problem with the editor 'gedit'.
> Please supply the message using either -m or -F option.
>
> Though this is not something that's going to happen very often, I'm
> not sure if this is to be considered. Just wanted to note what I
> observed.

See my earlier response to Eric Sunshine.

>> +static const char *close_notice = NULL;
>
> Just a little curious, what's the advantage of making 'close_notice'
> 'static' over 'auto' ?

A second call, if an earlier call to the function already determined
that our standard error output is going to a terminal and what kind
of terminal we are on, would just use the result from the earliser
call.

> Being curious again, is flushing 'stderr' required ? AFAIK, 'stderr'
> is unbuffered by default and I didn't notice any calls that changed
> the buffering mode of it along this code path.

"By default" is the key phrase.  The code is merely being defensive
to changes in other area of the code.

Re: [PATCH 4/4] builtin/branch: strip refs/heads/ using skip_prefix

[Eric Sunshine]

On Sat, Nov 18, 2017 at 12:26 PM, Kaartic Sivaraam
 wrote:
> Instead of hard-coding the offset strlen("refs/heads/") to skip
> the prefix "refs/heads/" use the skip_prefix() function which
> is more communicative and verifies that the string actually
> starts with that prefix.
>
> Though we don't check for the result of verification here as
> it's (almost) always the case that the string does start
> with "refs/heads", it's just better to avoid hard-coding and
> be more communicative.
>
> Signed-off-by: Kaartic Sivaraam 
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> @@ -462,6 +462,8 @@ static void copy_or_rename_branch(const char *oldname, 
> const char *newname, int
>  {
> struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = 
> STRBUF_INIT;
> struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
> +   const char *prefix_free_oldref = NULL;
> +   const char *prefix_free_newref = NULL;

A bit of a mouthful. Perhaps name these 'oldname' and 'newname' or something?

Re: [PATCH] git-send-email: honor $PATH

[Junio C Hamano]

"brian m. carlson"  writes:

> This patch adds support for PATH, but it also removes the fixed paths.
> On many systems, unprivileged users don't have /usr/sbin in their PATH,
> and I know of no systems which provide /usr/lib as a PATH value.
> Therefore, it's possible that this change will break automatic detection
> of sendmail for many users.

It is more than possible ;-) that this change alone is a regression.

> I think what you probably want to do is use entries in PATH first, and
> leave the two old values as backups at the end.

I do not think it would make things worse if the change were to do
the two standard places first and then try elements on the $PATH;
split of $PATH needs to be done carefully (Windows?), though.  

I would feel a lot more worried about trying elements on the $PATH
first and then using the two standard places as fallback.  If the
order of addition matters at all, that would mean that trying
elements on $PATH first and then falling back to the two standard
places *will* change the behaviour---for the affected users, we used
to pick one of these two, but now we would pick something different.
sendmail is usually installed out of the way of $PATH for regular
users for a reason, so picking anything whose name happens to be
sendmail that is on $PATH does not sound right.

Of course, for users who do not have sendmail at one of the two
standard places _and_ has one on one of the directories on $PATH,
the order in which we check would not make a difference, so my
suggestion would be to do the other way around.

[PATCH 2/2] Reduce performance cost of the trace if trace category is disabled

[gennady . kupava]

From: Gennady Kupava 

- Do the check if the trace key is enabled sooner in call chain.
- Move just enough code from trace.c into trace.h header so all code
  necessary to determine that trace is disabled could be inlined to
  calling functions.

Signed-off-by: Gennady Kupava 
---
 trace.c |  3 +--
 trace.h | 58 --
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/trace.c b/trace.c
index d47ea28e8..b7530b51a 100644
--- a/trace.c
+++ b/trace.c
@@ -25,6 +25,7 @@
 #include "quote.h"
 
 struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 };
+struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE);
 
 /* Get a trace file descriptor from "key" env variable. */
 static int get_trace_fd(struct trace_key *key)
@@ -172,8 +173,6 @@ void trace_strbuf_fl(const char *file, int line, struct 
trace_key *key,
print_trace_line(key, );
 }
 
-static struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE);
-
 static void trace_performance_vprintf_fl(const char *file, int line,
 uint64_t nanos, const char *format,
 va_list ap)
diff --git a/trace.h b/trace.h
index 24b32f8f4..cd9e280ba 100644
--- a/trace.h
+++ b/trace.h
@@ -14,6 +14,7 @@ struct trace_key {
 extern struct trace_key trace_default_key;
 
 #define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 }
+extern struct trace_key trace_perf_key;
 
 extern void trace_repo_setup(const char *prefix);
 extern int trace_want(struct trace_key *key);
@@ -79,24 +80,42 @@ extern void trace_performance_since(uint64_t start, const 
char *format, ...);
  * comma, but this is non-standard.
  */
 
-#define trace_printf(...) \
-   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, _default_key, 
__VA_ARGS__)
-
-#define trace_printf_key(key, ...) \
-   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, __VA_ARGS__)
-
-#define trace_argv_printf(argv, ...) \
-   trace_argv_printf_fl(TRACE_CONTEXT, __LINE__, argv, __VA_ARGS__)
-
-#define trace_strbuf(key, data) \
-   trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data)
-
-#define trace_performance(nanos, ...) \
-   trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos, __VA_ARGS__)
-
-#define trace_performance_since(start, ...) \
-   trace_performance_fl(TRACE_CONTEXT, __LINE__, getnanotime() - (start), \
-__VA_ARGS__)
+#define trace_printf_key(key, ...) \
+   do {\
+   if (trace_pass_fl(key)) \
+   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key,   \
+   __VA_ARGS__);   \
+   } while(0)
+
+#define trace_printf(...) trace_printf_key(_default_key, __VA_ARGS__);
+
+#define trace_argv_printf(argv, ...)   \
+   do {\
+   if (trace_pass_fl(_default_key))  \
+  trace_argv_printf_fl(TRACE_CONTEXT, __LINE__,\
+   argv, __VA_ARGS__); \
+   } while(0)
+
+#define trace_strbuf(key, data)
\
+   do {\
+   if (trace_pass_fl(key)) \
+   trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data);\
+   } while(0)
+
+#define trace_performance(nanos, ...)  \
+   do {\
+   if (trace_pass_fl(key)) \
+   trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos,\
+__VA_ARGS__);  \
+   } while(0)
+
+#define trace_performance_since(start, ...)\
+   do {\
+   if (trace_pass_fl(_perf_key)) \
+   trace_performance_fl(TRACE_CONTEXT, __LINE__,   \
+getnanotime() - (start),   \
+__VA_ARGS__);  \
+   } while(0)
 
 /* backend functions, use non-*fl macros instead */
 __attribute__((format (printf, 4, 5)))
@@ -110,6 +129,9 @@ extern void trace_strbuf_fl(const char *file, int line, 
struct trace_key *key,
 __attribute__((format (printf, 4, 5)))
 extern void trace_performance_fl(const char *file, int line,
 uint64_t nanos, const char *fmt, ...);
+inline int 

[PATCH 1/2] Simplify tracing code by removing trace key normalization concept

[gennady . kupava]

From: Gennady Kupava 

- to implement efficient traces with normalization, normalization
  implementation should be moved to header. as it seems better to not
  overload header file with this normalization logic, suggestion is
  just to remove it
- different macro exist specifically to handle traces with default key
- there is no use of normalization in current code
- it could be reintroduced if necessary

Signed-off-by: Gennady Kupava 
---
 trace.c | 24 
 trace.h |  4 +++-
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/trace.c b/trace.c
index cb1293ed3..d47ea28e8 100644
--- a/trace.c
+++ b/trace.c
@@ -24,26 +24,13 @@
 #include "cache.h"
 #include "quote.h"
 
-/*
- * "Normalize" a key argument by converting NULL to our trace_default,
- * and otherwise passing through the value. All caller-facing functions
- * should normalize their inputs in this way, though most get it
- * for free by calling get_trace_fd() (directly or indirectly).
- */
-static void normalize_trace_key(struct trace_key **key)
-{
-   static struct trace_key trace_default = { "GIT_TRACE" };
-   if (!*key)
-   *key = _default;
-}
+struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 };
 
 /* Get a trace file descriptor from "key" env variable. */
 static int get_trace_fd(struct trace_key *key)
 {
const char *trace;
 
-   normalize_trace_key();
-
/* don't open twice */
if (key->initialized)
return key->fd;
@@ -81,8 +68,6 @@ static int get_trace_fd(struct trace_key *key)
 
 void trace_disable(struct trace_key *key)
 {
-   normalize_trace_key();
-
if (key->need_close)
close(key->fd);
key->fd = 0;
@@ -128,7 +113,6 @@ static int prepare_trace_line(const char *file, int line,
 static void trace_write(struct trace_key *key, const void *buf, unsigned len)
 {
if (write_in_full(get_trace_fd(key), buf, len) < 0) {
-   normalize_trace_key();
warning("unable to write trace for %s: %s",
key->key, strerror(errno));
trace_disable(key);
@@ -167,13 +151,13 @@ static void trace_argv_vprintf_fl(const char *file, int 
line,
 {
struct strbuf buf = STRBUF_INIT;
 
-   if (!prepare_trace_line(file, line, NULL, ))
+   if (!prepare_trace_line(file, line, _default_key, ))
return;
 
strbuf_vaddf(, format, ap);
 
sq_quote_argv(, argv, 0);
-   print_trace_line(NULL, );
+   print_trace_line(_default_key, );
 }
 
 void trace_strbuf_fl(const char *file, int line, struct trace_key *key,
@@ -215,7 +199,7 @@ void trace_printf(const char *format, ...)
 {
va_list ap;
va_start(ap, format);
-   trace_vprintf_fl(NULL, 0, NULL, format, ap);
+   trace_vprintf_fl(NULL, 0, _default_key, format, ap);
va_end(ap);
 }
 
diff --git a/trace.h b/trace.h
index 179b249c5..24b32f8f4 100644
--- a/trace.h
+++ b/trace.h
@@ -11,6 +11,8 @@ struct trace_key {
unsigned int  need_close : 1;
 };
 
+extern struct trace_key trace_default_key;
+
 #define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 }
 
 extern void trace_repo_setup(const char *prefix);
@@ -78,7 +80,7 @@ extern void trace_performance_since(uint64_t start, const 
char *format, ...);
  */
 
 #define trace_printf(...) \
-   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, NULL, __VA_ARGS__)
+   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, _default_key, 
__VA_ARGS__)
 
 #define trace_printf_key(key, ...) \
trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, __VA_ARGS__)
-- 
2.14.1

[PATCH v3 2/3] worktree: make add dwim

[Thomas Gummerer]

Currently 'git worktree add  ', errors out when 'branch'
is not a local branch.   It has no additional dwim'ing features that one
might expect.

Make it behave more like 'git checkout ' when the branch doesn't
exist locally, but a remote tracking branch uniquely matches the desired
branch name, i.e. create a new branch from the remote tracking branch
and set the upstream to the remote tracking branch.

As 'git worktree add' currently just dies in this situation, there are
no backwards compatibility worries when introducing this feature.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt |  7 +++
 builtin/worktree.c | 15 ++
 t/t2025-worktree-add.sh| 46 ++
 3 files changed, 68 insertions(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b472acc356..abf48fecb8 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,6 +52,13 @@ is linked to the current repository, sharing everything 
except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as ``; it is synonymous with `@{-1}`.
 +
+If  is not found, and neither `-b` nor `-B` nor `--detach` are
+used, but there does exist a tracking branch in exactly one remote
+(call it ) with a matching name, treat as equivalent to
+
+$ git worktree add -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.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b9307aa58..05fc902bcc 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "checkout.h"
 #include "config.h"
 #include "builtin.h"
 #include "dir.h"
@@ -386,6 +387,20 @@ static int add(int ac, const char **av, const char *prefix)
opts.new_branch = xstrndup(s, n);
}
 
+   if (ac == 2 && !opts.new_branch && !opts.detach) {
+   struct object_id oid;
+   struct commit *commit;
+   const char *remote;
+
+   commit = lookup_commit_reference_by_name(branch);
+   if (!commit)
+   remote = unique_tracking_name(branch, );
+   if (!commit && remote) {
+   opts.new_branch = branch;
+   branch = remote;
+   }
+   }
+
if (opts.new_branch) {
struct child_process cp = CHILD_PROCESS_INIT;
cp.git_cmd = 1;
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index b5c47ac602..e3fc60dd1c 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -6,6 +6,16 @@ test_description='test git worktree add'
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+# Is branch "refs/heads/$1" set to pull from "$2/$3"?
+test_branch_upstream () {
+   printf "%s\n" "$2" "refs/heads/$3" >expect.upstream &&
+   {
+   git config "branch.$1.remote" &&
+   git config "branch.$1.merge"
+   } >actual.upstream &&
+   test_cmp expect.upstream actual.upstream
+}
+
 test_expect_success 'setup' '
test_commit init
 '
@@ -314,4 +324,40 @@ test_expect_success 'rename a branch under bisect not 
allowed' '
test_must_fail git branch -M under-bisect bisect-with-new-name
 '
 
+test_expect_success '"add"   fails' '
+   test_must_fail git worktree add foo non-existent
+'
+
+test_expect_success '"add"   dwims' '
+   test_when_finished rm -rf repo_upstream &&
+   test_when_finished rm -rf repo_dwim &&
+   test_when_finished rm -rf foo &&
+   git init repo_upstream &&
+   (
+   cd repo_upstream &&
+   test_commit upstream_master &&
+   git checkout -b foo &&
+   test_commit a_foo
+   ) &&
+   git init repo_dwim &&
+   (
+   cd repo_dwim &&
+   test_commit dwim_master &&
+   git remote add repo_upstream ../repo_upstream &&
+   git config remote.repo_upstream.fetch \
+ "refs/heads/*:refs/remotes/repo_upstream/*" &&
+   git fetch --all &&
+   test_must_fail git worktree add -b foo ../foo foo &&
+   test_must_fail git worktree add --detach ../foo foo &&
+   git worktree add ../foo foo
+   ) &&
+   (
+   cd foo &&
+   test_branch_upstream foo repo_upstream foo &&
+   git rev-parse repo_upstream/foo >expect &&
+   git rev-parse foo >actual &&
+   test_cmp expect actual
+   )
+'
+
 test_done
-- 
2.15.0.345.gf926f18f3

[PATCH v3 1/3] checkout: factor out functions to new lib file

[Thomas Gummerer]

Factor the functions out, so they can be re-used from other places.  In
particular these functions will be re-used in builtin/worktree.c to make
git worktree add dwim more.

While there add some docs to the function.

Signed-off-by: Thomas Gummerer 
---
 Makefile   |  1 +
 builtin/checkout.c | 41 +
 checkout.c | 42 ++
 checkout.h | 13 +
 4 files changed, 57 insertions(+), 40 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

diff --git a/Makefile b/Makefile
index cd75985991..8d603c7443 100644
--- a/Makefile
+++ b/Makefile
@@ -757,6 +757,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
+LIB_OBJS += checkout.o
 LIB_OBJS += color.o
 LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc4f8fd2ea..9e1cfd10b3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "config.h"
+#include "checkout.h"
 #include "lockfile.h"
 #include "parse-options.h"
 #include "refs.h"
@@ -874,46 +875,6 @@ static int git_checkout_config(const char *var, const char 
*value, void *cb)
return git_xmerge_config(var, value, NULL);
 }
 
-struct tracking_name_data {
-   /* const */ char *src_ref;
-   char *dst_ref;
-   struct object_id *dst_oid;
-   int unique;
-};
-
-static int check_tracking_name(struct remote *remote, void *cb_data)
-{
-   struct tracking_name_data *cb = cb_data;
-   struct refspec query;
-   memset(, 0, sizeof(struct refspec));
-   query.src = cb->src_ref;
-   if (remote_find_tracking(remote, ) ||
-   get_oid(query.dst, cb->dst_oid)) {
-   free(query.dst);
-   return 0;
-   }
-   if (cb->dst_ref) {
-   free(query.dst);
-   cb->unique = 0;
-   return 0;
-   }
-   cb->dst_ref = query.dst;
-   return 0;
-}
-
-static const char *unique_tracking_name(const char *name, struct object_id 
*oid)
-{
-   struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
-   cb_data.src_ref = xstrfmt("refs/heads/%s", name);
-   cb_data.dst_oid = oid;
-   for_each_remote(check_tracking_name, _data);
-   free(cb_data.src_ref);
-   if (cb_data.unique)
-   return cb_data.dst_ref;
-   free(cb_data.dst_ref);
-   return NULL;
-}
-
 static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
diff --git a/checkout.c b/checkout.c
new file mode 100644
index 00..b0c744d37a
--- /dev/null
+++ b/checkout.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "remote.h"
+
+struct tracking_name_data {
+   /* const */ char *src_ref;
+   char *dst_ref;
+   struct object_id *dst_oid;
+   int unique;
+};
+
+static int check_tracking_name(struct remote *remote, void *cb_data)
+{
+   struct tracking_name_data *cb = cb_data;
+   struct refspec query;
+   memset(, 0, sizeof(struct refspec));
+   query.src = cb->src_ref;
+   if (remote_find_tracking(remote, ) ||
+   get_oid(query.dst, cb->dst_oid)) {
+   free(query.dst);
+   return 0;
+   }
+   if (cb->dst_ref) {
+   free(query.dst);
+   cb->unique = 0;
+   return 0;
+   }
+   cb->dst_ref = query.dst;
+   return 0;
+}
+
+const char *unique_tracking_name(const char *name, struct object_id *oid)
+{
+   struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+   cb_data.src_ref = xstrfmt("refs/heads/%s", name);
+   cb_data.dst_oid = oid;
+   for_each_remote(check_tracking_name, _data);
+   free(cb_data.src_ref);
+   if (cb_data.unique)
+   return cb_data.dst_ref;
+   free(cb_data.dst_ref);
+   return NULL;
+}
diff --git a/checkout.h b/checkout.h
new file mode 100644
index 00..9980711179
--- /dev/null
+++ b/checkout.h
@@ -0,0 +1,13 @@
+#ifndef CHECKOUT_H
+#define CHECKOUT_H
+
+#include "cache.h"
+
+/*
+ * Check if the branch name uniquely matches a branch name on a remote
+ * tracking branch.  Return the name of the remote if such a branch
+ * exists, NULL otherwise.
+ */
+extern const char *unique_tracking_name(const char *name, struct object_id 
*oid);
+
+#endif /* CHECKOUT_H */
-- 
2.15.0.345.gf926f18f3

[PATCH v3 0/3] make git worktree add dwim more

[Thomas Gummerer]

Sorry about the noise with v2 and v3 so quickly one after another.  I
only too late realized that the new dwim for 'add  '
doesn't make much sense if -b or --detach are given, and it's better
to keep on erroring out in these cases.

The previous rounds were at 
https://public-inbox.org/git/20171112134305.3949-1-t.gumme...@gmail.com/
and https://public-inbox.org/git/20171118181103.28354-1-t.gumme...@gmail.com/.

In case anybody already started reading v2, an interdiff between the
two versions is below:

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 11cac104d9..eedead2c4c 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,9 +52,9 @@ is linked to the current repository, sharing everything 
except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as ``; it is synonymous with `@{-1}`.
 +
-If  is not found but there does exist a tracking branch in
-exactly one remote (call it ) with a matching name, treat as
-equivalent to
+If  is not found, and neither `-b` nor `-B` nor `--detach` are
+used, but there does exist a tracking branch in exactly one remote
+(call it ) with a matching name, treat as equivalent to
 
 $ git worktree add -b   /
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 82088415b8..b2a6dd020c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -396,7 +396,7 @@ static int add(int ac, const char **av, const char *prefix)
}
}
 
-   if (ac == 2) {
+   if (ac == 2 && !opts.new_branch && !opts.detach) {
struct object_id oid;
struct commit *commit;
const char *remote;
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index a566d867fe..87e233f812 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -347,6 +347,8 @@ test_expect_success '"add"   dwims' '
git config remote.repo_upstream.fetch \
  "refs/heads/*:refs/remotes/repo_upstream/*" &&
git fetch --all &&
+   test_must_fail git worktree add -b foo ../foo foo &&
+   test_must_fail git worktree add --detach ../foo foo &&
git worktree add ../foo foo
) &&
(

Thomas Gummerer (3):
  checkout: factor out functions to new lib file
  worktree: make add   dwim
  worktree: make add  dwim

 Documentation/git-worktree.txt |  22 +++--
 Makefile   |   1 +
 builtin/checkout.c |  41 +---
 builtin/worktree.c |  24 ++
 checkout.c |  42 
 checkout.h |  13 +
 t/t2025-worktree-add.sh| 106 +
 7 files changed, 206 insertions(+), 43 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

-- 
2.15.0.345.gf926f18f3

[PATCH v3 3/3] worktree: make add dwim

[Thomas Gummerer]

Currently 'git worktree add ' creates a new branch named after the
basename of the , that matches the HEAD of whichever worktree we
were on when calling "git worktree add ".

Make 'git worktree add  behave more like the dwim machinery in
'git checkout ', i.e. check if the new branch name uniquely
matches the branch name of a remote tracking branch, and if so check out
that branch and set the upstream to the remote tracking branch.

This is a change of behaviour compared to the current behaviour, where
we create a new branch matching HEAD.  However as 'git worktree' is
still an experimental feature, and it's easy to notice/correct the
behaviour in case it's not what the user desired it's probably okay to
break existing behaviour here.

In order to also satisfy users who want the current behaviour of
creating a new branch from HEAD, add a '--no-track' flag, which disables
the new behaviour, and keeps the old behaviour of creating a new branch
from the head of the current worktree.

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

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index abf48fecb8..eedead2c4c 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -60,9 +60,18 @@ $ git worktree add -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, if there exists a tracking branch in exactly
+one remote (call it ) matching the basename of the path
+(call it ), treat it as equivalent to
+
+$ git worktree add -b   /
+
+If no tracking branch exists in exactly one remote,  is
+created based on HEAD, as if `-b $(basename )` was specified.
++
+To disable the behaviour of trying to match the basename of  to
+a remote, and always create a new branch from HEAD, the `--no-track`
+flag can be passed to `git worktree add`.
 list::
 
 List details of each worktree.  The main worktree is listed first, followed by
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 05fc902bcc..b2a6dd020c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -342,6 +342,7 @@ static int add(int ac, const char **av, const char *prefix)
const char *new_branch_force = NULL;
char *path;
const char *branch;
+   int track_dwim = 1;
struct option options[] = {
OPT__FORCE(, N_("checkout  even if already 
checked out in other worktree")),
OPT_STRING('b', NULL, _branch, N_("branch"),
@@ -351,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix)
OPT_BOOL(0, "detach", , N_("detach HEAD at named 
commit")),
OPT_BOOL(0, "checkout", , N_("populate the new 
working tree")),
OPT_BOOL(0, "lock", _locked, N_("keep the new working 
tree locked")),
+   OPT_BOOL(0, "track", _dwim, N_("checkout upstream branch 
if there's a unique match")),
OPT_END()
};
 
@@ -385,6 +387,13 @@ static int add(int ac, const char **av, const char *prefix)
int n;
const char *s = worktree_basename(path, );
opts.new_branch = xstrndup(s, n);
+   if (track_dwim) {
+   struct object_id oid;
+   const char *remote =
+   unique_tracking_name(opts.new_branch, );
+   if (remote)
+   branch = remote;
+   }
}
 
if (ac == 2 && !opts.new_branch && !opts.detach) {
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index e3fc60dd1c..87e233f812 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -360,4 +360,64 @@ test_expect_success '"add"   dwims' '
)
 '
 
+test_expect_success 'git worktree add --no-track does not set up tracking' '
+   test_when_finished rm -rf repo_a &&
+   test_when_finished rm -rf repo_b &&
+   test_when_finished rm -rf foo &&
+   git init repo_a &&
+   (
+   cd repo_a &&
+   test_commit a_master &&
+   git checkout -b foo &&
+   test_commit a_foo
+   ) &&
+   git init repo_b &&
+   (
+   cd repo_b &&
+   test_commit b_master &&
+   git remote add repo_a ../repo_a &&
+   git config remote.repo_a.fetch \
+   "+refs/heads/*:refs/remotes/other_a/*" &&
+   git fetch --all &&
+   git worktree add --no-track ../foo
+   ) &&
+   (
+   cd foo &&
+   ! test_branch_upstream foo repo_a 

git archive --remote should generate tar.gz format indicated by -o filename

[git-scm]

git archive -o name.tar.gz generates a gzipped file without needing an
explicit --format switch.

However, git archive -o name.tar.gz --remote [url] generates a tar
file, which is unexpected, bandwidth-heavier, and additionally in some
cases it's not immediately obvious that this has happened.

git archive -o name.tar.gz --remote [url] --format tar.gz generates a
gzipped file, so there's obviously no limitation with e.g. git-upload-
archive.

Given the above, either git archive or git-upload-archive should apply
the same tar.gz filename heuristic and generate the expected format.

Presumably e.g. tar.xz support when using --remote would be more
problematic since, in the local case, it involves specifying an
arbitrary command.

[PATCH v2] git-send-email: honor $PATH for sendmail binary

[Florian Klink]

This extends git-send-email to also consider sendmail binaries in $PATH,
in addition to the (fixed) list of /usr/sbin and /usr/lib.fixed) list of
paths.

Signed-off-by: Florian Klink 
---
 Documentation/git-send-email.txt | 6 +++---
 git-send-email.perl  | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index bac9014ac..7af48f8eb 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -203,9 +203,9 @@ a password is obtained using 'git-credential'.
specify a full pathname of a sendmail-like program instead;
the program must support the `-i` option.  Default value can
be specified by the `sendemail.smtpServer` configuration
-   option; the built-in default is `/usr/sbin/sendmail` or
-   `/usr/lib/sendmail` if such program is available, or
-   `localhost` otherwise.
+   option; the built-in default is to search in $PATH,
+   then /usr/sbin and /usr/lib/sendmail afterwards if such program
+   is available, falling back to `localhost` otherwise.
 
 --smtp-server-port=::
Specifies a port different from the default port (SMTP
diff --git a/git-send-email.perl b/git-send-email.perl
index 2208dcc21..570f04079 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -885,7 +885,9 @@ if (defined $initial_reply_to) {
 }
 
 if (!defined $smtp_server) {
-   foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+   my @sendmail_paths = map {"$_/sendmail"} split /:/, $ENV{PATH};
+   push @sendmail_paths, qw( /usr/sbin/sendmail /usr/lib/sendmail );
+   foreach (@sendmail_paths) {
if (-x $_) {
$smtp_server = $_;
last;
-- 
2.15.0

Re: [PATCH v2 2/3] worktree: make add dwim

[Thomas Gummerer]

On 11/18, Thomas Gummerer wrote:
> Currently 'git worktree add  ', errors out when 'branch'
> is not a local branch.   It has no additional dwim'ing features that one
> might expect.
> 
> Make it behave more like 'git checkout ' when the branch doesn't
> exist locally, but a remote tracking branch uniquely matches the desired
> branch name, i.e. create a new branch from the remote tracking branch
> and set the upstream to the remote tracking branch.
> 
> As 'git worktree add' currently just dies in this situation, there are
> no backwards compatibility worries when introducing this feature.
> 
> Signed-off-by: Thomas Gummerer 
> ---
>  Documentation/git-worktree.txt |  7 +++
>  builtin/worktree.c | 15 ++
>  t/t2025-worktree-add.sh| 44 
> ++
>  3 files changed, 66 insertions(+)
> 
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index b472acc356..3c7c8c3cee 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -52,6 +52,13 @@ is linked to the current repository, sharing everything 
> except working
>  directory specific files such as HEAD, index, etc. `-` may also be
>  specified as ``; it is synonymous with `@{-1}`.
>  +
> +If  is not found but there does exist a tracking branch in
> +exactly one remote (call it ) with a matching name, treat as
> +equivalent to
> +
> +$ git worktree add -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.
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 7b9307aa58..92b583ae39 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -1,4 +1,5 @@
>  #include "cache.h"
> +#include "checkout.h"
>  #include "config.h"
>  #include "builtin.h"
>  #include "dir.h"
> @@ -386,6 +387,20 @@ static int add(int ac, const char **av, const char 
> *prefix)
>   opts.new_branch = xstrndup(s, n);
>   }
>  
> + if (ac == 2) {

Err I just realized this doesn't quite make sense.  Similar to the
dwim for 'git worktree add ', this condition should include
'!opts.new_branch && !opts.detach'.  In these cases it's still better
to error out, as the user explicitly asked for a new branch with a
different name/asked not to be put onto a branch.  I'll send a v3 with
this fixed in a bit.

> + struct object_id oid;
> + struct commit *commit;
> + const char *remote;
> +
> + commit = lookup_commit_reference_by_name(branch);
> + if (!commit)
> + remote = unique_tracking_name(branch, );
> + if (!commit && remote) {
> + opts.new_branch = branch;
> + branch = remote;
> + }
> + }
> +
>   if (opts.new_branch) {
>   struct child_process cp = CHILD_PROCESS_INIT;
>   cp.git_cmd = 1;
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index b5c47ac602..e5959800c0 100755
> --- a/t/t2025-worktree-add.sh
> +++ b/t/t2025-worktree-add.sh
> @@ -6,6 +6,16 @@ test_description='test git worktree add'
>  
>  . "$TEST_DIRECTORY"/lib-rebase.sh
>  
> +# Is branch "refs/heads/$1" set to pull from "$2/$3"?
> +test_branch_upstream () {
> + printf "%s\n" "$2" "refs/heads/$3" >expect.upstream &&
> + {
> + git config "branch.$1.remote" &&
> + git config "branch.$1.merge"
> + } >actual.upstream &&
> + test_cmp expect.upstream actual.upstream
> +}
> +
>  test_expect_success 'setup' '
>   test_commit init
>  '
> @@ -314,4 +324,38 @@ test_expect_success 'rename a branch under bisect not 
> allowed' '
>   test_must_fail git branch -M under-bisect bisect-with-new-name
>  '
>  
> +test_expect_success '"add"   fails' '
> + test_must_fail git worktree add foo non-existent
> +'
> +
> +test_expect_success '"add"   dwims' '
> + test_when_finished rm -rf repo_upstream &&
> + test_when_finished rm -rf repo_dwim &&
> + test_when_finished rm -rf foo &&
> + git init repo_upstream &&
> + (
> + cd repo_upstream &&
> + test_commit upstream_master &&
> + git checkout -b foo &&
> + test_commit a_foo
> + ) &&
> + git init repo_dwim &&
> + (
> + cd repo_dwim &&
> + test_commit dwim_master &&
> + git remote add repo_upstream ../repo_upstream &&
> + git config remote.repo_upstream.fetch \
> +   "refs/heads/*:refs/remotes/repo_upstream/*" &&
> + git fetch --all &&
> + git worktree add ../foo foo
> + ) &&
> + (
> + cd foo &&
> + test_branch_upstream foo repo_upstream foo &&
> + git rev-parse repo_upstream/foo >expect &&
> +

Re: [PATCH] git-send-email: honor $PATH

[Florian Klink]

This patch adds support for PATH, but it also removes the fixed paths.
On many systems, unprivileged users don't have /usr/sbin in their PATH,
and I know of no systems which provide /usr/lib as a PATH value.
Therefore, it's possible that this change will break automatic detection
of sendmail for many users.

I think what you probably want to do is use entries in PATH first, and
leave the two old values as backups at the end.


You're perfectly right - forgot about /usr/sbin not in PATH for unprivileged
users. Will send a new patch which appends to the original list.


signature.asc
Description: PGP signature

Re: [PATCH] git-send-email: honor $PATH

[brian m. carlson]

On Sat, Nov 18, 2017 at 01:42:49PM +0100, Florian Klink wrote:
> This will search $PATH for a sendmail binary, instead of the (previously
> fixed) list of paths.
> 
> Signed-off-by: Florian Klink 
> ---
>  Documentation/git-send-email.txt | 5 ++---
>  git-send-email.perl  | 3 ++-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-send-email.txt 
> b/Documentation/git-send-email.txt
> index bac9014ac..b9b1f2c41 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -203,9 +203,8 @@ a password is obtained using 'git-credential'.
>   specify a full pathname of a sendmail-like program instead;
>   the program must support the `-i` option.  Default value can
>   be specified by the `sendemail.smtpServer` configuration
> - option; the built-in default is `/usr/sbin/sendmail` or
> - `/usr/lib/sendmail` if such program is available, or
> - `localhost` otherwise.
> + option; the built-in default is to search in $PATH if such program is
> + available, or `localhost` otherwise.

This patch adds support for PATH, but it also removes the fixed paths.
On many systems, unprivileged users don't have /usr/sbin in their PATH,
and I know of no systems which provide /usr/lib as a PATH value.
Therefore, it's possible that this change will break automatic detection
of sendmail for many users.

I think what you probably want to do is use entries in PATH first, and
leave the two old values as backups at the end.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH] sha1: add gnutls as a sha1 provider

[Shawn Landden]

On Tue, Nov 14, 2017 at 11:47 AM, Todd Zullinger  wrote:
>
> Hi Shawn,
>
> Shawn Landden wrote:
>>
>> I think this is preferrable to bringing the assembly routines into the git 
>> code-base, as a way of getting access to these high-performance routines to 
>> a git available in Debian, Ubuntu, or Fedora (which all use BLK_SHA1=1 due 
>> to GPLv2 + OpenSSL license considerations, see Debian Bug #879459).
>
>
> While it seems like it could be useful to have the choice of using the fast 
> SHA1 implementation without concern about licensing issues, there's a few 
> details I thought were worth mentioning.
>
> Fedora moved from OpenSSL SHA1 to BLK_SHA1 to reduce the size of the binaries 
> and dependencies, not due to licensing issues (Fedora considers OpenSSL a 
> system library and allows linking GPLv2 code).
>
> Fedora now uses the default DC_SHA1 (the collision-detecting SHA1 
> implementation).  DC_SHA1 is not, as far as I know, as fast as the 
> OpenSSL/GnuTLS SHA1, but it's safer given the increasingly successful attacks 
> against SHA1.  I don't envision changing that to gain performance.  (And, of 
> course, the speed of SHA1 should become less of an issue once git moves to a 
> new, stronger hash.)
>
> It looks like the Debian packages use the default DC_SHA1 implementation as 
> well.  Regardless of the licensing concerns regarding OpenSSL in Debian, I 
> suspect they'll want to use the default, collision-detecting SHA1 
> implementation.  That doesn't mean a patch to add the option of GnuTLS isn't 
> useful though.
>
> Fedora does link with OpenSSL's libcrypto and libssl in Fedora for the 
> remote-curl helpers and imap-send.  I believe the remote-curl helpers just 
> link with curl, which happens to use OpenSSL on Fedora and could use GnuTLS 
> instead.  The imap-send command might also use curl and whatever crypto 
> library curl is built with too, but I'm not terribly familiar with imap-send. 
> (I think those are the only uses of libcrypto or libssl in Fedora's packages, 
> but I could be mistaken).
>
> That's a lot of text without having anything to say about the actual patch.  
> Hopefully it's at least mildly useful to you or others. :)
It is all appreciated. I just want to make note that I am still
interested in getting this patch in.

Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern

[René Scharfe]

Am 18.11.2017 um 18:52 schrieb Jeff King:
> On Sat, Nov 18, 2017 at 11:20:04AM +0100, René Scharfe wrote:
>> Reported-by: Jeff King 
>> Signed-off-by: Rene Scharfe 
> 
> I'm not sure I deserve a reported-by if I say "it looks fine" but am
> totally wrong. ;)

Right, wrong -- mere details.  You pointed out that there was work
to do. :)

René

Investissement privé

[Mrs Zhanna]

--
Cet e-mail sollicite strictement votre intérêt pour un partenariat avec 
moi pour un investissement énorme, répondez pour plus de détails.

Cordialement,
Khvostova Zhanna

[PATCH 6/6] grep: show non-empty lines before functions with -W

[René Scharfe]

Non-empty lines before a function definition are most likely comments
for that function and thus relevant.  Include them in function context.

Such a non-empty line might also belong to the preceding function if
there is no separating blank line.  Stop extending the context upwards
also at the next function line to make sure only one extra function body
is shown at most.

Signed-off-by: Rene Scharfe 
---
 grep.c  | 27 +++
 t/t7810-grep.sh |  2 +-
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/grep.c b/grep.c
index 2c55d10c55..a69c05edc2 100644
--- a/grep.c
+++ b/grep.c
@@ -1476,33 +1476,52 @@ static void show_funcname_line(struct grep_opt *opt, 
struct grep_source *gs,
}
 }
 
+static int is_empty_line(const char *bol, const char *eol);
+
 static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,
 char *bol, char *end, unsigned lno)
 {
unsigned cur = lno, from = 1, funcname_lno = 0, orig_from;
-   int funcname_needed = !!opt->funcname;
+   int funcname_needed = !!opt->funcname, comment_needed = 0;
 
if (opt->pre_context < lno)
from = lno - opt->pre_context;
if (from <= opt->last_shown)
from = opt->last_shown + 1;
orig_from = from;
-   if (opt->funcbody && !match_funcname(opt, gs, bol, end)) {
-   funcname_needed = 1;
+   if (opt->funcbody) {
+   if (match_funcname(opt, gs, bol, end))
+   comment_needed = 1;
+   else
+   funcname_needed = 1;
from = opt->last_shown + 1;
}
 
/* Rewind. */
while (bol > gs->buf && cur > from) {
+   char *next_bol = bol;
char *eol = --bol;
 
while (bol > gs->buf && bol[-1] != '\n')
bol--;
cur--;
+   if (comment_needed && (is_empty_line(bol, eol) ||
+  match_funcname(opt, gs, bol, eol))) {
+   comment_needed = 0;
+   from = orig_from;
+   if (cur < from) {
+   cur++;
+   bol = next_bol;
+   break;
+   }
+   }
if (funcname_needed && match_funcname(opt, gs, bol, eol)) {
funcname_lno = cur;
funcname_needed = 0;
-   from = orig_from;
+   if (opt->funcbody)
+   comment_needed = 1;
+   else
+   from = orig_from;
}
}
 
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 632b905611..c02ca735b9 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -785,7 +785,7 @@ test_expect_success 'grep -W with userdiff' '
git grep -W echo >function-context-userdiff-actual
 '
 
-test_expect_failure ' includes preceding comment' '
+test_expect_success ' includes preceding comment' '
grep "# Say hello" function-context-userdiff-actual
 '
 
-- 
2.15.0

Re: [PATCH v1 2/2] worktree: make add dwim

[Thomas Gummerer]

On 11/15, Thomas Gummerer wrote:
> On 11/14, Eric Sunshine wrote:
> > On Tue, Nov 14, 2017 at 3:14 PM, Eric Sunshine  
> > wrote:
> > > For my own edification...
> > > [...]
> > > git worktree add ../topic
> > >
> > > * Correctly errors out, refusing to create a new branch named "topic",
> > > if "topic" is already a branch.
> > 
> > By the way, there's an additional DWIM that could be done here instead
> > of erroring out. Specifically, for "git worktree add ../topic":
> > 
> > * If branch "topic" exists, check it out (rather than refusing to
> > create a new branch named "topic").
> 
> I think this would be a good improvement either way as I suspect this
> is what users would hope for, and as it currently just dies there are
> less backwards compatibility worries.

While I still think this would be an improvement, after thinking about
it a bit more I think this is somewhat orthogonal to what I'm trying
to achieve with this patch series.  Therefore I didn't implement this
yet, but I'm still thinking of implementing this in a separate topic.

> > * If origin/topic exists, DWIM local "topic" branch into existence.
> > 
> > * Otherwise, create new local branch "topic".
> > 
> > > * Creates a new branch named "topic" if no such local branch exists.
> > >
> > > The desired new DWIMing would change the second bullet point to:
> > >
> > > * If no local branch named "topic" exists, DWIM it from "origin/topic"
> > > if possible, else create a new local branch named "topic".

[PATCH 4/6] t7810: improve check of -W with user-defined function lines

[René Scharfe]

The check for function context (-W) together with user-defined function
line patterns reuses hello.c and pretends it's written in a language in
which function lines contain either "printf" or a trailing curly brace.
That's a bit obscure.

Make the test easier to read by adding a small PowerShell script, using
a simple, but meaningful expression, and separating out checks for
different aspects into dedicated tests instead of simply matching the
whole output byte for byte.

Also include a test for showing comments before function lines like git
diff -W does, which is currently failing because that functionality is
not implemented for git grep, yet.

Signed-off-by: Rene Scharfe 
---
 t/t7810-grep.sh | 41 +++--
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 2a6679c2f5..632b905611 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -60,6 +60,18 @@ test_expect_success setup '
echo " line with leading space3"
echo "line without leading space2"
} >space &&
+   cat >hello.ps1 <<-\EOF &&
+   # No-op.
+   function dummy() {}
+
+   # Say hello.
+   function hello() {
+ echo "Hello world."
+   } # hello
+
+   # Still a no-op.
+   function dummy() {}
+   EOF
git add . &&
test_tick &&
git commit -m initial
@@ -766,18 +778,27 @@ test_expect_success 'grep -W shows no trailing empty 
lines' '
test_cmp expected actual
 '
 
-cat >expected <.gitattributes &&
-   git grep -W return >actual &&
-   test_cmp expected actual
+   git config diff.custom.xfuncname "^function .*$" &&
+   echo "hello.ps1 diff=custom" >.gitattributes &&
+   git grep -W echo >function-context-userdiff-actual
+'
+
+test_expect_failure ' includes preceding comment' '
+   grep "# Say hello" function-context-userdiff-actual
+'
+
+test_expect_success ' includes function line' '
+   grep "=function hello" function-context-userdiff-actual
+'
+
+test_expect_success ' includes matching line' '
+   grep ":  echo" function-context-userdiff-actual
+'
+
+test_expect_success ' includes last line of the function' '
+   grep "} # hello" function-context-userdiff-actual
 '
 
 for threads in $(test_seq 0 10)
-- 
2.15.0

[PATCH 2/6] xdiff: factor out is_func_rec()

[René Scharfe]

Add a helper for checking if a given record is a function line.  It
frees callers from having to deal with the buffer arguments of
match_func_rec().

Signed-off-by: Rene Scharfe 
---
 xdiff/xemit.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 6881445e4a..c2d5bd004c 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -121,6 +121,12 @@ static long match_func_rec(xdfile_t *xdf, xdemitconf_t 
const *xecfg, long ri,
return xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv);
 }
 
+static int is_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri)
+{
+   char dummy[1];
+   return match_func_rec(xdf, xecfg, ri, dummy, sizeof(dummy)) >= 0;
+}
+
 struct func_line {
long len;
char buf[80];
@@ -178,7 +184,6 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, 
xdemitcb_t *ecb,
 
/* Appended chunk? */
if (i1 >= xe->xdf1.nrec) {
-   char dummy[1];
long i2 = xch->i2;
 
/*
@@ -186,8 +191,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, 
xdemitcb_t *ecb,
 * a whole function was added.
 */
while (i2 < xe->xdf2.nrec) {
-   if (match_func_rec(>xdf2, xecfg, i2,
-   dummy, sizeof(dummy)) >= 0)
+   if (is_func_rec(>xdf2, xecfg, i2))
goto post_context_calculation;
i2++;
}
-- 
2.15.0

[PATCH v2 2/3] worktree: make add dwim

[Thomas Gummerer]

Currently 'git worktree add  ', errors out when 'branch'
is not a local branch.   It has no additional dwim'ing features that one
might expect.

Make it behave more like 'git checkout ' when the branch doesn't
exist locally, but a remote tracking branch uniquely matches the desired
branch name, i.e. create a new branch from the remote tracking branch
and set the upstream to the remote tracking branch.

As 'git worktree add' currently just dies in this situation, there are
no backwards compatibility worries when introducing this feature.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-worktree.txt |  7 +++
 builtin/worktree.c | 15 ++
 t/t2025-worktree-add.sh| 44 ++
 3 files changed, 66 insertions(+)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b472acc356..3c7c8c3cee 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -52,6 +52,13 @@ is linked to the current repository, sharing everything 
except working
 directory specific files such as HEAD, index, etc. `-` may also be
 specified as ``; it is synonymous with `@{-1}`.
 +
+If  is not found but there does exist a tracking branch in
+exactly one remote (call it ) with a matching name, treat as
+equivalent to
+
+$ git worktree add -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.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b9307aa58..92b583ae39 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "checkout.h"
 #include "config.h"
 #include "builtin.h"
 #include "dir.h"
@@ -386,6 +387,20 @@ static int add(int ac, const char **av, const char *prefix)
opts.new_branch = xstrndup(s, n);
}
 
+   if (ac == 2) {
+   struct object_id oid;
+   struct commit *commit;
+   const char *remote;
+
+   commit = lookup_commit_reference_by_name(branch);
+   if (!commit)
+   remote = unique_tracking_name(branch, );
+   if (!commit && remote) {
+   opts.new_branch = branch;
+   branch = remote;
+   }
+   }
+
if (opts.new_branch) {
struct child_process cp = CHILD_PROCESS_INIT;
cp.git_cmd = 1;
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index b5c47ac602..e5959800c0 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -6,6 +6,16 @@ test_description='test git worktree add'
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+# Is branch "refs/heads/$1" set to pull from "$2/$3"?
+test_branch_upstream () {
+   printf "%s\n" "$2" "refs/heads/$3" >expect.upstream &&
+   {
+   git config "branch.$1.remote" &&
+   git config "branch.$1.merge"
+   } >actual.upstream &&
+   test_cmp expect.upstream actual.upstream
+}
+
 test_expect_success 'setup' '
test_commit init
 '
@@ -314,4 +324,38 @@ test_expect_success 'rename a branch under bisect not 
allowed' '
test_must_fail git branch -M under-bisect bisect-with-new-name
 '
 
+test_expect_success '"add"   fails' '
+   test_must_fail git worktree add foo non-existent
+'
+
+test_expect_success '"add"   dwims' '
+   test_when_finished rm -rf repo_upstream &&
+   test_when_finished rm -rf repo_dwim &&
+   test_when_finished rm -rf foo &&
+   git init repo_upstream &&
+   (
+   cd repo_upstream &&
+   test_commit upstream_master &&
+   git checkout -b foo &&
+   test_commit a_foo
+   ) &&
+   git init repo_dwim &&
+   (
+   cd repo_dwim &&
+   test_commit dwim_master &&
+   git remote add repo_upstream ../repo_upstream &&
+   git config remote.repo_upstream.fetch \
+ "refs/heads/*:refs/remotes/repo_upstream/*" &&
+   git fetch --all &&
+   git worktree add ../foo foo
+   ) &&
+   (
+   cd foo &&
+   test_branch_upstream foo repo_upstream foo &&
+   git rev-parse repo_upstream/foo >expect &&
+   git rev-parse foo >actual &&
+   test_cmp expect actual
+   )
+'
+
 test_done
-- 
2.15.0.345.gf926f18f3d

[PATCH v2 1/3] checkout: factor out functions to new lib file

[Thomas Gummerer]

Factor the functions out, so they can be re-used from other places.  In
particular these functions will be re-used in builtin/worktree.c to make
git worktree add dwim more.

While there add some docs to the function.

Signed-off-by: Thomas Gummerer 
---
The previous round of this series is at
https://public-inbox.org/git/20171112134305.3949-1-t.gumme...@gmail.com/.
Thanks Junio and Eric for the comments on the previous round!

 Makefile   |  1 +
 builtin/checkout.c | 41 +
 checkout.c | 42 ++
 checkout.h | 13 +
 4 files changed, 57 insertions(+), 40 deletions(-)
 create mode 100644 checkout.c
 create mode 100644 checkout.h

diff --git a/Makefile b/Makefile
index cd75985991..8d603c7443 100644
--- a/Makefile
+++ b/Makefile
@@ -757,6 +757,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
+LIB_OBJS += checkout.o
 LIB_OBJS += color.o
 LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc4f8fd2ea..9e1cfd10b3 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "config.h"
+#include "checkout.h"
 #include "lockfile.h"
 #include "parse-options.h"
 #include "refs.h"
@@ -874,46 +875,6 @@ static int git_checkout_config(const char *var, const char 
*value, void *cb)
return git_xmerge_config(var, value, NULL);
 }
 
-struct tracking_name_data {
-   /* const */ char *src_ref;
-   char *dst_ref;
-   struct object_id *dst_oid;
-   int unique;
-};
-
-static int check_tracking_name(struct remote *remote, void *cb_data)
-{
-   struct tracking_name_data *cb = cb_data;
-   struct refspec query;
-   memset(, 0, sizeof(struct refspec));
-   query.src = cb->src_ref;
-   if (remote_find_tracking(remote, ) ||
-   get_oid(query.dst, cb->dst_oid)) {
-   free(query.dst);
-   return 0;
-   }
-   if (cb->dst_ref) {
-   free(query.dst);
-   cb->unique = 0;
-   return 0;
-   }
-   cb->dst_ref = query.dst;
-   return 0;
-}
-
-static const char *unique_tracking_name(const char *name, struct object_id 
*oid)
-{
-   struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
-   cb_data.src_ref = xstrfmt("refs/heads/%s", name);
-   cb_data.dst_oid = oid;
-   for_each_remote(check_tracking_name, _data);
-   free(cb_data.src_ref);
-   if (cb_data.unique)
-   return cb_data.dst_ref;
-   free(cb_data.dst_ref);
-   return NULL;
-}
-
 static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
diff --git a/checkout.c b/checkout.c
new file mode 100644
index 00..b0c744d37a
--- /dev/null
+++ b/checkout.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "remote.h"
+
+struct tracking_name_data {
+   /* const */ char *src_ref;
+   char *dst_ref;
+   struct object_id *dst_oid;
+   int unique;
+};
+
+static int check_tracking_name(struct remote *remote, void *cb_data)
+{
+   struct tracking_name_data *cb = cb_data;
+   struct refspec query;
+   memset(, 0, sizeof(struct refspec));
+   query.src = cb->src_ref;
+   if (remote_find_tracking(remote, ) ||
+   get_oid(query.dst, cb->dst_oid)) {
+   free(query.dst);
+   return 0;
+   }
+   if (cb->dst_ref) {
+   free(query.dst);
+   cb->unique = 0;
+   return 0;
+   }
+   cb->dst_ref = query.dst;
+   return 0;
+}
+
+const char *unique_tracking_name(const char *name, struct object_id *oid)
+{
+   struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+   cb_data.src_ref = xstrfmt("refs/heads/%s", name);
+   cb_data.dst_oid = oid;
+   for_each_remote(check_tracking_name, _data);
+   free(cb_data.src_ref);
+   if (cb_data.unique)
+   return cb_data.dst_ref;
+   free(cb_data.dst_ref);
+   return NULL;
+}
diff --git a/checkout.h b/checkout.h
new file mode 100644
index 00..9980711179
--- /dev/null
+++ b/checkout.h
@@ -0,0 +1,13 @@
+#ifndef CHECKOUT_H
+#define CHECKOUT_H
+
+#include "cache.h"
+
+/*
+ * Check if the branch name uniquely matches a branch name on a remote
+ * tracking branch.  Return the name of the remote if such a branch
+ * exists, NULL otherwise.
+ */
+extern const char *unique_tracking_name(const char *name, struct object_id 
*oid);
+
+#endif /* CHECKOUT_H */
-- 
2.15.0.345.gf926f18f3d

[PATCH v2 3/3] worktree: make add dwim

[Thomas Gummerer]

Currently 'git worktree add ' creates a new branch named after the
basename of the , that matches the HEAD of whichever worktree we
were on when calling "git worktree add ".

Make 'git worktree add  behave more like the dwim machinery in
'git checkout ', i.e. check if the new branch name uniquely
matches the branch name of a remote tracking branch, and if so check out
that branch and set the upstream to the remote tracking branch.

This is a change of behaviour compared to the current behaviour, where
we create a new branch matching HEAD.  However as 'git worktree' is
still an experimental feature, and it's easy to notice/correct the
behaviour in case it's not what the user desired it's probably okay to
break existing behaviour here.

In order to also satisfy users who want the current behaviour of
creating a new branch from HEAD, add a '--no-track' flag, which disables
the new behaviour, and keeps the old behaviour of creating a new branch
from the head of the current worktree.

Signed-off-by: Thomas Gummerer 
---
I went back and forth between hiding this behing a flag, and making it
default and having a flag for getting the old behaviour back.

In the end I went for making the new behaviour the default, because
the 'worktree' feature is still experimental, and it's easy to correct
the behaviour if it's not what was desired.  I also think this is the
more intuitve behaviour, but clearly I'm biased because *I* want to it
to behave this way.

I'm happy to keep the old behaviour the default and hide the new
behaviour behind a flag if we feel this is too much of a change in
behaviour at this point.

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

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 3c7c8c3cee..11cac104d9 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -60,9 +60,18 @@ $ git worktree add -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, if there exists a tracking branch in exactly
+one remote (call it ) matching the basename of the path
+(call it ), treat it as equivalent to
+
+$ git worktree add -b   /
+
+If no tracking branch exists in exactly one remote,  is
+created based on HEAD, as if `-b $(basename )` was specified.
++
+To disable the behaviour of trying to match the basename of  to
+a remote, and always create a new branch from HEAD, the `--no-track`
+flag can be passed to `git worktree add`.
 list::
 
 List details of each worktree.  The main worktree is listed first, followed by
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 92b583ae39..82088415b8 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -342,6 +342,7 @@ static int add(int ac, const char **av, const char *prefix)
const char *new_branch_force = NULL;
char *path;
const char *branch;
+   int track_dwim = 1;
struct option options[] = {
OPT__FORCE(, N_("checkout  even if already 
checked out in other worktree")),
OPT_STRING('b', NULL, _branch, N_("branch"),
@@ -351,6 +352,7 @@ static int add(int ac, const char **av, const char *prefix)
OPT_BOOL(0, "detach", , N_("detach HEAD at named 
commit")),
OPT_BOOL(0, "checkout", , N_("populate the new 
working tree")),
OPT_BOOL(0, "lock", _locked, N_("keep the new working 
tree locked")),
+   OPT_BOOL(0, "track", _dwim, N_("checkout upstream branch 
if there's a unique match")),
OPT_END()
};
 
@@ -385,6 +387,13 @@ static int add(int ac, const char **av, const char *prefix)
int n;
const char *s = worktree_basename(path, );
opts.new_branch = xstrndup(s, n);
+   if (track_dwim) {
+   struct object_id oid;
+   const char *remote =
+   unique_tracking_name(opts.new_branch, );
+   if (remote)
+   branch = remote;
+   }
}
 
if (ac == 2) {
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index e5959800c0..a566d867fe 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -358,4 +358,64 @@ test_expect_success '"add"   dwims' '
)
 '
 
+test_expect_success 'git worktree add --no-track does not set up tracking' '
+   test_when_finished rm -rf repo_a &&
+   test_when_finished rm -rf repo_b &&
+   test_when_finished rm -rf foo &&
+   git init repo_a &&
+   (
+   cd repo_a &&
+   

[PATCH 5/6] grep: update boundary variable for pre-context

[René Scharfe]

Function context can be bigger than -A/-B/-C context.  To find the
beginning of the combined context we search backwards.  Currently we
check at each loop iteration what we're looking for and determine the
effective upper boundary based on that.

Simplify this a bit by setting the variable "from" to the lowest unshown
line number up front if we're looking for a function line and set it
back to the required -B/-C context line number when we find one.  This
prepares the ground for the next patch; no functional change intended.

Signed-off-by: Rene Scharfe 
---
 grep.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/grep.c b/grep.c
index d0b9b6cdfa..2c55d10c55 100644
--- a/grep.c
+++ b/grep.c
@@ -1479,20 +1479,21 @@ static void show_funcname_line(struct grep_opt *opt, 
struct grep_source *gs,
 static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,
 char *bol, char *end, unsigned lno)
 {
-   unsigned cur = lno, from = 1, funcname_lno = 0;
+   unsigned cur = lno, from = 1, funcname_lno = 0, orig_from;
int funcname_needed = !!opt->funcname;
 
-   if (opt->funcbody && !match_funcname(opt, gs, bol, end))
-   funcname_needed = 2;
-
if (opt->pre_context < lno)
from = lno - opt->pre_context;
if (from <= opt->last_shown)
from = opt->last_shown + 1;
+   orig_from = from;
+   if (opt->funcbody && !match_funcname(opt, gs, bol, end)) {
+   funcname_needed = 1;
+   from = opt->last_shown + 1;
+   }
 
/* Rewind. */
-   while (bol > gs->buf &&
-  cur > (funcname_needed == 2 ? opt->last_shown + 1 : from)) {
+   while (bol > gs->buf && cur > from) {
char *eol = --bol;
 
while (bol > gs->buf && bol[-1] != '\n')
@@ -1501,6 +1502,7 @@ static void show_pre_context(struct grep_opt *opt, struct 
grep_source *gs,
if (funcname_needed && match_funcname(opt, gs, bol, eol)) {
funcname_lno = cur;
funcname_needed = 0;
+   from = orig_from;
}
}
 
-- 
2.15.0

[PATCH 3/6] xdiff: show non-empty lines before functions with -W

[René Scharfe]

Non-empty lines before a function definition are most likely comments
for that function and thus relevant.  Include them in function context.

Such a non-empty line might also belong to the preceeding function if
there is no separating blank line.  Stop extending the context upwards
also at the next function line to make sure only one extra function body
is shown at most.

Original-patch-by: Vegard Nossum 
Signed-off-by: Rene Scharfe 
---
 t/t4051-diff-function-context.sh | 2 +-
 xdiff/xemit.c| 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 30fc5bf2b3..2d76a971c4 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -85,7 +85,7 @@ test_expect_success 'setup' '
 
 check_diff changed_hello 'changed function'
 
-test_expect_failure ' context includes comment' '
+test_expect_success ' context includes comment' '
grep "^ .*Hello comment" changed_hello.diff
 '
 
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index c2d5bd004c..7778dc2b19 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -204,6 +204,9 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, 
xdemitcb_t *ecb,
}
 
fs1 = get_func_line(xe, xecfg, NULL, i1, -1);
+   while (fs1 > 0 && !is_empty_rec(>xdf1, fs1 - 1) &&
+  !is_func_rec(>xdf1, xecfg, fs1 - 1))
+   fs1--;
if (fs1 < 0)
fs1 = 0;
if (fs1 < s1) {
-- 
2.15.0

[PATCH 1/6] t4051: add test for comments preceding function lines

[René Scharfe]

When showing function context it would be helpful to show comments
immediately before declarations, as they are most likely relevant.  Add
a test for that.

Signed-off-by: Rene Scharfe 
---
 t/t4051-diff-function-context.sh | 4 
 t/t4051/hello.c  | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 3e6b485ecb..30fc5bf2b3 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -85,6 +85,10 @@ test_expect_success 'setup' '
 
 check_diff changed_hello 'changed function'
 
+test_expect_failure ' context includes comment' '
+   grep "^ .*Hello comment" changed_hello.diff
+'
+
 test_expect_success ' context includes begin' '
grep "^ .*Begin of hello" changed_hello.diff
 '
diff --git a/t/t4051/hello.c b/t/t4051/hello.c
index 63b1a1e4ef..73e767e178 100644
--- a/t/t4051/hello.c
+++ b/t/t4051/hello.c
@@ -1,4 +1,7 @@
 
+/*
+ * Hello comment.
+ */
 static void hello(void)// Begin of hello
 {
/*
-- 
2.15.0

[PATCH 0/6] show non-empty lines before functions with diff/grep -W

[René Scharfe]

The option -W/--function-context lets git diff and git grep show the
whole surrounding function as context.  For the sake of simplicity and
performance they don't fully parse the files, but as a heuristic show
all lines from the preceding function line to the next one.

This series refines that heuristic and extends the context to include
any non-empty lines before the preceding function line as well.  They
most likely contain comments related to that function and are thus
relevant for reviewing diffs and search results.

Idea and original implementation for git diff by Vegard Nossum:
https://public-inbox.org/git/1484324112-17773-2-git-send-email-vegard.nos...@oracle.com/

  t4051: add test for comments preceeding function lines
  xdiff: factor out is_func_rec()
  xdiff: show non-empty lines before functions with -W
  t7810: improve check of -W with user-defined function lines
  grep: update boundary variable for pre-context
  grep: show non-empty lines before functions with -W

 grep.c   | 35 +++---
 t/t4051-diff-function-context.sh |  4 
 t/t4051/hello.c  |  3 +++
 t/t7810-grep.sh  | 41 ++--
 xdiff/xemit.c| 13 ++---
 5 files changed, 76 insertions(+), 20 deletions(-)

-- 
2.15.0

Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern

[Jeff King]

On Sat, Nov 18, 2017 at 11:20:04AM +0100, René Scharfe wrote:

> Am 17.11.2017 um 23:06 schrieb Jeff King:
> > There's one more case in write_section() that uses "==". That's not
> > actually wrong, but I wonder if we'd want to make it "< 0" for
> > consistency.
> 
> Actually it *is* wrong.

Thanks for digging, I didn't look beyond that single line.

> -- >8 --
> Subject: [PATCH] config: flip return value of write_section()
> 
> d9bd4cbb9cc (config: flip return value of store_write_*()) made
> write_section() follow the convention of write(2) to return -1 on error
> and the number of written bytes on success.  3b48045c6c7 (Merge branch
> 'sd/branch-copy') changed it back to returning 0 on error and 1 on
> success, but left its callers still checking for negative values.
> 
> Let write_section() follow the convention of write(2) again to meet the
> expectations of its callers.

Yikes. It looks like this slipped by on the tests because we always
check "< 0" in the callers, not non-zero. So success would not look like
failure, but failure would look like success. And write failure does not
happen regularly in the test suite.

So this looks correct, and well-explained.

> Reported-by: Jeff King 
> Signed-off-by: Rene Scharfe 

I'm not sure I deserve a reported-by if I say "it looks fine" but am
totally wrong. ;)

-Peff

Re: [PATCH] launch_editor(): indicate that Git waits for user input

[Kaartic Sivaraam]

On Thursday 16 November 2017 11:34 AM, Junio C Hamano wrote:


  * I tried this with "emacsclient" but it turns out that Emacs folks
have already thought about this issue, and the program says
"Waiting for Emacs..." while it does its thing, so from that
point of view, perhaps as Stefan said originally, the editor Lars
had trouble with is what is at fault and needs fixing?  I dunno.

Also, emacsclient seems to conclude its message, once the editing
is done, by emitting LF _before_ we have a chance to do the "go
back to the beginning and clear" dance, so it probably is not a
very good example to emulate the situation Lars had trouble with.
You cannot observe the nuking of the "launched, waiting..." with
it.



I tried this patch with 'gedit' and it seems to work fine except for one 
particular case. When the launched editor gets killed somehow when 
waiting for user input, the error message shown in the terminal seems to 
be following the "Launched editor..." message immediately, making it 
hard to identify it.


$ GIT_EDITOR=gedit ./git commit --allow-empty
Launched your editor, waiting...error: gedit died of signal 15
error: There was a problem with the editor 'gedit'.
Please supply the message using either -m or -F option.

Though this is not something that's going to happen very often, I'm not 
sure if this is to be considered. Just wanted to note what I observed.




  editor.c | 25 +
  1 file changed, 25 insertions(+)

diff --git a/editor.c b/editor.c
index 7519edecdc..1321944716 100644
--- a/editor.c
+++ b/editor.c
@@ -40,6 +40,28 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
const char *args[] = { editor, real_path(path), NULL };
struct child_process p = CHILD_PROCESS_INIT;
int ret, sig;
+   static const char *close_notice = NULL;


Just a little curious, what's the advantage of making 'close_notice' 
'static' over 'auto' ?




+
+   if (isatty(2) && !close_notice) {
+   char *term = getenv("TERM");
+
+   if (term && strcmp(term, "dumb"))
+   /*
+* go back to the beginning and erase the
+* entire line if the terminal is capable
+* to do so, to avoid wasting the vertical
+* space.
+*/
+   close_notice = "\r\033[K";
+   else
+   /* otherwise, complete and waste the line */
+   close_notice = "done.\n";
+   }
+
+   if (close_notice) {
+   fprintf(stderr, "Launched your editor, waiting...");
+   fflush(stderr);


Being curious again, is flushing 'stderr' required ? AFAIK, 'stderr' is 
unbuffered by default and I didn't notice any calls that changed the 
buffering mode of it along this code path.




+   }
  
  		p.argv = args;

p.env = env;
@@ -53,11 +75,14 @@ int launch_editor(const char *path, struct strbuf *buffer, 
const char *const *en
sig = ret - 128;
sigchain_pop(SIGINT);
sigchain_pop(SIGQUIT);
+
if (sig == SIGINT || sig == SIGQUIT)
raise(sig);
if (ret)
return error("There was a problem with the editor 
'%s'.",
editor);
+   if (close_notice)
+   fputs(close_notice, stderr);
}
  
  	if (!buffer)

[PATCH 1/4] branch: improve documentation and naming of create_branch() parameters

[Kaartic Sivaraam]

The documentation for 'create_branch()' was incomplete as it didn't say
what certain parameters were used for. Further a parameter name wasn't
very communicative.

So, add missing documentation for the sake of completeness and easy
reference. Also, rename the concerned parameter to make its name more
communicative.

Signed-off-by: Kaartic Sivaraam 
---
 branch.c | 4 ++--
 branch.h | 7 ++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/branch.c b/branch.c
index 62f7b0d8c..3e8d2f93f 100644
--- a/branch.c
+++ b/branch.c
@@ -228,7 +228,7 @@ N_("\n"
 "\"git push -u\" to set the upstream config as you push.");
 
 void create_branch(const char *name, const char *start_name,
-  int force, int reflog, int clobber_head,
+  int force, int reflog, int clobber_head_ok,
   int quiet, enum branch_track track)
 {
struct commit *commit;
@@ -244,7 +244,7 @@ void create_branch(const char *name, const char *start_name,
 
if (validate_new_branchname(name, , force,
track == BRANCH_TRACK_OVERRIDE ||
-   clobber_head)) {
+   clobber_head_ok)) {
if (!force)
dont_change_ref = 1;
else
diff --git a/branch.h b/branch.h
index b07788558..cb6411f84 100644
--- a/branch.h
+++ b/branch.h
@@ -15,12 +15,17 @@
  *
  *   - reflog creates a reflog for the branch
  *
+ *   - clobber_head_ok allows the currently checked out (hence existing)
+ * branch to be overwritten; without 'force', it has no effect.
+ *
+ *   - quiet suppresses tracking information
+ *
  *   - track causes the new branch to be configured to merge the remote branch
  * that start_name is a tracking branch for (if any).
  */
 void create_branch(const char *name, const char *start_name,
   int force, int reflog,
-  int clobber_head, int quiet, enum branch_track track);
+  int clobber_head_ok, int quiet, enum branch_track track);
 
 /*
  * Validates that the requested branch may be created, returning the
-- 
2.15.0.291.g0d8980c5d

[PATCH 3/4] branch: update warning message shown when copying a misnamed branch

[Kaartic Sivaraam]

When a user tries to rename a branch that has a "bad name" (e.g.,
starts with a '-') then we warn them that the misnamed branch has
been renamed "away". A similar message is shown when trying to create
a copy of a misnamed branch even though it doesn't remove the misnamed
branch. This is not correct and may confuse the user.

So, update the warning message shown to be more precise that only a copy
of the misnamed branch has been created. It's better to show the warning
message than not showing it at all as it makes the user aware of the
presence of a misnamed branch.

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

diff --git a/builtin/branch.c b/builtin/branch.c
index 4edef5baa..ca9d8abd0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -507,7 +507,7 @@ static void copy_or_rename_branch(const char *oldname, 
const char *newname, int
 
if (recovery) {
if (copy)
-   warning(_("Copied a misnamed branch '%s' away"),
+   warning(_("Created a copy of a misnamed branch '%s'"),
oldref.buf + 11);
else
warning(_("Renamed a misnamed branch '%s' away"),
-- 
2.15.0.291.g0d8980c5d

[PATCH 2/4] branch: group related arguments of create_branch()

[Kaartic Sivaraam]

39bd6f726 (Allow checkout -B  to update the current
branch, 2011-11-26) added 'clobber_head' (now, 'clobber_head_ok')
"before" 'track' as 'track' was closely related 'clobber_head' for
the purpose the commit wanted to achieve. Looking from the perspective
of how the arguments are used it turns out that 'clobber_head' is
more related to 'force' than it is to 'track'.

So, re-order the arguments to keep the related arguments close
to each other.

Signed-off-by: Kaartic Sivaraam 
---
 branch.c   | 2 +-
 branch.h   | 9 +
 builtin/branch.c   | 2 +-
 builtin/checkout.c | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/branch.c b/branch.c
index 3e8d2f93f..bd607ae97 100644
--- a/branch.c
+++ b/branch.c
@@ -228,7 +228,7 @@ N_("\n"
 "\"git push -u\" to set the upstream config as you push.");
 
 void create_branch(const char *name, const char *start_name,
-  int force, int reflog, int clobber_head_ok,
+  int force, int clobber_head_ok, int reflog,
   int quiet, enum branch_track track)
 {
struct commit *commit;
diff --git a/branch.h b/branch.h
index cb6411f84..f66536a10 100644
--- a/branch.h
+++ b/branch.h
@@ -13,19 +13,20 @@
  *
  *   - force enables overwriting an existing (non-head) branch
  *
- *   - reflog creates a reflog for the branch
- *
  *   - clobber_head_ok allows the currently checked out (hence existing)
  * branch to be overwritten; without 'force', it has no effect.
  *
+ *   - reflog creates a reflog for the branch
+ *
  *   - quiet suppresses tracking information
  *
  *   - track causes the new branch to be configured to merge the remote branch
  * that start_name is a tracking branch for (if any).
+ *
  */
 void create_branch(const char *name, const char *start_name,
-  int force, int reflog,
-  int clobber_head_ok, int quiet, enum branch_track track);
+  int force, int clobber_head_ok,
+  int reflog, int quiet, enum branch_track track);
 
 /*
  * Validates that the requested branch may be created, returning the
diff --git a/builtin/branch.c b/builtin/branch.c
index 33fd5fcfd..4edef5baa 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -806,7 +806,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
die(_("the '--set-upstream' option is no longer 
supported. Please use '--track' or '--set-upstream-to' instead."));
 
create_branch(argv[0], (argc == 2) ? argv[1] : head,
- force, reflog, 0, quiet, track);
+ force, 0, reflog, quiet, track);
 
} else
usage_with_options(builtin_branch_usage, options);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7d8bcc383..6068f8d8c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -640,8 +640,8 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
else
create_branch(opts->new_branch, new->name,
  opts->new_branch_force ? 1 : 0,
- opts->new_branch_log,
  opts->new_branch_force ? 1 : 0,
+ opts->new_branch_log,
  opts->quiet,
  opts->track);
new->name = opts->new_branch;
-- 
2.15.0.291.g0d8980c5d

[PATCH 4/4] builtin/branch: strip refs/heads/ using skip_prefix

[Kaartic Sivaraam]

Instead of hard-coding the offset strlen("refs/heads/") to skip
the prefix "refs/heads/" use the skip_prefix() function which
is more communicative and verifies that the string actually
starts with that prefix.

Though we don't check for the result of verification here as
it's (almost) always the case that the string does start
with "refs/heads", it's just better to avoid hard-coding and
be more communicative.

Signed-off-by: Kaartic Sivaraam 
---
 builtin/branch.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ca9d8abd0..8c546a958 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -462,6 +462,8 @@ static void copy_or_rename_branch(const char *oldname, 
const char *newname, int
 {
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = 
STRBUF_INIT;
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
+   const char *prefix_free_oldref = NULL;
+   const char *prefix_free_newref = NULL;
int recovery = 0;
int clobber_head_ok;
 
@@ -493,13 +495,17 @@ static void copy_or_rename_branch(const char *oldname, 
const char *newname, int
 
reject_rebase_or_bisect_branch(oldref.buf);
 
+   /* At this point it should be safe to believe that the refs have the
+  prefix "refs/heads" */
+   skip_prefix(oldref.buf, "refs/heads/", _free_oldref);
+   skip_prefix(newref.buf, "refs/heads/", _free_newref);
+
if (copy)
strbuf_addf(, "Branch: copied %s to %s",
oldref.buf, newref.buf);
else
strbuf_addf(, "Branch: renamed %s to %s",
oldref.buf, newref.buf);
-
if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf))
die(_("Branch rename failed"));
if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
@@ -508,10 +514,10 @@ static void copy_or_rename_branch(const char *oldname, 
const char *newname, int
if (recovery) {
if (copy)
warning(_("Created a copy of a misnamed branch '%s'"),
-   oldref.buf + 11);
+   prefix_free_oldref);
else
warning(_("Renamed a misnamed branch '%s' away"),
-   oldref.buf + 11);
+   prefix_free_oldref);
}
 
if (!copy &&
@@ -520,9 +526,9 @@ static void copy_or_rename_branch(const char *oldname, 
const char *newname, int
 
strbuf_release();
 
-   strbuf_addf(, "branch.%s", oldref.buf + 11);
+   strbuf_addf(, "branch.%s", prefix_free_oldref);
strbuf_release();
-   strbuf_addf(, "branch.%s", newref.buf + 11);
+   strbuf_addf(, "branch.%s", prefix_free_newref);
strbuf_release();
if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) 
< 0)
die(_("Branch is renamed, but update of config-file failed"));
-- 
2.15.0.291.g0d8980c5d

[PATCH 0/4] cleanups surrounding branch

[Kaartic Sivaraam]

On the process of making 'git' give more useful error messages
when trying to rename a branch[0], I found a few things that could
be cleaned up. After noticing that the cleanup commits exceeded
the commits that are related to the series, I thought it would
be better to separate the cleanups into an independent series
to keep that topic focused on improving the error messages.

1/4 and 2/4 were part of that series until v3. The others are new
cleanups.

This series goes on top of 'master' and can be found in my fork
as branch 'work/branch-cleanups'[1].

Note: 1/4 might possibly have some conflicts with jc/branch-name-sanity

Footnotes:

[0]: cf. <20171102065407.25404-1-kaartic.sivar...@gmail.com>

[1]: https://github.com/sivaraam/git/tree/work/branch-cleanups


Kaartic Sivaraam (4):
  branch: improve documentation and naming of create_branch() parameters
  branch: group related arguments of create_branch()
  branch: update warning message shown when copying a misnamed branch
  builtin/branch: strip refs/heads/ using skip_prefix

 branch.c   |  4 ++--
 branch.h   | 10 --
 builtin/branch.c   | 20 +---
 builtin/checkout.c |  2 +-
 4 files changed, 24 insertions(+), 12 deletions(-)

-- 
2.15.0.291.g0d8980c5d

[PATCH] git-rebase: clean up dashed-usages in messages

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam 
---
 git-rebase.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 6344e8d5e..42a485aaa 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -9,7 +9,7 @@ OPTIONS_STUCKLONG=t
 OPTIONS_SPEC="\
 git rebase [-i] [options] [--exec ] [--onto ] [] 
[]
 git rebase [-i] [options] [--exec ] [--onto ] --root []
-git-rebase --continue | --abort | --skip | --edit-todo
+git rebase --continue | --abort | --skip | --edit-todo
 --
  Available options are
 v,verbose! display a diffstat of what changed upstream
@@ -216,7 +216,7 @@ run_pre_rebase_hook () {
 }
 
 test -f "$apply_dir"/applying &&
-   die "$(gettext "It looks like git-am is in progress. Cannot rebase.")"
+   die "$(gettext "It looks like you are in the middle of an am session. 
Cannot rebase.")"
 
 if test -d "$apply_dir"
 then
-- 
2.15.0.291.g0d8980c5d

Re: [PATCH v3 0/8] sequencer: don't fork git commit

[Phillip Wood]

On 18/11/17 03:57, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> Phillip Wood  writes:
>>
>>> From: Phillip Wood 
>>>
>>> I've updated these based on the feedback for v2. I've dropped the
>>> patch that stopped print_commit_summary() from dying as I think it is
>>> better to die than return an error (see the commit message of the
>>> patch that adds print_commit_summary() for the reasoning). Apart from
>>> that they're minor changes - style fixes and a reworded a commit message.
>>
>> Thanks for further polishing this topic; I found nothing in the
>> update that was questionable.  Will replace.
>>
>> With this, perhaps it is ready for 'next'?
> 
> Not really.  I needed at least this to get it even compile, which
> hints that I do not yet know what _else_ I missed by skimming this
> round of the series.

My apologies, it seems that I was half alseep yesterday morning and in
my haste to update these patches I wasn't paying proper attention to
what I was doing. For some reason after I dropped the patch that
converted print_commit_summary() to return an error rather than die I
forgot to amend the last patch to match and then compounded the mistake
by forgetting to compile and test properly before sending them. I think
they're okay now but I'll double check the changes before sending again
in case there are any other embarrassing mistakes lurking.

As the white rabbit said "The hurrier I go the behinder I get"

Phillip


>  sequencer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 37460db6b1..63cfb6ddd9 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1139,8 +1139,8 @@ static int do_commit(const char *msg_file, const char 
> *author,
>   unlink(git_path_cherry_pick_head());
>   unlink(git_path_merge_msg());
>   if (!is_rebase_i(opts))
> - res = print_commit_summary(NULL, ,
> - SUMMARY_SHOW_AUTHOR_DATE);
> + print_commit_summary(NULL, ,
> +  SUMMARY_SHOW_AUTHOR_DATE);
>   return res;
>   }
>   }
> 

[PATCH] git-send-email: honor $PATH

[Florian Klink]

This will search $PATH for a sendmail binary, instead of the (previously
fixed) list of paths.

Signed-off-by: Florian Klink 
---
 Documentation/git-send-email.txt | 5 ++---
 git-send-email.perl  | 3 ++-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index bac9014ac..b9b1f2c41 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -203,9 +203,8 @@ a password is obtained using 'git-credential'.
specify a full pathname of a sendmail-like program instead;
the program must support the `-i` option.  Default value can
be specified by the `sendemail.smtpServer` configuration
-   option; the built-in default is `/usr/sbin/sendmail` or
-   `/usr/lib/sendmail` if such program is available, or
-   `localhost` otherwise.
+   option; the built-in default is to search in $PATH if such program is
+   available, or `localhost` otherwise.
 
 --smtp-server-port=::
Specifies a port different from the default port (SMTP
diff --git a/git-send-email.perl b/git-send-email.perl
index 2208dcc21..8e357aeab 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -885,7 +885,8 @@ if (defined $initial_reply_to) {
 }
 
 if (!defined $smtp_server) {
-   foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
+   my @sendmail_paths = map {"$_/sendmail"} split /:/, $ENV{PATH};
+   foreach (@sendmail_paths) {
if (-x $_) {
$smtp_server = $_;
last;
-- 
2.15.0

Re: Is it not bug git stash -- does not work at non-root directory?

[Junio C Hamano]

小川恭史  writes:

> I upgraded the version of git from 2.13.1 to 2.15.0 on Mac and fixed my issue.
> Thanks.

Ah, yes, that bug was fixed in the 2.14.0 timeframe but was
backported to 2.13.2 and onwards (it was a bug in 2.13.0, I think).

Re: [PATCH v3 0/8] sequencer: don't fork git commit

[Phillip Wood]

On 18/11/17 03:57, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> Phillip Wood  writes:
>>
>>> From: Phillip Wood 
>>>
>>> I've updated these based on the feedback for v2. I've dropped the
>>> patch that stopped print_commit_summary() from dying as I think it is
>>> better to die than return an error (see the commit message of the
>>> patch that adds print_commit_summary() for the reasoning). Apart from
>>> that they're minor changes - style fixes and a reworded a commit message.
>>
>> Thanks for further polishing this topic; I found nothing in the
>> update that was questionable.  Will replace.
>>
>> With this, perhaps it is ready for 'next'?
> 
> Not really.  I needed at least this to get it even compile, which
> hints that I do not yet know what _else_ I missed by skimming this
> round of the series.

Sorry I'm not sure what happened there, by branch has the missing 'res =
' something must have happened to the patches. I'll sort it out and resend

Phillip

>  sequencer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 37460db6b1..63cfb6ddd9 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1139,8 +1139,8 @@ static int do_commit(const char *msg_file, const char 
> *author,
>   unlink(git_path_cherry_pick_head());
>   unlink(git_path_merge_msg());
>   if (!is_rebase_i(opts))
> - res = print_commit_summary(NULL, ,
> - SUMMARY_SHOW_AUTHOR_DATE);
> + print_commit_summary(NULL, ,
> +  SUMMARY_SHOW_AUTHOR_DATE);
>   return res;
>   }
>   }
> 

Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern

[Phillip Wood]

On 17/11/17 22:06, Jeff King wrote:
> On Wed, Nov 15, 2017 at 12:40:43PM +, Phillip Wood wrote:
> 
>> From: Phillip Wood 
>>
>> As explained in commit 06f46f237 (avoid "write_in_full(fd, buf, len)
>> != len" pattern, 2017–09–13) the return value of write_in_full() is
>> either -1 or the requested number of bytes. As such comparing the
>> return value to an unsigned value such as strbuf.len will fail to
>> catch errors. Change the code to use the preferred '< 0' check.
> 
> Thanks for catching this. I wondered at first how I missed these obvious
> cases, but the answer is that they were added after my commit. :)
> 
> There's one more case in write_section() that uses "==". That's not
> actually wrong, but I wonder if we'd want to make it "< 0" for
> consistency.

Yes, I noticed that but didn't get round to looking at it properly the
other day. Rene's fix looks good to me.

Best Wishes

Phillip

Re: [PATCH] config: avoid "write_in_full(fd, buf, len) != len" pattern

[René Scharfe]

Am 17.11.2017 um 23:06 schrieb Jeff King:
> There's one more case in write_section() that uses "==". That's not
> actually wrong, but I wonder if we'd want to make it "< 0" for
> consistency.

Actually it *is* wrong.

-- >8 --
Subject: [PATCH] config: flip return value of write_section()

d9bd4cbb9cc (config: flip return value of store_write_*()) made
write_section() follow the convention of write(2) to return -1 on error
and the number of written bytes on success.  3b48045c6c7 (Merge branch
'sd/branch-copy') changed it back to returning 0 on error and 1 on
success, but left its callers still checking for negative values.

Let write_section() follow the convention of write(2) again to meet the
expectations of its callers.

Reported-by: Jeff King 
Signed-off-by: Rene Scharfe 
---
 config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.c b/config.c
index 903abf9533..3f079c77ad 100644
--- a/config.c
+++ b/config.c
@@ -2315,7 +2315,7 @@ static ssize_t write_section(int fd, const char *key)
struct strbuf sb = store_create_section(key);
ssize_t ret;
 
-   ret = write_in_full(fd, sb.buf, sb.len) == sb.len;
+   ret = write_in_full(fd, sb.buf, sb.len);
strbuf_release();
 
return ret;
-- 
2.15.0

Re: Is it not bug git stash -- does not work at non-root directory?

[小川恭史]

I upgraded the version of git from 2.13.1 to 2.15.0 on Mac and fixed my issue.
Thanks.

2017-11-18 16:56 GMT+09:00 Junio C Hamano :
> 小川恭史  writes:
>
>>> Please make it a habit (not limited to when interacting with
>>> _this_ project) to state a bit more than "does not work";
>>> instead, say "it is expected to do X, but instead it does Y, and
>>> the difference between X and Y I perceive is Z".
>>
>> Thanks. I'll rewrite the issue.
>>
>> Assuming that we have sub/something and something is not included anywhere 
>> else,
>>
>> cd sub && git stash -- something
>>
>>  is expected to make a stash for sub/something but instead returns error like
>>
>> error: pathspec 'something' did not match any file(s) known to git.
>> Did you forget to 'git add'?
>>
>> .
>>
>> I don't know what I should write about 'the difference between X and Y is Z'.
>
> If the difference between X and Y is obvious there is no need.
>
> I just tried it and I do not see the command is broken in the way
> you describe.
>
> Trial #1 -- the command fully spelled out.
>
> $ git.git/master: cd Documentation
> $ Documentation/master: echo >>Makefile
> $ Documentation/master: git stash push -m "doc-make" -- Makefile
> Saved working directory and index state On master: doc-make
> $ Documentation/master: git stash show --stat
>  Documentation/Makefile | 1 +
>  1 file changed, 1 insertion(+:
>
> Trial #2 -- lazily issue the command without subcommand.
>
> $ git.git/master: cd Documentation
> $ Documentation/master: echo >>Makefile
> $ Documentation/master: git stash -- Makefile
> Saved working directory and index state WIP on master: 89ea799ffc Sync 
> with maint
> $ Documentation/master: git stash show --stat
>  Documentation/Makefile | 1 +
>  1 file changed, 1 insertion(+:
>
> Trial #3 -- make sure having files with the same name is not hiding any bug.
>
> $ git.git/master: cd Documentation
> $ Documentation/master: echo >>CodingGuidelines
> $ Documentation/master: git stash -- CodingGuidelines
> Saved working directory and index state WIP on master: 89ea799ffc
> $ Documentation/master: git stash show --stat
>  Documentation/CodingGuidelines | 1 +
>   1 file changed, 1 insertion(+)
>
> Trial #4 -- simulate a PEBKAC
>
> $ git.git/master: cd Documentation
> $ Documentation/master: echo >>no-such-file
> $ Documentation/master: git stash -- no-such-file
> error: pathspec 'Documentation/no-such-file' did not match any file(s) 
> known to git.
> Did you forget to 'git add'?
>
> The last one is an expected result---the pathspec given to the
> command does not match anything tracked, so without first adding the
> file, there is nothing for the command to do.
>

Re: Improved error handling (Was: [PATCH 1/2] sequencer: factor out rewrite_file())

[Johannes Sixt]

Am 17.11.2017 um 23:33 schrieb Jeff King:

On Mon, Nov 06, 2017 at 05:13:15PM +0100, Simon Ruderich wrote:

On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote:

Yes, I think what you've written here (and below) is quite close to the
error_context patches I linked elsewhere in the thread. In other
words, I think it's a sane approach.


In contrast to error_context I'd like to keep all exiting
behavior (die, ignore, etc.) in the hand of the caller and not
use any callbacks as that makes the control flow much harder to
follow.


Yeah, I have mixed feelings on that. I think it does make the control
flow less clear. At the same time, what I found was that handlers like
die/ignore/warn were the thing that gave the most reduction in
complexity in the callers.


Would you not consider switching over to C++? With exceptions, you get 
the error context without cluttering the API. (Did I mention that 
librarification would become a breeze? Do not die in library routines: 
not a problem anymore, just catch the exception. die_on_error 
parameters? Not needed anymore. Not to mention that resource leaks would 
be much, MUCH simpler to treat.)


-- Hannes

Hello Beautiful,

[jack]

Good day dear, i hope this mail meets you well? my name is Jack, from the U.S. 
I know this may seem inappropriate so i ask for your forgiveness but i wish to 
get to know you better, if I may be so bold. I consider myself an easy-going 
man, adventurous, honest and fun loving person but I am currently looking for a 
relationship in which I will feel loved. I promise to answer any question that 
you may want to ask me...all i need is just your attention and the chance to 
know you more.

Please tell me more about yourself, if you do not mind. Hope to hear back from 
you soon.

Jack.

hi

[Mrs. Nicole Maoris]

--
I am Mrs Nicole  i have a pending project of fulfillment to put in your 
hand, i will need your support to make this ream come through, could 
you le me know your interest to enable me give you further information, 
and I hereby advice that you send the below mentioned information


I decided to will/donate the sum of  2.7 million Euro to you for the 
good work of god, and also to help the motherless and less privilege 
and also forth assistance of the widows. At the moment I cannot take 
any telephone calls right now due to the fact that my relatives (that 
have squandered the funds agave them for this purpose before) are 
around me and my health status also. I have adjusted my will and my 
lawyer is aware.

I have willed those properties to you by quoting my personal file 
routing and account information. And I have also notified the bank that 
I am willing that properties to you for a good, effective and prudent 
work. I know I don't know you but I have been directed to do this by 
god.ok Please contact this woman for more details you might not get me 
on line in time contact this

Your full name..
Your private telephone number..
Your passport or identity card
Your country... ...
Your occupation..
Thank you as i wait your reply.

Yoursfaithful friand,

Mrs. Nicole Maoris
--