Git Gui 0.20 messes up UTF-8 author names when amending a commit

[Roessner Silvester SGD AS-ER/QMM1 *]

Hi,

first I want to thank all involved in developing git.
It is really one of the - no! - it is the greatest tool I ever worked with!

So I want to help improving it at least with this bug report.

Since git version 2.9.0 git git-gui messes up my author name
- which contains UTF-8 characters: Silvester Rößner -
when I amend a commit.

When I do the same with git-bash it's all ok.

git log after first amend:

$ git log
commit 8ac8115c3af04bd8f5171452141398b0cfb57f87
Author: Silvester Rö�ner 
Date:   Wed Jun 29 07:31:46 2016 +0200

asdf

commit ae34f23a9a3d7f673ad3ed57dbea5af302d4f0c1
Author: Silvester Rößner 
Date:   Wed Jun 29 07:31:21 2016 +0200

test


git log after second amend:

$ git log
commit 9240372e8e7336a2c523f27a75b3ac4e07315039
Author: Silvester R�¶�?ner 
Date:   Wed Jun 29 07:31:46 2016 +0200

asdf

commit ae34f23a9a3d7f673ad3ed57dbea5af302d4f0c1
Author: Silvester Rößner 
Date:   Wed Jun 29 07:31:21 2016 +0200

test


To reproduce the bug

1. create an empty repo,
2. set up the author name to "Silvester Rößner"
3. commit a file with git gui
4. amend the commit with git gui

I use this version

git-gui version 0.20.GITGUI
git version 2.9.0.windows.1

Tcl/Tk version 8.6.5

The versions I used before just work great!

Mit freundlichen Grüßen / Best regards

Silvester Rößner

Automotive Steering, Electric Power Steering Rack, Quality Processes & 
Engineering (AS-ER/QMM1)
Robert Bosch Automotive Steering GmbH | 73527 Schwäbisch Gmünd | GERMANY | 
www.bosch-automotive-steering.com
Tel. +49 7171 31-4120 | Fax +49 7171 31-64120 | silvester.roess...@bosch.com

--
Robert Bosch Automotive Steering GmbH, Richard-Bullinger-Str. 77, 73527 
Schwäbisch Gmünd, Germany
Vorsitzender des Aufsichtsrats/Chairman of the Supervisory Board: Dirk Hoheisel
Geschäftsführung/Management Board: Christian Sobottka (Vorsitz/Chairman), Hanns 
Bernd Ketteler, Marcus Parche, Henning Wagner
Sitz der Gesellschaft/Headquarters: Schwäbisch Gmünd - Handelsregistereintrag: 
Amtsgericht Ulm HRB 701678/Trade register of the municipal court of Ulm HRB 
701678
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: preview: What's cooking in git.git (Jun 2016, #10; Tue, 28)

[Johannes Sixt]

Am 29.06.2016 um 03:43 schrieb Jeff King:

Another is to just put the posix/ksh schemes into the helper function,
and let Windows people sort it out later if they want to.


Let's do this.

-- Hannes

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

Re: [PATCH] builtin/worktree.c: add option for setting worktree name

[Barret Rennie]

> On Jun 27, 2016, at 5:11 PM, Eric Sunshine  wrote:
> 
> [snip]
> 
> My knee-jerk reaction is that the directory name under .git/worktrees
> is an implementation detail (and could easily have been an arbitrary
> ID, such as .git/worktrees/7ba84ec0) and rather than exposing it
> further and encouraging people to muck around in it manually, we
> should be providing higher-level solutions so that they don't have to.
> 
> Even without the higher-level solutions, it seems like "git rev-parse
> --git-dir" should satisfy your needs, and if someone enhances "git
> worktree list" to provide the additional worktree tag name (as
> envisioned all along), then you'd likewise have sufficient information
> to "fix" your worktrees.
> 
> As an example of higher-level solutions, Duy's "git worktree move"
> series[1] would, I think, be exactly what you need to avoid such
> breakage in the first place (assuming you can retrain your fingers or
> fix your scripts, if they are doing the worktree renaming).
> 
> I don't know how Duy and Junio feel about it, but my response to this
> patch and what it wants to accomplish (even with Junio's input) is
> fairly negative. I'd much rather see more missing high-level worktree
> features implemented rather than see patches further exposing
> git-worktree's internals.
> 
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/298194
Yes, it does seem like `git worktree move` will  solve all my issues and
is a much more elegant solution. Thanks for pointing it out to me and
looking at this patch.

I may write a patch to print out the worktree tag name instead and submit
that later.

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

Re: [PATCH v2 0/2] Config conditional include

[Duy Nguyen]

On Tue, Jun 28, 2016 at 10:28 PM, Jeff King  wrote:
> On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:
>
>> There's a surprise about core.ignorecase. We are matching paths, so we
>> should match case-insensitively if core.ignorecase tells us so. And it
>> gets a bit tricky if core.ignorecase is defined in the same config
>> file. I don't think we have ever told the user that keys are processed
>> from top down. We do now.
>
> Hrm. I'm not excited about introducing ordering issues into the config
> parsing. But I think it's actually even more complicated than that.
>
> core.ignorecase is generally about the working tree, not the git
> repository directory, which may reside on another filesystem entirely
> (though I would not be surprised if we've blurred that line already).
>
> I wonder if it would be that bad to just punt on the issue, and say that
> these include-match globs are always case-insensitive, or something.
> True, that does not allow one to distinguish between config for "foo"
> and "Foo" directories, but I find it unlikely anybody would ever want
> to. And if we define it that way from day one, then nobody expects it to
> work.

You already opened a path for this with your gitdir/regexp suggestion:
we could support case-sensitive match with "gitdir:" then
case-insensitive match with "gitdir/i:". Everybody is happy.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] config: add conditional include

[Duy Nguyen]

On Tue, Jun 28, 2016 at 10:49 PM, Jeff King  wrote:
>> +static int prepare_include_condition_pattern(struct strbuf *pat)
>> +{
>> + struct strbuf path = STRBUF_INIT;
>> + int prefix = 0;
>> +
>> + /* TODO: maybe support ~user/ too */
>> + if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {
>> + const char *home = getenv("HOME");
>> +
>> + if (!home)
>> + return error(_("$HOME is not defined"));
>> +
>> + strbuf_add_absolute_path(, home);
>> + strbuf_splice(pat, 0, 1, path.buf, path.len);
>> + prefix = path.len + 1 /*slash*/;
>
> Why did you drop expand_user_path() here?
>
> I guess it's to do with knowing the length of the prefix portion? I'm
> not sure I understand why the prefix is necessary. I thought the goal
> was to construct a wildmatch pattern that could be used against
> get_git_dir().

We need to make sure any '*', '?' and '[' in the $HOME (or `pwd`)
portion are not automatically promoted to wildcards. One option is
split the pattern in two parts, the prefix part is matched literally
then the rest passed to wildmatch(). The other option is escape
$HOME/`pwd`, but I think that's more complicated (or at least slower).
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > It's just that if you take the latter, then the conditional after
> > the loop exits (i.e. the last transmission was an incomplete line)
> > cannot be "is outbuf empty?", as your base state is "has PREFIX and
> > can never be empty".  I was working back from that if statement.
> 
> Let's try this again.  How does this look?

Still broken.

> In this version:
> 
>  - "outbuf" is where we keep the (possibly partial) data collected
>to be eventually shown;
> 
>  - output of pending (possibly partial) data is handled by a helper
>function drain().  It is responsible for prepending of the
>PREFIX, which is treated purely as a cosmetic thing.  It also is
>responsible for completing an incomplete line at the end of the
>transmission (e.g. flushing of the buffered input upon reception of
>the emergency exit packet).

It uses strbuf_complete_line() which destroys the intended result for 
lines that end with '\r'.

>  - locally generated errors go directly to fprintf(stderr),
>bypassing outbuf (hence drain()).
> 
>  sideband.c | 43 +--
>  1 file changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/sideband.c b/sideband.c
> index 226a8c2..6873137 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -18,6 +18,16 @@
>  #define ANSI_SUFFIX "\033[K"
>  #define DUMB_SUFFIX ""
>  
> +static void drain(struct strbuf *outbuf)
> +{
> + if (!outbuf->len)
> + return;
> + strbuf_splice(outbuf, 0, 0, PREFIX, strlen(PREFIX));
> + strbuf_complete_line(outbuf);
> + fwrite(outbuf->buf, 1, outbuf->len, stderr);
> + strbuf_reset(outbuf);
> +}
> +
>  int recv_sideband(const char *me, int in_stream, int out)
>  {
>   const char *term, *suffix;
> @@ -26,20 +36,21 @@ int recv_sideband(const char *me, int in_stream, int out)
>   const char *b, *brk;
>   int retval = 0;
>  
> - strbuf_addf(, "%s", PREFIX);
>   term = getenv("TERM");
>   if (isatty(2) && term && strcmp(term, "dumb"))
>   suffix = ANSI_SUFFIX;
>   else
>   suffix = DUMB_SUFFIX;
>  
> - while (retval == 0) {
> + while (!retval) {
>   int band, len;
>   len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
> 0);
>   if (len == 0)
>   break;
>   if (len < 1) {
> - fprintf(stderr, "%s: protocol error: no band 
> designator\n", me);
> + drain();
> + fprintf(stderr, "%s: protocol error: no band 
> designator\n",
> + me);
>   retval = SIDEBAND_PROTOCOL_ERROR;
>   break;
>   }
> @@ -48,7 +59,8 @@ int recv_sideband(const char *me, int in_stream, int out)
>   len--;
>   switch (band) {
>   case 3:
> - fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
> + drain();
> + strbuf_addf(, "%s\n", buf + 1);
>   retval = SIDEBAND_REMOTE_ERROR;
>   break;
>   case 2:
> @@ -58,13 +70,12 @@ int recv_sideband(const char *me, int in_stream, int out)
>* Append a suffix to each nonempty line to clear the
>* end of the screen line.
>*
> -  * The output is accumulated in a buffer and each line
> -  * is printed to stderr using fprintf() with a single
> -  * conversion specifier. This is a "best effort"
> -  * approach to supporting both inter-process atomicity
> -  * (single conversion specifiers are likely to end up
> -  * in single atomic write() system calls) and the ANSI
> -  * control code emulation under Windows.
> +  * The output is accumulated in a buffer and
> +  * each line is printed to stderr using
> +  * fwrite(3).  This is a "best effort"
> +  * approach to support inter-process atomicity
> +  * (single fwrite(3) call is likely to end up
> +  * in single atomic write() system calls).
>*/
>   while ((brk = strpbrk(b, "\n\r"))) {
>   int linelen = brk - b;
> @@ -75,11 +86,7 @@ int recv_sideband(const char *me, int in_stream, int out)
>   } else {
>   strbuf_addf(, "%c", *brk);
>   }
> - fprintf(stderr, "%.*s", (int)outbuf.len,
> - outbuf.buf);
> - strbuf_reset();
> - 

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Junio C Hamano  writes:

> It's just that if you take the latter, then the conditional after
> the loop exits (i.e. the last transmission was an incomplete line)
> cannot be "is outbuf empty?", as your base state is "has PREFIX and
> can never be empty".  I was working back from that if statement.

Let's try this again.  How does this look?

In this version:

 - "outbuf" is where we keep the (possibly partial) data collected
   to be eventually shown;

 - output of pending (possibly partial) data is handled by a helper
   function drain().  It is responsible for prepending of the
   PREFIX, which is treated purely as a cosmetic thing.  It also is
   responsible for completing an incomplete line at the end of the
   transmission (e.g. flushing of the buffered input upon reception of
   the emergency exit packet).

 - locally generated errors go directly to fprintf(stderr),
   bypassing outbuf (hence drain()).

 sideband.c | 43 +--
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/sideband.c b/sideband.c
index 226a8c2..6873137 100644
--- a/sideband.c
+++ b/sideband.c
@@ -18,6 +18,16 @@
 #define ANSI_SUFFIX "\033[K"
 #define DUMB_SUFFIX ""
 
+static void drain(struct strbuf *outbuf)
+{
+   if (!outbuf->len)
+   return;
+   strbuf_splice(outbuf, 0, 0, PREFIX, strlen(PREFIX));
+   strbuf_complete_line(outbuf);
+   fwrite(outbuf->buf, 1, outbuf->len, stderr);
+   strbuf_reset(outbuf);
+}
+
 int recv_sideband(const char *me, int in_stream, int out)
 {
const char *term, *suffix;
@@ -26,20 +36,21 @@ int recv_sideband(const char *me, int in_stream, int out)
const char *b, *brk;
int retval = 0;
 
-   strbuf_addf(, "%s", PREFIX);
term = getenv("TERM");
if (isatty(2) && term && strcmp(term, "dumb"))
suffix = ANSI_SUFFIX;
else
suffix = DUMB_SUFFIX;
 
-   while (retval == 0) {
+   while (!retval) {
int band, len;
len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
0);
if (len == 0)
break;
if (len < 1) {
-   fprintf(stderr, "%s: protocol error: no band 
designator\n", me);
+   drain();
+   fprintf(stderr, "%s: protocol error: no band 
designator\n",
+   me);
retval = SIDEBAND_PROTOCOL_ERROR;
break;
}
@@ -48,7 +59,8 @@ int recv_sideband(const char *me, int in_stream, int out)
len--;
switch (band) {
case 3:
-   fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
+   drain();
+   strbuf_addf(, "%s\n", buf + 1);
retval = SIDEBAND_REMOTE_ERROR;
break;
case 2:
@@ -58,13 +70,12 @@ int recv_sideband(const char *me, int in_stream, int out)
 * Append a suffix to each nonempty line to clear the
 * end of the screen line.
 *
-* The output is accumulated in a buffer and each line
-* is printed to stderr using fprintf() with a single
-* conversion specifier. This is a "best effort"
-* approach to supporting both inter-process atomicity
-* (single conversion specifiers are likely to end up
-* in single atomic write() system calls) and the ANSI
-* control code emulation under Windows.
+* The output is accumulated in a buffer and
+* each line is printed to stderr using
+* fwrite(3).  This is a "best effort"
+* approach to support inter-process atomicity
+* (single fwrite(3) call is likely to end up
+* in single atomic write() system calls).
 */
while ((brk = strpbrk(b, "\n\r"))) {
int linelen = brk - b;
@@ -75,11 +86,7 @@ int recv_sideband(const char *me, int in_stream, int out)
} else {
strbuf_addf(, "%c", *brk);
}
-   fprintf(stderr, "%.*s", (int)outbuf.len,
-   outbuf.buf);
-   strbuf_reset();
-   strbuf_addf(, "%s", PREFIX);
-
+   drain();
b = brk + 1;
}
 
@@ -90,6 +97,7 @@ int recv_sideband(const char *me, int in_stream, int out)

Re: [PATCH v4] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Nicolas Pitre  writes:
> 
> >> The basic structure of the code (without the "SQUASH" we discussed)
> >> looks like this:
> >> 
> >>strbuf_addf(, "%s", PREFIX);
> >>while (retval == 0) {
> >>len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
> >> 0);
> >>...
> >>band = buf[0] & 0xff;
> >>switch (band) {
> >>case 3:
> >>... /* emergency exit */
> >>case 2:
> >>while ((brk = strpbrk(b, "\n\r"))) {
> >>int linelen = brk - b;
> >> 
> >>if (linelen > 0) {
> >>strbuf_addf(, "%.*s%s%c",
> >>linelen, b, suffix, *brk);
> >>} else {
> >>strbuf_addf(, "%c", *brk);
> >>}
> >>fprintf(stderr, "%.*s", (int)outbuf.len,
> >>outbuf.buf);
> >>strbuf_reset();
> >>strbuf_addf(, "%s", PREFIX);
> >>b = brk + 1;
> >>}
> >>if (*b)
> >>strbuf_addf(, "%s", b);
> >>break;
> >>...
> >>}
> >>}
> >> 
> >>if (outbuf.len > 0)
> >>fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
> >>  ...
> > That won't work. If at this point there is the beginning of a partial 
> > line queued in the buffer from the previous round waiting to see its 
> > line break, you just deleted the beginning of that line.
> 
> Ahh, OK, a single logical line split into two and sent in two
> packets--the first one would not result in any output (just does "if
> (*b) strbuf_addf(...)" to buffer it) and then the second one finally
> finds a LF.  Yeah, that won't work if we cleared.
> 
> But then my observation still holds, no?

I think it does... but I'm no longer sure of what you meant.

To make it clearer, here's a patch on top of pu that fixes all the 
issues I think are remaining. All tests pass now.

diff --git a/sideband.c b/sideband.c
index 36a032f..0e6c6df 100644
--- a/sideband.c
+++ b/sideband.c
@@ -23,10 +23,8 @@ int recv_sideband(const char *me, int in_stream, int out)
const char *term, *suffix;
char buf[LARGE_PACKET_MAX + 1];
struct strbuf outbuf = STRBUF_INIT;
-   const char *b, *brk;
int retval = 0;
 
-   strbuf_addf(, "%s", PREFIX);
term = getenv("TERM");
if (isatty(2) && term && strcmp(term, "dumb"))
suffix = ANSI_SUFFIX;
@@ -34,14 +32,15 @@ int recv_sideband(const char *me, int in_stream, int out)
suffix = DUMB_SUFFIX;
 
while (!retval) {
+   const char *b, *brk;
int band, len;
len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
0);
if (len == 0)
break;
if (len < 1) {
strbuf_addf(,
-   "\n%s: protocol error: no band 
designator\n",
-   me);
+   "%s%s: protocol error: no band designator",
+   outbuf.len ? "\n" : "", me);
retval = SIDEBAND_PROTOCOL_ERROR;
break;
}
@@ -50,7 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out)
len--;
switch (band) {
case 3:
-   strbuf_addf(, "\n%s%s\n", PREFIX, buf + 1);
+   strbuf_addf(, "%s%s%s", outbuf.len ? "\n" : "",
+   PREFIX, buf + 1);
retval = SIDEBAND_REMOTE_ERROR;
break;
case 2:
@@ -70,6 +70,8 @@ int recv_sideband(const char *me, int in_stream, int out)
while ((brk = strpbrk(b, "\n\r"))) {
int linelen = brk - b;
 
+   if (!outbuf.len)
+   strbuf_addf(, "%s", PREFIX);
if (linelen > 0) {
strbuf_addf(, "%.*s%s%c",
linelen, b, suffix, *brk);
@@ -78,27 +80,29 @@ int recv_sideband(const char *me, int in_stream, int out)
}
fwrite(outbuf.buf, 1, outbuf.len, stderr);
strbuf_reset();
-   strbuf_addf(, "%s", PREFIX);
 
b = brk + 1;
}
 
if (*b)
-  

Re: preview: What's cooking in git.git (Jun 2016, #10; Tue, 28)

[Junio C Hamano]

Jeff King  writes:

> Johannes pointed out that matching "3" for Windows is overly broad (it
> only happens due to raise(), not regular signal death).
>
> What do we want to do there?
>
> I _think_ matching "3" just makes us slightly less careful, and will not
> cause false positives (it may cause false negatives in very specific
> cases, though). So one option is to proceed as-is.
>
> Another is to just put the posix/ksh schemes into the helper function,
> and let Windows people sort it out later if they want to.

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

Re: preview: What's cooking in git.git (Jun 2016, #10; Tue, 28)

[Jeff King]

On Tue, Jun 28, 2016 at 04:36:25PM -0700, Junio C Hamano wrote:

> * jk/test-match-signal (2016-06-24) 4 commits
>  - t/lib-git-daemon: use test_match_signal
>  - test_must_fail: use test_match_signal
>  - t0005: use test_match_signal as appropriate
>  - tests: factor portable signal check out of t0005
> 
>  The test framework learned a new helper test_match_signal to check
>  an exit code from getting killed by an expected signal.
> 
>  Will merge to 'next'.

Johannes pointed out that matching "3" for Windows is overly broad (it
only happens due to raise(), not regular signal death).

What do we want to do there?

I _think_ matching "3" just makes us slightly less careful, and will not
cause false positives (it may cause false negatives in very specific
cases, though). So one option is to proceed as-is.

Another is to just put the posix/ksh schemes into the helper function,
and let Windows people sort it out later if they want to.

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

preview: What's cooking in git.git (Jun 2016, #10; Tue, 28)

[Junio C Hamano]

Quite a few topics that have been in 'pu' are now in 'next',
following yesterday's batch of topics that were merged to 'master'.

I'll do the real #10th issue tomorrow, but here is a snapshot for
today.

--
[New Topics]

* cc/apply-am (2016-06-28) 41 commits
 - apply: use error_errno() where possible
 - builtin/am: use apply api in run_apply()
 - apply: change error_routine when be_silent is set
 - usage: add get_error_routine() and get_warn_routine()
 - usage: add set_warn_routine()
 - apply: don't print on stdout when be_silent is set
 - apply: make 'be_silent' incompatible with 'apply_verbosely'
 - apply: add 'be_silent' variable to 'struct apply_state'
 - write_or_die: use warning() instead of fprintf(stderr, ...)
 - environment: add set_index_file()
 - apply: make some parsing functions static again
 - apply: move libified code from builtin/apply.c to apply.{c,h}
 - apply: rename and move opt constants to apply.h
 - builtin/apply: rename option parsing functions
 - builtin/apply: make create_one_file() return -1 on error
 - builtin/apply: make try_create_file() return -1 on error
 - builtin/apply: make write_out_results() return -1 on error
 - builtin/apply: make write_out_one_result() return -1 on error
 - builtin/apply: make create_file() return -1 on error
 - builtin/apply: make add_index_file() return -1 on error
 - builtin/apply: make add_conflicted_stages_file() return -1 on error
 - builtin/apply: make remove_file() return -1 on error
 - builtin/apply: make build_fake_ancestor() return -1 on error
 - builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
 - builtin/apply: make gitdiff_*() return -1 on error
 - builtin/apply: make gitdiff_*() return 1 at end of header
 - builtin/apply: make parse_traditional_patch() return -1 on error
 - builtin/apply: make apply_all_patches() return 128 or 1 on error
 - builtin/apply: move check_apply_state() to apply.c
 - builtin/apply: make check_apply_state() return -1 instead of die()ing
 - apply: make init_apply_state() return -1 instead of exit()ing
 - builtin/apply: move init_apply_state() to apply.c
 - builtin/apply: make parse_ignorewhitespace_option() return -1 instead of 
die()ing
 - builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
 - builtin/apply: make parse_single_patch() return -1 on error
 - builtin/apply: make parse_chunk() return a negative integer on error
 - builtin/apply: make find_header() return -128 instead of die()ing
 - builtin/apply: read_patch_file() return -1 instead of die()ing
 - builtin/apply: make apply_patch() return -1 or -128 instead of die()ing
 - apply: move 'struct apply_state' to apply.h
 - apply: make some names more specific

 "git am" has been taught to make an internal call to "git apply"'s
 innards without spawning the latter as a separate process.

 Needs review.


* dp/autoconf-curl-ssl (2016-06-28) 1 commit
 - ./configure.ac: detect SSL in libcurl using curl-config

 The ./configure script generated from configure.ac was taught how
 to detect support of SSL by libcurl better.

 Needs review.


* js/color-on-windows-comment (2016-06-28) 1 commit
  (merged to 'next' on 2016-06-28 at 38a2ea1)
 + color.h: remove obsolete comment about limitations on Windows

 For a long time, we carried an in-code comment that said our
 colored output would work only when we use fprintf/fputs on
 Windows, which no longer is the case for the past few years.

 Will merge to 'master'.

--
[Stalled]

* jh/clean-smudge-annex (2016-06-22) 10 commits
 - SQUASH???
 - use smudgeToFile filter in recursive merge
 - use smudgeToFile filter in git am
 - better recovery from failure of smudgeToFile filter
 - warn on unusable smudgeToFile/cleanFromFile config
 - use smudgeToFile in git checkout etc
 - use cleanFromFile in git add
 - add smudgeToFile and cleanFromFile filter configs
 - clarify %f documentation
 - Merge branch 'tb/convert-peek-in-index' into jh/clean-smudge-annex
 (this branch uses tb/convert-peek-in-index.)

 The interface to "clean/smudge" filters require Git to feed the
 whole contents via pipe, which is suboptimal for some
 applications.  "cleanFromFile/smudgeToFile" commands are the moral
 equilvalents for these filters but they interact with the files on
 the filesystem directly.


* dk/blame-move-no-reason-for-1-line-context (2016-05-29) 1 commit
 - blame: require 0 context lines while finding moved lines with -M

 "git blame -M" missed a single line that was moved within the file.

 We may want to squash a test or two to this commit.  Volunteers?


* tb/convert-peek-in-index (2016-06-07) 3 commits
 - correct ce_compare_data() in a middle of a merge
 - read-cache: factor out get_sha1_from_index() helper
 - convert: unify the "auto" handling of CRLF
 (this branch is used by jh/clean-smudge-annex.)

 Needs review.


* sb/bisect (2016-04-15) 22 commits
 - SQUASH???
 - bisect: get back halfway 

Re: [PATCH v2 2/2] config: add conditional include

[Eric Sunshine]

On Tue, Jun 28, 2016 at 1:26 PM, Nguyễn Thái Ngọc Duy  wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/config.c b/config.c
> @@ -140,9 +141,89 @@ static int handle_path_include(const char *path, struct 
> config_include_data *inc
> +static int include_condition_is_true(const char *cond, int cond_len)
> +{
> +   const char *value;
> +   size_t value_len;
> +
> +   /* no condition (i.e., "include.path") is always true */
> +   if (!cond)
> +   return 1;
> +
> +   if (skip_prefix_mem(cond, cond_len, "gitdir:", , _len)) {
> +   struct strbuf text = STRBUF_INIT;
> +   struct strbuf pattern = STRBUF_INIT;
> +   int ret, prefix;
> +
> +   strbuf_add_absolute_path(, get_git_dir());
> +   strbuf_add(, value, value_len);
> +   prefix = prepare_include_condition_pattern();
> +
> +   if (prefix < 0)
> +   return 0;
> +
> +   if (prefix > 0 &&
> +   (text.len < prefix ||
> +fspathncmp(pattern.buf, text.buf, prefix)))
> +   return 0;

Are the above two "return"s leaking 'text' and 'pattern' strbufs?

> +
> +   ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,
> +ignore_case ? WM_CASEFOLD : 0,
> +NULL);
> +   strbuf_release();
> +   strbuf_release();
> +   return ret;
> +   }
> +
> +   error(_("unrecognized include condition: %.*s"), cond_len, cond);
> +   /* unknown conditionals are always false */
> +   return 0;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Junio C Hamano  writes:

> But then my observation still holds, no?
>
> ...
> if (outbuf.len) {
>   /* we still have something to say */
> strbuf_splice(, 0, 0, PREFIX,
>   strlen(PREFIX));
> fwrite(...);
>   }

I guess these two are more or less equivalent.

I view "outbuf" more as "holdbuf", i.e. the payload that has been
received and yet to be shown, and from that point of view, having
and keeping PREFIX which is not something we received in there while
looping to grab more input makes me feel dirty.  IOW, I consider
"empty" is the correct base state of the hold buffer before anything
happens.

I understand that you view "outbuf" as what has been prepared to be
shown but not ready to be shown due to lack of LF, and from that
point of view, the base state of the out buffer before anything
happens could be "it has PREFIX and nothing else".

It's just that if you take the latter, then the conditional after
the loop exits (i.e. the last transmission was an incomplete line)
cannot be "is outbuf empty?", as your base state is "has PREFIX and
can never be empty".  I was working back from that if statement.


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

Re: What's happening to the index

[Andy Falanga (afalanga)]

On 06/28/2016 09:17 AM, Matthieu Moy wrote:
> "Andy Falanga (afalanga)"  writes:
>
>> After the line calling increlnum is executed, I often have issues with
>> make unable to spawn the next command because it can't read the current
>> directory info.
> This may happen if you delete the current directory, even if your
> re-create it afterwards. For example:
>
> /tmp/test$ rm -fr /tmp/test && mkdir /tmp/test
> /tmp/test$ touch foo
> touch: cannot touch ‘foo’: No such file or directory
> /tmp/test$ cd /tmp/test
> /tmp/test$ touch foo
> /tmp/test$
>
> This is unrelated from Git, but maybe you asked Git to delete a
> directory (by switching to a branch which doesn't contain a directory
> for example).
>
>> If I do: cd .. && cd -; all is well.
> This is a typical symptom of the issue above.
>
Thank you for the insight: very interesting.  After asking another 
colleague how he solved this issue, I've re-written my increlnum script 
to, instead of working within my working tree, clone a temporary of this 
one branch only.  Then, it increments the number and pushes back to the 
origin.  Once completed, the temporary clone is deleted.

The strange thing now is, after the script exits, I then call "git 
fetch" in the recipe.  I can see from the output of make that the remote 
db is fetched.  However, when I call "git show 
origin/rpm:path/to/rpm_build_num" from the makefile I get the *previous* 
number.  Yet, as soon as the make process exits, I call "git show 
origin/rpm:path/to/rpm_build_num" and it shows the correct number!  What 
gives?  Is there some sort of strange file caching that happening when 
make starts that, although the local db is updated, I don't get what I'm 
after?

Andy

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Nicolas Pitre  writes:

>> The basic structure of the code (without the "SQUASH" we discussed)
>> looks like this:
>> 
>>  strbuf_addf(, "%s", PREFIX);
>>  while (retval == 0) {
>>  len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
>> 0);
>>  ...
>>  band = buf[0] & 0xff;
>>  switch (band) {
>>  case 3:
>>  ... /* emergency exit */
>>  case 2:
>>  while ((brk = strpbrk(b, "\n\r"))) {
>>  int linelen = brk - b;
>> 
>>  if (linelen > 0) {
>>  strbuf_addf(, "%.*s%s%c",
>>  linelen, b, suffix, *brk);
>>  } else {
>>  strbuf_addf(, "%c", *brk);
>>  }
>>  fprintf(stderr, "%.*s", (int)outbuf.len,
>>  outbuf.buf);
>>  strbuf_reset();
>>  strbuf_addf(, "%s", PREFIX);
>>  b = brk + 1;
>>  }
>>  if (*b)
>>  strbuf_addf(, "%s", b);
>>  break;
>>  ...
>>  }
>>  }
>> 
>>  if (outbuf.len > 0)
>>  fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
>>  ...
> That won't work. If at this point there is the beginning of a partial 
> line queued in the buffer from the previous round waiting to see its 
> line break, you just deleted the beginning of that line.

Ahh, OK, a single logical line split into two and sent in two
packets--the first one would not result in any output (just does "if
(*b) strbuf_addf(...)" to buffer it) and then the second one finally
finds a LF.  Yeah, that won't work if we cleared.

But then my observation still holds, no?

while () {
read();
switch () {
case 2:
while ((brk = strpbfk())) {
/* we have LF! we'll say something! */
strbuf_splice(, 0, 0,
  PREFIX, strlen(PREFIX));
strbuf_addf(, ...);
fwrite(...);
b = brk + 1;
}
if (*b)
strbuf_addstr(, b);
break;
}
}

if (outbuf.len) {
/* we still have something to say */
strbuf_splice(, 0, 0, PREFIX,
strlen(PREFIX));
fwrite(...);
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] Config conditional include

[Junio C Hamano]

Honestly, I'd really prefer to see those with topics in 'pu' that
has seen reviews but not yet updated to go back to and polish them
to help move things forward, with the goal to have them in 'next'
sooner so that we can have fixes and features that are sufficiently
vetted and tested in the next release, before starting yet another
new topic that will be left hanging in 'pu'.

In your case, I am talking about nd/icase, nd/fetch-ref-summary, and
nd/connect-ssh-command-config topics.

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

Re: [PATCH v4] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Nicolas Pitre  writes:
> 
> >> There is something else going on.  I cannot quite explain why I am
> >> getting this failure from t5401-update-hooks.sh, for example:
> >> 
> >> --- expect  2016-06-28 19:46:24.564937075 +
> >> +++ actual  2016-06-28 19:46:24.564937075 +
> >> @@ -9,3 +9,4 @@
> >>  remote: STDERR post-receive
> >>  remote: STDOUT post-update
> >>  remote: STDERR post-update
> >> +remote: To ./victim.git
> >> not ok 12 - send-pack stderr contains hook messages
> >> 
> >> ... goes and looks what v2.9.0 produces, which ends like this:
> >> 
> >> ...
> >> remote: STDERR post-receive
> >> remote: STDOUT post-update
> >> remote: STDERR post-update
> >> To ./victim.git
> >>e4822ab..2b65bd1  master -> master
> >>  ! [remote rejected] tofail -> tofail (hook declined)
> >> 
> >> The test checks if lines prefixed with "remote: " match the expected
> >> output, and the difference is an indication that the new code is
> >> showing an extra incomplete-line "remote: " before other parts of
> >> the code says "To ./victim.git" to report where the push is going.
> >
> > Ah...  I think I know what's going on.
> >
> > The leftover data in the strbuf is normally (when there is no errors) an 
> > unterminated line. So instead of doing:
> >
> > -   fprintf(stderr, "%s: protocol error: no band 
> > designator\n", me);
> > +   strbuf_addf(,
> > +   "\n%s: protocol error: no band 
> > designator\n",
> > +   me);
> >
> > you could omit the final \n in the format string and:
> >
> > -   if (outbuf.len > 0)
> > -   fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
> > +   if (outbuf.len)
> > +   fwrite(outbuf.buf, 1, outbuf.len, stderr);
> > strbuf_release();
> >
> > and here a \n could be added before writing out the buffer.
> 
> Unfortunately, that is not it.
> 
> The basic structure of the code (without the "SQUASH" we discussed)
> looks like this:
> 
>   strbuf_addf(, "%s", PREFIX);
>   while (retval == 0) {
>   len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
> 0);
>   ...
>   band = buf[0] & 0xff;
>   switch (band) {
>   case 3:
>   ... /* emergency exit */
>   case 2:
>   while ((brk = strpbrk(b, "\n\r"))) {
>   int linelen = brk - b;
> 
>   if (linelen > 0) {
>   strbuf_addf(, "%.*s%s%c",
>   linelen, b, suffix, *brk);
>   } else {
>   strbuf_addf(, "%c", *brk);
>   }
>   fprintf(stderr, "%.*s", (int)outbuf.len,
>   outbuf.buf);
>   strbuf_reset();
>   strbuf_addf(, "%s", PREFIX);
>   b = brk + 1;
>   }
>   if (*b)
>   strbuf_addf(, "%s", b);
>   break;
>   ...
>   }
>   }
> 
>   if (outbuf.len > 0)
>   fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
> 
> Imagine we are reading from band #2 and we find a complete line.  We
> concatenate the payload up to the LF at the end of the line to the
> PREFIX we prepared outside the loop and emit it, and then we ASSUME
> that we further have something after strpbrk() and add PREFIX to the
> buffer, before going to the next line in the payload.
> 
> But there may not be anything after the LF.  outbuf.len is still
> counting the PREFIX and we end up showing it, without termination.

You're right.  Although my previous observations still apply.

> This takes us back to what I said in my review of an earlier round,
> in $gmane/297332, where I said:
> 
> Instead of doing "we assume outbuf already has PREFIX when we add
> contents from buf[]", the code structure would be better if you:
> 
>  * make outbuf.buf contain PREFIX at the beginning of this innermost
>loop; lose the reset/addf from here.
> 
>  * move strbuf_reset() at the end of the next if (*b) block
>to just before "continue;"
> 
> perhaps?
> 
> I think the strbuf_addf(PREFIX) above the loop should be removed,
> and instead the code should use the PREFIX only when it decides that
> there is something worth emitting, i.e.
> 
>   while (!retval) {
>   len = packet_read();
> ...
> band = buf[0] & 0xff;
> switch (band) {
> case 3:
>   

Re: [PATCH v8 33/41] write_or_die: use warning() instead of fprintf(stderr, ...)

[Junio C Hamano]

Christian Couder  writes:

> diff --git a/write_or_die.c b/write_or_die.c
> index 49e80aa..c29f677 100644
> --- a/write_or_die.c
> +++ b/write_or_die.c
> @@ -87,8 +87,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t 
> count, const char *msg)
>  {
>   if (write_in_full(fd, buf, count) < 0) {
>   check_pipe(errno);
> - fprintf(stderr, "%s: write error (%s)\n",
> - msg, strerror(errno));
> + warning("%s: write error (%s)\n", msg, strerror(errno));
>   return 0;
>   }
>  
> @@ -98,8 +97,7 @@ int write_or_whine_pipe(int fd, const void *buf, size_t 
> count, const char *msg)
>  int write_or_whine(int fd, const void *buf, size_t count, const char *msg)
>  {
>   if (write_in_full(fd, buf, count) < 0) {
> - fprintf(stderr, "%s: write error (%s)\n",
> - msg, strerror(errno));
> + warning("%s: write error (%s)\n", msg, strerror(errno));
>   return 0;
>   }

I do not think you call write_or_whine() at all.  As another topic
in flight removes the last caller of this function, this hunk is
very much unwelcome.  The only effect of it is to force me resolve
unnecessary merge conflicts.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

subtree bug: removing and re-adding a subtree screws up the push

[Christian Zommerfelds]

Hi,

Today I realized that when you add a subtree with `git subtree add`,
then remove it with `git rm -r`, then re-add it again, pushing to the
subtree behaves weirdly: it adds the whole history of the main
repository to the sub repository.

Below is a list of commands to reproduce the issue:

```
mkdir myproj
mkdir mylib

cd mylib
touch mylib-file1
git init
git add mylib-file1
git commit -m "mylib initial commit"

cd ../myproj
touch myproj-file1
git init
git add myproj-file1
git commit -m "myproj initial commit"
git subtree add --squash -P contrib/mylib -m "added lib" ../mylib master
git rm -r contrib/mylib
git commit -m "removed lib"
git subtree add --squash -P contrib/mylib -m "re-adding lib" ../mylib master

git subtree push -P contrib/mylib ../mylib test

cd ../mylib
git checkout test
git log | cat
# expected: only "mylib initial commit"
# actual: full history of myproj
```

If you replace the last two occurrences of contrib/mylib by a
different folder such as contrib2/mylib, the push works as usual. I
suspect that subtree splits using the first commit in the subfolder in
the whole history, rather than the first commit after the folder was
last added.

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

Re: [PATCH] rebase -i: restore autostash on abort

[Junio C Hamano]

Matthieu Moy  writes:

> It is "interesting" if you mean "matches real-life use-case", as it
> corresponds to the case where the user killed the editor (as reported by
> Daniel Hahler indeed, "Abort with ":cq", which will make Vim exit
> non-zero").

Yes.  It is an interesting failure mode in that sense.  But breakage
of such a basic mode is something an end-user is likely to notice
immediately, so in that sense, having such a test alone is not all
that interesting.

> If you mean "likely to trigger nasty bugs", then indeed testing the case
> when apply_autostash fails is interesting: for example, calling
> die_abort when "stash apply" fails is tempting, but would lead to
> infinite recursion (it doesn't seem to be the case, but a test would be
> nice). Setting the editor to something that modifies uncommited-content
> before 'false' should do the trick.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Nicolas Pitre  writes:

>> There is something else going on.  I cannot quite explain why I am
>> getting this failure from t5401-update-hooks.sh, for example:
>> 
>> --- expect  2016-06-28 19:46:24.564937075 +
>> +++ actual  2016-06-28 19:46:24.564937075 +
>> @@ -9,3 +9,4 @@
>>  remote: STDERR post-receive
>>  remote: STDOUT post-update
>>  remote: STDERR post-update
>> +remote: To ./victim.git
>> not ok 12 - send-pack stderr contains hook messages
>> 
>> ... goes and looks what v2.9.0 produces, which ends like this:
>> 
>> ...
>> remote: STDERR post-receive
>> remote: STDOUT post-update
>> remote: STDERR post-update
>> To ./victim.git
>>e4822ab..2b65bd1  master -> master
>>  ! [remote rejected] tofail -> tofail (hook declined)
>> 
>> The test checks if lines prefixed with "remote: " match the expected
>> output, and the difference is an indication that the new code is
>> showing an extra incomplete-line "remote: " before other parts of
>> the code says "To ./victim.git" to report where the push is going.
>
> Ah...  I think I know what's going on.
>
> The leftover data in the strbuf is normally (when there is no errors) an 
> unterminated line. So instead of doing:
>
> -   fprintf(stderr, "%s: protocol error: no band 
> designator\n", me);
> +   strbuf_addf(,
> +   "\n%s: protocol error: no band 
> designator\n",
> +   me);
>
> you could omit the final \n in the format string and:
>
> -   if (outbuf.len > 0)
> -   fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
> +   if (outbuf.len)
> +   fwrite(outbuf.buf, 1, outbuf.len, stderr);
> strbuf_release();
>
> and here a \n could be added before writing out the buffer.

Unfortunately, that is not it.

The basic structure of the code (without the "SQUASH" we discussed)
looks like this:

strbuf_addf(, "%s", PREFIX);
while (retval == 0) {
len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
0);
...
band = buf[0] & 0xff;
switch (band) {
case 3:
... /* emergency exit */
case 2:
while ((brk = strpbrk(b, "\n\r"))) {
int linelen = brk - b;

if (linelen > 0) {
strbuf_addf(, "%.*s%s%c",
linelen, b, suffix, *brk);
} else {
strbuf_addf(, "%c", *brk);
}
fprintf(stderr, "%.*s", (int)outbuf.len,
outbuf.buf);
strbuf_reset();
strbuf_addf(, "%s", PREFIX);
b = brk + 1;
}
if (*b)
strbuf_addf(, "%s", b);
break;
...
}
}

if (outbuf.len > 0)
fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);

Imagine we are reading from band #2 and we find a complete line.  We
concatenate the payload up to the LF at the end of the line to the
PREFIX we prepared outside the loop and emit it, and then we ASSUME
that we further have something after strpbrk() and add PREFIX to the
buffer, before going to the next line in the payload.

But there may not be anything after the LF.  outbuf.len is still
counting the PREFIX and we end up showing it, without termination.

This takes us back to what I said in my review of an earlier round,
in $gmane/297332, where I said:

Instead of doing "we assume outbuf already has PREFIX when we add
contents from buf[]", the code structure would be better if you:

 * make outbuf.buf contain PREFIX at the beginning of this innermost
   loop; lose the reset/addf from here.

 * move strbuf_reset() at the end of the next if (*b) block
   to just before "continue;"

perhaps?

I think the strbuf_addf(PREFIX) above the loop should be removed,
and instead the code should use the PREFIX only when it decides that
there is something worth emitting, i.e.

while (!retval) {
len = packet_read();
...
band = buf[0] & 0xff;
switch (band) {
case 3:
... /* emergency exit */
case 2:
while ((brk = ...)) {
/* we have something to say */
strbuf_reset();
strbuf_addstr(, PREFIX);
   

Re: [PATCH v2 0/2] Config conditional include

[Jeff King]

On Tue, Jun 28, 2016 at 10:51:15PM +0200, Matthieu Moy wrote:

> Jeff King  writes:
> 
> > On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:
> >
> >> There's a surprise about core.ignorecase. We are matching paths, so we
> >> should match case-insensitively if core.ignorecase tells us so. And it
> >> gets a bit tricky if core.ignorecase is defined in the same config
> >> file. I don't think we have ever told the user that keys are processed
> >> from top down. We do now.
> >
> > Hrm. I'm not excited about introducing ordering issues into the config
> > parsing.
> 
> There's already at least one case of ordering-sensitive variables, that
> we encountered when writting the config cache during Tanay Abhra's GSoC:
> diff..funcname Vs diff..xfuncname. Git applies the "last
> one wins" policy, which is the normal rule for a single-valued variable,
> but in this case, a "funcname" definition can override an "xfuncname"
> def. To preserve this behavior we had to introduce ordering in the
> cache, but to me this was a design mistake to rely on order.
> 
> In short: we already have one, but I'm not excited either about
> introducing new ones.

I still see funcname versus xfuncname as fundamentally a "last one wins"
scenario; it's just that the two options are sort-of synonyms. But we
are still talking about the same linear-ish parsing scheme, and I think
it just makes the implementation a little more complicated.

I'm much more worried about something that impacts how we parse the
config, and is set up in a possibly unrelated config-parsing sequence.
So whether ignorecase will work depends on more variables:

 - are we doing our config parse before or after somebody has called
   git_config() at the start of a program?

 - if before (or during), does our callback call git_default_core_config()?

 - if so, did core.ignorecase appear before our include? (Almost
   certainly not, if our include is in ~/.gitconfig, because we parse
   from least-specific to most-specific).

So here it is not the implementation that is complicated, but the
user-facing behavior. It's very difficult to predict when your include
will kick in, and there is a good chance it will behave differently for
different programs.

In general I think the best bet here is to lazy-load such values from
the config-cache (so we _know_ that we got a complete parse before we
look at the value). But that creates a recursion problem when we try to
lazy-load from inside the config parser itself.

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

Re: [PATCH v4] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Nicolas Pitre  writes:
> 
> > Without this, the error and remaining buffer would be reversed as 
> > mentioned previously.  With this, the order is restored, but a newline 
> > is added to unterminated lines whereas the error was simply appended to 
> > the output before Lukas' patch.
> >
> > In any case the new behavior is probably better and I'd simply adjust 
> > the test expectations.
> 
> There is something else going on.  I cannot quite explain why I am
> getting this failure from t5401-update-hooks.sh, for example:
> 
> --- expect  2016-06-28 19:46:24.564937075 +
> +++ actual  2016-06-28 19:46:24.564937075 +
> @@ -9,3 +9,4 @@
>  remote: STDERR post-receive
>  remote: STDOUT post-update
>  remote: STDERR post-update
> +remote: To ./victim.git
> not ok 12 - send-pack stderr contains hook messages
> 
> ... goes and looks what v2.9.0 produces, which ends like this:
> 
> ...
> remote: STDERR post-receive
> remote: STDOUT post-update
> remote: STDERR post-update
> To ./victim.git
>e4822ab..2b65bd1  master -> master
>  ! [remote rejected] tofail -> tofail (hook declined)
> 
> The test checks if lines prefixed with "remote: " match the expected
> output, and the difference is an indication that the new code is
> showing an extra incomplete-line "remote: " before other parts of
> the code says "To ./victim.git" to report where the push is going.

Ah...  I think I know what's going on.

The leftover data in the strbuf is normally (when there is no errors) an 
unterminated line. So instead of doing:

-   fprintf(stderr, "%s: protocol error: no band 
designator\n", me);
+   strbuf_addf(,
+   "\n%s: protocol error: no band 
designator\n",
+   me);

you could omit the final \n in the format string and:

-   if (outbuf.len > 0)
-   fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
+   if (outbuf.len)
+   fwrite(outbuf.buf, 1, outbuf.len, stderr);
strbuf_release();

and here a \n could be added before writing out the buffer.


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

Re: [PATCH v2 0/2] Config conditional include

[Matthieu Moy]

Jeff King  writes:

> On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:
>
>> There's a surprise about core.ignorecase. We are matching paths, so we
>> should match case-insensitively if core.ignorecase tells us so. And it
>> gets a bit tricky if core.ignorecase is defined in the same config
>> file. I don't think we have ever told the user that keys are processed
>> from top down. We do now.
>
> Hrm. I'm not excited about introducing ordering issues into the config
> parsing.

There's already at least one case of ordering-sensitive variables, that
we encountered when writting the config cache during Tanay Abhra's GSoC:
diff..funcname Vs diff..xfuncname. Git applies the "last
one wins" policy, which is the normal rule for a single-valued variable,
but in this case, a "funcname" definition can override an "xfuncname"
def. To preserve this behavior we had to introduce ordering in the
cache, but to me this was a design mistake to rely on order.

In short: we already have one, but I'm not excited either about
introducing new ones.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] config: add conditional include

[Jeff King]

On Tue, Jun 28, 2016 at 07:26:41PM +0200, Nguyễn Thái Ngọc Duy wrote:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 58673cf..c8ad0bf 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -91,6 +91,46 @@ found at the location of the include directive. If the 
> value of the
>  relative to the configuration file in which the include directive was
>  found.  See below for examples.
>  
> +Included files can be grouped into subsections where subsectino name
> +is the condition when the files are included. The condition starts
> +with an condition type string, followed by a colon and a pattern.

s/subsectino/subsection/
s/an condition/a condition/

I think the first sentence may parse more easily as:

  Included files can be grouped into subsections, where the subsection
  name is a condition that must be met for the files to be included.

I wonder if it would make more sense to refer to "condition type string"
as a "condition keyword" or something.

I think we should probably also claim only that the bit to the right of
the colon is keyword-specific data. For some conditions it will not be a
glob, and may not even be a pattern at all. And then each keyword can
describe its syntax (we may end up wanting to factor out the particular
set of rules, but I think that can wait until we have a second keyword
that uses them).

> +Only "gitdir" type is supported, where files are included if
> +`$GIT_DIR` matches the specified pattern. For example,

Maybe start this as a list like:

The currently supported keywords are:

`gitdir`::
...

to make it clear that the "only" is potentially temporary. We should
probably also document that unknown keywords are always treated as
false.

> + * If the pattern starts with `~/`, `~` will be substitued with the
> +   environment variable `HOME`.

s/substitued/substituted/

> + * If the pattern starts with `./`, it is replaced with the directory
> +   where the current config file is. For example if the config file

Perhaps replace "directory where the current config file is" with
"directory containing the current config file". Also, perhaps "config
file that contains the include directive" is more descriptive than
"current" (we use similar language when describing relative include
paths).

> +static int prepare_include_condition_pattern(struct strbuf *pat)
> +{
> + struct strbuf path = STRBUF_INIT;
> + int prefix = 0;
> +
> + /* TODO: maybe support ~user/ too */
> + if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {
> + const char *home = getenv("HOME");
> +
> + if (!home)
> + return error(_("$HOME is not defined"));
> +
> + strbuf_add_absolute_path(, home);
> + strbuf_splice(pat, 0, 1, path.buf, path.len);
> + prefix = path.len + 1 /*slash*/;

Why did you drop expand_user_path() here?

I guess it's to do with knowing the length of the prefix portion? I'm
not sure I understand why the prefix is necessary. I thought the goal
was to construct a wildmatch pattern that could be used against
get_git_dir().

> +static int include_condition_is_true(const char *cond, int cond_len)
> +{
> + const char *value;
> + size_t value_len;
> +
> + /* no condition (i.e., "include.path") is always true */
> + if (!cond)
> + return 1;
> +
> + if (skip_prefix_mem(cond, cond_len, "gitdir:", , _len)) {

It's not wrong to use extra variables, but it's safe to feed the
originals, like:

  if (skip_prefix_mem(cond, cond_len, "gitdir:", , _len))

The variables are guaranteed to be untouched if we didn't match (so your
error() below is fine).

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

Re: [PATCH] rebase -i: restore autostash on abort

[Matthieu Moy]

Junio C Hamano  writes:

> Patrick Steinhardt  writes:
>
>> +test_expect_success 'restore autostash on editor failure' '
>> +test_when_finished "git reset --hard" &&
>> +echo uncommited-content >file0 &&
>> +(
>> +test_set_editor "false" &&
>> +test_must_fail git rebase -i --autostash HEAD^
>> +) &&
>> +echo uncommited-content >expected &&
>
> While making sure this case works is crucial, it is not an
> interesting failure mode, is it? Can we also have "does not apply
> cleanly anymore" case, too?

It is "interesting" if you mean "matches real-life use-case", as it
corresponds to the case where the user killed the editor (as reported by
Daniel Hahler indeed, "Abort with ":cq", which will make Vim exit
non-zero").

If you mean "likely to trigger nasty bugs", then indeed testing the case
when apply_autostash fails is interesting: for example, calling
die_abort when "stash apply" fails is tempting, but would lead to
infinite recursion (it doesn't seem to be the case, but a test would be
nice). Setting the editor to something that modifies uncommited-content
before 'false' should do the trick.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/2] Config conditional include

[Jeff King]

On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:

> There's a surprise about core.ignorecase. We are matching paths, so we
> should match case-insensitively if core.ignorecase tells us so. And it
> gets a bit tricky if core.ignorecase is defined in the same config
> file. I don't think we have ever told the user that keys are processed
> from top down. We do now.

Hrm. I'm not excited about introducing ordering issues into the config
parsing. But I think it's actually even more complicated than that.

core.ignorecase is generally about the working tree, not the git
repository directory, which may reside on another filesystem entirely
(though I would not be surprised if we've blurred that line already).

I wonder if it would be that bad to just punt on the issue, and say that
these include-match globs are always case-insensitive, or something.
True, that does not allow one to distinguish between config for "foo"
and "Foo" directories, but I find it unlikely anybody would ever want
to. And if we define it that way from day one, then nobody expects it to
work.

> It makes me wonder if we should allow users the option to match case
> insensitively regardless of filesystem too. Something close to
> pathspec magic. But that probably has little use in real world for a
> lot more work.

Yeah, if we had a builtin syntax for "add these options to the
wildmatch", it wouldn't be so bad, but I think we'd be inventing that
syntax.

> The '/' vs '\\' battle on Windows, I'll leave it to Windows guys again.

Oof. Me too. :)

> To leave room for future expansion, perhaps we should declare that ':'
> can't appear in the pattern, so we can add more prefix later, and even
> stack them, e.g. "regexp:gitdir:". The prefix can't be one char
> long. That should be enough for windows to specify full path
> including the drive letter.

It seems unnecessarily restrictive to place rules about what can go in
the pattern provided by the user, when we can easily restrict the
characters on the keyword side. For example "gitdir/regexp:" is
a natural extension of "gitdir:", and is backwards compatible
as long as we use something besides ":" for the option separator.

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

Re: Shallow submodule efficiency

[Stefan Beller]

On Tue, Jun 28, 2016 at 12:08 PM, Martin von Gagern
 wrote:
> Hi Stefan,
>
> On 28.06.2016 19:20, Stefan Beller wrote:
>>> I have the feeling that “git submodule update --depth 1” is less clever
>>> than it could be. Here is one example I observed with git 2.0.0:
>>
>> 2.9.0 (as "Direct fetching of " is not part of 2.0.0 IIRC) ?
>
> Yes, sorry. I had this tested with 2.8.3 at first, then waited for my
> update to 2.9.0 to reproduce, and garbled the text while adjusting it.
>
>> Makes sense! I think the easiest way forward to implement this will be:
>>
>> * `git clone` learns a (maybe undocumented internal) option `--get-sha1`
>>   `--branch` looks similar to what we want, but doesn't quite fit as we do 
>> not
>>   know, whether we're on a tag or not. The submodule tells us just the
>>   recorded sha1, not the branch/tag. So maybe we'd end up calling it
>>   `--detach-at=`,
>
> That name makes a lot of sense to me.
>
>>   that will
>>   -> inspect the ls-remote for the sha1 being there
>>   -> if the sha1 is there (at least once) clone as if --branch  was 
>> given
>
> Clone but detach, to be consistent. Yes.

Oh, right.

>
>>   -> if not found and the server advertised  allowReachableSHA1InWant,
>> try again inside the clone
>
> All of this has to pass through transport and get-pack, right?

Yeah we have to go through the transport layer, i.e. from builtin/clone.c we
manipulate the transport object as defined in transport.h and code in
transport.c
What do you mean by get-pack, though?

>
>> * `submodule--helper update-clone` passes the  `--get-sha1` to the
>> clones of the submodules
>>
>> * cmd_update() in git-submodule.sh will only checkout submodules and
>> not try again
>>   to fetch them if `just_cloned` is set as the cloning did the best it could.
>
> Sounds like a very reasonable roadmap to me.
>
> Do you think there will be someone volunteering to tackle this?

I have it on my (low priority) TODO list, so if you want it sooner
than later, go for
it yourself. Otherwise just wait. Maybe Duy has some interest as well.

Thanks,
Stefan

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

Re: git svn clone segmentation faul issue

[Eric Wong]

ioannis.kap...@rbs.com wrote:
> Fortunately, a patch has already been submitted to subversion
> with (github) revision
> a074af86c8764404b28ce99d0bedcb668a321408 (at
> https://github.com/apache/subversion/commit/a074af86c8764404b28ce99d0bedcb668a321408
> ) on the trunk to handle this and a couple of other similar
> cases. But by the looks of it has not been picked up yet in
> the latest subversion 1.9.4 release or the 1.9.x branch,
> perhaps because this patch was identified in sanity checks
> rather than coming out from a perceivable production issue?

Thank you for documenting this.  Curious, does this affect older
SVN versions or only 1.9.x?

I don't know Perl internals well or SWIG at all; so reports
like these are very much appreciated.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Nicolas Pitre  writes:

> Without this, the error and remaining buffer would be reversed as 
> mentioned previously.  With this, the order is restored, but a newline 
> is added to unterminated lines whereas the error was simply appended to 
> the output before Lukas' patch.
>
> In any case the new behavior is probably better and I'd simply adjust 
> the test expectations.

There is something else going on.  I cannot quite explain why I am
getting this failure from t5401-update-hooks.sh, for example:

--- expect  2016-06-28 19:46:24.564937075 +
+++ actual  2016-06-28 19:46:24.564937075 +
@@ -9,3 +9,4 @@
 remote: STDERR post-receive
 remote: STDOUT post-update
 remote: STDERR post-update
+remote: To ./victim.git
not ok 12 - send-pack stderr contains hook messages

... goes and looks what v2.9.0 produces, which ends like this:

...
remote: STDERR post-receive
remote: STDOUT post-update
remote: STDERR post-update
To ./victim.git
   e4822ab..2b65bd1  master -> master
 ! [remote rejected] tofail -> tofail (hook declined)

The test checks if lines prefixed with "remote: " match the expected
output, and the difference is an indication that the new code is
showing an extra incomplete-line "remote: " before other parts of
the code says "To ./victim.git" to report where the push is going.

It appeasrs that the "Refector"ed logic needs to be a bit more
careful when relaying an empty payload.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Shallow submodule efficiency

[Martin von Gagern]

Hi Stefan,

On 28.06.2016 19:20, Stefan Beller wrote:
>> I have the feeling that “git submodule update --depth 1” is less clever
>> than it could be. Here is one example I observed with git 2.0.0:
> 
> 2.9.0 (as "Direct fetching of " is not part of 2.0.0 IIRC) ?

Yes, sorry. I had this tested with 2.8.3 at first, then waited for my
update to 2.9.0 to reproduce, and garbled the text while adjusting it.

> Makes sense! I think the easiest way forward to implement this will be:
> 
> * `git clone` learns a (maybe undocumented internal) option `--get-sha1`
>   `--branch` looks similar to what we want, but doesn't quite fit as we do not
>   know, whether we're on a tag or not. The submodule tells us just the
>   recorded sha1, not the branch/tag. So maybe we'd end up calling it
>   `--detach-at=`,

That name makes a lot of sense to me.

>   that will
>   -> inspect the ls-remote for the sha1 being there
>   -> if the sha1 is there (at least once) clone as if --branch  was given

Clone but detach, to be consistent. Yes.

>   -> if not found and the server advertised  allowReachableSHA1InWant,
> try again inside the clone

All of this has to pass through transport and get-pack, right?

> * `submodule--helper update-clone` passes the  `--get-sha1` to the
> clones of the submodules
> 
> * cmd_update() in git-submodule.sh will only checkout submodules and
> not try again
>   to fetch them if `just_cloned` is set as the cloning did the best it could.

Sounds like a very reasonable roadmap to me.

Do you think there will be someone volunteering to tackle this?

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

Re: [PATCH] rebase -i: restore autostash on abort

[Junio C Hamano]

Patrick Steinhardt  writes:

> When we abort an interactive rebase we do so by calling
> `die_abort`, which cleans up after us by removing the rebase
> state directory. If the user has requested to use the autostash
> feature, though, the state directory may also contain a reference
> to the autostash, which will now be deleted.
>
> Fix the issue by trying to re-apply the autostash in `die_abort`.
> This will also handle the case where the autostash does not apply
> cleanly anymore by recording it in a user-visible stash.

I do not do autostash myself, but it is a good thing to try not to
lose information ;-)

> +test_expect_success 'restore autostash on editor failure' '
> + test_when_finished "git reset --hard" &&
> + echo uncommited-content >file0 &&
> + (
> + test_set_editor "false" &&
> + test_must_fail git rebase -i --autostash HEAD^
> + ) &&
> + echo uncommited-content >expected &&

While making sure this case works is crucial, it is not an
interesting failure mode, is it?  Can we also have "does not apply
cleanly anymore" case, too?


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

Re: [PATCH] connect: read $GIT_SSH_COMMAND from config file

[Eric Sunshine]

On Tue, Jun 28, 2016 at 1:58 PM, Junio C Hamano  wrote:
> Let's then squash this in.
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -444,11 +444,11 @@ This is useful for excluding servers inside a firewall 
> from
>  core.sshCommand::
> -   If this variable is set then 'git fetch' and 'git push' will
> -   use the specified command instead of 'ssh' when they need to
> +   If this variable is set, `git fetch` and `git push` will
> +   use the specified command instead of `ssh` when they need to
> connect to a remote system. The command is in the same form as
> -   'GIT_SSH_COMMAND' environment variable and is overriden when
> -   the environment variable is set.
> +   the `GIT_SSH_COMMAND` environment variable and is overriden

s/overriden/overridden/

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

Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp

[Junio C Hamano]

Johannes Sixt  writes:

> Am 27.06.2016 um 09:26 schrieb larsxschnei...@gmail.com:
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap):
>>   self.useClientSpec_from_options = False
>>   self.clientSpecDirs = None
>>   self.tempBranches = []
>> -self.tempBranchLocation = "git-p4-tmp"
>> +self.tempBranchLocation = "refs/git-p4-tmp"
>>   self.largeFileSystem = None
>>
>>   if gitConfig('git-p4.largeFileSystem'):
>> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
>> index 0aafd03..8f28ed2 100755
>> --- a/t/t9801-git-p4-branch.sh
>> +++ b/t/t9801-git-p4-branch.sh
>> @@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' '
>>  test_path_is_file file2 &&
>>  test_path_is_file file3 &&
>>  ! grep update file2 &&
>> -test_path_is_missing .git/git-p4-tmp
>> +test_path_is_missing .git/ref/git-p4-tmp
>
> This should be .git/refs/git-p4-tmp, no? Otherwise, this does not test
> what it should test.

Yes, and it probably should use "git show-ref --verify" to
future-proof, instead of assuming the file-based ref backend.

>
>>  )
>>   '
>>
>> @@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches 
>> in the same changelist'
>>  test_path_is_file file2 &&
>>  test_path_is_file file3 &&
>>  ! grep update file2 &&
>> -test_path_is_missing .git/git-p4-tmp
>> +test_path_is_missing .git/ref/git-p4-tmp
>
> Same here.
>
>>  )
>>   '
>
> -- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Nicolas Pitre  writes:
> 
> >> When we exit the loop because we set retval to a non-zero value,
> >> should we still drain the outbuf?
> >
> > I would think so.  Anything that the remote sent before any error should 
> > be printed nevertheless.  The clue for the error might be in the pending 
> > buffer.
> >
> > However in this case the actual error printout and the pending buffer 
> > will appear reversed.
> >
> > So what I'd suggest is actually something like this:
> >
> > if (len < 1) {
> > strbuf_addf(, "\n%s: protocol error: no band 
> > designator\n", me);
> > retval = SIDEBAND_PROTOCOL_ERROR;
> > break;
> >
> > And so on for the other error cases.
> 
> Makes sense.
> 
> Here is what I have as a "SQUASH" on top of Lukas's change to be
> queued on 'pu'.

Looks good to me.

Acked-by: Nicolas Pitre 

> It appears that a few tests get their expectations broken, with or
> without this "SQUASH" change, though X-<.

Without this, the error and remaining buffer would be reversed as 
mentioned previously.  With this, the order is restored, but a newline 
is added to unterminated lines whereas the error was simply appended to 
the output before Lukas' patch.

In any case the new behavior is probably better and I'd simply adjust 
the test expectations.


> 
>  sideband.c | 30 +++---
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/sideband.c b/sideband.c
> index 226a8c2..082dfc6 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -33,13 +33,15 @@ int recv_sideband(const char *me, int in_stream, int out)
>   else
>   suffix = DUMB_SUFFIX;
>  
> - while (retval == 0) {
> + while (!retval) {
>   int band, len;
>   len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
> 0);
>   if (len == 0)
>   break;
>   if (len < 1) {
> - fprintf(stderr, "%s: protocol error: no band 
> designator\n", me);
> + strbuf_addf(,
> + "\n%s: protocol error: no band 
> designator\n",
> + me);
>   retval = SIDEBAND_PROTOCOL_ERROR;
>   break;
>   }
> @@ -48,7 +50,7 @@ int recv_sideband(const char *me, int in_stream, int out)
>   len--;
>   switch (band) {
>   case 3:
> - fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
> + strbuf_addf(, "\n%s%s\n", PREFIX, buf + 1);
>   retval = SIDEBAND_REMOTE_ERROR;
>   break;
>   case 2:
> @@ -58,13 +60,12 @@ int recv_sideband(const char *me, int in_stream, int out)
>* Append a suffix to each nonempty line to clear the
>* end of the screen line.
>*
> -  * The output is accumulated in a buffer and each line
> -  * is printed to stderr using fprintf() with a single
> -  * conversion specifier. This is a "best effort"
> -  * approach to supporting both inter-process atomicity
> -  * (single conversion specifiers are likely to end up
> -  * in single atomic write() system calls) and the ANSI
> -  * control code emulation under Windows.
> +  * The output is accumulated in a buffer and
> +  * each line is printed to stderr using
> +  * fwrite(3).  This is a "best effort"
> +  * approach to support inter-process atomicity
> +  * (single fwrite(3) call is likely to end up
> +  * in single atomic write() system calls).
>*/
>   while ((brk = strpbrk(b, "\n\r"))) {
>   int linelen = brk - b;
> @@ -75,8 +76,7 @@ int recv_sideband(const char *me, int in_stream, int out)
>   } else {
>   strbuf_addf(, "%c", *brk);
>   }
> - fprintf(stderr, "%.*s", (int)outbuf.len,
> - outbuf.buf);
> + fwrite(outbuf.buf, 1, outbuf.len, stderr);
>   strbuf_reset();
>   strbuf_addf(, "%s", PREFIX);
>  
> @@ -90,15 +90,15 @@ int recv_sideband(const char *me, int in_stream, int out)
>   write_or_die(out, buf + 1, len);
>   break;
>   default:
> - fprintf(stderr, "%s: protocol error: bad band #%d\n",
> + strbuf_addf(, "\n%s: protocol error: bad 

[PATCHv2] submodule: test moving recursive submodule

[Stefan Beller]

This reproduces the error as pointed out in [1], but the fix is not easy,
so punt on it for now and just document what needs to be done.

[1] 
http://stackoverflow.com/questions/32782382/git-moving-submodules-recursively-nested-submodules

Signed-off-by: Stefan Beller 
---

  Bart,
  
  I don't have the time fixing this properly,
  so this is the best I can come up with for now. 
  
  Thanks,
  Stefan

 builtin/mv.c  |  4 
 t/t7001-mv.sh | 34 ++
 2 files changed, 38 insertions(+)

diff --git a/builtin/mv.c b/builtin/mv.c
index a201426..36dd2fd 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -264,6 +264,10 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
connect_work_tree_and_git_dir(dst, 
submodule_gitfile[i]);
if (!update_path_in_gitmodules(src, dst))
gitmodules_modified = 1;
+   /**
+* NEEDSWORK: We need to recurse into the submodule
+* and fix the nested submodules as well.
+*/
}
 
if (mode == WORKING_DIRECTORY)
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 4a2570e..fe933f1 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -295,6 +295,28 @@ test_expect_success 'setup submodule' '
mkdir -p deep/directory/hierachy &&
git submodule add ./. deep/directory/hierachy/sub &&
git commit -m "added another submodule" &&
+   mkdir inner_sub &&
+   (
+   cd inner_sub &&
+   git init &&
+   test_commit initial
+   ) &&
+   mkdir outer_sub &&
+   (
+   cd outer_sub &&
+   git init &&
+   test_commit initial &&
+   git submodule add ../inner_sub &&
+   git commit -a -m "add an inner submodule"
+   ) &&
+   git submodule add ./outer_sub ./deep/outer_sub &&
+   git commit -a -m "add outer sub" &&
+   git -C deep ls-tree HEAD |cut -f 2 >actual &&
+   cat >expect <<-EOF &&
+   directory
+   outer_sub
+   EOF
+   test_cmp expect actual &&
git branch submodule
 '
 
@@ -488,6 +510,18 @@ test_expect_success 'moving a submodule in nested 
directories' '
git config -f ../.gitmodules 
submodule.deep/directory/hierachy/sub.path >../actual &&
echo "directory/hierachy/sub" >../expect
) &&
+   test_cmp actual expect &&
+   git commit -a -m "mv a submodule in nested dir"
+'
+
+test_expect_failure 'moving a submodule with a nested submodule' '
+   git submodule update --init --recursive &&
+   git mv deep/outer_sub outer_sub_moved &&
+   # git status would fail if the update of linking git dir to
+   # work dir of the submodule failed.
+   git status &&
+   git config -f .gitmodules submodule.deep/outer_sub.path >actual &&
+   echo "outer_sub_moved" >expect &&
test_cmp actual expect
 '
 
-- 
2.9.0.138.g8a4fcb8.dirty

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

Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp

[Johannes Sixt]

Am 27.06.2016 um 09:26 schrieb larsxschnei...@gmail.com:

--- a/git-p4.py
+++ b/git-p4.py
@@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap):
  self.useClientSpec_from_options = False
  self.clientSpecDirs = None
  self.tempBranches = []
-self.tempBranchLocation = "git-p4-tmp"
+self.tempBranchLocation = "refs/git-p4-tmp"
  self.largeFileSystem = None

  if gitConfig('git-p4.largeFileSystem'):
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 0aafd03..8f28ed2 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' '
test_path_is_file file2 &&
test_path_is_file file3 &&
! grep update file2 &&
-   test_path_is_missing .git/git-p4-tmp
+   test_path_is_missing .git/ref/git-p4-tmp


This should be .git/refs/git-p4-tmp, no? Otherwise, this does not test 
what it should test.



)
  '

@@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in 
the same changelist'
test_path_is_file file2 &&
test_path_is_file file3 &&
! grep update file2 &&
-   test_path_is_missing .git/git-p4-tmp
+   test_path_is_missing .git/ref/git-p4-tmp


Same here.


)
  '


-- Hannes

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

[RFC/PATCH v2 10/10] Add t0410 to test external ODB transfer

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t0410-transfer-e-odb.sh | 136 ++
 1 file changed, 136 insertions(+)
 create mode 100755 t/t0410-transfer-e-odb.sh

diff --git a/t/t0410-transfer-e-odb.sh b/t/t0410-transfer-e-odb.sh
new file mode 100755
index 000..868b55d
--- /dev/null
+++ b/t/t0410-transfer-e-odb.sh
@@ -0,0 +1,136 @@
+#!/bin/sh
+
+test_description='basic tests for transfering external ODBs'
+
+. ./test-lib.sh
+
+ORIG_SOURCE="$PWD/.git"
+export ORIG_SOURCE
+
+ALT_SOURCE1="$PWD/alt-repo1/.git"
+export ALT_SOURCE1
+write_script odb-helper1 <<\EOF
+die() {
+   printf >&2 "%s\n" "$@"
+   exit 1
+}
+GIT_DIR=$ALT_SOURCE1; export GIT_DIR
+case "$1" in
+have)
+   git cat-file --batch-check --batch-all-objects |
+   awk '{print $1 " " $3 " " $2}'
+   ;;
+get)
+   cat "$GIT_DIR"/objects/$(echo $2 | sed 's#..#&/#')
+   ;;
+put)
+   sha1="$2"
+   size="$3"
+   kind="$4"
+   writen=$(git hash-object -w -t "$kind" --stdin)
+   test "$writen" = "$sha1" || die "bad sha1 passed '$sha1' vs writen 
'$writen'"
+   ref_hash=$(echo "$sha1 $size $kind" | GIT_DIR=$ORIG_SOURCE 
GIT_NO_EXTERNAL_ODB=1 git hash-object -w -t blob --stdin) || exit
+   GIT_DIR=$ORIG_SOURCE git update-ref refs/odbs/magic/"$sha1" "$ref_hash"
+   ;;
+*)
+   die "unknown command '$1'"
+   ;;
+esac
+EOF
+HELPER1="\"$PWD\"/odb-helper1"
+
+OTHER_SOURCE="$PWD/.git"
+export OTHER_SOURCE
+
+ALT_SOURCE2="$PWD/alt-repo2/.git"
+export ALT_SOURCE2
+write_script odb-helper2 <<\EOF
+die() {
+   printf >&2 "%s\n" "$@"
+   exit 1
+}
+GIT_DIR=$ALT_SOURCE2; export GIT_DIR
+case "$1" in
+have)
+   GIT_DIR=$OTHER_SOURCE git for-each-ref --format='%(objectname)' 
refs/odbs/magic/ | GIT_DIR=$OTHER_SOURCE xargs git show
+   ;;
+get)
+   OBJ_FILE="$GIT_DIR"/objects/$(echo $2 | sed 's#..#&/#')
+   if ! test -f "$OBJ_FILE"
+   then
+   # "Download" the missing object by copying it from alt-repo1
+   OBJ_DIR=$(echo $2 | sed 's/\(..\).*/\1/')
+   OBJ_BASE=$(basename "$OBJ_FILE")
+   ALT_OBJ_DIR1="$ALT_SOURCE1/objects/$OBJ_DIR"
+   ALT_OBJ_DIR2="$ALT_SOURCE2/objects/$OBJ_DIR"
+   mkdir -p "$ALT_OBJ_DIR2" || die "Could not mkdir 
'$ALT_OBJ_DIR2'"
+   OBJ_SRC="$ALT_OBJ_DIR1/$OBJ_BASE"
+   cp "$OBJ_SRC" "$ALT_OBJ_DIR2" ||
+   die "Could not cp '$OBJ_SRC' into '$ALT_OBJ_DIR2'"
+   fi
+   cat "$OBJ_FILE" || die "Could not cat '$OBJ_FILE'"
+   ;;
+put)
+   sha1="$2"
+   size="$3"
+   kind="$4"
+   writen=$(git hash-object -w -t "$kind" --stdin)
+   test "$writen" = "$sha1" || die "bad sha1 passed '$sha1' vs writen 
'$writen'"
+   ref_hash=$(echo "$sha1 $size $kind" | GIT_DIR=$OTHER_SOURCE 
GIT_NO_EXTERNAL_ODB=1 git hash-object -w -t blob --stdin) || exit
+   GIT_DIR=$OTHER_SOURCE git update-ref refs/odbs/magic/"$sha1" "$ref_hash"
+   ;;
+*)
+   die "unknown command '$1'"
+   ;;
+esac
+EOF
+HELPER2="\"$PWD\"/odb-helper2"
+
+test_expect_success 'setup first alternate repo' '
+   git init alt-repo1 &&
+   test_commit zero &&
+   git config odb.magic.command "$HELPER1"
+'
+
+test_expect_success 'setup other repo and its alternate repo' '
+   git init other-repo &&
+   git init alt-repo2 &&
+   (cd other-repo &&
+git remote add origin .. &&
+git pull origin master &&
+git checkout master &&
+git log)
+'
+
+test_expect_success 'new blobs are put in first object store' '
+   test_commit one &&
+   hash1=$(git ls-tree HEAD | grep one.t | cut -f1 | cut -d\  -f3) &&
+   content=$(cd alt-repo1 && git show "$hash1") &&
+   test "$content" = "one" &&
+   test_commit two &&
+   hash2=$(git ls-tree HEAD | grep two.t | cut -f1 | cut -d\  -f3) &&
+   content=$(cd alt-repo1 && git show "$hash2") &&
+   test "$content" = "two"
+'
+
+test_expect_success 'other repo gets the blobs from object store' '
+   (cd other-repo &&
+git fetch origin "refs/odbs/magic/*:refs/odbs/magic/*" &&
+test_must_fail git cat-file blob "$hash1" &&
+test_must_fail git cat-file blob "$hash2" &&
+git config odb.magic.command "$HELPER2" &&
+git cat-file blob "$hash1" &&
+git cat-file blob "$hash2"
+   )
+'
+
+test_expect_success 'other repo gets everything else' '
+   (cd other-repo &&
+git fetch origin &&
+content=$(git show "$hash1") &&
+test "$content" = "one" &&
+content=$(git show "$hash2") &&
+test "$content" = "two")
+'
+
+test_done
-- 
2.9.0.rc2.11.g990c140

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

[RFC/PATCH v2 05/10] t0400: add test for 'put' command

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t0400-external-odb.sh | 9 +
 1 file changed, 9 insertions(+)

diff --git a/t/t0400-external-odb.sh b/t/t0400-external-odb.sh
index 0f1bb97..6c6da5c 100755
--- a/t/t0400-external-odb.sh
+++ b/t/t0400-external-odb.sh
@@ -57,4 +57,13 @@ test_expect_success 'helper can retrieve alt objects' '
test_cmp expect actual
 '
 
+test_expect_success 'helper can add objects to alt repo' '
+   hash=$(echo "Hello odb!" | git hash-object -w -t blob --stdin) &&
+   test -f .git/objects/$(echo $hash | sed "s#..#&/#") &&
+   size=$(git cat-file -s "$hash") &&
+   git cat-file blob "$hash" | ./odb-helper put "$hash" "$size" blob &&
+   alt_size=$(cd alt-repo && git cat-file -s "$hash") &&
+   test "$size" -eq "$alt_size"
+'
+
 test_done
-- 
2.9.0.rc2.11.g990c140

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

[RFC/PATCH v2 04/10] t0400: add 'put' command to odb-helper script

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t0400-external-odb.sh | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/t/t0400-external-odb.sh b/t/t0400-external-odb.sh
index fe85413..0f1bb97 100755
--- a/t/t0400-external-odb.sh
+++ b/t/t0400-external-odb.sh
@@ -7,6 +7,10 @@ test_description='basic tests for external object databases'
 ALT_SOURCE="$PWD/alt-repo/.git"
 export ALT_SOURCE
 write_script odb-helper <<\EOF
+die() {
+   printf >&2 "%s\n" "$@"
+   exit 1
+}
 GIT_DIR=$ALT_SOURCE; export GIT_DIR
 case "$1" in
 have)
@@ -16,6 +20,16 @@ have)
 get)
cat "$GIT_DIR"/objects/$(echo $2 | sed 's#..#&/#')
;;
+put)
+   sha1="$2"
+   size="$3"
+   kind="$4"
+   writen=$(git hash-object -w -t "$kind" --stdin)
+   test "$writen" = "$sha1" || die "bad sha1 passed '$sha1' vs writen 
'$writen'"
+   ;;
+*)
+   die "unknown command '$1'"
+   ;;
 esac
 EOF
 HELPER="\"$PWD\"/odb-helper"
-- 
2.9.0.rc2.11.g990c140

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

[RFC/PATCH v2 06/10] external odb: add write support

[Christian Couder]

Signed-off-by: Christian Couder 
---
 external-odb.c | 15 +++
 external-odb.h |  2 ++
 odb-helper.c   | 41 +
 odb-helper.h   |  3 +++
 sha1_file.c|  2 ++
 5 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/external-odb.c b/external-odb.c
index 42978a3..bb70fe3 100644
--- a/external-odb.c
+++ b/external-odb.c
@@ -127,3 +127,18 @@ int external_odb_for_each_object(each_external_object_fn 
fn, void *data)
}
return 0;
 }
+
+int external_odb_write_object(const void *buf, unsigned long len,
+ const char *type, unsigned char *sha1)
+{
+   struct odb_helper *o;
+
+   external_odb_init();
+
+   for (o = helpers; o; o = o->next) {
+   int r = odb_helper_write_object(o, buf, len, type, sha1);
+   if (r <= 0)
+   return r;
+   }
+   return 1;
+}
diff --git a/external-odb.h b/external-odb.h
index cea8570..55d291d 100644
--- a/external-odb.h
+++ b/external-odb.h
@@ -10,5 +10,7 @@ typedef int (*each_external_object_fn)(const unsigned char 
*sha1,
   unsigned long size,
   void *data);
 int external_odb_for_each_object(each_external_object_fn, void *);
+int external_odb_write_object(const void *buf, unsigned long len,
+ const char *type, unsigned char *sha1);
 
 #endif /* EXTERNAL_ODB_H */
diff --git a/odb-helper.c b/odb-helper.c
index 045cf6f..677a5e7 100644
--- a/odb-helper.c
+++ b/odb-helper.c
@@ -33,9 +33,10 @@ static void prepare_helper_command(struct argv_array *argv, 
const char *cmd,
strbuf_release();
 }
 
-__attribute__((format (printf,3,4)))
+__attribute__((format (printf,4,5)))
 static int odb_helper_start(struct odb_helper *o,
struct odb_helper_cmd *cmd,
+   int use_stdin,
const char *fmt, ...)
 {
va_list ap;
@@ -52,7 +53,10 @@ static int odb_helper_start(struct odb_helper *o,
 
cmd->child.argv = cmd->argv.argv;
cmd->child.use_shell = 1;
-   cmd->child.no_stdin = 1;
+   if (use_stdin)
+   cmd->child.in = -1;
+   else
+   cmd->child.no_stdin = 1;
cmd->child.out = -1;
 
if (start_command(>child) < 0) {
@@ -109,7 +113,7 @@ static void odb_helper_load_have(struct odb_helper *o)
return;
o->have_valid = 1;
 
-   if (odb_helper_start(o, , "have") < 0)
+   if (odb_helper_start(o, , 0, "have") < 0)
return;
 
fh = xfdopen(cmd.child.out, "r");
@@ -164,7 +168,7 @@ int odb_helper_fetch_object(struct odb_helper *o, const 
unsigned char *sha1,
if (!obj)
return -1;
 
-   if (odb_helper_start(o, , "get %s", sha1_to_hex(sha1)) < 0)
+   if (odb_helper_start(o, , 0, "get %s", sha1_to_hex(sha1)) < 0)
return -1;
 
memset(, 0, sizeof(stream));
@@ -252,3 +256,32 @@ int odb_helper_for_each_object(struct odb_helper *o,
 
return 0;
 }
+
+int odb_helper_write_object(struct odb_helper *o,
+   const void *buf, unsigned long len,
+   const char *type, unsigned char *sha1)
+{
+   struct odb_helper_cmd cmd;
+
+   if (odb_helper_start(o, , 1, "put %s %lu %s",
+sha1_to_hex(sha1), len, type) < 0)
+   return -1;
+
+   do {
+   int w = xwrite(cmd.child.in, buf, len);
+   if (w < 0) {
+   error("unable to write to odb helper '%s': %s",
+ o->name, strerror(errno));
+   close(cmd.child.in);
+   close(cmd.child.out);
+   odb_helper_finish(o, );
+   return -1;
+   }
+   len -= w;
+   } while (len > 0);
+
+   close(cmd.child.in);
+   close(cmd.child.out);
+   odb_helper_finish(o, );
+   return 0;
+}
diff --git a/odb-helper.h b/odb-helper.h
index 8c3916d..af31cc2 100644
--- a/odb-helper.h
+++ b/odb-helper.h
@@ -25,5 +25,8 @@ int odb_helper_fetch_object(struct odb_helper *o, const 
unsigned char *sha1,
int fd);
 int odb_helper_for_each_object(struct odb_helper *o,
   each_external_object_fn, void *);
+int odb_helper_write_object(struct odb_helper *o,
+   const void *buf, unsigned long len,
+   const char *type, unsigned char *sha1);
 
 #endif /* ODB_HELPER_H */
diff --git a/sha1_file.c b/sha1_file.c
index a707bc1..90f19de 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3192,6 +3192,8 @@ int write_sha1_file(const void *buf, unsigned long len, 
const char *type, unsign
 * it out into .git/objects/??/?{38} file.
 */
write_sha1_file_prepare(buf, len, type, sha1, hdr, );

[RFC/PATCH v2 09/10] Add GIT_NO_EXTERNAL_ODB env variable

[Christian Couder]

Signed-off-by: Christian Couder 
---
 cache.h| 9 +
 environment.c  | 4 
 external-odb.c | 6 ++
 sha1_file.c| 3 +++
 4 files changed, 22 insertions(+)

diff --git a/cache.h b/cache.h
index cc0a934..b0fe2bc 100644
--- a/cache.h
+++ b/cache.h
@@ -420,6 +420,7 @@ static inline enum object_type object_type(unsigned int 
mode)
 #define CEILING_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
 #define NO_REPLACE_OBJECTS_ENVIRONMENT "GIT_NO_REPLACE_OBJECTS"
 #define GIT_REPLACE_REF_BASE_ENVIRONMENT "GIT_REPLACE_REF_BASE"
+#define NO_EXTERNAL_ODB_ENVIRONMENT "GIT_NO_EXTERNAL_ODB"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
@@ -678,6 +679,14 @@ int get_shared_repository(void);
 extern int check_replace_refs;
 extern char *git_replace_ref_base;
 
+/*
+ * Do external odbs need to be used this run?  This variable is
+ * initialized to true unless $GIT_NO_EXTERNAL_ODB is set, but it
+ * maybe set to false by some commands that do not want external
+ * odbs to be active.
+ */
+extern int use_external_odb;
+
 extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_apply_sparse_checkout;
diff --git a/environment.c b/environment.c
index ca72464..1799339 100644
--- a/environment.c
+++ b/environment.c
@@ -48,6 +48,7 @@ const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 int check_replace_refs = 1;
 char *git_replace_ref_base;
+int use_external_odb = 1;
 enum eol core_eol = EOL_UNSET;
 enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
@@ -119,6 +120,7 @@ const char * const local_repo_env[] = {
INDEX_ENVIRONMENT,
NO_REPLACE_OBJECTS_ENVIRONMENT,
GIT_REPLACE_REF_BASE_ENVIRONMENT,
+   NO_EXTERNAL_ODB_ENVIRONMENT,
GIT_PREFIX_ENVIRONMENT,
GIT_SHALLOW_FILE_ENVIRONMENT,
GIT_COMMON_DIR_ENVIRONMENT,
@@ -183,6 +185,8 @@ static void setup_git_env(void)
replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
  : "refs/replace/");
+   if (getenv(NO_EXTERNAL_ODB_ENVIRONMENT))
+   use_external_odb = 0;
namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
namespace_len = strlen(namespace);
shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
diff --git a/external-odb.c b/external-odb.c
index 6dd7b25..a980fbf 100644
--- a/external-odb.c
+++ b/external-odb.c
@@ -63,6 +63,9 @@ int external_odb_has_object(const unsigned char *sha1)
 {
struct odb_helper *o;
 
+   if (!use_external_odb)
+   return 0;
+
external_odb_init();
 
for (o = helpers; o; o = o->next)
@@ -133,6 +136,9 @@ int external_odb_write_object(const void *buf, unsigned 
long len,
 {
struct odb_helper *o;
 
+   if (!use_external_odb)
+   return 1;
+
/* For now accept only blobs */
if (strcmp(type, "blob"))
return 1;
diff --git a/sha1_file.c b/sha1_file.c
index 90f19de..13d4d75 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -448,6 +448,9 @@ void prepare_external_alt_odb(void)
static int linked_external;
const char *path;
 
+   if (!use_external_odb)
+   return;
+
if (linked_external)
return;
 
-- 
2.9.0.rc2.11.g990c140

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

[RFC/PATCH v2 00/10] Add initial experimental external ODB support

[Christian Couder]

Goal


Git can store its objects only in the form of loose objects in
separate files or packed objects in a pack file.

To be able to better handle some kind of objects, for example big
blobs, it would be nice if Git could store its objects in other object
databases (ODB).

To do that, this patch series makes it possible to register commands,
using "odb..command" config variables, to access external
ODBs where objects can be stored and retrieved.

External ODBs should be able to tranfer information about the blobs
they store. This patch series show how this is possible using kind of
replace refs.

Design
~~

* Registered command

Each registered command manages access to one external ODB and will be
called the following ways:

  - " have": the command should output the sha1, size and
type of all the objects the external ODB contains, one object per
line.

  - " get ": the command should then read from the
external ODB the content of the object corresponding to  and
output it on stdout.

  - " put   ": the command should then read
from stdin an object and store it in the external ODB.

* Transfer

To tranfer information about the blobs stored in external ODB, some
special refs, called "odb ref", similar as replace refs, are used.

For now there should be one odb ref per blob. Each ref name should be
refs/odbs// where  is the sha1 of the blob stored
in the external odb named .

These odb refs should all point to a blob that should be stored in the
Git repository and contain information about the blob stored in the
external odb. This information can be specific to the external odb.
The repos can then share this information using commands like:

`git fetch origin "refs/odbs//*:refs/odbs//*"`

* Other

This RFC patch series for now does not address the following important
part of a complete solution:

  - No real external ODB has been interfaced with Git. The tests use
another git repo in a separate directory for this purpose which is
probably useless in the real world.

Design discussion about performance
~~~

Yeah, it is not efficient to fork/exec a command to just read or write
one object to or from the external ODB. Batch calls and/or using a
daemon and/or RPC should be used instead to be able to store regular
objects in an external ODB. But for now the external ODB would be all
about really big files, where the cost of a fork+exec should not
matter much. If we later want to extend usage of external ODBs, yeah
we will probably need to design other mechanisms.

Here are some related explanations from Peff:

{{{
Because this "external odb" essentially acts as a git alternate, we
would hit it only when we couldn't find an object through regular means.
Git would then make the object available in the usual on-disk format
(probably as a loose object).

So in most processes, we would not need to consult the odb command at
all. And when we do, the first thing would be to get its "have" list,
which would at most run once per process.

So the per-object cost is really calling "get", and my assumption there
was that the cost of actually retrieving the object over the network
would dwarf the fork/exec cost.

I also waffled on having git cache the output of " have" in
some fast-lookup format to save even the single fork/exec. But I figured
that was something that could be added later if needed.

You'll note that this is sort of a "fault-in" model. Another model would
be to treat external odb updates similar to fetches. I.e., we touch the
network only during a special update operation, and then try to work
locally with whatever the external odb has. IMHO this policy could
actually be up to the external odb itself (i.e., its "have" command
could serve from a local cache if it likes).
}}}

Implementation
~~

* Mechanism to call the registered commands

This series adds a set of function in external-odb.{c,h} that are
called by the rest of Git to manage all the external ODBs.

These functions use 'struct odb_helper' and its associated functions
defined in odb-helper.{c,h} to talk to the different external ODBs by
launching the configured "odb..command" commands and writing
to or reading from them.

The tests in this series creates an odb-helper script that is
registered using the "odb.magic.command" config variable, and then
called to read from and write to the external ODB.

* ODB refs

For now odb ref management is only implemented in a registered command
in t0410, but maybe this or some parts of it could be done by Git
itself.

When a new blob is added to an external odb, its sha1, size and type
are writen in another new blob and the odb ref is created.

When the list of existing blobs is requested from the external odb,
the content of the blobs pointed to by the odb refs can also be used
by the odb to claim that it can get the objects.

When a blob is actually requested from the external odb, it can use
the content stored in the blobs pointed to by the odb refs to get 

[RFC/PATCH v2 02/10] external odb foreach

[Christian Couder]

From: Jeff King 

---
 external-odb.c | 14 ++
 external-odb.h |  6 ++
 odb-helper.c   | 15 +++
 odb-helper.h   |  4 
 4 files changed, 39 insertions(+)

diff --git a/external-odb.c b/external-odb.c
index 1ccfa99..42978a3 100644
--- a/external-odb.c
+++ b/external-odb.c
@@ -113,3 +113,17 @@ int external_odb_fetch_object(const unsigned char *sha1)
 
return -1;
 }
+
+int external_odb_for_each_object(each_external_object_fn fn, void *data)
+{
+   struct odb_helper *o;
+
+   external_odb_init();
+
+   for (o = helpers; o; o = o->next) {
+   int r = odb_helper_for_each_object(o, fn, data);
+   if (r)
+   return r;
+   }
+   return 0;
+}
diff --git a/external-odb.h b/external-odb.h
index 2397477..cea8570 100644
--- a/external-odb.h
+++ b/external-odb.h
@@ -5,4 +5,10 @@ const char *external_odb_root(void);
 int external_odb_has_object(const unsigned char *sha1);
 int external_odb_fetch_object(const unsigned char *sha1);
 
+typedef int (*each_external_object_fn)(const unsigned char *sha1,
+  enum object_type type,
+  unsigned long size,
+  void *data);
+int external_odb_for_each_object(each_external_object_fn, void *);
+
 #endif /* EXTERNAL_ODB_H */
diff --git a/odb-helper.c b/odb-helper.c
index 7029a59..045cf6f 100644
--- a/odb-helper.c
+++ b/odb-helper.c
@@ -235,5 +235,20 @@ int odb_helper_fetch_object(struct odb_helper *o, const 
unsigned char *sha1,
return -1;
}
 
+   return 0;
+}
+
+int odb_helper_for_each_object(struct odb_helper *o,
+  each_external_object_fn fn,
+  void *data)
+{
+   int i;
+   for (i = 0; i < o->have_nr; i++) {
+   struct odb_helper_object *obj = >have[i];
+   int r = fn(obj->sha1, obj->type, obj->size, data);
+   if (r)
+   return r;
+   }
+
return 0;
 }
diff --git a/odb-helper.h b/odb-helper.h
index 0f704f9..8c3916d 100644
--- a/odb-helper.h
+++ b/odb-helper.h
@@ -1,6 +1,8 @@
 #ifndef ODB_HELPER_H
 #define ODB_HELPER_H
 
+#include "external-odb.h"
+
 struct odb_helper {
const char *name;
const char *cmd;
@@ -21,5 +23,7 @@ struct odb_helper *odb_helper_new(const char *name, int 
namelen);
 int odb_helper_has_object(struct odb_helper *o, const unsigned char *sha1);
 int odb_helper_fetch_object(struct odb_helper *o, const unsigned char *sha1,
int fd);
+int odb_helper_for_each_object(struct odb_helper *o,
+  each_external_object_fn, void *);
 
 #endif /* ODB_HELPER_H */
-- 
2.9.0.rc2.11.g990c140

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

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Nicolas Pitre  writes:

>> When we exit the loop because we set retval to a non-zero value,
>> should we still drain the outbuf?
>
> I would think so.  Anything that the remote sent before any error should 
> be printed nevertheless.  The clue for the error might be in the pending 
> buffer.
>
> However in this case the actual error printout and the pending buffer 
> will appear reversed.
>
> So what I'd suggest is actually something like this:
>
> if (len < 1) {
> strbuf_addf(, "\n%s: protocol error: no band 
> designator\n", me);
> retval = SIDEBAND_PROTOCOL_ERROR;
> break;
>
> And so on for the other error cases.

Makes sense.

Here is what I have as a "SQUASH" on top of Lukas's change to be
queued on 'pu'.

It appears that a few tests get their expectations broken, with or
without this "SQUASH" change, though X-<.

 sideband.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/sideband.c b/sideband.c
index 226a8c2..082dfc6 100644
--- a/sideband.c
+++ b/sideband.c
@@ -33,13 +33,15 @@ int recv_sideband(const char *me, int in_stream, int out)
else
suffix = DUMB_SUFFIX;
 
-   while (retval == 0) {
+   while (!retval) {
int band, len;
len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
0);
if (len == 0)
break;
if (len < 1) {
-   fprintf(stderr, "%s: protocol error: no band 
designator\n", me);
+   strbuf_addf(,
+   "\n%s: protocol error: no band 
designator\n",
+   me);
retval = SIDEBAND_PROTOCOL_ERROR;
break;
}
@@ -48,7 +50,7 @@ int recv_sideband(const char *me, int in_stream, int out)
len--;
switch (band) {
case 3:
-   fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
+   strbuf_addf(, "\n%s%s\n", PREFIX, buf + 1);
retval = SIDEBAND_REMOTE_ERROR;
break;
case 2:
@@ -58,13 +60,12 @@ int recv_sideband(const char *me, int in_stream, int out)
 * Append a suffix to each nonempty line to clear the
 * end of the screen line.
 *
-* The output is accumulated in a buffer and each line
-* is printed to stderr using fprintf() with a single
-* conversion specifier. This is a "best effort"
-* approach to supporting both inter-process atomicity
-* (single conversion specifiers are likely to end up
-* in single atomic write() system calls) and the ANSI
-* control code emulation under Windows.
+* The output is accumulated in a buffer and
+* each line is printed to stderr using
+* fwrite(3).  This is a "best effort"
+* approach to support inter-process atomicity
+* (single fwrite(3) call is likely to end up
+* in single atomic write() system calls).
 */
while ((brk = strpbrk(b, "\n\r"))) {
int linelen = brk - b;
@@ -75,8 +76,7 @@ int recv_sideband(const char *me, int in_stream, int out)
} else {
strbuf_addf(, "%c", *brk);
}
-   fprintf(stderr, "%.*s", (int)outbuf.len,
-   outbuf.buf);
+   fwrite(outbuf.buf, 1, outbuf.len, stderr);
strbuf_reset();
strbuf_addf(, "%s", PREFIX);
 
@@ -90,15 +90,15 @@ int recv_sideband(const char *me, int in_stream, int out)
write_or_die(out, buf + 1, len);
break;
default:
-   fprintf(stderr, "%s: protocol error: bad band #%d\n",
+   strbuf_addf(, "\n%s: protocol error: bad band 
#%d\n",
me, band);
retval = SIDEBAND_PROTOCOL_ERROR;
break;
}
}
 
-   if (outbuf.len > 0)
-   fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
+   if (outbuf.len)
+   fwrite(outbuf.buf, 1, outbuf.len, stderr);
strbuf_release();
return retval;
 }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo 

[RFC/PATCH v2 03/10] t0400: use --batch-all-objects to get all objects

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t0400-external-odb.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/t/t0400-external-odb.sh b/t/t0400-external-odb.sh
index 2b01617..fe85413 100755
--- a/t/t0400-external-odb.sh
+++ b/t/t0400-external-odb.sh
@@ -10,9 +10,7 @@ write_script odb-helper <<\EOF
 GIT_DIR=$ALT_SOURCE; export GIT_DIR
 case "$1" in
 have)
-   git rev-list --all --objects |
-   cut -d' ' -f1 |
-   git cat-file --batch-check |
+   git cat-file --batch-check --batch-all-objects |
awk '{print $1 " " $3 " " $2}'
;;
 get)
-- 
2.9.0.rc2.11.g990c140

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

[RFC/PATCH v2 01/10] Add initial external odb support

[Christian Couder]

From: Jeff King 

Signed-off-by: Christian Couder 
---
 Makefile|   2 +
 cache.h |   9 ++
 external-odb.c  | 115 +++
 external-odb.h  |   8 ++
 odb-helper.c| 239 
 odb-helper.h|  25 +
 sha1_file.c |  64 ++---
 t/t0400-external-odb.sh |  48 ++
 8 files changed, 495 insertions(+), 15 deletions(-)
 create mode 100644 external-odb.c
 create mode 100644 external-odb.h
 create mode 100644 odb-helper.c
 create mode 100644 odb-helper.h
 create mode 100755 t/t0400-external-odb.sh

diff --git a/Makefile b/Makefile
index de5a030..8641318 100644
--- a/Makefile
+++ b/Makefile
@@ -726,6 +726,7 @@ LIB_OBJS += ewah/ewah_bitmap.o
 LIB_OBJS += ewah/ewah_io.o
 LIB_OBJS += ewah/ewah_rlw.o
 LIB_OBJS += exec_cmd.o
+LIB_OBJS += external-odb.o
 LIB_OBJS += fetch-pack.o
 LIB_OBJS += fsck.o
 LIB_OBJS += gettext.o
@@ -757,6 +758,7 @@ LIB_OBJS += notes-cache.o
 LIB_OBJS += notes-merge.o
 LIB_OBJS += notes-utils.o
 LIB_OBJS += object.o
+LIB_OBJS += odb-helper.o
 LIB_OBJS += pack-bitmap.o
 LIB_OBJS += pack-bitmap-write.o
 LIB_OBJS += pack-check.o
diff --git a/cache.h b/cache.h
index 6049f86..cc0a934 100644
--- a/cache.h
+++ b/cache.h
@@ -864,6 +864,12 @@ const char *git_path_shallow(void);
  */
 extern const char *sha1_file_name(const unsigned char *sha1);
 
+/*
+ * Like sha1_file_name, but return the filename within a specific alternate
+ * object directory. Shares the same static buffer with sha1_file_name.
+ */
+extern const char *sha1_file_name_alt(const char *objdir, const unsigned char 
*sha1);
+
 /*
  * Return the name of the (local) packfile with the specified sha1 in
  * its name.  The return value is a pointer to memory that is
@@ -1093,6 +1099,8 @@ extern int do_check_packed_object_crc;
 
 extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned 
long size, const char *type);
 
+extern int create_object_tmpfile(struct strbuf *tmp, const char *filename);
+extern void close_sha1_file(int fd);
 extern int finalize_object_file(const char *tmpfile, const char *filename);
 
 extern int has_sha1_pack(const unsigned char *sha1);
@@ -1338,6 +1346,7 @@ extern void read_info_alternates(const char * 
relative_base, int depth);
 extern void add_to_alternates_file(const char *reference);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 extern int foreach_alt_odb(alt_odb_fn, void*);
+extern void prepare_external_alt_odb(void);
 
 struct pack_window {
struct pack_window *next;
diff --git a/external-odb.c b/external-odb.c
new file mode 100644
index 000..1ccfa99
--- /dev/null
+++ b/external-odb.c
@@ -0,0 +1,115 @@
+#include "cache.h"
+#include "external-odb.h"
+#include "odb-helper.h"
+
+static struct odb_helper *helpers;
+static struct odb_helper **helpers_tail = 
+
+static struct odb_helper *find_or_create_helper(const char *name, int len)
+{
+   struct odb_helper *o;
+
+   for (o = helpers; o; o = o->next)
+   if (!strncmp(o->name, name, len) && !o->name[len])
+   return o;
+
+   o = odb_helper_new(name, len);
+   *helpers_tail = o;
+   helpers_tail = >next;
+
+   return o;
+}
+
+static int external_odb_config(const char *var, const char *value, void *data)
+{
+   struct odb_helper *o;
+   const char *key, *dot;
+
+   if (!skip_prefix(var, "odb.", ))
+   return 0;
+   dot = strrchr(key, '.');
+   if (!dot)
+   return 0;
+
+   o = find_or_create_helper(key, dot - key);
+   key = dot + 1;
+
+   if (!strcmp(key, "command"))
+   return git_config_string(>cmd, var, value);
+
+   return 0;
+}
+
+static void external_odb_init(void)
+{
+   static int initialized;
+
+   if (initialized)
+   return;
+   initialized = 1;
+
+   git_config(external_odb_config, NULL);
+}
+
+const char *external_odb_root(void)
+{
+   static const char *root;
+   if (!root)
+   root = git_pathdup("objects/external");
+   return root;
+}
+
+int external_odb_has_object(const unsigned char *sha1)
+{
+   struct odb_helper *o;
+
+   external_odb_init();
+
+   for (o = helpers; o; o = o->next)
+   if (odb_helper_has_object(o, sha1))
+   return 1;
+   return 0;
+}
+
+int external_odb_fetch_object(const unsigned char *sha1)
+{
+   struct odb_helper *o;
+   const char *path;
+
+   if (!external_odb_has_object(sha1))
+   return -1;
+
+   path = sha1_file_name_alt(external_odb_root(), sha1);
+   safe_create_leading_directories_const(path);
+   prepare_external_alt_odb();
+
+   for (o = helpers; o; o = o->next) {
+   struct strbuf tmpfile = STRBUF_INIT;
+   int ret;
+   int fd;
+
+   if 

[RFC/PATCH v2 07/10] external-odb: accept only blobs for now

[Christian Couder]

Signed-off-by: Christian Couder 
---
 external-odb.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/external-odb.c b/external-odb.c
index bb70fe3..6dd7b25 100644
--- a/external-odb.c
+++ b/external-odb.c
@@ -133,6 +133,10 @@ int external_odb_write_object(const void *buf, unsigned 
long len,
 {
struct odb_helper *o;
 
+   /* For now accept only blobs */
+   if (strcmp(type, "blob"))
+   return 1;
+
external_odb_init();
 
for (o = helpers; o; o = o->next) {
-- 
2.9.0.rc2.11.g990c140

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

[RFC/PATCH v2 08/10] t0400: add test for external odb write support

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t0400-external-odb.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t0400-external-odb.sh b/t/t0400-external-odb.sh
index 6c6da5c..3c868ca 100755
--- a/t/t0400-external-odb.sh
+++ b/t/t0400-external-odb.sh
@@ -66,4 +66,12 @@ test_expect_success 'helper can add objects to alt repo' '
test "$size" -eq "$alt_size"
 '
 
+test_expect_success 'commit adds objects to alt repo' '
+   test_config odb.magic.command "$HELPER" &&
+   test_commit three &&
+   hash3=$(git ls-tree HEAD | grep three.t | cut -f1 | cut -d\  -f3) &&
+   content=$(cd alt-repo && git show "$hash3") &&
+   test "$content" = "three"
+'
+
 test_done
-- 
2.9.0.rc2.11.g990c140

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

Re: [PATCH] doc: git-htmldocs.googlecode.com is no more

[Junio C Hamano]

Matthieu Moy  writes:

> Junio C Hamano  writes:
>
>> On Wed, Jun 22, 2016 at 12:00 PM, Eric Wong  wrote:
>>>
>>> Just wondering, who updates
>>> https://kernel.org/pub/software/scm/git/docs/
>>> and why hasn't it been updated in a while?
>>> (currently it says Last updated 2015-06-06 at the bottom)
>>
>> Nobody. It is too cumbersome to use their upload tool to update many
>> files (it is geared towards updating a handful of tarballs at a time).
>
> Then, I guess it would make sense to remove it to avoid pointing users
> to outdated docs?

Actually, it turns out that k.org folks have an auto-builder that
updates the documentation set for each release.  It hasn't been run
since v2.5 days, though, until very recently.

I was told that the above URL currently should be showing v2.9.0
docs.

The rule for https://kernel.org/pub/software/scm/git/docs/ currently
is that it shows documentation set for a released, but this is
likely to change in the future (s/a released/the latest release/ at
the minimum, with some definition of "the latest").

Also https://kernel.org/pub/software/scm/git/docs/v2.4.0/git.html
would give you the documentation set for that version.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] connect: read $GIT_SSH_COMMAND from config file

[Junio C Hamano]

Matthieu Moy  writes:

> Jeff King  writes:
>
>> On Sun, Jun 26, 2016 at 01:16:35PM +0200, Nguyễn Thái Ngọc Duy wrote:
>>
>>> +   use the specified command instead of 'ssh' when they need to
>>> +   connect to a remote system. The command is in the same form as
>>> +   'GIT_SSH_COMMAND' environment variable and is overriden when
>>> +   the environment variable is set.
>>
>> Probably s/'GIT_SSH_COMMAND'/the &/.
>
> I think so. I'd write either "same form as `GIT_SSH_COMMAND`" or "same
> form as the `GIT_SSH_COMMAND` environment variable`.

Let's then squash this in.

 Documentation/config.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bd32069..fe7c9c4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -444,11 +444,11 @@ This is useful for excluding servers inside a firewall 
from
 proxy use, while defaulting to a common proxy for external domains.
 
 core.sshCommand::
-   If this variable is set then 'git fetch' and 'git push' will
-   use the specified command instead of 'ssh' when they need to
+   If this variable is set, `git fetch` and `git push` will
+   use the specified command instead of `ssh` when they need to
connect to a remote system. The command is in the same form as
-   'GIT_SSH_COMMAND' environment variable and is overriden when
-   the environment variable is set.
+   the `GIT_SSH_COMMAND` environment variable and is overriden
+   when the environment variable is set.
 
 core.ignoreStat::
If true, Git will avoid using lstat() calls to detect if files have
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] rebase -i: restore autostash on abort

[Patrick Steinhardt]

When we abort an interactive rebase we do so by calling
`die_abort`, which cleans up after us by removing the rebase
state directory. If the user has requested to use the autostash
feature, though, the state directory may also contain a reference
to the autostash, which will now be deleted.

Fix the issue by trying to re-apply the autostash in `die_abort`.
This will also handle the case where the autostash does not apply
cleanly anymore by recording it in a user-visible stash.

Reported-by: Daniel Hahler 
Signed-off-by: Patrick Steinhardt 
---
 git-rebase--interactive.sh  |  1 +
 t/t3420-rebase-autostash.sh | 11 +++
 2 files changed, 12 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 05f22e4..4f499d2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -212,6 +212,7 @@ exit_with_patch () {
 }
 
 die_abort () {
+   apply_autostash
rm -rf "$state_dir"
die "$1"
 }
diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 944154b..2e1171e 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -192,4 +192,15 @@ test_expect_success 'abort rebase -i with --autostash' '
test_cmp expected file0
 '
 
+test_expect_success 'restore autostash on editor failure' '
+   test_when_finished "git reset --hard" &&
+   echo uncommited-content >file0 &&
+   (
+   test_set_editor "false" &&
+   test_must_fail git rebase -i --autostash HEAD^
+   ) &&
+   echo uncommited-content >expected &&
+   test_cmp expected file0
+'
+
 test_done
-- 
2.9.0

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

Re: [PATCH] sha1_name.c: add an option to abort on ambiguous refs

[Duy Nguyen]

Bringing this up again after I've seen another person accidentally
create a refs/heads/origin/some-branch and get really confused because
"git push" reports up-to-date but the remote branch is not updated. I
don't know how he got into that situation, but I hope we should be
able to catch it and suggest a way out.

On Wed, Mar 23, 2016 at 2:30 PM, Nguyễn Thái Ngọc Duy  wrote:
> There are cases when a warning on ambiguous refs may go unnoticed
> (e.g. git-log filling up the whole screen). There are also cases when
> people want to catch ambiguity early (e.g. it happens deep in some
> script). In either case, aborting the program would accomplish it.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  Documentation/config.txt |  3 ++-
>  config.c |  5 -
>  sha1_name.c  | 10 --
>  3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 2cd6bdd..4172f59 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -522,7 +522,8 @@ core.sharedRepository::
>
>  core.warnAmbiguousRefs::
> If true, Git will warn you if the ref name you passed it is ambiguous
> -   and might match multiple refs in the repository. True by default.
> +   and might match multiple refs in the repository. If set to "fatal",
> +   the program will abort on ambiguous refs. True by default.
>
>  core.compression::
> An integer -1..9, indicating a default compression level.
> diff --git a/config.c b/config.c
> index 9ba40bc..79f1ea5 100644
> --- a/config.c
> +++ b/config.c
> @@ -738,7 +738,10 @@ static int git_default_core_config(const char *var, 
> const char *value)
> }
>
> if (!strcmp(var, "core.warnambiguousrefs")) {
> -   warn_ambiguous_refs = git_config_bool(var, value);
> +   if (!strcasecmp(value, "fatal"))
> +   warn_ambiguous_refs = 2;
> +   else
> +   warn_ambiguous_refs = git_config_bool(var, value);
> return 0;
> }
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 3acf221..a0f0ab9 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -480,6 +480,8 @@ static int get_sha1_basic(const char *str, int len, 
> unsigned char *sha1,
> warning(warn_msg, len, str);
> if (advice_object_name_warning)
> fprintf(stderr, "%s\n", 
> _(object_name_msg));
> +   if (warn_ambiguous_refs > 1)
> +   die(_("cannot continue with ambiguous 
> refs"));
> }
> free(real_ref);
> }
> @@ -537,8 +539,12 @@ static int get_sha1_basic(const char *str, int len, 
> unsigned char *sha1,
>
> if (warn_ambiguous_refs && !(flags & GET_SHA1_QUIETLY) &&
> (refs_found > 1 ||
> -!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY)))
> -   warning(warn_msg, len, str);
> +!get_short_sha1(str, len, tmp_sha1, GET_SHA1_QUIETLY))) {
> +   if (warn_ambiguous_refs > 1)
> +   die(warn_msg, len, str);
> +   else
> +   warning(warn_msg, len, str);
> +   }
>
> if (reflog_len) {
> int nth, i;
> --
> 2.8.0.rc0.210.gd302cd2
>



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

Re: [PATCH] submodule: test moving recursive submodule

[Stefan Beller]

On Mon, Jun 27, 2016 at 11:28 PM, Bart Bogaerts  wrote:
> I dit some more testing.
>
> It is important for the bug to occur that all submodules are initialised.
> So my suggestion is not yet complete, you need to add the line

Oh right. I was completely oblivious to that.

>
> git submodule update --recursive --init

Adding the initialization makes my test cases fail as well.

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

Re: [PATCH v4] Refactor recv_sideband()

[Nicolas Pitre]

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> And then that made me stare at the patch even more.  We still write
> some error messages to stderr in the updated code (without my crap
> SQUASH) inside "while (!retval)" loop:
> 
>   while (retval == 0) {
>   int band, len;
>   len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
> 0);
>   if (len == 0)
>   break;
>   if (len < 1) {
>   fprintf(stderr, "%s: protocol error: no band 
> designator\n", me);
>   retval = SIDEBAND_PROTOCOL_ERROR;
>   break;
>   }
>   band = buf[0] & 0xff;
>   buf[len] = '\0';
>   len--;
>   switch (band) {
>   case 3:
>   fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
>   retval = SIDEBAND_REMOTE_ERROR;
>   break;
>   case 2:
>   ...
>   while ((brk = strpbrk(b, "\n\r"))) {
>   ...
>   write(2, outbuf.buf, outbuf.len);
>   ...
>   }
> 
>   if (*b)
>   strbuf_addf(, "%s", b);
>   break;
>   case 1:
>   write_or_die(out, buf + 1, len);
>   break;
>   default:
>   fprintf(stderr, "%s: protocol error: bad band #%d\n",
>   me, band);
>   retval = SIDEBAND_PROTOCOL_ERROR;
>   break;
>   }
>   }
> 
>   if (outbuf.len > 0)
>   write(2, outbuf.buf, outbuf.len);
> 
> In general, mixing stdio and raw file descriptor access is not such
> a good idea, but these remaining calls to fprintf(stderr, ...) above
> are for error-exit codepath, so from that point of view, the above
> illustration may be acceptable, but there is still one niggle.
> 
> When we exit the loop because we set retval to a non-zero value,
> should we still drain the outbuf?

I would think so.  Anything that the remote sent before any error should 
be printed nevertheless.  The clue for the error might be in the pending 
buffer.

However in this case the actual error printout and the pending buffer 
will appear reversed.

So what I'd suggest is actually something like this:

if (len < 1) {
strbuf_addf(, "\n%s: protocol error: no band 
designator\n", me);
retval = SIDEBAND_PROTOCOL_ERROR;
break;

And so on for the other error cases.


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

[PATCH v2 1/2] add skip_prefix_mem helper

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

From: Jeff King 

The skip_prefix function has been very useful for
simplifying pointer arithmetic and avoiding repeated magic
numbers, but we have no equivalent for length-limited
buffers. So we're stuck with:

  if (3 <= len && skip_prefix(buf, "foo", ))
  len -= 3;

That's not that complicated, but it needs to use magic
numbers for the length of the prefix (or else write out
strlen("foo"), repeating the string). By using a helper, we
can get the string length behind the scenes (and often at
compile time for string literals).

Signed-off-by: Jeff King 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 git-compat-util.h | 17 +
 1 file changed, 17 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 49d4029..c99cddc 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -473,6 +473,23 @@ static inline int skip_prefix(const char *str, const char 
*prefix,
return 0;
 }
 
+/*
+ * Like skip_prefix, but promises never to read past "len" bytes of the input
+ * buffer, and returns the remaining number of bytes in "out" via "outlen".
+ */
+static inline int skip_prefix_mem(const char *buf, size_t len,
+ const char *prefix,
+ const char **out, size_t *outlen)
+{
+   size_t prefix_len = strlen(prefix);
+   if (prefix_len <= len && !memcmp(buf, prefix, prefix_len)) {
+   *out = buf + prefix_len;
+   *outlen = len - prefix_len;
+   return 1;
+   }
+   return 0;
+}
+
 /*
  * If buf ends with suffix, return 1 and subtract the length of the suffix
  * from *len. Otherwise, return 0 and leave *len untouched.
-- 
2.8.2.531.gd073806

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

Re: What's cooking in git.git (Jun 2016, #05; Thu, 16)

[Junio C Hamano]

Michael Haggerty  writes:

> (I suppose that `REFNAME_ALLOW_ONELEVEL` was
> meant for checking partial reference names, for example to vet "foo"
> that the caller wants to pass to `git branch`, which automatically turns
> it into `refs/heads/foo`.)

Correct.

> In summary, `check_refname_format()` is in some ways less strict than
> `refname_is_safe()`. For example, it allows reference names like
> `git-p4-tmp/6` or even `git-p4-tmp`.

Again, correct.

> I don't know what I was thinking. Long ago, when I refactored and
> documented check_refname_format(), I realized that the checks are
> surprisingly lax and that the `REFNAME_ALLOW_ONELEVEL` option is
> misleading. But I was new to the project and wasn't brave or energetic
> enough to do something about it. Meanwhile I guess I forgot that it
> never got fixed. Commit
>
> d0f810f0 2014-09-03 refs.c: allow listing and deleting badly named refs
>
> , which introduced `refname_is_safe()`, perhaps muddled the situation by
> adding a "fallback" check that is not strictly laxer than the main check.
>
> Where does that leave us?
>
> * It is currently possible to create and delete references with
>   names that are neither within `refs/` nor ALL_CAPS (though not
>   remotely).
>
> * Such references do not participate in reachability checks, so
>   they have to be considered semi-broken.
>
> * Users could create chaos (though not remotely) by creating a
>   loose "reference" whose name coincides with that of another
>   file under `$GIT_DIR`.
>   (`git update-ref objects/info/alternates HEAD` anyone?)

All correct.

> * `git-p4` is apparently the only code within the Git project that
>   was making use of this questionable freedom.

True.

> * Presumably there is user code in the wild that creates and uses
>   such references.

I doubt it, if they value their data.  .git/p4-tmp or .git/p4-tmp/6
are not just "partly broken"--they are just as transient as things
like MERGE_HEAD in that the objects pointed by them are subject to
be pruned.

> I think we need to get this under control, but given that we've allowed
> such references (albeit partly broken) for a long time, we probably need
> to provide an escape hatch to aid the transition. I'm skeptical that the
> mh/split-under-lock patch series is the best place to start such a
> campaign. On the other hand, I don't want that patch series to open up
> any new avenues to creating references that escape `$GIT_DIR`.
>
> Let me think for a bit about the next step. Input is welcome.
>
> In any case, I think that Lars's patch to `git-p4` is a good thing.

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

[PATCH v2 2/2] config: add conditional include

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

Main description is already in config.txt. Here is a dev-only note
about Windows support.

While prepare_include_condition_pattern() is Windows-friendly (because
it does not hard code '/'). The reality could be uglier because
internally get_git_dir() may return a path with '/' only or worse, a
mix of '/' and '\\'.

At some point, we need to teach wildmatch() that '/' and '\' should be
treated the same way (via a flag) as well. Then we could care less
about '/' vs '\\'. But a Windows dev probably has to do it.

Helped-by: Jeff King 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/config.txt  | 40 +
 config.c  | 89 +--
 t/t1305-config-include.sh | 45 
 3 files changed, 172 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 58673cf..c8ad0bf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -91,6 +91,46 @@ found at the location of the include directive. If the value 
of the
 relative to the configuration file in which the include directive was
 found.  See below for examples.
 
+Included files can be grouped into subsections where subsectino name
+is the condition when the files are included. The condition starts
+with an condition type string, followed by a colon and a pattern.
+
+Only "gitdir" type is supported, where files are included if
+`$GIT_DIR` matches the specified pattern. For example,
+
+
+   [include "gitdir:/path/to/foo.git"]
+   path = /path/to/foo.inc
+
+would only include "/path/to/foo.inc" if `$GIT_DIR` is
+/path/to/foo.git.
+
+The following pattern is a wildcard pattern with two additional
+wildcards `**/` and `/**`. See linkgit:gitignore[5] for more
+information. For convenience:
+
+ * If the pattern ends with '/', '**' will be automatically added. For
+   example, the pattern 'foo/' becomes 'foo/**'. In other words, it
+   matches "foo" and everything inside, recursively.
+
+ * If the pattern starts with `~/`, `~` will be substitued with the
+   environment variable `HOME`.
+
+ * If the pattern starts with `./`, it is replaced with the directory
+   where the current config file is. For example if the config file
+   that contains the "include" subsection is `$HOME/.gitconfig` then
+   the pattern `./foo` would match the path `$HOME/foo`
+
+A few more notes:
+
+ * Symlinks in `$GIT_DIR` are not resolved before matching.
+
+ * Note that "../" is not special and will match literally, which is
+   unlikely what you want.
+
+ * On case-insensitive file systems, you may need to specify
+   core.ignoreCase before the `include` subsections in order to match
+   case-insensitively if core.ignoreCase is declared in the same file.
 
 Example
 ~~~
diff --git a/config.c b/config.c
index f51c56b..97c450e 100644
--- a/config.c
+++ b/config.c
@@ -13,6 +13,7 @@
 #include "hashmap.h"
 #include "string-list.h"
 #include "utf8.h"
+#include "dir.h"
 
 struct config_source {
struct config_source *prev;
@@ -140,9 +141,89 @@ static int handle_path_include(const char *path, struct 
config_include_data *inc
return ret;
 }
 
+static int prepare_include_condition_pattern(struct strbuf *pat)
+{
+   struct strbuf path = STRBUF_INIT;
+   int prefix = 0;
+
+   /* TODO: maybe support ~user/ too */
+   if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {
+   const char *home = getenv("HOME");
+
+   if (!home)
+   return error(_("$HOME is not defined"));
+
+   strbuf_add_absolute_path(, home);
+   strbuf_splice(pat, 0, 1, path.buf, path.len);
+   prefix = path.len + 1 /*slash*/;
+   } else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {
+   const char *slash;
+
+   if (!cf || !cf->path)
+   return error(_("relative config include "
+  "conditionals must come from files"));
+
+   /* TODO: escape wildcards */
+   strbuf_add_absolute_path(, cf->path);
+   slash = find_last_dir_sep(path.buf);
+   if (!slash)
+   die("BUG: how is this possible?");
+   strbuf_splice(pat, 0, 1, path.buf, slash - path.buf);
+   prefix = slash - path.buf + 1 /* slash */;
+   } else if (!is_absolute_path(pat->buf))
+   strbuf_insert(pat, 0, "**/", 3);
+
+   if (pat->len && is_dir_sep(pat->buf[pat->len - 1]))
+   strbuf_addstr(pat, "**");
+
+   strbuf_release();
+   return prefix;
+}
+
+static int include_condition_is_true(const char *cond, int cond_len)
+{
+   const char *value;
+   size_t value_len;
+
+   /* no condition (i.e., "include.path") is always true */
+   if (!cond)
+   return 1;
+
+   if (skip_prefix_mem(cond, cond_len, "gitdir:", , _len)) {
+  

[PATCH v2 0/2] Config conditional include

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

Second try. The anchoring rules now look a lot better. I cherry picked
Jeff's skip_prefix_mem() for now to avoid dependency.

There's a surprise about core.ignorecase. We are matching paths, so we
should match case-insensitively if core.ignorecase tells us so. And it
gets a bit tricky if core.ignorecase is defined in the same config
file. I don't think we have ever told the user that keys are processed
from top down. We do now.

It makes me wonder if we should allow users the option to match case
insensitively regardless of filesystem too. Something close to
pathspec magic. But that probably has little use in real world for a
lot more work.

The '/' vs '\\' battle on Windows, I'll leave it to Windows guys again.

I'm also not adding "worktree:" condition. If that happens, it
probably happens in the per-worktree config file series.

To leave room for future expansion, perhaps we should declare that ':'
can't appear in the pattern, so we can add more prefix later, and even
stack them, e.g. "regexp:gitdir:". The prefix can't be one char
long. That should be enough for windows to specify full path
including the drive letter.

Jeff King (1):
  add skip_prefix_mem helper

Nguyễn Thái Ngọc Duy (1):
  config: add conditional include

 Documentation/config.txt  | 40 +
 config.c  | 89 +--
 git-compat-util.h | 17 +
 t/t1305-config-include.sh | 45 
 4 files changed, 189 insertions(+), 2 deletions(-)

-- 
2.8.2.531.gd073806

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

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Junio C Hamano  writes:

> With input from Dscho that recent Git-for-Windows does the right
> thing without limiting us to use only a subset of stdio, perhaps we
> would want to squash something like this in.
>
> diff --git a/sideband.c b/sideband.c
> index 226a8c2..72e2c5c 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -58,13 +58,12 @@ int recv_sideband(const char *me, int in_stream, int out)
>* Append a suffix to each nonempty line to clear the
>* end of the screen line.
>*
> -  * The output is accumulated in a buffer and each line
> -  * is printed to stderr using fprintf() with a single
> -  * conversion specifier. This is a "best effort"
> -  * approach to supporting both inter-process atomicity
> -  * (single conversion specifiers are likely to end up
> -  * in single atomic write() system calls) and the ANSI
> -  * control code emulation under Windows.
> +  * The output is accumulated in a buffer and
> +  * each line is printed to stderr using
> +  * fwrite(3).  This is a "best effort"
> +  * approach to suppor inter-process atomicity
> +  * (single fwrite(3) call is likely to end up
> +  * in single atomic write() system calls).
>*/
>   while ((brk = strpbrk(b, "\n\r"))) {
>   int linelen = brk - b;

That may be good, but the remainder is crap.  Sorry about the typo;
s/output/outbuf/g is needed at least.

> @@ -75,8 +74,7 @@ int recv_sideband(const char *me, int in_stream, int out)
>   } else {
>   strbuf_addf(, "%c", *brk);
>   }
> - fprintf(stderr, "%.*s", (int)outbuf.len,
> - outbuf.buf);
> + fwrite(output.buf, 1, output.len, stderr);
>   strbuf_reset();
>   strbuf_addf(, "%s", PREFIX);
>  
> @@ -98,7 +96,7 @@ int recv_sideband(const char *me, int in_stream, int out)
>   }
>  
>   if (outbuf.len > 0)
> - fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
> + fwrite(output.buf, 1, output.len, stderr);
>   strbuf_release();
>   return retval;
>  }

But we would be better off using the real write(2), now Windows port
does not limit us to stdio.

And then that made me stare at the patch even more.  We still write
some error messages to stderr in the updated code (without my crap
SQUASH) inside "while (!retval)" loop:

while (retval == 0) {
int band, len;
len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX, 
0);
if (len == 0)
break;
if (len < 1) {
fprintf(stderr, "%s: protocol error: no band 
designator\n", me);
retval = SIDEBAND_PROTOCOL_ERROR;
break;
}
band = buf[0] & 0xff;
buf[len] = '\0';
len--;
switch (band) {
case 3:
fprintf(stderr, "%s%s\n", PREFIX, buf + 1);
retval = SIDEBAND_REMOTE_ERROR;
break;
case 2:
...
while ((brk = strpbrk(b, "\n\r"))) {
...
write(2, outbuf.buf, outbuf.len);
...
}

if (*b)
strbuf_addf(, "%s", b);
break;
case 1:
write_or_die(out, buf + 1, len);
break;
default:
fprintf(stderr, "%s: protocol error: bad band #%d\n",
me, band);
retval = SIDEBAND_PROTOCOL_ERROR;
break;
}
}

if (outbuf.len > 0)
write(2, outbuf.buf, outbuf.len);

In general, mixing stdio and raw file descriptor access is not such
a good idea, but these remaining calls to fprintf(stderr, ...) above
are for error-exit codepath, so from that point of view, the above
illustration may be acceptable, but there is still one niggle.

When we exit the loop because we set retval to a non-zero value,
should we still drain the outbuf?

IOW, shouldn't the if statement after "while (!retval)" loop be more
like this?

if (!retval && outbuf.len)
write(2, outbuf.buf, outbuf,len);

Re: Shallow submodule efficiency

[Stefan Beller]

On Mon, Jun 27, 2016 at 10:39 PM, Martin von Gagern
 wrote:
> Hi!
>
> I have the feeling that “git submodule update --depth 1” is less clever
> than it could be. Here is one example I observed with git 2.0.0:

2.9.0 (as "Direct fetching of " is not part of 2.0.0 IIRC) ?

>
>   git init foo
>   cd foo
>   git clone --single-branch \
> -b v0.99 https://github.com/git/git.git git-scm
>   git submodule add https://github.com/git/git.git git-scm
>   git commit -m Submod
>   git clone --dissociate . ../bar
>   cd ../bar
>   git submodule update --init --depth 1 git-scm
>
> This will download quite a bit of history, then result in an error message:
>
>   error: no such remote ref a3eb250f996bf5e12376ec88622c4ccaabf20ea8
>   Fetched in submodule path 'git-scm', but it did not contain
>   a3eb250f996bf5e12376ec88622c4ccaabf20ea8. Direct fetching of that
>   commit failed.

Yeah there are a few things going on, which try to cover up an error
in design IMO.

* The depth is measured from the tip of a branch in the submodule,
   not from the sha1 that the superproject points to.
* Shallowness is treated separately in the superproject and submodules as they
  have a strong notion of being independent. It would be cool to have a thing
  `git clone --recurse-submodules --depth=15
--submodule-depth-as-reachable-from-superproject`
  which would obtain the submodules as shallow as possible, but it
includes all versions that
  the 15 commits in the superproject points to. (may be 1 up to 15
  different non-sequential versions)


>
> That seems so avoidable, since the commit in question is a tag, so it
> would be perfectly possible to fetch that specific commit from the
> server directly. Something like the following commands would do the trick:
>
>   git fetch $url $(git ls-remote $url | \
>awk /$sha1/'{print $2}' | sed 's/\^{}//')
>

* `git submodule update --init --depth 1` is using clone instead of fetch
  currently when the submodule doesn't exist yet. The clone is buried in
  the `submodule--helper update-clone` that is a mixture of listing
the submodules
  and cloning multiple submodules in parallel if possible. So I would
assume it is
  easier to teach git clone to behave correctly and then stop retrying
in git-submodule.sh
  if `just_cloned` is set in the `cmd_update()`.

> If the commit in question is NOT a ref, then whether asking for it by
> unlisted SHA1 is supported will probably depend on the server's
> uploadpack.allowReachableSHA1InWant setting. I guess this is a reason
> why fb43e31 made the fetch for a specific SHA1 a fallback after the
> fetch for the default branch. Nevertheless, in case of “--depth 1” I
> think it would make sense to abort early: if none of the listed refs
> matches the requested one, and asking by SHA1 isn't supported by the
> server, then there is no point in fetching anything, since we won't be
> able to satisfy the submodule requirement either way.

Makes sense! I think the easiest way forward to implement this will be:

* `git clone` learns a (maybe undocumented internal) option `--get-sha1`
  `--branch` looks similar to what we want, but doesn't quite fit as we do not
  know, whether we're on a tag or not. The submodule tells us just the
  recorded sha1, not the branch/tag. So maybe we'd end up calling it
  `--detach-at=`, that will
  -> inspect the ls-remote for the sha1 being there
  -> if the sha1 is there (at least once) clone as if --branch  was given
  -> if not found and the server advertised  allowReachableSHA1InWant,
try again inside the clone

* `submodule--helper update-clone` passes the  `--get-sha1` to the
clones of the submodules

* cmd_update() in git-submodule.sh will only checkout submodules and
not try again
  to fetch them if `just_cloned` is set as the cloning did the best it could.


>
> For the case of “--depth n” with n > 1, I was wondering whether it would
> make sense to prefer the branch listed in submodule.‹name›.branch over
> the default branch.

Makes sense to me.

>
> I think shallow submodules would be very useful to embed libraries into
> projects, without too much care for history (and without the download
> times getting it entails), but with efficient updates to affected files
> only in case of a change in library version. But not being able to get a
> specific tag as a shallow submodule is a major showstopper here, I think.

Thanks for taking your time to point this out and start this discussion!

Thanks,
Stefan

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

Re: [PATCH v4] Refactor recv_sideband()

[Junio C Hamano]

Lukas Fleischer  writes:

> Before this patch, we used character buffer manipulations to split
> messages from the sideband at line breaks and insert "remote: " at the
> beginning of each line, using the packet size to determine the end of a
> message. However, since it is safe to assume that diagnostic messages
> from the sideband never contain NUL characters, we can also
> NUL-terminate the buffer, use strpbrk() for splitting lines and use
> format strings to insert the prefix.
>
> A strbuf is used for accumulating the output which is then printed using
> a single fprintf() call with a single conversion specifier per line,
> such that the atomicity of the output is preserved. See 9ac13ec (atomic
> write for sideband remote messages, 2006-10-11) for details.
>
> Helped-by: Jeff King 
> Helped-by: Junio C Hamano 
> Helped-by: Nicolas Pitre 
> Signed-off-by: Lukas Fleischer 
> ---
> Changes since v3:
> * The new code always frees the strbuf used for the output.
> * Switched back to fprintf() to support ANSI codes under Windows.
> * Added a comment on the tradeoff between atomicity and Windows support.

With input from Dscho that recent Git-for-Windows does the right
thing without limiting us to use only a subset of stdio, perhaps we
would want to squash something like this in.

diff --git a/sideband.c b/sideband.c
index 226a8c2..72e2c5c 100644
--- a/sideband.c
+++ b/sideband.c
@@ -58,13 +58,12 @@ int recv_sideband(const char *me, int in_stream, int out)
 * Append a suffix to each nonempty line to clear the
 * end of the screen line.
 *
-* The output is accumulated in a buffer and each line
-* is printed to stderr using fprintf() with a single
-* conversion specifier. This is a "best effort"
-* approach to supporting both inter-process atomicity
-* (single conversion specifiers are likely to end up
-* in single atomic write() system calls) and the ANSI
-* control code emulation under Windows.
+* The output is accumulated in a buffer and
+* each line is printed to stderr using
+* fwrite(3).  This is a "best effort"
+* approach to suppor inter-process atomicity
+* (single fwrite(3) call is likely to end up
+* in single atomic write() system calls).
 */
while ((brk = strpbrk(b, "\n\r"))) {
int linelen = brk - b;
@@ -75,8 +74,7 @@ int recv_sideband(const char *me, int in_stream, int out)
} else {
strbuf_addf(, "%c", *brk);
}
-   fprintf(stderr, "%.*s", (int)outbuf.len,
-   outbuf.buf);
+   fwrite(output.buf, 1, output.len, stderr);
strbuf_reset();
strbuf_addf(, "%s", PREFIX);
 
@@ -98,7 +96,7 @@ int recv_sideband(const char *me, int in_stream, int out)
}
 
if (outbuf.len > 0)
-   fprintf(stderr, "%.*s", (int)outbuf.len, outbuf.buf);
+   fwrite(output.buf, 1, output.len, stderr);
strbuf_release();
return retval;
 }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Remove obsolete comment from color.h

[Jeff King]

On Tue, Jun 28, 2016 at 06:24:05PM +0200, Johannes Schindelin wrote:

> Essentially, the caveat in color.h that one should use fprintf() and
> printf() when outputting color sequences to a terminal no longer holds
> true. fwrite() or write() work just as well.

Cool. One less thing to worry about.

Thanks for digging into this.

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

Re: [PATCH] Remove obsolete comment from color.h

[Junio C Hamano]

On Tue, Jun 28, 2016 at 9:24 AM, Johannes Schindelin
 wrote:
>
> On Tue, 28 Jun 2016, Junio C Hamano wrote:
>>
>> So as long as we write via stdio to stdout/stderr, you can show
>> colors?  Or is it now stronger, in that as long as we do anything
>> that ends up writing to file descriptors 1 or 2, you can show
>> colors?
>
> Essentially, the caveat in color.h that one should use fprintf() and
> printf() when outputting color sequences to a terminal no longer holds
> true. fwrite() or write() work just as well.

In short, the answer is "the latter"? That is a great news. It means that
Lukas can use write(2) in the recv_sideband() patch to ensure atomicity,
without having to rely on the observed behaviour of fprintf(stderr, "%s", ...)
that as long as the output is within reasonable size the call seems to
result in a single write(2).

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

Re: [PATCH] ./configure.ac: Detect SSL in libcurl using curl-config

[Junio C Hamano]

Дилян Палаузов   writes:

> The API of libcurl does not mention Curl_ssl_init() and when curl is built
> with -flto, the Curl_ssl_init symbol is not exported.
>
> https://curl.haxx.se/libcurl/using/ suggests calling
>   curl-config --feature | grep SSL
> to see, if the installed curl has SSL support.  Another approach would
> be calling curl_version_info and checking the returned struct.
>
> This patch removes the check for the Curl_ssl_init exported symbol from
> libcurl and uses curl-config to detect SSL support in libcurl.
>
> Signed-Off-By: Дилян Палаузов 
> ---

This is a tangent, but the patch made me notice [*1*] that a user
cannot build Git without libcurl support if curl is installed on the
system, with something like:

$ ./configure NO_CURL=NoThanks

I do not know if that is a problem (it certainly is NOT a new
problem introduced by your change).

Anyway, will queue.

Thanks.

[Footnote]

*1* The updated code does not have any branch based on $NO_CURL the
original used to have, even though it does branch on
$NO_OPENSSL, which is why I got curious.

>  configure.ac | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index c279025..5e9ba59 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -528,16 +528,6 @@ AC_CHECK_LIB([curl], [curl_global_init],
>  [NO_CURL=],
>  [NO_CURL=YesPlease])
>  
> -if test -z "${NO_CURL}" && test -z "${NO_OPENSSL}"; then
> -
> -AC_CHECK_LIB([curl], [Curl_ssl_init],
> -[NEEDS_SSL_WITH_CURL=YesPlease],
> -[NEEDS_SSL_WITH_CURL=])
> -
> -GIT_CONF_SUBST([NEEDS_SSL_WITH_CURL])
> -
> -fi
> -
>  GIT_UNSTASH_FLAGS($CURLDIR)
>  
>  GIT_CONF_SUBST([NO_CURL])
> @@ -550,6 +540,17 @@ AC_CHECK_PROG([CURL_CONFIG], [curl-config],
>  
>  if test $CURL_CONFIG != no; then
>  GIT_CONF_SUBST([CURL_CONFIG])
> +if test -z "${NO_OPENSSL}"; then
> +  AC_MSG_CHECKING([if Curl supports SSL])
> +  if test $(curl-config --features|grep SSL) = SSL; then
> + NEEDS_SSL_WITH_CURL=YesPlease
> + AC_MSG_RESULT([yes])
> +  else
> + NEEDS_SSL_WITH_CURL=
> + AC_MSG_RESULT([no])
> +  fi
> +  GIT_CONF_SUBST([NEEDS_SSL_WITH_CURL])
> +fi
>  fi
>  
>  fi
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git svn clone segmentation faul issue

[Johannes Schindelin]

Hi Yannis,

On Tue, 28 Jun 2016, Ioannis Kappas wrote:

> Hi Johanes,

Almost ;-)

> Johannes Schindelin  gmx.de> writes:
> 
> > Would you mind giving them a whirl?
> 
> The patch in "source code (zip)" seems to be missing the line in the prepare
> () section of PKGBUILD to actually apply the fix:
> 
> diff --git a/subversion/PKGBUILD b/subversion/PKGBUILD
> --- a/subversion/PKGBUILD
> +++ b/subversion/PKGBUILD
> 
> @@ -101,6 +103,7 @@ prepare() {
>patch -p1 -i ${srcdir}/17-fix-test-link.patch
>patch -p1 -i ${srcdir}/18-fix-serf-config.patch
>patch -p1 -i ${srcdir}/19-remove-contrib-from-configure.patch
> +  patch -p1 -i ${srcdir}/20-fix-stack-corruption-in-swig-wrappers.patch
>patch -p1 -i ${srcdir}/subversion-1.9.1-msys2.patch
>patch -p1 -i ${srcdir}/remove-checking-symlink.patch
>patch -p1 -i ${srcdir}/90-use-copy-instead-symlink.patch
> 
> 
> Would you be so kind to add the above and rebuild. 

Oh bummer. Sorry for that! I will fix it and rebuild tomorrow.

> I have just tested it locally to work successfully, but I do not mind 
> retesting if you wish to provide another build :)

Thanks, that would be much appreciated! I'll send a reply when I rebuilt
the packages.

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

Re: [PATCH] Remove obsolete comment from color.h

[Johannes Schindelin]

Hi Junio,

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > Originally, ANSI color sequences were supported on Windows only by
> > overriding the printf() and fprintf() functions, as mentioned in e7821d7
> > (Add a notice that only certain functions can print color escape codes,
> > 2009-11-27).
> >
> > As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),
> > however, this is no longer the case, as the ANSI color sequence support
> > code needed to be replaced with a thread-safe version, one side effect
> > being that stdout and stderr handled no matter which function is used to
> > write to it.
> 
> So as long as we write via stdio to stdout/stderr, you can show
> colors?  Or is it now stronger, in that as long as we do anything
> that ends up writing to file descriptors 1 or 2, you can show
> colors?

It means that we override stdout/stderr with a pipe that is parsed for the
color sequences. If stdout or stderr are reopened later, this color
handling will not apply to the new file descriptor/stream.

Essentially, the caveat in color.h that one should use fprintf() and
printf() when outputting color sequences to a terminal no longer holds
true. fwrite() or write() work just as well.

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

Re: [PATCH v2] Refactor recv_sideband()

[Johannes Schindelin]

Hi Junio,

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > Erm, sorry...
> >
> > On Tue, 28 Jun 2016, Johannes Schindelin wrote:
> >
> >> [...] we actually do not override fprintf() at all anymore [*1*] [...]
> >
> > ... forgot the
> >
> > Footnote *1*: In Git for Windows, we actually *do* override fprintf(),
> > thanks to using gettext, but it is *gettext* that is doing the overriding
> > now, not Git.
> 
> That's nice to know but is not particularly actionable.  

Right. It was only meant as a footnote for interested parties. In Git for
Windows, we do not need to override fprintf() to support color any longer.

> Do you mean
> 
>  - We do color correctly without having to override fprintf() these
>days, so feel free and safe to use either fwrite() or write()

This one.

>  - Because we do not override fprintf(), we no longer do colors
> 
>  - Or something in between, e.g. "we don't override fprintf()
>ourselves but gettext does X for us (for unknown value of X), so
>you will get color as long as you do Y (for unknown value of Y)"?
> 
> In other words, based on this piece of knowledge you shared with us,
> what is your recommendation for Lukas's patch to do?

Sorry for having been so obscure. I tried to clarify something that did
not need clarification (it is of nobody's concern that fprintf() is
overridden by gettext).

The short version is: do not worry about using fwrite().

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

Re: git svn clone segmentation faul issue

[Ioannis Kappas]

Hi Johanes,

Johannes Schindelin  gmx.de> writes:

> 
> Hi Ioannis,
> 
> On Tue, 28 Jun 2016, Johannes Schindelin wrote:
> 
> > On Tue, 28 Jun 2016, Ioannis.Kappas  rbs.com wrote:
> > 
> > > Git can fail with a "malformed index nnn" error or cause a 
segmentation
> > > fault when executing the "git svn clone" command. 
> > >
> > > [...]
> > >
> > > Fortunately, a patch has already been submitted to subversion with
> > > (github) revision a074af86c8764404b28ce99d0bedcb668a321408 (at
> > > https://github.com/apache/subversion/commit/a074af86c8) on the trunk 
to
> > > handle this and a couple of other similar cases. But by the looks of 
it
> > > has not been picked up yet in the latest subversion 1.9.4 release or 
the
> > > 1.9.x branch, perhaps because this patch was identified in sanity 
checks
> > > rather than coming out from a perceivable production issue?
> > 
> > This is an excellent analysis and a silver lining on the horizon to
> > resolve those vexing git svn issues we keep having in Git for Windows.
> > 
> > Do you have a test case that is reliably reproducing the issue?
> 
> I hope you do! 

Yes, my codebase always fails on git svn clone with something like:

fatal: malformed index info 100644 a6d6543f625fb4a7200e
error: git-svn died of signal 11


> I patched the MSYS2 build script to apply a074af86c8 before
> compiling, and uploaded the resulting packages for i686 and x86_64
> architectures to
> 
>   https://github.com/dscho/MSYS2-
packages/releases/tag/subversion-1.9.4-2
> 
> Would you mind giving them a whirl?

The patch in "source code (zip)" seems to be missing the line in the prepare
() section of PKGBUILD to actually apply the fix:

diff --git a/subversion/PKGBUILD b/subversion/PKGBUILD
--- a/subversion/PKGBUILD
+++ b/subversion/PKGBUILD

@@ -101,6 +103,7 @@ prepare() {
   patch -p1 -i ${srcdir}/17-fix-test-link.patch
   patch -p1 -i ${srcdir}/18-fix-serf-config.patch
   patch -p1 -i ${srcdir}/19-remove-contrib-from-configure.patch
+  patch -p1 -i ${srcdir}/20-fix-stack-corruption-in-swig-wrappers.patch
   patch -p1 -i ${srcdir}/subversion-1.9.1-msys2.patch
   patch -p1 -i ${srcdir}/remove-checking-symlink.patch
   patch -p1 -i ${srcdir}/90-use-copy-instead-symlink.patch


Would you be so kind to add the above and rebuild. 

I have just tested it locally to work successfully, but I do not mind 
retesting if you wish to provide another build :)

Thanks,
Yannis

> 
> Thanks,
> Johannes
> 




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

Re: [RFC] Native access to Git LFS cache

[Duy Nguyen]

On Tue, Jun 28, 2016 at 3:43 PM, Lars Schneider
 wrote:
>
>> On 28 Jun 2016, at 15:14, Johannes Schindelin  
>> wrote:
>>
>> Hi Duy,
>>
>> On Tue, 28 Jun 2016, Duy Nguyen wrote:
>>
>>> On Tue, Jun 28, 2016 at 11:40 AM, Johannes Schindelin
>>>  wrote:

 On Mon, 27 Jun 2016, Duy Nguyen wrote:

> On Mon, Jun 27, 2016 at 7:38 AM,   wrote:
>> ## Proposed solution
>> Git LFS caches its objects under .git/lfs/objects. Most of the time
>> Git LFS objects are already available in the cache (e.g. if you
>> switch branches back and forth). I implemented these "cache hits"
>> natively in Git.  Please note that this implementation is just a
>> quick and dirty proof of concept. If the Git community agrees that
>> this kind of approach would be acceptable then I will start to work
>> on a proper patch series with cross platform support and unit
>> tests.
>
> Would it be possible to move all this code to a separate daemon?
> Instead of spawning a new process to do the filtering, you send a
> command "convert this" over maybe unix socket and either receive the
> whole result over the socket, or receive a path of the result.

 Unix sockets are not really portable...
>>>
>>> It's the same situation as index-helper. I expect you guys will
>>> replace the transport with named pipe or similar.
>>
>> Yes, I will have to work on that. But I might need to ask for a change in
>> the design if I hit some obstacle there: named pipes are not the same at
>> all as Unix sockets.
>>
>> Read: it will be painful, and not a general solution. So every new Unix
>> socket that you introduce will introduce new problems for me.
>
> Thanks Duy for your suggestion. I considered a daemon, but a daemon makes
> it always harder for the user as the user needs to ensure the daemon is
> running! Plus, Dscho's concerns regarding Windows.
>
> I think the core problem is that we invoke the filter for every file:
> https://github.com/git/git/blob/master/convert.c#L461-L475
>
> Couldn't we start the filter executable at the beginning of the Git process
> and communicate with it via stdin/stdout whenever we hit the Git filter
> code? Would that work?

Yeah if one filter process per one git process still brings
significant perf. gain for you, why not, it's simpler than daemon.
Though you may want to look at Christian's external odb first in the
other mail. Note though that external odb may still spawn process a
lot (because the design is you cache objects locally once and you
don't have to spawn again). Whether that fits in lfs scheme, I have no
idea (I have never used git-lfs myself).

> Alternatively, do you see a way to add a "plugin" system to Git? Where Git
> could be configured to dynamically load a "filter" library?

I don't think plugins as .so files are welcome because it would force
us to freeze some ABI. So far all git extension has always been via an
external process.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] Native access to Git LFS cache

[Duy Nguyen]

On Tue, Jun 28, 2016 at 3:14 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Tue, 28 Jun 2016, Duy Nguyen wrote:
>
>> On Tue, Jun 28, 2016 at 11:40 AM, Johannes Schindelin
>>  wrote:
>> >
>> > On Mon, 27 Jun 2016, Duy Nguyen wrote:
>> >
>> >> On Mon, Jun 27, 2016 at 7:38 AM,   wrote:
>> >> > ## Proposed solution
>> >> > Git LFS caches its objects under .git/lfs/objects. Most of the time
>> >> > Git LFS objects are already available in the cache (e.g. if you
>> >> > switch branches back and forth). I implemented these "cache hits"
>> >> > natively in Git.  Please note that this implementation is just a
>> >> > quick and dirty proof of concept. If the Git community agrees that
>> >> > this kind of approach would be acceptable then I will start to work
>> >> > on a proper patch series with cross platform support and unit
>> >> > tests.
>> >>
>> >> Would it be possible to move all this code to a separate daemon?
>> >> Instead of spawning a new process to do the filtering, you send a
>> >> command "convert this" over maybe unix socket and either receive the
>> >> whole result over the socket, or receive a path of the result.
>> >
>> > Unix sockets are not really portable...
>>
>> It's the same situation as index-helper. I expect you guys will
>> replace the transport with named pipe or similar.
>
> Yes, I will have to work on that. But I might need to ask for a change in
> the design if I hit some obstacle there: named pipes are not the same at
> all as Unix sockets.
>
> Read: it will be painful, and not a general solution. So every new Unix
> socket that you introduce will introduce new problems for me.

I thought we could have a drop-in replacement (or maybe a higher
abstraction that would be sufficient for git). Thanks for pointing it
out.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Remove obsolete comment from color.h

[Junio C Hamano]

Johannes Schindelin  writes:

> Originally, ANSI color sequences were supported on Windows only by
> overriding the printf() and fprintf() functions, as mentioned in e7821d7
> (Add a notice that only certain functions can print color escape codes,
> 2009-11-27).
>
> As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),
> however, this is no longer the case, as the ANSI color sequence support
> code needed to be replaced with a thread-safe version, one side effect
> being that stdout and stderr handled no matter which function is used to
> write to it.

So as long as we write via stdio to stdout/stderr, you can show
colors?  Or is it now stronger, in that as long as we do anything
that ends up writing to file descriptors 1 or 2, you can show
colors?

> So let's just remove the comment that is now obsolete.

Thanks.

> Signed-off-by: Johannes Schindelin 
> ---
> Published-As: https://github.com/dscho/git/releases/tag/winansi-color-v1
>  color.h | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/color.h b/color.h
> index e155d13..b9ead16 100644
> --- a/color.h
> +++ b/color.h
> @@ -18,11 +18,6 @@ struct strbuf;
>   */
>  #define COLOR_MAXLEN 70
>  
> -/*
> - * IMPORTANT: Due to the way these color codes are emulated on Windows,
> - * write them only using printf(), fprintf(), and fputs(). In particular,
> - * do not use puts() or write().
> - */
>  #define GIT_COLOR_NORMAL ""
>  #define GIT_COLOR_RESET  "\033[m"
>  #define GIT_COLOR_BOLD   "\033[1m"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] doc: typeset long command-line options as literal

[Junio C Hamano]

Matthieu Moy  writes:

> Sorry, I forgot to mention when sending the series: this is to be
> applied on top of tr/doc-tt (in next, marked "will merge to master").

Yup, I figured it out and these are queued there.  tr/doc-tt is part
of 'master' now.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's happening to the index

[Matthieu Moy]

"Andy Falanga (afalanga)"  writes:

> After the line calling increlnum is executed, I often have issues with 
> make unable to spawn the next command because it can't read the current 
> directory info.

This may happen if you delete the current directory, even if your
re-create it afterwards. For example:

/tmp/test$ rm -fr /tmp/test && mkdir /tmp/test
/tmp/test$ touch foo
touch: cannot touch ‘foo’: No such file or directory
/tmp/test$ cd /tmp/test
/tmp/test$ touch foo   
/tmp/test$ 

This is unrelated from Git, but maybe you asked Git to delete a
directory (by switching to a branch which doesn't contain a directory
for example).

> If I do: cd .. && cd -; all is well.

This is a typical symptom of the issue above.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Refactor recv_sideband()

[Junio C Hamano]

Johannes Schindelin  writes:

> Erm, sorry...
>
> On Tue, 28 Jun 2016, Johannes Schindelin wrote:
>
>> [...] we actually do not override fprintf() at all anymore [*1*] [...]
>
> ... forgot the
>
> Footnote *1*: In Git for Windows, we actually *do* override fprintf(),
> thanks to using gettext, but it is *gettext* that is doing the overriding
> now, not Git.

That's nice to know but is not particularly actionable.  

Do you mean

 - We do color correctly without having to override fprintf() these
   days, so feel free and safe to use either fwrite() or write()

 - Because we do not override fprintf(), we no longer do colors

 - Or something in between, e.g. "we don't override fprintf()
   ourselves but gettext does X for us (for unknown value of X), so
   you will get color as long as you do Y (for unknown value of Y)"?

In other words, based on this piece of knowledge you shared with us,
what is your recommendation for Lukas's patch to do?

Thanks.

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

What's happening to the index

[Andy Falanga (afalanga)]

Hi,

I'm using git version 1.8.3.1.  I have a process for building RPMs in my 
repository.  The RPMs are versioned using a "build number". In order to 
maintain uniqueness for this build number, I have stored this number to 
a file which exists on only a unique branch.

The build process, for an actual release, is intended to be done by 
branching on a tag.  During that process, a BASH script is called with 
checks out "rpm", fetches the repo from remote, merges with origin/rpm, 
increments the number and pushes back to origin.  It then returns the 
branch to the original branch in which the script was called.  The make 
recipe looks like this:


release:
 make clean
 cd ../..  && ../tools/increlnum && cd -
 if [[ $(TAG) = . ]]; then \
 git checkout -b rpm_build_$(TAG) $(TAG); \
 fi
 make rpm RPM_BUILD_NUM=$(shell git show rpm:./rpm_build_num)

After the line calling increlnum is executed, I often have issues with 
make unable to spawn the next command because it can't read the current 
directory info.  Make stops with errors and I'm done.  I have the branch 
name displayed in my PS1 prompt.  I've noticed that, when make errors at 
this point, the branch isn't displayed.  It's as if the index has become 
unstable (or something similar).  If I do: cd .. && cd -; all is well.

What is the problem (I'd really like to understand) and what might I do 
to correct it?

AndyN�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [RFC] Native access to Git LFS cache

[Christian Couder]

On Tue, Jun 28, 2016 at 3:22 PM, Lars Schneider
 wrote:
>
> @Christian/Peff:
> Is there a place to look for more info about your remote-object-store idea?

You may want to take a look at:

https://github.com/chriscool/git/commits/external-odb

I just updated it and I may send an updated RFC series from this
branch to the list soon.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] Native access to Git LFS cache

[Lars Schneider]

> On 28 Jun 2016, at 15:14, Johannes Schindelin  
> wrote:
> 
> Hi Duy,
> 
> On Tue, 28 Jun 2016, Duy Nguyen wrote:
> 
>> On Tue, Jun 28, 2016 at 11:40 AM, Johannes Schindelin
>>  wrote:
>>> 
>>> On Mon, 27 Jun 2016, Duy Nguyen wrote:
>>> 
 On Mon, Jun 27, 2016 at 7:38 AM,   wrote:
> ## Proposed solution
> Git LFS caches its objects under .git/lfs/objects. Most of the time
> Git LFS objects are already available in the cache (e.g. if you
> switch branches back and forth). I implemented these "cache hits"
> natively in Git.  Please note that this implementation is just a
> quick and dirty proof of concept. If the Git community agrees that
> this kind of approach would be acceptable then I will start to work
> on a proper patch series with cross platform support and unit
> tests.
 
 Would it be possible to move all this code to a separate daemon?
 Instead of spawning a new process to do the filtering, you send a
 command "convert this" over maybe unix socket and either receive the
 whole result over the socket, or receive a path of the result.
>>> 
>>> Unix sockets are not really portable...
>> 
>> It's the same situation as index-helper. I expect you guys will
>> replace the transport with named pipe or similar.
> 
> Yes, I will have to work on that. But I might need to ask for a change in
> the design if I hit some obstacle there: named pipes are not the same at
> all as Unix sockets.
> 
> Read: it will be painful, and not a general solution. So every new Unix
> socket that you introduce will introduce new problems for me.

Thanks Duy for your suggestion. I considered a daemon, but a daemon makes
it always harder for the user as the user needs to ensure the daemon is 
running! Plus, Dscho's concerns regarding Windows.

I think the core problem is that we invoke the filter for every file:
https://github.com/git/git/blob/master/convert.c#L461-L475

Couldn't we start the filter executable at the beginning of the Git process
and communicate with it via stdin/stdout whenever we hit the Git filter 
code? Would that work?

Alternatively, do you see a way to add a "plugin" system to Git? Where Git
could be configured to dynamically load a "filter" library?

@Dscho:
Do you have a recommendation for interprocess communication that works 
without trouble on Windows? 

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

Re: git svn clone segmentation faul issue

[Johannes Schindelin]

Hi Ioannis,

On Tue, 28 Jun 2016, Johannes Schindelin wrote:

> On Tue, 28 Jun 2016, ioannis.kap...@rbs.com wrote:
> 
> > Git can fail with a "malformed index nnn" error or cause a segmentation
> > fault when executing the "git svn clone" command. 
> >
> > [...]
> >
> > Fortunately, a patch has already been submitted to subversion with
> > (github) revision a074af86c8764404b28ce99d0bedcb668a321408 (at
> > https://github.com/apache/subversion/commit/a074af86c8) on the trunk to
> > handle this and a couple of other similar cases. But by the looks of it
> > has not been picked up yet in the latest subversion 1.9.4 release or the
> > 1.9.x branch, perhaps because this patch was identified in sanity checks
> > rather than coming out from a perceivable production issue?
> 
> This is an excellent analysis and a silver lining on the horizon to
> resolve those vexing git svn issues we keep having in Git for Windows.
> 
> Do you have a test case that is reliably reproducing the issue?

I hope you do! I patched the MSYS2 build script to apply a074af86c8 before
compiling, and uploaded the resulting packages for i686 and x86_64
architectures to

https://github.com/dscho/MSYS2-packages/releases/tag/subversion-1.9.4-2

Would you mind giving them a whirl?

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

Re: [RFC] Native access to Git LFS cache

[Lars Schneider]

> On 27 Jun 2016, at 18:09, Junio C Hamano  wrote:
> 
> larsxschnei...@gmail.com writes:
> 
>> Unfortunately that fix helps only with cloning. Any local Git operation
>> that invokes the clean/smudge filter (e.g. switching branches) is still
>> slow.
> 
> Do you know where the slowness comes from?  Does Joey's new
> clean/smudge interface help GitLFS?

I am pretty sure the startup time of the external clean/smudge process
causes the slowness and consequently I don't think Joey's patch would help. 
The following tests makes me believe that:

I ran the same test as in my original email using the repo with 15,000 
LFS files. Instead of the LFS binary I use the fast and simple shell 
built-in `true` command:

$ git -c filter.lfs.smudge=true -c filter.lfs.clean=true clone 
https://github.com/larsxschneider/lfstest-manyfiles.git
$ cd lfstest-manyfiles/
$ time git -c filter.lfs.smudge=true -c filter.lfs.clean=true checkout 
removed-files

real0m47.030s
user0m29.521s
sys 0m16.993s

It still takes 47 seconds to switch the branch. Does this test prove my
point or do you see a flaw in the test?


> You are not likely to get anything that knows that a blob object may
> be named as anything other than SHA-1("blob " + ) to
> Git core.  The remote-object-store idea that was floated by Peff and
> Christian started running with at least maintains that object naming
> property and has a better chance of interacting better with the core,
> but LFS, Annex or anything that would not preserve the object naming
> would not.
> 
> Personally, I view a surrogate blob left by LFS in the tree object
> and filtered via clean/smudge a "smarter" kind of symbolic link that
> points outside what Git controls.  The area outside what Git
> controls is left to be managed by whatever the add-on does; Git
> shouldn't even be aware of how they are structured and/or managed.

I understand and somewhat anticipated your point of view. I will try
to find a less intrusive solution.

@Christian/Peff: 
Is there a place to look for more info about your remote-object-store idea? 

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

Re: [RFC] Native access to Git LFS cache

[Johannes Schindelin]

Hi Duy,

On Tue, 28 Jun 2016, Duy Nguyen wrote:

> On Tue, Jun 28, 2016 at 11:40 AM, Johannes Schindelin
>  wrote:
> >
> > On Mon, 27 Jun 2016, Duy Nguyen wrote:
> >
> >> On Mon, Jun 27, 2016 at 7:38 AM,   wrote:
> >> > ## Proposed solution
> >> > Git LFS caches its objects under .git/lfs/objects. Most of the time
> >> > Git LFS objects are already available in the cache (e.g. if you
> >> > switch branches back and forth). I implemented these "cache hits"
> >> > natively in Git.  Please note that this implementation is just a
> >> > quick and dirty proof of concept. If the Git community agrees that
> >> > this kind of approach would be acceptable then I will start to work
> >> > on a proper patch series with cross platform support and unit
> >> > tests.
> >>
> >> Would it be possible to move all this code to a separate daemon?
> >> Instead of spawning a new process to do the filtering, you send a
> >> command "convert this" over maybe unix socket and either receive the
> >> whole result over the socket, or receive a path of the result.
> >
> > Unix sockets are not really portable...
> 
> It's the same situation as index-helper. I expect you guys will
> replace the transport with named pipe or similar.

Yes, I will have to work on that. But I might need to ask for a change in
the design if I hit some obstacle there: named pipes are not the same at
all as Unix sockets.

Read: it will be painful, and not a general solution. So every new Unix
socket that you introduce will introduce new problems for me.

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

Re: [RFC] Native access to Git LFS cache

[Duy Nguyen]

On Tue, Jun 28, 2016 at 11:40 AM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Mon, 27 Jun 2016, Duy Nguyen wrote:
>
>> On Mon, Jun 27, 2016 at 7:38 AM,   wrote:
>> > ## Proposed solution
>> > Git LFS caches its objects under .git/lfs/objects. Most of the time
>> > Git LFS objects are already available in the cache (e.g. if you switch
>> > branches back and forth). I implemented these "cache hits" natively in
>> > Git.  Please note that this implementation is just a quick and dirty
>> > proof of concept. If the Git community agrees that this kind of
>> > approach would be acceptable then I will start to work on a proper
>> > patch series with cross platform support and unit tests.
>>
>> Would it be possible to move all this code to a separate daemon?
>> Instead of spawning a new process to do the filtering, you send a
>> command "convert this" over maybe unix socket and either receive the
>> whole result over the socket, or receive a path of the result.
>
> Unix sockets are not really portable...

It's the same situation as index-helper. I expect you guys will
replace the transport with named pipe or similar.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ./configure.ac: Detect SSL in libcurl using curl-config

[Дилян Палаузов]

The API of libcurl does not mention Curl_ssl_init() and when curl is built
with -flto, the Curl_ssl_init symbol is not exported.

https://curl.haxx.se/libcurl/using/ suggests calling
  curl-config --feature | grep SSL
to see, if the installed curl has SSL support.  Another approach would
be calling curl_version_info and checking the returned struct.

This patch removes the check for the Curl_ssl_init exported symbol from
libcurl and uses curl-config to detect SSL support in libcurl.

Signed-Off-By: Дилян Палаузов 
---
 configure.ac | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index c279025..5e9ba59 100644
--- a/configure.ac
+++ b/configure.ac
@@ -528,16 +528,6 @@ AC_CHECK_LIB([curl], [curl_global_init],
 [NO_CURL=],
 [NO_CURL=YesPlease])
 
-if test -z "${NO_CURL}" && test -z "${NO_OPENSSL}"; then
-
-AC_CHECK_LIB([curl], [Curl_ssl_init],
-[NEEDS_SSL_WITH_CURL=YesPlease],
-[NEEDS_SSL_WITH_CURL=])
-
-GIT_CONF_SUBST([NEEDS_SSL_WITH_CURL])
-
-fi
-
 GIT_UNSTASH_FLAGS($CURLDIR)
 
 GIT_CONF_SUBST([NO_CURL])
@@ -550,6 +540,17 @@ AC_CHECK_PROG([CURL_CONFIG], [curl-config],
 
 if test $CURL_CONFIG != no; then
 GIT_CONF_SUBST([CURL_CONFIG])
+if test -z "${NO_OPENSSL}"; then
+  AC_MSG_CHECKING([if Curl supports SSL])
+  if test $(curl-config --features|grep SSL) = SSL; then
+ NEEDS_SSL_WITH_CURL=YesPlease
+ AC_MSG_RESULT([yes])
+  else
+ NEEDS_SSL_WITH_CURL=
+ AC_MSG_RESULT([no])
+  fi
+  GIT_CONF_SUBST([NEEDS_SSL_WITH_CURL])
+fi
 fi
 
 fi
-- 
2.9.0

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

Re: git svn clone segmentation faul issue

[Johannes Schindelin]

Hi Ioannis,

On Tue, 28 Jun 2016, ioannis.kap...@rbs.com wrote:

> Git can fail with a "malformed index nnn" error or cause a segmentation
> fault when executing the "git svn clone" command. 
>
> [...]
>
> Fortunately, a patch has already been submitted to subversion with
> (github) revision a074af86c8764404b28ce99d0bedcb668a321408 (at
> https://github.com/apache/subversion/commit/a074af86c8) on the trunk to
> handle this and a couple of other similar cases. But by the looks of it
> has not been picked up yet in the latest subversion 1.9.4 release or the
> 1.9.x branch, perhaps because this patch was identified in sanity checks
> rather than coming out from a perceivable production issue?

This is an excellent analysis and a silver lining on the horizon to
resolve those vexing git svn issues we keep having in Git for Windows.

Do you have a test case that is reliably reproducing the issue?

> Although this issue is not in the Git source code, I thought reporting
> this error here first since it is Git that stresses the subversion perl
> bindings in such a level to cause the failure. Perhaps the patch can be
> applied first in those git binary releases that include subversion (I
> believe Git for windows is such a case) while in the mean time I submit
> this case for consideration in the subversion bug tracking system.

Yes, absolutely. Thank you for reporting this!

> The Royal Bank of Scotland plc. Registered in Scotland No 90312. 

Oh. I am sorry!

> This e-mail message is confidential and for use by the addressee only. 

This footer does not make much sense when sending a mail to a public
mailing list ;-)

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

[PATCH v2 0/7] literal formatting in documentation

[Matthieu Moy]

This should address all comments from Peff.

The series is now based on master (which now contains tr/doc-tt).

Content changes since v1:

* First patch (fix indentation prior to actual change to avoid 'git
  am' warning) is new.

* One buggy replacement (`grep.patternType` -> 'grep.patternType')
  removed.

Other fixes are typos and details in commit messages.

Matthieu Moy (7):
  Documentation/git-mv.txt: fix whitespace indentation
  doc: typeset short command-line options as literal
  doc: typeset long command-line options as literal
  doc: typeset '--' as literal
  doc: typeset long options with argument as literal
  CodingGuidelines: formatting HEAD in documentation
  doc: typeset HEAD and variants as literal

 Documentation/CodingGuidelines   |  5 ++--
 Documentation/config.txt | 40 
 Documentation/diff-config.txt|  2 +-
 Documentation/diff-format.txt|  8 +++
 Documentation/diff-generate-patch.txt|  6 ++---
 Documentation/fetch-options.txt  |  6 ++---
 Documentation/git-am.txt |  4 ++--
 Documentation/git-bisect.txt |  2 +-
 Documentation/git-branch.txt |  6 ++---
 Documentation/git-cat-file.txt   | 12 +-
 Documentation/git-checkout.txt   |  4 ++--
 Documentation/git-cherry-pick.txt|  2 +-
 Documentation/git-clean.txt  |  2 +-
 Documentation/git-commit-tree.txt|  2 +-
 Documentation/git-commit.txt |  4 ++--
 Documentation/git-config.txt | 28 +++---
 Documentation/git-credential-store.txt   |  2 +-
 Documentation/git-cvsimport.txt  | 12 +-
 Documentation/git-cvsserver.txt  | 12 +-
 Documentation/git-daemon.txt |  8 +++
 Documentation/git-describe.txt   |  2 +-
 Documentation/git-diff-index.txt |  4 ++--
 Documentation/git-diff-tree.txt  | 20 
 Documentation/git-difftool.txt   |  2 +-
 Documentation/git-fast-import.txt|  4 ++--
 Documentation/git-fetch-pack.txt |  4 ++--
 Documentation/git-filter-branch.txt  |  6 ++---
 Documentation/git-for-each-ref.txt   |  2 +-
 Documentation/git-fsck.txt   |  2 +-
 Documentation/git-grep.txt   | 10 
 Documentation/git-gui.txt|  2 +-
 Documentation/git-help.txt   |  6 ++---
 Documentation/git-http-push.txt  |  4 ++--
 Documentation/git-interpret-trailers.txt |  2 +-
 Documentation/git-ls-files.txt   |  2 +-
 Documentation/git-ls-tree.txt|  8 +++
 Documentation/git-mktree.txt |  2 +-
 Documentation/git-mv.txt |  4 ++--
 Documentation/git-notes.txt  |  2 +-
 Documentation/git-p4.txt | 22 +-
 Documentation/git-push.txt   |  2 +-
 Documentation/git-rebase.txt | 10 
 Documentation/git-remote.txt | 10 
 Documentation/git-repack.txt |  4 ++--
 Documentation/git-revert.txt |  4 ++--
 Documentation/git-send-email.txt | 24 +--
 Documentation/git-send-pack.txt  | 10 
 Documentation/git-shell.txt  |  4 ++--
 Documentation/git-show-branch.txt|  2 +-
 Documentation/git-show-ref.txt   |  6 ++---
 Documentation/git-svn.txt| 26 ++---
 Documentation/git-tag.txt|  2 +-
 Documentation/git-update-index.txt   | 14 +--
 Documentation/git-web--browse.txt|  2 +-
 Documentation/git.txt| 10 
 Documentation/gitcore-tutorial.txt   |  2 +-
 Documentation/gitdiffcore.txt|  4 ++--
 Documentation/gitk.txt   |  2 +-
 Documentation/gitmodules.txt |  2 +-
 Documentation/gitremote-helpers.txt  | 10 
 Documentation/rev-list-options.txt   |  2 +-
 Documentation/revisions.txt  | 26 ++---
 62 files changed, 228 insertions(+), 227 deletions(-)

-- 
2.8.2.397.gbe91ebf.dirty

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

[PATCH v2 6/7] CodingGuidelines: formatting HEAD in documentation

[Matthieu Moy]

The current practice is:

git/Documentation$ git grep "'HEAD'" | wc -l
24
git/Documentation$ git grep "\`HEAD\`" | wc -l
66

Let's adopt the majority as a guideline.

Signed-off-by: Matthieu Moy 
---
 Documentation/CodingGuidelines | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 7f4769a..4cd95da 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -527,12 +527,13 @@ Writing Documentation:
  or commands:
 
  Literal examples (e.g. use of command-line options, command names,
- configuration and environment variables) must be typeset in monospace (i.e.
- wrapped with backticks):
+ branch names, configuration and environment variables) must be
+ typeset in monospace (i.e. wrapped with backticks):
`--pretty=oneline`
`git rev-list`
`remote.pushDefault`
`GIT_DIR`
+   `HEAD`
 
  An environment variable must be prefixed with "$" only when referring to its
  value and not when referring to the variable itself, in this case there is
-- 
2.8.2.397.gbe91ebf.dirty

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

[PATCH v2 3/7] doc: typeset long command-line options as literal

[Matthieu Moy]

Similarly to the previous commit, use backquotes instead of
forward-quotes, for long options.

This was obtained with:

  perl -pi -e "s/'(--[a-z][a-z=<>-]*)'/\`\$1\`/g" *.txt

and manual tweak to remove false positive in ascii-art (o'--o'--o' to
describe rewritten history).

Signed-off-by: Matthieu Moy 
---
 Documentation/config.txt | 32 
 Documentation/diff-format.txt|  2 +-
 Documentation/diff-generate-patch.txt|  4 ++--
 Documentation/fetch-options.txt  |  6 +++---
 Documentation/git-am.txt |  4 ++--
 Documentation/git-branch.txt |  2 +-
 Documentation/git-cat-file.txt   |  2 +-
 Documentation/git-cherry-pick.txt|  2 +-
 Documentation/git-commit.txt |  2 +-
 Documentation/git-config.txt | 26 +-
 Documentation/git-credential-store.txt   |  2 +-
 Documentation/git-cvsserver.txt  |  2 +-
 Documentation/git-daemon.txt |  6 +++---
 Documentation/git-describe.txt   |  2 +-
 Documentation/git-diff-index.txt |  4 ++--
 Documentation/git-diff-tree.txt  |  6 +++---
 Documentation/git-difftool.txt   |  2 +-
 Documentation/git-fetch-pack.txt |  2 +-
 Documentation/git-filter-branch.txt  |  4 ++--
 Documentation/git-for-each-ref.txt   |  2 +-
 Documentation/git-fsck.txt   |  2 +-
 Documentation/git-grep.txt   |  8 
 Documentation/git-help.txt   |  6 +++---
 Documentation/git-http-push.txt  |  4 ++--
 Documentation/git-interpret-trailers.txt |  2 +-
 Documentation/git-ls-files.txt   |  2 +-
 Documentation/git-p4.txt | 20 ++--
 Documentation/git-push.txt   |  2 +-
 Documentation/git-rebase.txt | 10 +-
 Documentation/git-remote.txt | 10 +-
 Documentation/git-revert.txt |  4 ++--
 Documentation/git-send-email.txt | 24 
 Documentation/git-send-pack.txt  | 10 +-
 Documentation/git-show-branch.txt|  2 +-
 Documentation/git-show-ref.txt   |  6 +++---
 Documentation/git-svn.txt| 26 +-
 Documentation/git-tag.txt|  2 +-
 Documentation/git-update-index.txt   | 14 +++---
 Documentation/git-web--browse.txt|  2 +-
 Documentation/git.txt|  8 
 Documentation/gitcore-tutorial.txt   |  2 +-
 Documentation/gitdiffcore.txt|  4 ++--
 Documentation/gitk.txt   |  2 +-
 Documentation/gitmodules.txt |  2 +-
 Documentation/gitremote-helpers.txt  |  8 
 Documentation/revisions.txt  |  2 +-
 46 files changed, 149 insertions(+), 149 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6d5e5ba..556ed0e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -140,7 +140,7 @@ boolean::
false;; Boolean false can be spelled as `no`, `off`,
`false`, or `0`.
 +
-When converting value to the canonical form using '--bool' type
+When converting value to the canonical form using `--bool` type
 specifier; 'git config' will ensure that the output is "true" or
 "false" (spelled in lowercase).
 
@@ -481,7 +481,7 @@ core.worktree::
If `GIT_COMMON_DIR` environment variable is set, core.worktree
is ignored and not used for determining the root of working tree.
This can be overridden by the `GIT_WORK_TREE` environment
-   variable and the '--work-tree' command-line option.
+   variable and the `--work-tree` command-line option.
The value can be an absolute path or relative to the path to
the .git directory, which is either specified by --git-dir
or GIT_DIR, or automatically discovered.
@@ -779,7 +779,7 @@ core.abbrev::
 add.ignoreErrors::
 add.ignore-errors (deprecated)::
Tells 'git add' to continue adding files when some files cannot be
-   added due to indexing errors. Equivalent to the '--ignore-errors'
+   added due to indexing errors. Equivalent to the `--ignore-errors`
option of linkgit:git-add[1].  `add.ignore-errors` is deprecated,
as it does not follow the usual naming convention for configuration
variables.
@@ -805,9 +805,9 @@ from the original current directory. See 
linkgit:git-rev-parse[1].
 
 am.keepcr::
If true, git-am will call git-mailsplit for patches in mbox format
-   with parameter '--keep-cr'. In this case git-mailsplit will
+   with parameter `--keep-cr`. In this case git-mailsplit will
not remove `\r` from lines ending with `\r\n`. Can be overridden
-   by giving '--no-keep-cr' from the command line.
+   by giving `--no-keep-cr` from the command line.
See linkgit:git-am[1], linkgit:git-mailsplit[1].
 

[PATCH v2 2/7] doc: typeset short command-line options as literal

[Matthieu Moy]

It was common in our documentation to surround short option names with
forward quotes, which renders as italic in HTML. Instead, use backquotes
which renders as monospace. This is one more step toward conformance to
Documentation/CodingGuidelines.

This was obtained with:

  perl -pi -e "s/'(-[a-z])'/\`\$1\`/g" *.txt

Signed-off-by: Matthieu Moy 
---
 Documentation/config.txt  |  8 
 Documentation/diff-config.txt |  2 +-
 Documentation/diff-format.txt |  8 
 Documentation/diff-generate-patch.txt |  4 ++--
 Documentation/git-cat-file.txt| 12 ++--
 Documentation/git-checkout.txt|  4 ++--
 Documentation/git-clean.txt   |  2 +-
 Documentation/git-commit-tree.txt |  2 +-
 Documentation/git-commit.txt  |  2 +-
 Documentation/git-cvsimport.txt   |  6 +++---
 Documentation/git-cvsserver.txt   |  8 
 Documentation/git-diff-tree.txt   | 14 +++---
 Documentation/git-fetch-pack.txt  |  2 +-
 Documentation/git-filter-branch.txt   |  2 +-
 Documentation/git-grep.txt|  2 +-
 Documentation/git-help.txt|  4 ++--
 Documentation/git-ls-tree.txt |  4 ++--
 Documentation/git-mktree.txt  |  2 +-
 Documentation/git-mv.txt  |  2 +-
 Documentation/git-notes.txt   |  2 +-
 Documentation/git-repack.txt  |  4 ++--
 Documentation/git-shell.txt   |  4 ++--
 Documentation/git.txt |  4 ++--
 23 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 626243f..6d5e5ba 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -930,7 +930,7 @@ browser..cmd::
 
 browser..path::
Override the path for the given tool that may be used to
-   browse HTML help (see '-w' option in linkgit:git-help[1]) or a
+   browse HTML help (see `-w` option in linkgit:git-help[1]) or a
working repository in gitweb (see linkgit:git-instaweb[1]).
 
 clean.requireForce::
@@ -1429,9 +1429,9 @@ gitcvs.logFile::
 
 gitcvs.usecrlfattr::
If true, the server will look up the end-of-line conversion
-   attributes for files to determine the '-k' modes to use. If
+   attributes for files to determine the `-k` modes to use. If
the attributes force Git to treat a file as text,
-   the '-k' mode will be left blank so CVS clients will
+   the `-k` mode will be left blank so CVS clients will
treat it as text. If they suppress text conversion, the file
will be set with '-kb' mode, which suppresses any newline munging
the client might otherwise do. If the attributes do not allow
@@ -1501,7 +1501,7 @@ gitweb.snapshot::
See linkgit:gitweb.conf[5] for description.
 
 grep.lineNumber::
-   If set to true, enable '-n' option by default.
+   If set to true, enable `-n` option by default.
 
 grep.patternType::
Set the default matching behavior. Using a value of 'basic', 'extended',
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index f1101c7..d5a5b17 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -105,7 +105,7 @@ diff.orderFile::
 
 diff.renameLimit::
The number of files to consider when performing the copy/rename
-   detection; equivalent to the 'git diff' option '-l'.
+   detection; equivalent to the 'git diff' option `-l`.
 
 diff.renames::
Whether and how Git detects renames.  If set to "false",
diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
index 85b0890..f10fd54 100644
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -46,11 +46,11 @@ That is, from the left to the right:
 . sha1 for "dst"; 0\{40\} if creation, unmerged or "look at work tree".
 . a space.
 . status, followed by optional "score" number.
-. a tab or a NUL when '-z' option is used.
+. a tab or a NUL when `-z` option is used.
 . path for "src"
-. a tab or a NUL when '-z' option is used; only exists for C or R.
+. a tab or a NUL when `-z` option is used; only exists for C or R.
 . path for "dst"; only exists for C or R.
-. an LF or a NUL when '-z' option is used, to terminate the record.
+. an LF or a NUL when `-z` option is used, to terminate the record.
 
 Possible status letters are:
 
@@ -86,7 +86,7 @@ diff format for merges
 --
 
 "git-diff-tree", "git-diff-files" and "git-diff --raw"
-can take '-c' or '--cc' option
+can take `-c` or '--cc' option
 to generate diff output also for merge commits.  The output differs
 from the format described above in the following way:
 
diff --git a/Documentation/diff-generate-patch.txt 
b/Documentation/diff-generate-patch.txt
index c91afee..18608f5 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -2,7 +2,7 @@ Generating patches with -p
 --
 
 

[PATCH v2 4/7] doc: typeset '--' as literal

[Matthieu Moy]

This was obtained with:

  perl -pi -e "s/'--'/\`--\`/g" *.txt

Signed-off-by: Matthieu Moy 
---
 Documentation/git-fast-import.txt   | 4 ++--
 Documentation/git-filter-branch.txt | 2 +-
 Documentation/rev-list-options.txt  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index 644df99..2b76265 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -1056,7 +1056,7 @@ relative-marks::
 no-relative-marks::
 force::
Act as though the corresponding command-line option with
-   a leading '--' was passed on the command line
+   a leading `--` was passed on the command line
(see OPTIONS, above).
 
 import-marks::
@@ -1107,7 +1107,7 @@ options the user may specify to git fast-import itself.
 
 The `` part of the command may contain any of the options
 listed in the OPTIONS section that do not change import semantics,
-without the leading '--' and is treated in the same way.
+without the leading `--` and is treated in the same way.
 
 Option commands must be the first commands on the input (not counting
 feature commands), to give an option command after any non-option
diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index 060ebb3..0a09698 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -197,7 +197,7 @@ to other tags will be rewritten to point to the underlying 
commit.
 ...::
Arguments for 'git rev-list'.  All positive refs included by
these options are rewritten.  You may also specify options
-   such as `--all`, but you must use '--' to separate them from
+   such as `--all`, but you must use `--` to separate them from
the 'git filter-branch' options. Implies <>.
 
 
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4f009d4..c5bd218 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -193,7 +193,7 @@ endif::git-rev-list[]
 
 --stdin::
In addition to the '' listed on the command
-   line, read them from the standard input. If a '--' separator is
+   line, read them from the standard input. If a `--` separator is
seen, stop reading commits and start reading paths to limit the
result.
 
-- 
2.8.2.397.gbe91ebf.dirty

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

[PATCH v2 5/7] doc: typeset long options with argument as literal

[Matthieu Moy]

We previously reformatted '--option' to `--option`. This patch reformats
'--option ' to `--option `. Obtained with:

  perl -pi -e "s/'(--[a-z][a-z=<>-]* <[^>]*>)'/\`\$1\`/g" *.txt

Signed-off-by: Matthieu Moy 
---
 Documentation/git-config.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 79905fb..f163113 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -47,12 +47,12 @@ checks or transformations are performed on the value.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and '--file ' can be
+`--system`, `--global`, `--local` and `--file ` can be
 used to tell the command to read from only that location (see <>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-'--file ' can be used to tell the command to write to
+`--file ` can be used to tell the command to write to
 that location (you can say `--local` but that is the default).
 
 This command will fail with non-zero status upon error.  Some exit
-- 
2.8.2.397.gbe91ebf.dirty

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

[PATCH v2 7/7] doc: typeset HEAD and variants as literal

[Matthieu Moy]

This is an application of the newly added CodingGuidelines to HEAD and
variants like FETCH_HEAD. It was obtained with:

  perl -pi -e "s/'([A-Z_]*HEAD)'/\`\$1\`/g" *.txt

Signed-off-by: Matthieu Moy 
---
 Documentation/git-bisect.txt|  2 +-
 Documentation/git-branch.txt|  4 ++--
 Documentation/git-cvsimport.txt |  6 +++---
 Documentation/git-cvsserver.txt |  2 +-
 Documentation/git-daemon.txt|  2 +-
 Documentation/git-gui.txt   |  2 +-
 Documentation/git-ls-tree.txt   |  4 ++--
 Documentation/git-p4.txt|  2 +-
 Documentation/git-tag.txt   |  2 +-
 Documentation/gitremote-helpers.txt |  2 +-
 Documentation/revisions.txt | 24 
 11 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index d9f960b..2bb9a57 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -358,7 +358,7 @@ OPTIONS
 --no-checkout::
 +
 Do not checkout the new working tree at each iteration of the bisection
-process. Instead just update a special reference named 'BISECT_HEAD' to make
+process. Instead just update a special reference named `BISECT_HEAD` to make
 it point to the commit that should be tested.
 +
 This option may be useful when the test you would perform in each step
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 123144f..1fe7344 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -39,10 +39,10 @@ named commit).  With `--merged`, only branches merged into 
the named
 commit (i.e. the branches whose tip commits are reachable from the named
 commit) will be listed.  With `--no-merged` only branches not merged into
 the named commit will be listed.  If the  argument is missing it
-defaults to 'HEAD' (i.e. the tip of the current branch).
+defaults to `HEAD` (i.e. the tip of the current branch).
 
 The command's second form creates a new branch head named 
-which points to the current 'HEAD', or  if given.
+which points to the current `HEAD`, or  if given.
 
 Note that this will create the new branch, but it will not switch the
 working tree to it; use "git checkout " to switch to the
diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index bbf1c2b..41207a2 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -74,10 +74,10 @@ OPTIONS
akin to the way 'git clone' uses 'origin' by default.
 
 -o ::
-   When no remote is specified (via -r) the 'HEAD' branch
+   When no remote is specified (via -r) the `HEAD` branch
from CVS is imported to the 'origin' branch within the Git
-   repository, as 'HEAD' already has a special meaning for Git.
-   When a remote is specified the 'HEAD' branch is named
+   repository, as `HEAD` already has a special meaning for Git.
+   When a remote is specified the `HEAD` branch is named
remotes//master mirroring 'git clone' behaviour.
Use this option if you want to import into a different
branch.
diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index a1a5234..a336ae5 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -332,7 +332,7 @@ To get a checkout with the Eclipse CVS client:
 3. Browse the 'modules' available. It will give you a list of the heads in
the repository. You will not be able to browse the tree from there. Only
the heads.
-4. Pick 'HEAD' when it asks what branch/tag to check out. Untick the
+4. Pick `HEAD` when it asks what branch/tag to check out. Untick the
"launch commit wizard" to avoid committing the .project file.
 
 Protocol notes: If you are using anonymous access via pserver, just select 
that.
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 7901c54..3c91db7 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -296,7 +296,7 @@ they correspond to these IP addresses.
 selectively enable/disable services per repository::
To enable 'git archive --remote' and disable 'git fetch' against
a repository, have the following in the configuration file in the
-   repository (that is the file 'config' next to 'HEAD', 'refs' and
+   repository (that is the file 'config' next to `HEAD`, 'refs' and
'objects').
 +
 
diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
index 8144527..c1a3e8b 100644
--- a/Documentation/git-gui.txt
+++ b/Documentation/git-gui.txt
@@ -35,7 +35,7 @@ blame::
 
 browser::
Start a tree browser showing all files in the specified
-   commit (or 'HEAD' by default).  Files selected through the
+   commit (or `HEAD` by default).  Files selected through the
browser are opened in the blame viewer.
 
 citool::
diff --git 

[PATCH v2 1/7] Documentation/git-mv.txt: fix whitespace indentation

[Matthieu Moy]

Replace spaces with tabs to avoid a warning when further patches change
these lines.

Signed-off-by: Matthieu Moy 
---
 Documentation/git-mv.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt
index e453132..6dcb8b2 100644
--- a/Documentation/git-mv.txt
+++ b/Documentation/git-mv.txt
@@ -32,10 +32,10 @@ OPTIONS
 --force::
Force renaming or moving of a file even if the target exists
 -k::
-Skip move or rename actions which would lead to an error
+   Skip move or rename actions which would lead to an error
condition. An error happens when a source is neither existing nor
controlled by Git, or when it would overwrite an existing
-file unless '-f' is given.
+   file unless '-f' is given.
 -n::
 --dry-run::
Do nothing; only show what would happen
-- 
2.8.2.397.gbe91ebf.dirty

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

[PATCH v6 1/1] contrib/subtree: Add a test for subtree rebase that loses commits

[David Greene]

From: David A. Greene 

This test merges an external tree in as a subtree, makes some commits
on top of it and splits it back out.  In the process the added commits
are lost or the rebase aborts with an internal error.  The tests are
marked to expect failure so that we don't forget to fix it.

Signed-off-by: David A. Greene 
---

Notes:
Change History:

v1 - Initial version
v2 - Additional tests and code cleanup
v3 - Remove check_equal, mark comments on failure and remove
 test_debug statements
v4 - Send correct v3 test (botched v3)
v5 - Fix use of verbose
v6 - Add individual tests for each potentially dropped commit

 t/t3427-rebase-subtree.sh | 119 ++
 1 file changed, 119 insertions(+)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
new file mode 100755
index 000..3780877
--- /dev/null
+++ b/t/t3427-rebase-subtree.sh
@@ -0,0 +1,119 @@
+#!/bin/sh
+
+test_description='git rebase tests for -Xsubtree
+
+This test runs git rebase and tests the subtree strategy.
+'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+commit_message() {
+   git log --pretty=format:%s -1 "$1"
+}
+
+test_expect_success 'setup' '
+   test_commit README &&
+   mkdir files &&
+   (
+   cd files &&
+   git init &&
+   test_commit master1 &&
+   test_commit master2 &&
+   test_commit master3
+   ) &&
+   git fetch files master &&
+   git branch files-master FETCH_HEAD &&
+   git read-tree --prefix=files_subtree files-master &&
+   git checkout -- files_subtree &&
+   tree=$(git write-tree) &&
+   head=$(git rev-parse HEAD) &&
+   rev=$(git rev-parse --verify files-master^0) &&
+   commit=$(git commit-tree -p $head -p $rev -m "Add subproject master" 
$tree) &&
+   git update-ref HEAD $commit &&
+   (
+   cd files_subtree &&
+   test_commit master4
+   ) &&
+   test_commit files_subtree/master5
+'
+
+# FAILURE: Does not preserve master4.
+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
+   reset_rebase &&
+   git checkout -b rebase-preserve-merges-4 master &&
+   git filter-branch --prune-empty -f --subdirectory-filter files_subtree 
&&
+   git commit -m "Empty commit" --allow-empty &&
+   git rebase -Xsubtree=files_subtree --preserve-merges --onto 
files-master master &&
+   verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
+'
+
+# FAILURE: Does not preserve master5.
+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
+   reset_rebase &&
+   git checkout -b rebase-preserve-merges-5 master &&
+   git filter-branch --prune-empty -f --subdirectory-filter files_subtree 
&&
+   git commit -m "Empty commit" --allow-empty &&
+   git rebase -Xsubtree=files_subtree --preserve-merges --onto 
files-master master &&
+   verbose test "$(commit_message HEAD)" = "files_subtree/master5"
+'
+
+# FAILURE: Does not preserve master4.
+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto 
commit 4' '
+   reset_rebase &&
+   git checkout -b rebase-keep-empty-4 master &&
+   git filter-branch --prune-empty -f --subdirectory-filter files_subtree 
&&
+   git commit -m "Empty commit" --allow-empty &&
+   git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges 
--onto files-master master &&
+   verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
+'
+
+# FAILURE: Does not preserve master5.
+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto 
commit 5' '
+   reset_rebase &&
+   git checkout -b rebase-keep-empty-5 master &&
+   git filter-branch --prune-empty -f --subdirectory-filter files_subtree 
&&
+   git commit -m "Empty commit" --allow-empty &&
+   git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges 
--onto files-master master &&
+   verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
+'
+
+# FAILURE: Does not preserve Empty.
+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto 
empty commit' '
+   reset_rebase &&
+   git checkout -b rebase-keep-empty-empty master &&
+   git filter-branch --prune-empty -f --subdirectory-filter files_subtree 
&&
+   git commit -m "Empty commit" --allow-empty &&
+   git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges 
--onto files-master master &&
+   verbose test "$(commit_message HEAD)" = "Empty commit"
+'
+
+# FAILURE: fatal: Could not parse object
+test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
+   reset_rebase &&
+   git checkout -b rebase-onto-4 master &&
+   git filter-branch --prune-empty -f --subdirectory-filter files_subtree 
&&
+   git commit -m "Empty commit" 

Re: [PATCH v5 1/1] contrib/subtree: Add a test for subtree rebase that loses commits

[David A. Greene]

Junio C Hamano  writes:

> gree...@obbligato.org (David A. Greene) writes:
>
>>> I also notice that files_subtree/master4 does not appear in any of
>>> the verification in the three tests that use the history being
>>> prepared here, i.e. if master4 is silently dropped while master5 is
>>> kept, such a bug won't be caught by them.
>>
>> Ah, good catch.  I should add a test for that.
>>
>> Let me do a re-roll of this since I think you bring up some excellent
>> points.  Might be a few days due to work obbligations.
>
> A friendly ping to see if I missed anything that happened after this
> message...

Just sent it.  I guess it took more than a few days... :)

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

Re: [PATCH] contrib/subtree: Remove --annotate

[David A. Greene]

Junio C Hamano  writes:

> gree...@obbligato.org (David A. Greene) writes:
>
>> Just to clarify, what is the expectation of things in contrib?
>> Basically the same as other code?
>
> That heavily depends on your exit strategy.
>
> If the aspiration is to move up to exit, then the quality and
> stability expectation is basically the same as stuff in core, and we
> need to strive to keep it stable and high quality.

This is the strategy I was planning to pursue.  After extensive
experience with git-subtree and some local enhancements I have in
real-world work, I am convinced it is a great complementary tool to
git-submodule.  It seems odd to me to have one in core and one not.

>  * If the integration between "git subtree" and the rest of the
>system is loose (in other words, if your improved version of "git
>subtree" taken from Git 2.8 is dropped into an even newer version
>of Git 2.13, or an older version like Git 2.4 for that matter, is
>it expected to work, given the promise of interface stability git
>core gives you?), there is not much technical reason why it must
>stay in core.  Of course, your improvements may need to take
>advantage of improvements on the core side and your new "git
>subtree" may start to require at least Git 2.8, or you may even
>send patches to the core side to extend and enhance the services
>you use from the core side, but as long as that happens only
>occasionally and the dependency does not require lock-step
>upgrade, we can still call such an integration "loose" and moving
>out will still be a viable possibility.

The enhancements to git-subtree that I have and/or am planning to
implement will probably require some changes to core, mostly bugfixes.
Some of the rebase tests I've sent are heading in that direction.  They
are problems I discovered while trying to enhance subtree.

>  * If you expect the pace of improvement would be far faster than
>the release schedule of git core (usually a cycle lasts for 8 to
>12 weeks), moving out would give users a shorter turnaround for
>getting new and improved "git subtree".

I don't think this is a concern.

>  * It may even turn out that the users are a lot more tolerant for
>instability (e.g. removal of rarely used features) in "git
>subtree" than they require the git core proper to be stable, in
>which case moving up (rather than moving out) to apply the same
>stability requirement to "git subtree" as the rest of the system
>would be undesirable.

That's a fair point.  Besides than removing this --annotate option, I
anticipate two other potentially breaking changes:

1. Reorganizing metadata to be more useful - The metadata tags are
   somewhat misleading at the moment and there is additional metadata
   I've thought about adding.

2. Changing the split algorithm to reuse more of git core -
   Specifically, I would like to leverage filter-branch to eliminate a
   bunch of custom code in the split algorithm.  In fact doing so would
   fix a couple of bugs that have come in.  My intent for this change is
   to not alter the resulting history from what split does now (except
   fixing the known bugs) but I can't absolutely guarantee that will be
   the case until I implement it and try it out.

>  * Moving up and staying in has a big social implication. It gives
>the version that comes with git core an appearance of being
>authoritative, even when other people fork the project.
>
>- This discourages incompatible forks (e.g. when one such fork
>  finds the need to improve the "metadata" left by merge
>  operation and used by split, the resulting repository managed
>  by it may no longer usable by other variants of "git subtree",
>  and if there is one in-tree "authoritative" one that is
>  maintained, such a fork will not get wide adoption without
>  taking compatibility issues into account).

Other than the metadata rework mentioned above, I personally don't
anticipate a lot of change to it.  Some ideas have come in from
elsewhere but I'm not yet convinced they're necessary.  My guess is that
any future metadata changes will be more for convenience than any core
fnctionality.  Thus, they could be added in a backward-compatible way.

>- On the other hand, if the "authoritative" one moves too slowly,
>  that may hinder progress.  An exit by "moving out" to become
>  one of the projects that help people's Git-life would result in
>  two or more honestly competing forks of "git subtree", which
>  might give users a better end-result after a few years, even
>  though the users who happened to have picked the losing side
>  during these few years may end up having to rewrite the
>  history.

That's an important point, especially given the time constraints I have.
Despite all my efforts, work is still taking the majority of my coding
time.  That may improve given some other 

Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp

[Michael Haggerty]

On 06/27/2016 09:26 AM, larsxschnei...@gmail.com wrote:
> Git-P4 used to place temporary refs under "git-p4-tmp". Since 3da1f37
> Git checks that all refs are placed under "ref". Instruct Git-P4 to
> place temporary refs under "ref/git-p4-tmp". There are no backwards
> compatibility considerations as these refs are transient.
> 
> All refs under "ref" are shared across all worktrees. This is not
> desired for temporary Git-P4 refs and will be adressed in a later patch.

Thanks for working on this, Lars! Your change looks about what I would
expect, and hits the three places in the source where the string
`git-p4-tmp` appears, so it seems like a very plausible fix.

Michael

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

Re: [PATCH v13 21/20] unix-socket.c: add stub implementation when unix sockets are not supported

[Johannes Schindelin]

Hi Duym

On Mon, 27 Jun 2016, Duy Nguyen wrote:

> On Mon, Jun 27, 2016 at 2:14 PM, Johannes Schindelin
>  wrote:
> >
> > On Sun, 26 Jun 2016, Nguyễn Thái Ngọc Duy wrote:
> >
> >> This keeps #ifdef at the callee instead of caller, it's less messier.
> >>
> >> The caller in question is in read-cache.c which, unlike other
> >> unix-socket callers so far, is always built regardless of unix socket
> >> support. No extra handling (for ENOSYS) is needed because in this
> >> build, index-helper does not exist, $GIT_DIR/index-helper.sock does
> >> not exist, so no unix socket call is made by read-cache.c in the first
> >> place.
> >>
> >> Signed-off-by: Nguyễn Thái Ngọc Duy 
> >> ---
> >
> > Heh, I made something very similar (although I did not update the errno as
> > your patch does): https://github.com/git-for-windows/git/commit/919cb1d79
> 
> Yours lacks the important "else" line in Makefile, so pick mine! :-D

Oh, I definitely prefer your patch over mine. I just meant to concur that
this needs fixing.

Ciao,
Dscho

Re: [PATCH v2] Refactor recv_sideband()

[Johannes Schindelin]

Erm, sorry...

On Tue, 28 Jun 2016, Johannes Schindelin wrote:

> [...] we actually do not override fprintf() at all anymore [*1*] [...]

... forgot the

Footnote *1*: In Git for Windows, we actually *do* override fprintf(),
thanks to using gettext, but it is *gettext* that is doing the overriding
now, not Git.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Refactor recv_sideband()

[Johannes Schindelin]

Hi Peff,

On Mon, 27 Jun 2016, Jeff King wrote:

> On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote:
> 
> > It's just you used xwrite() there that introduced a different issue.
> > Wouldn't replacing it with fwrite(stderr) without changing anything
> > else solve that?
> 
> I am having trouble actually seeing how the ANSI-emulation code gets
> triggered, but the comment in color.h implies that it is only printf,
> fprintf, and fputs that have the desired effect. So fwrite() may not be
> sufficient, and we may need fprintf("%.*s", len, buf) or something.

I meant to clarify how fprintf() works as oposed to fwrite(), so I had a
look and came up empty-handed. I dug a bit further and it turns out that
we actually do not override fprintf() at all anymore [*1*], so I just sent
out a patch to remove the now-obsolete comment.

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

[PATCH] Remove obsolete comment from color.h

[Johannes Schindelin]

Originally, ANSI color sequences were supported on Windows only by
overriding the printf() and fprintf() functions, as mentioned in e7821d7
(Add a notice that only certain functions can print color escape codes,
2009-11-27).

As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),
however, this is no longer the case, as the ANSI color sequence support
code needed to be replaced with a thread-safe version, one side effect
being that stdout and stderr handled no matter which function is used to
write to it.

So let's just remove the comment that is now obsolete.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/winansi-color-v1
 color.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/color.h b/color.h
index e155d13..b9ead16 100644
--- a/color.h
+++ b/color.h
@@ -18,11 +18,6 @@ struct strbuf;
  */
 #define COLOR_MAXLEN 70
 
-/*
- * IMPORTANT: Due to the way these color codes are emulated on Windows,
- * write them only using printf(), fprintf(), and fputs(). In particular,
- * do not use puts() or write().
- */
 #define GIT_COLOR_NORMAL   ""
 #define GIT_COLOR_RESET"\033[m"
 #define GIT_COLOR_BOLD "\033[1m"
-- 
2.9.0.118.g0e1a633

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