diff --color-words breaks spacing

[Phil Hord]

I noticed some weird spacing when comparing files with git diff
--color-words.  The space before a colored word disappears sometimes.

$ git --version
git version 2.11.0.485.g4e59582ff

echo "FOO foo; foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b
FOOfoo; foo = barbaz

There should be a space after FOO in the diff, even if git doesn't
think "foo" and "foo;" are the same word.

If I remove the semicolon, it looks better, but in fact it only moves
the error later. The missing space is now between the two "foo" words.

echo "FOO foo foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b
FOO foofoo = barbaz

Here's the same with the color codes changed to text for purposes of this email:

echo "FOO foo; foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b | tr  \\033 E
FOOE[31mfoo;E[m foo = E[31mbarE[mE[32mbazE[m

echo "FOO foo foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b | tr  \\033 E
FOO fooE[31mfooE[m = E[31mbarE[mE[32mbazE[m

Re: [DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)

[René Scharfe]

Am 23.01.2017 um 20:07 schrieb Junio C Hamano:
> René Scharfe  writes:
> 
>> Implement qsort_s() as a wrapper to the GNU version of qsort_r(1) and
>> use it on Linux.  Performance increases slightly:
>>
>> Test HEAD^ HEAD
>> 
>> 0071.2: sort(1)  0.10(0.20+0.02)   0.10(0.21+0.01) +0.0%
>> 0071.3: string_list_sort()   0.17(0.15+0.01)   0.16(0.15+0.01) -5.9%
>>
>> Additionally the unstripped size of compat/qsort_s.o falls from 24576
>> to 16544 bytes in my build.
>>
>> IMHO these savings aren't worth the increased complexity of having to
>> support two implementations.
> 
> I do worry about having to support more implementations in the
> future that have different function signature for the comparison
> callbacks, which will make things ugly, but this addition alone
> doesn't look too bad to me.

It is unfair of me to show a 5% speedup and then recommend to not
include it. ;-)  That difference won't be measurable in real use cases
and the patch is not necessary.  This patch is simple, but the overall
complexity (incl. #ifdefs etc.) will be lower without it.

But here's another one, with even higher performance and with an even
bigger recommendation to not include it! :)  It veers off into another
direction: Parallel execution.  It requires thread-safe comparison
functions, which might surprise callers.  The value 1000 for the minimum
number of items before threading kicks in is just a guess, not based on
measurements.  So it's quite raw -- and I'm not sure why it's still a
bit slower than sort(1).

Test HEAD^ HEAD
-
0071.2: sort(1)  0.10(0.18+0.03)   0.10(0.20+0.02) +0.0%
0071.3: string_list_sort()   0.17(0.14+0.02)   0.11(0.18+0.02) -35.3%

---
 compat/qsort_s.c | 76 ++--
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/compat/qsort_s.c b/compat/qsort_s.c
index 52d1f0a73d..1304a089af 100644
--- a/compat/qsort_s.c
+++ b/compat/qsort_s.c
@@ -1,4 +1,5 @@
 #include "../git-compat-util.h"
+#include "../thread-utils.h"
 
 /*
  * A merge sort implementation, simplified from the qsort implementation
@@ -6,29 +7,58 @@
  * Added context pointer, safety checks and return value.
  */
 
-static void msort_with_tmp(void *b, size_t n, size_t s,
-  int (*cmp)(const void *, const void *, void *),
-  char *t, void *ctx)
+#define MIN_ITEMS_FOR_THREAD 1000
+
+struct work {
+   int nr_threads;
+   void *base;
+   size_t nmemb;
+   size_t size;
+   char *tmp;
+   int (*cmp)(const void *, const void *, void *);
+   void *ctx;
+};
+
+static void *msort_with_tmp(void *work)
 {
+   struct work one, two, *w = work;
char *tmp;
char *b1, *b2;
size_t n1, n2;
+   size_t s, n;
 
-   if (n <= 1)
-   return;
+   if (w->nmemb <= 1)
+   return NULL;
 
-   n1 = n / 2;
-   n2 = n - n1;
-   b1 = b;
-   b2 = (char *)b + (n1 * s);
+   one = two = *w;
+   one.nr_threads /= 2;
+   two.nr_threads -= one.nr_threads;
+   n = one.nmemb;
+   s = one.size;
+   n1 = one.nmemb = n / 2;
+   n2 = two.nmemb = n - n1;
+   b1 = one.base;
+   b2 = two.base = b1 + n1 * s;
+   two.tmp += n1 * s;
 
-   msort_with_tmp(b1, n1, s, cmp, t, ctx);
-   msort_with_tmp(b2, n2, s, cmp, t, ctx);
+#ifndef NO_PTHREADS
+   if (one.nr_threads && n > MIN_ITEMS_FOR_THREAD) {
+   pthread_t thread;
+   int err = pthread_create(, NULL, msort_with_tmp, );
+   msort_with_tmp();
+   if (err || pthread_join(thread, NULL))
+   msort_with_tmp();
+   } else
+#endif
+   {
+   msort_with_tmp();
+   msort_with_tmp();
+   }
 
-   tmp = t;
+   tmp = one.tmp;
 
while (n1 > 0 && n2 > 0) {
-   if (cmp(b1, b2, ctx) <= 0) {
+   if (one.cmp(b1, b2, one.ctx) <= 0) {
memcpy(tmp, b1, s);
tmp += s;
b1 += s;
@@ -42,7 +72,8 @@ static void msort_with_tmp(void *b, size_t n, size_t s,
}
if (n1 > 0)
memcpy(tmp, b1, n1 * s);
-   memcpy(b, t, (n - n2) * s);
+   memcpy(one.base, one.tmp, (n - n2) * s);
+   return NULL;
 }
 
 int git_qsort_s(void *b, size_t n, size_t s,
@@ -50,20 +81,29 @@ int git_qsort_s(void *b, size_t n, size_t s,
 {
const size_t size = st_mult(n, s);
char buf[1024];
+   struct work w;
 
if (!n)
return 0;
if (!b || !cmp)
return -1;
 
+   w.nr_threads = online_cpus();
+   w.base = b;
+   w.nmemb = n;
+   w.size = s;
+   w.cmp = cmp;
+   

Re: [PATCH v2 0/5] string-list: make string_list_sort() reentrant

[René Scharfe]

Am 24.01.2017 um 00:54 schrieb Jeff King:

The speed looks like a reasonable outcome. I'm torn on the qsort_r()
demo patch. I don't think it looks too bad. OTOH, I don't think I would
want to deal with the opposite-argument-order versions.


The code itself may look OK, but it's not really necessary and the 
special implementation for Linux makes increases maintenance costs.  Can 
we save it for later and first give the common implemention a chance to 
prove itself?



Is there any interest in people adding the ISO qsort_s() to their libc
implementations? It seems like it's been a fair number of years by now.


https://sourceware.org/ml/libc-alpha/2014-12/msg00513.html is the last 
post mentioning qsort_s on the glibc mailing list, but it didn't even 
make it into https://sourceware.org/glibc/wiki/Development_Todo/Master.
Not sure what's planned in BSD land, didn't find anything (but didn't 
look too hard).


René

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Phil Hord]

On Tue, Jan 24, 2017 at 1:54 AM Stefan Hajnoczi  wrote:
> > The use of "git show" you are demonstrating is still about showing
> > the commit object, whose behaviour is defined to show the log
> > message and the diff relative to its sole parent, limited to the
> > paths that match the pathspec.
> >
> > It is perfectly fine and desirable that "git show :"
> > and "git show  -- " behaves differently.  These are
> > two completely different features.
>
> Thanks for explaining guys.  It all makes logical sense.  I just hope I
> remember the distinctions in that table for everyday git use.


Familiar itch; previous discussion here:

https://public-inbox.org/git/1377528372-31206-1-git-send-email-ho...@cisco.com/

Phil

[PATCH v2 0/4] urlmatch: allow regexp-based matches

[Patrick Steinhardt]

Hi,

This is version two of my patch series.

The use case is to be able to configure an HTTP proxy for all
subdomains of a domain where there are hundreds of subdomains.

Previously, I have been using complete regular expressions with
an escape-mechanism to match the configuration key's URLs.
According to Junio's comments, I changed this mechanism to a much
simpler one, where the user is only allowed to use globbing for
the host part of the URL. That is a user can now specify a key
`http.https://*.example.com` to match all sub-domains of
`example.com`. For now I've decided to implement it such that a
single `*` matches a single subdomain only, so for example
`https://foo.bar.example.com` would not match in this case. This
is similar to how shell-globbing works usually, so it should not
be of much surprise. It's also highlighted in the documentation.

I did not include an interdiff as too much has changed between
the two versions.

Regards
Patrick

Patrick Steinhardt (4):
  mailmap: add Patrick Steinhardt's work address
  urlmatch: enable normalization of URLs with globs
  urlmatch: split host and port fields in `struct url_info`
  urlmatch: allow globbing for the URL host part

 .mailmap |  1 +
 Documentation/config.txt |  5 +++-
 t/t1300-repo-config.sh   | 36 +
 urlmatch.c   | 68 +---
 urlmatch.h   |  9 ---
 5 files changed, 104 insertions(+), 15 deletions(-)

-- 
2.11.0

[PATCH v2 1/4] mailmap: add Patrick Steinhardt's work address

[Patrick Steinhardt]

Signed-off-by: Patrick Steinhardt 
---
 .mailmap | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index 9c87a3840..ea59205b9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -177,6 +177,7 @@ Paolo Bonzini  
 Pascal Obry  
 Pascal Obry  
 Pat Notz  
+Patrick Steinhardt  
 Paul Mackerras  
 Paul Mackerras  
 Peter Baumann  

-- 
2.11.0

Re: [PATCH v2 4/4] urlmatch: allow globbing for the URL host part

[Philip Oakley]

From: "Patrick Steinhardt" 

a quick comment on the documentation part ..


The URL matching function computes for two URLs whether they match not.
The match is performed by splitting up the URL into different parts and
then doing an exact comparison with the to-be-matched URL.

The main user of `urlmatch` is the configuration subsystem. It allows to
set certain configurations based on the URL which is being connected to
via keys like `http..*`. A common use case for this is to set
proxies for only some remotes which match the given URL. Unfortunately,
having exact matches for all parts of the URL can become quite tedious
in some setups. Imagine for example a corporate network where there are
dozens or even hundreds of subdomains, which would have to be configured
individually.

This commit introduces the ability to use globbing in the host-part of
the URLs. A user can simply specify a `*` as part of the host name to
match all subdomains at this level. For example adding a configuration
key `http.https://*.example.com.proxy` will match all subdomains of
`https://example.com`.

Signed-off-by: Patrick Steinhardt 
---
Documentation/config.txt |  5 -
t/t1300-repo-config.sh   | 36 
urlmatch.c   | 38 ++
3 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 506431267..a78921c2b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1914,7 +1914,10 @@ http..*::
  must match exactly between the config key and the URL.

. Host/domain name (e.g., `example.com` in `https://example.com/`).
-  This field must match exactly between the config key and the URL.
+  This field must match between the config key and the URL. It is
+  possible to use globs in the config key to match all subdomains, e.g.
+  `https://*.example.com/` to match all subdomains of `example.com`. Note
+  that a glob only every matches a single part of the hostname.


[s/every/ever/ ?]

the "match all subdomains" appears to contradict the "a glob only ever 
matches a single part ".


Maybe borrow the example from the 0/4 cover letter
"so for example `https://foo.bar.example.com` would not match in the case of 
`http.https://*.example.com` " (If I understood it correctly.


A simple example often clarifies much better than more words.
--
Philip



. Port number (e.g., `8080` in `http://example.com:8080/`).
  This field must match exactly between the config key and the URL.
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a2..ec545e092 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,6 +1177,42 @@ test_expect_success 'urlmatch' '
 test_cmp expect actual
'

+test_expect_success 'glob-based urlmatch' '
+ cat >.git/config <<-\EOF &&
+ [http]
+ sslVerify
+ [http "https://*.example.com;]
+ sslVerify = false
+ cookieFile = /tmp/cookie.txt
+ EOF
+
+ test_expect_code 1 git config --bool --get-urlmatch doesnt.exist 
https://good.example.com >actual &&

+ test_must_be_empty actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://example.com 
 >actual &&

+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://good-example.com 
 >actual &&

+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.sslverify 
https://deep.nested.example.com >actual &&

+ test_cmp expect actual &&
+
+ echo false >expect &&
+ git config --bool --get-urlmatch http.sslverify https://good.example.com 
 >actual &&

+ test_cmp expect actual &&
+
+ {
+ echo http.cookiefile /tmp/cookie.txt &&
+ echo http.sslverify false
+ } >expect &&
+ git config --get-urlmatch HTTP https://good.example.com >actual &&
+ test_cmp expect actual
+'
+
# good section hygiene
test_expect_failure 'unsetting the last key in a section removes header' '
 cat >.git/config <<-\EOF &&
diff --git a/urlmatch.c b/urlmatch.c
index e328905eb..53ff972a6 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,6 +63,38 @@ static int append_normalized_escapes(struct strbuf 
*buf,

 return 1;
}

+static int match_host(const struct url_info *url_info,
+   const struct url_info *pattern_info)
+{
+ char *url = xmemdupz(url_info->url + url_info->host_off, 
url_info->host_len);
+ char *pat = xmemdupz(pattern_info->url + pattern_info->host_off, 
pattern_info->host_len);

+ char *url_tok, *pat_tok, *url_save, *pat_save;
+ int matching;
+
+ url_tok = strtok_r(url, ".", _save);
+ pat_tok = strtok_r(pat, ".", _save);
+
+ for (; url_tok && pat_tok; url_tok = strtok_r(NULL, ".", _save),
+pat_tok = strtok_r(NULL, ".", _save)) {
+ if (!strcmp(pat_tok, "*"))
+ continue; /* a simple glob matches everything */
+
+ if (strcmp(url_tok, pat_tok)) {
+ /* subdomains do not match */
+ matching = 0;
+ break;
+ }
+ }
+
+ /* matching if both URL and 

[PATCH v2 4/4] urlmatch: allow globbing for the URL host part

[Patrick Steinhardt]

The URL matching function computes for two URLs whether they match not.
The match is performed by splitting up the URL into different parts and
then doing an exact comparison with the to-be-matched URL.

The main user of `urlmatch` is the configuration subsystem. It allows to
set certain configurations based on the URL which is being connected to
via keys like `http..*`. A common use case for this is to set
proxies for only some remotes which match the given URL. Unfortunately,
having exact matches for all parts of the URL can become quite tedious
in some setups. Imagine for example a corporate network where there are
dozens or even hundreds of subdomains, which would have to be configured
individually.

This commit introduces the ability to use globbing in the host-part of
the URLs. A user can simply specify a `*` as part of the host name to
match all subdomains at this level. For example adding a configuration
key `http.https://*.example.com.proxy` will match all subdomains of
`https://example.com`.

Signed-off-by: Patrick Steinhardt 
---
 Documentation/config.txt |  5 -
 t/t1300-repo-config.sh   | 36 
 urlmatch.c   | 38 ++
 3 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 506431267..a78921c2b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1914,7 +1914,10 @@ http..*::
   must match exactly between the config key and the URL.
 
 . Host/domain name (e.g., `example.com` in `https://example.com/`).
-  This field must match exactly between the config key and the URL.
+  This field must match between the config key and the URL. It is
+  possible to use globs in the config key to match all subdomains, e.g.
+  `https://*.example.com/` to match all subdomains of `example.com`. Note
+  that a glob only every matches a single part of the hostname.
 
 . Port number (e.g., `8080` in `http://example.com:8080/`).
   This field must match exactly between the config key and the URL.
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a2..ec545e092 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,6 +1177,42 @@ test_expect_success 'urlmatch' '
test_cmp expect actual
 '
 
+test_expect_success 'glob-based urlmatch' '
+   cat >.git/config <<-\EOF &&
+   [http]
+   sslVerify
+   [http "https://*.example.com;]
+   sslVerify = false
+   cookieFile = /tmp/cookie.txt
+   EOF
+
+   test_expect_code 1 git config --bool --get-urlmatch doesnt.exist 
https://good.example.com >actual &&
+   test_must_be_empty actual &&
+
+   echo true >expect &&
+   git config --bool --get-urlmatch http.SSLverify https://example.com 
>actual &&
+   test_cmp expect actual &&
+
+   echo true >expect &&
+   git config --bool --get-urlmatch http.SSLverify 
https://good-example.com >actual &&
+   test_cmp expect actual &&
+
+   echo true >expect &&
+   git config --bool --get-urlmatch http.sslverify 
https://deep.nested.example.com >actual &&
+   test_cmp expect actual &&
+
+   echo false >expect &&
+   git config --bool --get-urlmatch http.sslverify 
https://good.example.com >actual &&
+   test_cmp expect actual &&
+
+   {
+   echo http.cookiefile /tmp/cookie.txt &&
+   echo http.sslverify false
+   } >expect &&
+   git config --get-urlmatch HTTP https://good.example.com >actual &&
+   test_cmp expect actual
+'
+
 # good section hygiene
 test_expect_failure 'unsetting the last key in a section removes header' '
cat >.git/config <<-\EOF &&
diff --git a/urlmatch.c b/urlmatch.c
index e328905eb..53ff972a6 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,6 +63,38 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
 }
 
+static int match_host(const struct url_info *url_info,
+ const struct url_info *pattern_info)
+{
+   char *url = xmemdupz(url_info->url + url_info->host_off, 
url_info->host_len);
+   char *pat = xmemdupz(pattern_info->url + pattern_info->host_off, 
pattern_info->host_len);
+   char *url_tok, *pat_tok, *url_save, *pat_save;
+   int matching;
+
+   url_tok = strtok_r(url, ".", _save);
+   pat_tok = strtok_r(pat, ".", _save);
+
+   for (; url_tok && pat_tok; url_tok = strtok_r(NULL, ".", _save),
+  pat_tok = strtok_r(NULL, ".", _save)) {
+   if (!strcmp(pat_tok, "*"))
+   continue; /* a simple glob matches everything */
+
+   if (strcmp(url_tok, pat_tok)) {
+   /* subdomains do not match */
+   matching = 0;
+   break;
+   }
+   }
+
+   /* matching if both URL and pattern are at their ends */
+   

[PATCH v2 2/4] urlmatch: enable normalization of URLs with globs

[Patrick Steinhardt]

The `url_normalize` function is used to validate and normalize URLs. As
such, it does not allow for some special characters to be part of the
URLs that are to be normalized. As we want to allow using globs in some
configuration keys making use of URLs, namely `http..`, but
still normalize them, we need to somehow enable some additional allowed
characters.

To do this without having to change all callers of `url_normalize`,
where most do not actually want globbing at all, we split off another
function `url_normalize_1`. This function accepts an additional
parameter `allow_globs`, which is subsequently called by `url_normalize`
with `allow_globs=0`.

As of now, this function is not used with globbing enabled. A caller
will be added in the following commit.

Signed-off-by: Patrick Steinhardt 
---
 urlmatch.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/urlmatch.c b/urlmatch.c
index 132d342bc..d350478c0 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,7 +63,7 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
 }
 
-char *url_normalize(const char *url, struct url_info *out_info)
+static char *url_normalize_1(const char *url, struct url_info *out_info, char 
allow_globs)
 {
/*
 * Normalize NUL-terminated url using the following rules:
@@ -191,7 +191,12 @@ char *url_normalize(const char *url, struct url_info 
*out_info)
strbuf_release();
return NULL;
}
-   spanned = strspn(url, URL_HOST_CHARS);
+
+   if (allow_globs)
+   spanned = strspn(url, URL_HOST_CHARS "*");
+   else
+   spanned = strspn(url, URL_HOST_CHARS);
+
if (spanned < colon_ptr - url) {
/* Host name has invalid characters */
if (out_info) {
@@ -380,6 +385,11 @@ char *url_normalize(const char *url, struct url_info 
*out_info)
return result;
 }
 
+char *url_normalize(const char *url, struct url_info *out_info)
+{
+   return url_normalize_1(url, out_info, 0);
+}
+
 static size_t url_match_prefix(const char *url,
   const char *url_prefix,
   size_t url_prefix_len)
-- 
2.11.0

[PATCH v2 3/4] urlmatch: split host and port fields in `struct url_info`

[Patrick Steinhardt]

The `url_info` structure contains information about a normalized URL
with the URL's components being represented by different fields. The
host and port part though are to be accessed by the same `host` field,
so that getting the host and/or port separately becomes more involved
than really necessary.

To make the port more readily accessible, split up the host and port
fields. Namely, the `host_len` will not include the port length anymore
and a new `port_off` field has been added which includes the offset to
the port, if available.

The only user of these fields is `url_normalize_1`. This change makes it
easier later on to treat host and port differently when introducing
globs for domains.

Signed-off-by: Patrick Steinhardt 
---
 urlmatch.c | 16 
 urlmatch.h |  9 +
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/urlmatch.c b/urlmatch.c
index d350478c0..e328905eb 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -104,7 +104,7 @@ static char *url_normalize_1(const char *url, struct 
url_info *out_info, char al
struct strbuf norm;
size_t spanned;
size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
-   size_t host_off=0, host_len=0, port_len=0, path_off, path_len, 
result_len;
+   size_t host_off=0, host_len=0, port_off=0, port_len=0, path_off, 
path_len, result_len;
const char *slash_ptr, *at_ptr, *colon_ptr, *path_start;
char *result;
 
@@ -263,6 +263,7 @@ static char *url_normalize_1(const char *url, struct 
url_info *out_info, char al
return NULL;
}
strbuf_addch(, ':');
+   port_off = norm.len;
strbuf_add(, url, slash_ptr - url);
port_len = slash_ptr - url;
}
@@ -270,7 +271,7 @@ static char *url_normalize_1(const char *url, struct 
url_info *out_info, char al
url = slash_ptr;
}
if (host_off)
-   host_len = norm.len - host_off;
+   host_len = norm.len - host_off - (port_len ? port_len + 1 : 0);
 
 
/*
@@ -378,6 +379,7 @@ static char *url_normalize_1(const char *url, struct 
url_info *out_info, char al
out_info->passwd_len = passwd_len;
out_info->host_off = host_off;
out_info->host_len = host_len;
+   out_info->port_off = port_off;
out_info->port_len = port_len;
out_info->path_off = path_off;
out_info->path_len = path_len;
@@ -464,11 +466,17 @@ static int match_urls(const struct url_info *url,
usermatched = 1;
}
 
-   /* check the host and port */
+   /* check the host */
if (url_prefix->host_len != url->host_len ||
strncmp(url->url + url->host_off,
url_prefix->url + url_prefix->host_off, url->host_len))
-   return 0; /* host names and/or ports do not match */
+   return 0; /* host names do not match */
+
+   /* check the port */
+   if (url_prefix->port_len != url->port_len ||
+   strncmp(url->url + url->port_off,
+   url_prefix->url + url_prefix->port_off, url->port_len))
+   return 0; /* ports do not match */
 
/* check the path */
pathmatchlen = url_match_prefix(
diff --git a/urlmatch.h b/urlmatch.h
index 528862adc..0ea812b03 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -18,11 +18,12 @@ struct url_info {
size_t passwd_len;  /* length of passwd; if passwd_off != 0 but
   passwd_len == 0, an empty passwd was given */
size_t host_off;/* offset into url to start of host name (0 => 
none) */
-   size_t host_len;/* length of host name; this INCLUDES any 
':portnum';
+   size_t host_len;/* length of host name;
 * file urls may have host_len == 0 */
-   size_t port_len;/* if a portnum is present (port_len != 0), it 
has
-* this length (excluding the leading ':') at 
the
-* end of the host name (always 0 for file 
urls) */
+   size_t port_off;/* offset into url to start of port number (0 
=> none) */
+   size_t port_len;/* if a portnum is present (port_off != 0), it 
has
+* this length (excluding the leading ':') 
starting
+* from port_off (always 0 for file urls) */
size_t path_off;/* offset into url to the start of the url path;
 * this will always point to a '/' character
 * after the url has been normalized */
-- 
2.11.0

Re: [PATCH 7/7] completion: recognize more long-options

[Cornelius Weig]

On 01/24/2017 08:15 AM, Johannes Sixt wrote:
> If at all possible, please use your real email address as the From
> address. It is pointless to hide behind a fake address because as Git
> contributor you will have to reveal your identity anyway.

These are both real addresses, but for send-mail I would not want to use
my work account. I hope this is not a problem.

> Please study item (5) "Sign your work" in
> Documentation/SubmittingPatches and sign off your work.

I followed the recommendations to submitting work, and in the first
round signing is discouraged.

> AFAIR, it was a deliberate decision that potentially destructive command
> line options are not included in command completions. In the list given,
> I find these:
>
>>  - reset: --merge --mixed --hard --soft --patch --keep

My bad, I only added --keep, which should be fine. As to these options

>>  - apply: --unsafe-paths
>>  - rm: --force

let's wait for further comments, but I won't cling to it.

> Additionally, these options are only for internal purposes, but I may be
> wrong:
>
>>  - archive: --remote= --exec=

These may in fact be too exotic and just clutter the command line. Best
they are removed.

-- Cornelius

Re: submodule network operations [WAS: Re: [RFC/PATCH 0/4] working tree operations: support superprefix]

[Stefan Beller]

On Sat, Jan 21, 2017 at 7:53 AM, Brian J. Davis  wrote:
>
> On 1/19/2017 7:22 PM, Stefan Beller wrote:

 Between the init and the update step you can modify the URLs.
 These commands are just a repetition from the first email, but the
 git commands can be viewed as moving from one state to another
 for submodules; submodules itself can be seen as a state machine
 according to that proposed documentation. Maybe such a state machine
 makes it easier to understand for some people.
>>>
>>>
>>> "Between the init and the update step you can modify the URLs."  Yes I
>>> can
>>> and have to... wish it was not this way.
>>
>> So how would yo u rather want to do it?
>> look at the .gitmodules file beforehand and then run a "submodule update"
>> ?
>> Or a thing like
>>
>>  git -c url.https://internal.insteadOf git://github.com/ \
>>  -c submodule.record-rewritten-urls submodule update
>>
>> (no need for init there as theoretically there is not
>> need for such an intermediate step)
>>
> "Yes please and thank you" ... both.
>
> My thought was to simply allow addition to .gitmodules.  If I understand
> correctly you are proposing, to override these at the command line and
> possibly rewrite them on submodule update, but maybe not save or add to
> .gitmodules. I would then propose both.
> 1) allow user to add to .gitmodules for those who do not care that
> "outsiders" know the internal dev server
> and
> 2) allow to rewrite while not saving to .gitmodules on fresh clone and
> submodule update for thoes that do not want ousiders to known internal dev
> server.
> and possibly:
> 3) allow at command line to add remote to .gitmodules on submodule commands
> (note add optoin in -c  =  pair)
>
> .gitmodules before:
>
> [submodule "subprojects/wrangler"]
> path = subprojects/wrangler
> url = git://github.com/
>
> Then your adapted command:
>
> git -c url.https://internal.insteadOf git://github.com/ \
> -c submodule.record-rewritten-urls=add,internal --add submodule
> update
>
> would produce
>
> [submodule "subprojects/projname"]
> path = subprojects/projname
> remote.origin.url = git://github.com/
> remote.internal.url =https://internal.insteadOf
>
> Or similar support.

I think this was avoided until now as it would rewrite/add history.
So what if you want ot "just mirror" a large project with a lot
of submodules? You would want to do that without touching
the history, hence we'd need to do such a configuration in a separate
place. IIRC there was a proposal to have a ref e.g.
"refs/submodule/config", that can overwrite/extend the .gitmodules
file with your own configuration. It is a ref, such that mirroring would
work, but not part of the main history, such that yoiu can still change it.

I think to get it right we need to enable a workflow that allows easy
"multi-step" mirroring, e.g. A (source of truth) can be mirrored to B,
who can overlay the .gitmodules to point to server-B, which then can
be mirrored by C, who can have its own serverC.

When C forgot to configure all the submodules, it should fall back to
serverB as that was closest, and if that is unconfigured it should
fallback to A IMO.



>
 [remote "origin"]
 url = https://github.com/..
 [remote "inhouse"]
 url = https://inhouse.corp/..

 But where do we clone it from?
 (Or do we just do a "git init" on that submodule and fetch
 from both remotes? in which order?)
>>>
>>> origin by default and inhouse if specified. There is already a implied
>>> default (origin). The idea was not to do both but rather what is
>>> specified.
>>> Origin and inhouse are just names for remotes. If one wanted a
>>> "--all-remotes" could pull from everywhere in the Ether if feature was to
>>> be
>>> implemented.
>>
>> How is origin implied to be the default?
>> Should there be an order (e.g. if you cannot find it at inhouse get it
>> from github,
>> if they are down, get it from kernel.org)
>
> As it is in the Highlander series... "there can be only one" (remote).   So
> that is what I mean by origin.  The only remote allowed is the "origin"
> unless changed by the user... but there can still only be one *currently*.
> Though I see your point as it is not labeled "origin".  It is not labeled at
> all.  Apologies for confusion there.

"origin" is just a common name for a remote, like "master" is a common name
for branches. There is nothing inherently special about these except for
their automatic setup after cloning/initializing a repo.

So you can delete the master branch (which I did in a project
that I am not the authoritative maintainer of; I only have feature
branches), and
the repository just works fine.

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Jeff King]

On Sat, Jan 21, 2017 at 08:08:02PM +, Thomas Gummerer wrote:

> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 2e9cef06e6..0ad5335a3e 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -47,8 +47,9 @@ OPTIONS
>  
>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
> [-q|--quiet] []::
>  
> - Save your local modifications to a new 'stash', and run `git reset
> - --hard` to revert them.  The  part is optional and gives
> + Save your local modifications to a new 'stash', and revert the
> + the changes in the working tree to match the index.
> + The  part is optional and gives

Hrm. "git reset --hard" doesn't just make the working tree match the
index. It also resets the index to HEAD.  So either the original or your
new description is wrong.

I think it's the latter. We really do reset the index unless
--keep-index is specified.

I also wondered if it was worth avoiding the word "revert", as "git
revert" has a much different meaning (as opposed to "svn revert", which
does what you're talking about here). But I see that "git add -i"
already uses the word revert in this way (and there are probably
others).

So it may not be worth worrying about, but "set" or "reset" probably
serves the same purpose.

-Peff

Re: [PATCH 08/12] add oidset API

[Ramsay Jones]

On 24/01/17 00:46, Jeff King wrote:
> This is similar to many of our uses of sha1-array, but it
> overcomes one limitation of a sha1-array: when you are
> de-duplicating a large input with relatively few unique
> entries, sha1-array uses 20 bytes per non-unique entry.
> Whereas this set will use memory linear in the number of
> unique entries (albeit a few more than 20 bytes due to
> hashmap overhead).
> 
> Signed-off-by: Jeff King 
> ---
> This may be overkill. You can get roughly the same thing by making
> actual object structs via lookup_unknown_object(). But see the next
> patch for some comments on that.
> 
>  Makefile |  1 +
>  oidset.c | 49 +
>  oidset.h | 45 +
>  3 files changed, 95 insertions(+)
>  create mode 100644 oidset.c
>  create mode 100644 oidset.h
> 
> diff --git a/Makefile b/Makefile
> index 27afd0f37..e41efc2d8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -774,6 +774,7 @@ LIB_OBJS += notes-cache.o
>  LIB_OBJS += notes-merge.o
>  LIB_OBJS += notes-utils.o
>  LIB_OBJS += object.o
> +LIB_OBJS += oidset.o
>  LIB_OBJS += pack-bitmap.o
>  LIB_OBJS += pack-bitmap-write.o
>  LIB_OBJS += pack-check.o
> diff --git a/oidset.c b/oidset.c
> new file mode 100644
> index 0..6094cff8c
> --- /dev/null
> +++ b/oidset.c
> @@ -0,0 +1,49 @@
> +#include "cache.h"
> +#include "oidset.h"
> +
> +struct oidset_entry {
> + struct hashmap_entry hash;
> + struct object_id oid;
> +};
> +
> +int oidset_hashcmp(const void *va, const void *vb,

static int oidset_hashcmp( ...

ATB,
Ramsay Jones

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Jakub Narębski]

W dniu 21.01.2017 o 21:08, Thomas Gummerer pisze:
> Don't mention git reset --hard in the documentation for git stash save.
> It's an implementation detail that doesn't matter to the end user and
> thus shouldn't be exposed to them.
> 
> Signed-off-by: Thomas Gummerer 
> ---
>  Documentation/git-stash.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 2e9cef06e6..0ad5335a3e 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -47,8 +47,9 @@ OPTIONS
>  
>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
> [-q|--quiet] []::
>  
> - Save your local modifications to a new 'stash', and run `git reset
> - --hard` to revert them.  The  part is optional and gives
> + Save your local modifications to a new 'stash', and revert the
> + the changes in the working tree to match the index.

I think the following might be better:

..., and set the working tree to match the index.

Or not, as it ignores problem of untracked files.

Anyway, removing internal implementation detail looks like a good idea.
OTOH the reader should be familiar with what `git reset --hard` does,
and if not, he knows where to find the information.

> + The  part is optional and gives
>   the description along with the stashed state.  For quickly making
>   a snapshot, you can omit _both_ "save" and , but giving
>   only  does not trigger this action to prevent a misspelled
> 

-- 
Jakub Narębski

Re: What's cooking in git.git (Jan 2017, #03; Thu, 19)

[Thomas Braun]

> Junio C Hamano  hat am 20. Januar 2017 um 00:37
> geschrieben:

[snip]
  
> * rh/mergetool-regression-fix (2017-01-10) 14 commits
>   (merged to 'next' on 2017-01-10 at e8e00c798b)
>  + mergetool: fix running in subdir when rerere enabled
>  + mergetool: take the "-O" out of $orderfile
>  + t7610: add test case for rerere+mergetool+subdir bug
>  + t7610: spell 'git reset --hard' consistently
>  + t7610: don't assume the checked-out commit
>  + t7610: always work on a test-specific branch
>  + t7610: delete some now-unnecessary 'git reset --hard' lines
>  + t7610: run 'git reset --hard' after each test to clean up
>  + t7610: don't rely on state from previous test
>  + t7610: use test_when_finished for cleanup tasks
>  + t7610: move setup code to the 'setup' test case
>  + t7610: update branch names to match test number
>  + rev-parse doc: pass "--" to rev-parse in the --prefix example
>  + .mailmap: record canonical email for Richard Hansen
> 
>  "git mergetool" without any pathspec on the command line that is
>  run from a subdirectory became no-op in Git v2.11 by mistake, which
>  has been fixed.

Hi Junio, 

Sorry for asking a maybe obvious question.
Will that be merged into maint as well?
It is a regression in 2.11 so I would have expected to see that in maint.

Thanks,
Thomas

Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)

[Lars Schneider]

> On 24 Jan 2017, at 01:18, Junio C Hamano  wrote:
> 
> * jk/fsck-connectivity-check-fix (2017-01-17) 6 commits
>  (merged to 'next' on 2017-01-23 at e8e9b76b84)
> + fsck: check HAS_OBJ more consistently
> + fsck: do not fallback "git fsck " to "git fsck"
> + fsck: tighten error-checks of "git fsck "
> + fsck: prepare dummy objects for --connectivity-check
> + fsck: report trees as dangling
> + t1450: clean up sub-objects in duplicate-entry test
> 
> "git fsck --connectivity-check" was not working at all.
> 
> Will merge to 'master'.

"fsck: prepare dummy objects for --connectivity-check" seems to
make t1450-fsck.sh fail on macOS with TravisCI:

Initialized empty Git repository in /Users/travis/build/git/git/t/trash 
directory.t1450-fsck/connectivity-only/.git/
[master (root-commit) 86520b7] empty
 Author: A U Thor 
 2 files changed, 1 insertion(+)
 create mode 100644 empty
 create mode 100644 empty.t
override r--r--r--  travis/staff for 
.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391? (y/n [n]) not 
overwritten
dangling blob c21c9352f7526e9576892a6631e0e8cf1fccd34d
test_must_fail: command succeeded: git fsck --strict
not ok 58 - fsck --connectivity-only

More test output: https://travis-ci.org/git/git/jobs/194663620

For some reason I am not able to replicate that behavior on my local
macOS machine. I found the commit using bisect on TravisCI:
https://api.travis-ci.org/jobs/194746454/log.txt?deansi=true

Any idea what might be wrong?

- Lars

Re: [PATCH v1 0/2] urlmatch: allow regexp-based matches

[Patrick Steinhardt]

On Mon, Jan 23, 2017 at 11:53:43AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt  writes:
> 
> > This patch is mostly a request for comments. The use case is to
> > be able to configure an HTTP proxy for all subdomains of a
> > certain domain where there are hundreds of subdomains. The most
> > flexible way I could imagine was by using regular expressions for
> > the matching, which is how I implemented it for now. So users can
> > now create a configuration key like
> > `http.?http://.*\\.example\\.com.*` to apply settings for all
> > subdomains of `example.com`.
> 
> While reading 2/2, I got an impression that this is "too" flexible
> and possibly operates at a wrong level.  I would have expected that
> the wildcarding to be limited to the host part only and hook into
> match_urls(), allowing the users of the new feature to still take
> advantage of the existing support of "http://m...@example.com; that
> limits the match to the case that the connection is authenticated
> for a user, for example, by newly allowing "http://me@*.example.com;
> or something like that.
> 
> Because you cannot have a literal '*' in your hostname, I would
> imagine that supporting a match pattern "http://me@*.example.com;
> would be already backward compatible without requiring a leading
> question-mark.
> 
> I also personally would prefer these textual matching to be done
> with glob not with regexp, by the way, as the above description of
> mine shows.
> 
> Thanks.

Thanks for your feedback. Using globs in the hostname only was my
first intent, as well. I later on took regular expressions
instead so as to allow further flexibility for the user. The
reasoning was that there might be other use cases which cannot
actually be solved with using globs only, even if I myself wasn't
aware of different ones. So this might be indeed over-engineered
when using regular expressions.

There are several questions though regarding semantics with
globs, where I'd like to have additional opinions on.

- should a glob only be allowed for actual subdomains, allowing
  "http://*.example.com; but not "http://*example.com;?

- should a glob also match multiple nestings of subdomains? E.g.
  "http://*.example.com; would match "http://foo.example.com; but
  not "http://foo.bar.example.com;?

I'll send a version 2 soon-ish.

Regards
Patrick


signature.asc
Description: PGP signature

Re: [RFC 1/2] grep: only add delimiter if there isn't one already

[Stefan Hajnoczi]

On Fri, Jan 20, 2017 at 10:16:31AM -0800, Junio C Hamano wrote:
> Stefan Hajnoczi  writes:
> 
> > v2.9.3::Makefile may convey that the user originally provided v2.9.3:
> > but is that actually useful information?
> 
> You are either asking a wrong question, or asking a wrong person
> (i.e. Git) the question.  The real question is why the user added a
> colon at the end, when "v2.9.3" alone would have sufficed.  What did
> the user want to get out of giving an extra colon like "v2.9.3:"?
> 
> One answer I can think of is that it is a degenerate case of [2/2],
> i.e. if "v2.9.3:t" were an appropriate way to look in the subtree
> "t" of "v2.9.3", "v2.9.3:" would be the appropriate way to look in
> the whole tree of "v2.9.3".
> 
> I understand, from your response to my comment in the thread for
> [2/2], that the reason why "v2.9.3:t" was used in the example was
> because you felt uncomfortable with using pathspec.  
> 
> That's superstition.
> 
> My only piece of advice to folks who feel that way is to learn Git
> more and get comfortable.  You can do neat things like
> 
>$ git grep -e pattern rev -- t ':!t/helper/'
> 
> that you cannot do with "rev:t", for example ;-)

Neat, thanks for showing the path exclusion syntax.  I wasn't aware of
it.

> All examples we saw so far are the ones that the user used the colon
> syntax when it is more natural to express the command without it.  I
> am hesitant to see the behaviour of the command changed to appease
> such suboptimal use cases if it requires us to discard a bit of
> information, when we haven't established it is OK to lose.
> 
> There may be a case
> 
>  (1) where the colon syntax is the most natural thing to use, AND
>  script reading the output that used that syntax is forced to do
>  unnecessary work because "git grep" parrots the colon
>  literally, instread of being more intelligent about it
>  (i.e. omitting or substituting with slash when the input is a
>  tree object, not a tree-ish, as discussed in the thread).
> 
>  (2) where the colon syntax is the most natural thing, AND script
>  reading the output WANTS to see the distinction in the input
>  (remember, "git grep" can take more than one input tree).
> 
> We haven't seen either of the above two in the discussion, so the
> discussion so far is not sufficient to support the change being
> proposed in this RFC, which requires that it is safe to assume that
> (1) is the only case where the input is a raw tree (or the input
> contains a colon) and (2) will never happen.
> 
> So I am still mildly negative on the whole thing.

Avoiding the colon syntax avoids the whole issue for my use case.

I still think git-grep(1)'s output would be more useful if it used valid
git rev:path syntax in all cases.


signature.asc
Description: PGP signature

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Stefan Hajnoczi]

On Fri, Jan 20, 2017 at 02:56:26PM -0800, Junio C Hamano wrote:
> Jeff King  writes:
> 
> > It's not ignored; just as with git-log, it's a pathspec to limit the
> > diff. E.g.:
> >
> >   $ git show --name-status v2.9.3
> >   ...
> >   M   Documentation/RelNotes/2.9.3.txt
> >   M   Documentation/git.txt
> >   M   GIT-VERSION-GEN
> >
> >   $ git show --name-status v2.9.3 -- Documentation
> >   M   Documentation/RelNotes/2.9.3.txt
> >   M   Documentation/git.txt
> >
> > That's typically less useful than it is with log (where limiting the
> > diff also kicks in history simplification and omits some commits
> > entirely). But it does do something.
> 
> I think Stefan is missing the fact that the argument to "git show
> :" actually is naming a blob that sits at the 
> in the .  In other words, "show" is acting as a glorified
> "git -p cat-file blob", in that use.
> 
> The use of "git show" you are demonstrating is still about showing
> the commit object, whose behaviour is defined to show the log
> message and the diff relative to its sole parent, limited to the
> paths that match the pathspec.
> 
> It is perfectly fine and desirable that "git show :"
> and "git show  -- " behaves differently.  These are
> two completely different features.

Thanks for explaining guys.  It all makes logical sense.  I just hope I
remember the distinctions in that table for everyday git use.

Stefan


signature.asc
Description: PGP signature

Re: [RFC] Case insensitive Git attributes

[Lars Schneider]

> On 23 Jan 2017, at 20:38, Junio C Hamano  wrote:
> 
> Junio C Hamano  writes:
> 
>> So you are worried about the case where somebody on a case
>> insensitive but case preserving system would do
>> 
>>   $ edit file.txt
>>   $ edit .gitattributes
>>   $ git add file.txt .gitattributes
>> 
>> and adds "*.TXT  someattr=true" to the attributes file, which
>> would set someattr to true on his system for file.txt, but when the
>> result is checked out on a case sensitive system, it would behave
>> differently because "*.TXT" does not match "file.txt"?

Correct!


>> How do other systems address it?  Your Java, Ruby, etc. sources may
>> refer to another file with "import" and the derivation of the file
>> names from class names or package names would have the same issue,
>> isn't it?  Do they have an option that lets you say
>> 
>>   Even though the import statements may say "import a.b.C", we
>>   know that the source tarball was prepared on a case insensitive
>>   system, and I want you to look for a/b/C.java and a/b/c.java and
>>   use what was found.
>> 
>> or something like that?  Same for anything that records other
>> filenames in the content to refer to them, like "Makefile".
>> 
>> My knee jerk reaction to that is that .gitattributes and .gitignore
>> should not be instructed to go case insensitive on case sensitive
>> systems.  If the system is case insensitive but case preserving,
>> it probably would make sense not to do case insensitive matching,
>> which would prevent the issue from happening in the first place.
> 
> Sorry, but there is a slight leap in the above that makes it hard to
> track my thought, so let me clarify a bit.  
> 
> In the above, I am guessing the answer to the "How do other systems
> address it?" question to be "nothing".  And that leads to the
> conclusion that it is better to do "nothing on case sensitive
> systems, and probably become evem more strict on case insensitive
> but case preserving systems", because that will give us a chance to
> expose the problem earlier, hopefully even on the originating
> system.

I agree: Git attributes should behave the same on all platforms independent
of the file system type. I dug a bit deeper and realized that this is actually
already the case. However, the default (?) core.ignorecase=1 config on Win/Mac
generates the behavior explained above. I wonder if 6eba621 ("attr.c: respect 
core.ignorecase when matching attribute patterns", 2011-10-11) was a good idea.

AFAIK disabling core.ignorecase entirely on Win/Mac is no solution as this would
generate other trouble.

Git users can already create case insensitive gitattributes pattern. E.g.:
*.[tT][xX][tT]

However, based on my dayjob experience no Win/Mac developer does that as it
makes the gitattributes file unreadable. Consequently, Linux developers are 
screwed. Therefore, I wonder if it would make sense to introduce a shortcut
for the case insensitive glob pattern. E.g.:

*.txt ignorecase

If Git detects the ignorecase attribute then it could generate *.[tT][xX][tT]
automatically.

Re: [PATCH 0/3] stash: support filename argument

[Johannes Schindelin]

Hi Thomas,

On Sat, 21 Jan 2017, Thomas Gummerer wrote:

> This is the first try to implement the RFC I posted a week ago [1].  It
> introduces a new push verb for git stash.  I couldn't come up with
> any better name that wasn't already taken.  If anyone has ideas I'd be
> very happy to hear them.

I would have preferred a series of patches that essentially adds a new and
improved `save` syntax:

git stash [save] [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
  [-u|--include-untracked] [-a|--all] [-m ]]
  [-- ...]

and keeps the legacy syntax, but deprecates it:

git stash [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
  [-u|--include-untracked] [-a|--all] []]

The problem with that is, of course, that 3c2eb80fe3 (stash: simplify
defaulting to "save" and reject unknown options, 2009-08-18) in its
infinite wisdom *already* introduced the `--` separator to drop out of
option parsing.

On a positive note, it is a thorn in Git's CUI that `git stash` implies
the `save` command, and that `save` is not at all the opposite of `apply`
or `pop`. Your introduction of the `push` command will fix that flaw, and
we can *still* deprecate the `save` command.

Ciao,
Johannes

Re: GSoC 2017: application open, deadline = February 9, 2017

[Johannes Schindelin]

Hi Matthieu,

On Mon, 23 Jan 2017, Matthieu Moy wrote:

> * Who's willing to mentor?

As in the years before, I am willing to mentor.

Ciao,
Johannes

Assalamu`Alaikum.

[mohammad ouattara]

Dear Sir/Madam.

Assalamu`Alaikum.

I am Dr mohammad ouattara, I have  ($10.6 Million us dollars) to transfer into 
your account,

I will send you more details about this deal and the procedures to follow when 
I receive a positive response from you, 

Have a great day,
Dr mohammad ouattara.

Re: [PATCH] blame: add option to print tips (--tips)

[Pranit Bauva]

Hey Junio,

On Tue, Jan 24, 2017 at 12:06 AM, Junio C Hamano  wrote:
> Pranit Bauva  writes:
>
>> We can probably make it useful with some extended efforts. I use
>> git-blame and I sometimes find that I don't need things like the name
>> of the author, time, timezone and not even the file name and I have to
>> use a bigger terminal. If we could somehow remove those fields then
>> maybe this would be a useful feature.
>
> I admit that I didn't recall the option until somebody else told me,
> but I think "blame -s" or something like that for that purpose ;-)

Ah! Thanks a lot!

Regards,
Pranit Bauva

Re: [PATCH v2 0/5] string-list: make string_list_sort() reentrant

[Johannes Schindelin]

Hi Peff,

On Mon, 23 Jan 2017, Jeff King wrote:

> Is there any interest in people adding the ISO qsort_s() to their libc
> implementations? It seems like it's been a fair number of years by now.

Visual C supports it *at least* since Visual Studio 2005:

https://msdn.microsoft.com/en-us/library/4xc60xas(v=vs.80).aspx

With René's patch, we have an adapter for GNU libc, and if anybody comes
up with the (equally trivial) adapter for BSD libc's qsort_r(), we have a
lot of bases covered.

Ciao,
Johannes

Re: [DEMO][PATCH v2 6/5] compat: add a qsort_s() implementation based on GNU's qsort_r(1)

[Jeff King]

On Tue, Jan 24, 2017 at 07:00:03PM +0100, René Scharfe wrote:

> > I do worry about having to support more implementations in the
> > future that have different function signature for the comparison
> > callbacks, which will make things ugly, but this addition alone
> > doesn't look too bad to me.
> 
> It is unfair of me to show a 5% speedup and then recommend to not
> include it. ;-)  That difference won't be measurable in real use cases
> and the patch is not necessary.  This patch is simple, but the overall
> complexity (incl. #ifdefs etc.) will be lower without it.

I care less about squeezing out the last few percent performance and
more that somebody libc qsort_r() might offer some other improvement.
For instance, it could sort in-place to lower memory use for some cases,
or do some clever thing that gives more than a few percent in the real
world (something like TimSort).

I don't know to what degree we should care about that.

> But here's another one, with even higher performance and with an even
> bigger recommendation to not include it! :)  It veers off into another
> direction: Parallel execution.  It requires thread-safe comparison
> functions, which might surprise callers.  The value 1000 for the minimum
> number of items before threading kicks in is just a guess, not based on
> measurements.  So it's quite raw -- and I'm not sure why it's still a
> bit slower than sort(1).

Fun, but probably insane for our not-very-threadsafe code base. :)

-Peff

Re: What's cooking in git.git (Jan 2017, #03; Thu, 19)

[Junio C Hamano]

Thomas Braun  writes:

>> * rh/mergetool-regression-fix (2017-01-10) 14 commits
>>   (merged to 'next' on 2017-01-10 at e8e00c798b)
>>  + mergetool: fix running in subdir when rerere enabled
>>  + mergetool: take the "-O" out of $orderfile
>>  + t7610: add test case for rerere+mergetool+subdir bug
>>  + t7610: spell 'git reset --hard' consistently
>>  + t7610: don't assume the checked-out commit
>>  + t7610: always work on a test-specific branch
>>  + t7610: delete some now-unnecessary 'git reset --hard' lines
>>  + t7610: run 'git reset --hard' after each test to clean up
>>  + t7610: don't rely on state from previous test
>>  + t7610: use test_when_finished for cleanup tasks
>>  + t7610: move setup code to the 'setup' test case
>>  + t7610: update branch names to match test number
>>  + rev-parse doc: pass "--" to rev-parse in the --prefix example
>>  + .mailmap: record canonical email for Richard Hansen
> ...
> Sorry for asking a maybe obvious question.
> Will that be merged into maint as well?

A good way to tell is to compare outputs from these two:

$ git log --first-parent maint..$tip_of_the_topic
$ git log --first-parent master..$tip_of_the_topic

I actually wasn't planning to.  We (or distro packagers) may want to
cherry-pick the essential bits from the topic and backport to 'maint'.

Re: [PATCH 08/12] add oidset API

[Jeff King]

On Tue, Jan 24, 2017 at 08:26:40PM +, Ramsay Jones wrote:

> > +++ b/oidset.c
> > @@ -0,0 +1,49 @@
> > +#include "cache.h"
> > +#include "oidset.h"
> > +
> > +struct oidset_entry {
> > +   struct hashmap_entry hash;
> > +   struct object_id oid;
> > +};
> > +
> > +int oidset_hashcmp(const void *va, const void *vb,
> 
> static int oidset_hashcmp( ...

Yep, thanks.

-Peff

Re: [PATCH v2 0/5] string-list: make string_list_sort() reentrant

[Jeff King]

On Tue, Jan 24, 2017 at 07:00:07PM +0100, René Scharfe wrote:

> Am 24.01.2017 um 00:54 schrieb Jeff King:
> > The speed looks like a reasonable outcome. I'm torn on the qsort_r()
> > demo patch. I don't think it looks too bad. OTOH, I don't think I would
> > want to deal with the opposite-argument-order versions.
> 
> The code itself may look OK, but it's not really necessary and the special
> implementation for Linux makes increases maintenance costs.  Can we save it
> for later and first give the common implemention a chance to prove itself?

Sure, I'm OK with leaving it out for now.

> > Is there any interest in people adding the ISO qsort_s() to their libc
> > implementations? It seems like it's been a fair number of years by now.
> 
> https://sourceware.org/ml/libc-alpha/2014-12/msg00513.html is the last post
> mentioning qsort_s on the glibc mailing list, but it didn't even make it
> into https://sourceware.org/glibc/wiki/Development_Todo/Master.
> Not sure what's planned in BSD land, didn't find anything (but didn't look
> too hard).

So it sounds like "no, not really". I think that's OK. I was mostly
curious if we could expect our custom implementation to age out over
time.

-Peff

[PATCH] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves

[Stefan Beller]

Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
When nested is already absorbed into sub, but sub is not absorbed into
its superproject, then we need to fixup the gitfile and core.worktree
setting for 'nested' when absorbing 'sub', but we do not need to move
its git dir around.

Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".

An alternative I considered to do this work lazily, i.e. when resolving
"../.git/modules/nested", we would notice the ".git" being a gitfile
linking to another path.  That seemed to be robuster by design, but harder
to get the implementation right.  Maybe we have to do that anyway once we
try to have submodules and worktrees working nicely together, but for now
just produce 'correct' (i.e. direct) pointers.

Signed-off-by: Stefan Beller 
---

  cc Jeff and Brandon, who worked on the setup code recently,
  and the alternative design mentioned was messing around a lot in setup.c.
  
  Thanks,
  Stefan

 submodule.c| 41 +++---
 t/t7412-submodule-absorbgitdirs.sh | 27 +
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/submodule.c b/submodule.c
index 4c4f033e8a..7deb0fca6a 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1393,16 +1393,9 @@ static void 
relocate_single_git_dir_into_superproject(const char *prefix,
char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = 
NULL;
const char *new_git_dir;
const struct submodule *sub;
-
-   if (submodule_uses_worktrees(path))
-   die(_("relocate_gitdir for submodule '%s' with "
- "more than one worktree not supported"), path);
+   int err_code;
 
old_git_dir = xstrfmt("%s/.git", path);
-   if (read_gitfile(old_git_dir))
-   /* If it is an actual gitfile, it doesn't need migration. */
-   return;
-
real_old_git_dir = real_pathdup(old_git_dir);
 
sub = submodule_from_path(null_sha1, path);
@@ -1414,6 +1407,24 @@ static void 
relocate_single_git_dir_into_superproject(const char *prefix,
die(_("could not create directory '%s'"), new_git_dir);
real_new_git_dir = real_pathdup(new_git_dir);
 
+   if (read_gitfile_gently(old_git_dir, _code) ||
+   err_code == READ_GITFILE_ERR_NOT_A_REPO) {
+   /*
+* If it is an actual gitfile, it doesn't need migration,
+* however in case of a recursively nested submodule, the
+* gitfile content may be stale, as its superproject
+* (which may be a submodule of another superproject)
+* may have been moved. So expect a bogus pointer to be read,
+* which materializes as error READ_GITFILE_ERR_NOT_A_REPO.
+*/
+   connect_work_tree_and_git_dir(path, real_new_git_dir);
+   return;
+   }
+
+   if (submodule_uses_worktrees(path))
+   die(_("relocate_gitdir for submodule '%s' with "
+ "more than one worktree not supported"), path);
+
if (!prefix)
prefix = get_super_prefix();
 
@@ -1437,22 +1448,14 @@ void absorb_git_dir_into_superproject(const char 
*prefix,
  const char *path,
  unsigned flags)
 {
-   const char *sub_git_dir, *v;
-   char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
struct strbuf gitdir = STRBUF_INIT;
-
strbuf_addf(, "%s/.git", path);
-   sub_git_dir = resolve_gitdir(gitdir.buf);
 
/* Not populated? */
-   if (!sub_git_dir)
+   if (!file_exists(gitdir.buf))
goto out;
 
-   /* Is it already absorbed into the superprojects git dir? */
-   real_sub_git_dir = real_pathdup(sub_git_dir);
-   real_common_git_dir = real_pathdup(get_git_common_dir());
-   if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))
-   relocate_single_git_dir_into_superproject(prefix, path);
+   relocate_single_git_dir_into_superproject(prefix, path);
 
if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
struct child_process cp = CHILD_PROCESS_INIT;
@@ -1481,6 +1484,4 @@ void absorb_git_dir_into_superproject(const char *prefix,
 
 out:
strbuf_release();
-   free(real_sub_git_dir);
-   free(real_common_git_dir);
 }
diff --git a/t/t7412-submodule-absorbgitdirs.sh 
b/t/t7412-submodule-absorbgitdirs.sh
index 1c47780e2b..e2bbb449b6 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested 
submodule' '
test_cmp expect.2 actual.2
 '
 
+test_expect_success 're-setup nested submodule' '
+   # un-absorb the direct 

Re: [PATCH] difftool.c: mark a file-local symbol with static

[Junio C Hamano]

Jeff King  writes:

>> > As ugly as warning("%s", "") is, I think it may be the thing that annoys
>> > the smallest number of people.
>> > 
>> > -Peff
>> 
>> How about using warning(" ") instead?
>> 
>> For difftool.c specifically, the following is a fine solution,
>> and doesn't require that we change our warning flags just for
>> this one file.
>
> I dunno. As ugly as the "%s" thing is in the source, at least it doesn't
> change the output. Not that an extra space is the end of the world, but
> it seems like it's letting the problem escape from the source code.
>
> Do people still care about resolving this? -Wno-format-zero-length is in
> the DEVELOPER options. It wasn't clear to me if that was sufficient, or
> if we're going to get a bunch of reports from people that need to be
> directed to the right compiler options.

I view both as ugly, but probably "%s", "" is lessor of the two
evils.

Perhaps

#define JUST_SHOW_EMPTY_LINE "%s", ""

...
warning(JUST_SHOW_EMPTY_LINE);
...

or something silly like that?

Re: [RFC 1/2] grep: only add delimiter if there isn't one already

[Philip Oakley]

From: "Jakub Narębski" 

W dniu 23.01.2017 o 14:15, Stefan Hajnoczi pisze:

On Fri, Jan 20, 2017 at 10:16:31AM -0800, Junio C Hamano wrote:



My only piece of advice to folks who feel that way is to learn Git
more and get comfortable.  You can do neat things like

   $ git grep -e pattern rev -- t ':!t/helper/'

that you cannot do with "rev:t", for example ;-)


Neat, thanks for showing the path exclusion syntax.  I wasn't aware of
it.


That reminds me of mu TODO item: moving extended pathspec information
from gitglossary(7) manpage (sic!) to to-be-created gitpathspec(7).



Good to see someone else also had it on a ToDo list..

Attached is my collation of all the different path spec info I found from 
trawling the man & guide pages to satisfy my ignorance...

--
Philip 

gitpathspec(7)


NAME

gitpathspec - How to specify a path or file to git

SYNOPSIS

$HOME/.config/git/ignore, $GIT_DIR/info/exclude, .gitignore

DESCRIPTION
---

Pathspecs are used in a range of git functions.
.gitignore
.gitexclude
gitsparse
git-add -- pathspec
git-checkout -- pathspec (after the double-dash)
git grep (active/non-active wild card matching)
git log (L#:<> pathspec limiters ?? what does it mean)

git rerere (uncontentious)
git status (uncontentious)
gitk (uncontentious, but see 'log' above)
'git' itself --
--literal-pathspecs
Treat pathspecs literally (i.e. no globbing, no pathspec magic). This is 
equivalent to setting the GIT_LITERAL_PATHSPECS environment variable to 1.

--glob-pathspecs
Add "glob" magic to all pathspec. This is equivalent to setting the GIT_GLOB_PATHSPECS 
environment variable to 1. Disabling globbing on individual pathspecs can be done using pathspec 
magic ":(literal)"

--noglob-pathspecs
Add "literal" magic to all pathspec. This is equivalent to setting the 
GIT_NOGLOB_PATHSPECS environment variable to 1. Enabling globbing on individual pathspecs can be 
done using pathspec magic ":(glob)"

--icase-pathspecs
Add "icase" magic to all pathspec. This is equivalent to setting the 
GIT_ICASE_PATHSPECS environment variable to 1.

see glossary-content
pathspec
Pattern used to limit paths in Git commands.

Pathspecs are used on the command line of "git ls-files", "git ls-tree", "git add", "git grep", 
"git diff", "git checkout", and many other commands to limit the scope of operations to some subset of the tree or 
worktree. See the documentation of each command for whether paths are relative to the current directory or toplevel. The pathspec syntax is 
as follows:

any path matches itself

the pathspec up to the last slash represents a directory prefix. The scope of 
that pathspec is limited to that subtree.

the rest of the pathspec is a pattern for the remainder of the pathname. Paths 
relative to the directory prefix will be matched against that pattern using 
fnmatch(3); in particular, * and ? can match directory separators.

For example, Documentation/*.jpg will match all .jpg files in the Documentation 
subtree, including Documentation/chapter_1/figure_1.jpg.

A pathspec that begins with a colon : has special meaning. In the short form, the leading colon : is followed by zero 
or more "magic signature" letters (which optionally is terminated by another colon :), and the remainder is 
the pattern to match against the path. The "magic signature" consists of ASCII symbols that are neither 
alphanumeric, glob, regex special charaters nor colon. The optional colon that terminates the "magic 
signature" can be omitted if the pattern begins with a character that does not belong to "magic 
signature" symbol set and is not a colon.

In the long form, the leading colon : is followed by a open parenthesis (, a 
comma-separated list of zero or more "magic words", and a close parentheses ), 
and the remainder is the pattern to match against the path.

A pathspec with only a colon means "there is no pathspec". This form should not 
be combined with other pathspec.

top
The magic word top (magic signature: /) makes the pattern match from the root 
of the working tree, even when you are running the command from inside a 
subdirectory.

literal
Wildcards in the pattern such as * or ? are treated as literal characters.

icase
Case insensitive match.

glob
Git treats the pattern as a shell glob suitable for consumption by fnmatch(3) with the FNM_PATHNAME flag: wildcards in 
the pattern will not match a / in the pathname. For example, "Documentation/*.html" matches 
"Documentation/git.html" but not "Documentation/ppc/ppc.html" or 
"tools/perf/Documentation/perf.html".

Two consecutive asterisks ("**") in patterns matched against full pathname may 
have special meaning:

A leading "**" followed by a slash means match in all directories. For example, "**/foo" matches file or directory 
"foo" anywhere, the same as pattern "foo". "**/foo/bar" matches file or directory "bar" anywhere that is 
directly under directory "foo".

A trailing "/**" matches everything inside. For 

Re: [PATCH] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves

[Brandon Williams]

On 01/24, Stefan Beller wrote:
> + if (read_gitfile_gently(old_git_dir, _code) ||
> + err_code == READ_GITFILE_ERR_NOT_A_REPO) {
> + /*
> +  * If it is an actual gitfile, it doesn't need migration,
> +  * however in case of a recursively nested submodule, the
> +  * gitfile content may be stale, as its superproject
> +  * (which may be a submodule of another superproject)
> +  * may have been moved. So expect a bogus pointer to be read,
> +  * which materializes as error READ_GITFILE_ERR_NOT_A_REPO.
> +  */
> + connect_work_tree_and_git_dir(path, real_new_git_dir);

So connect_work_tree_and_git_dir() will update the .gitfile if it is
stale.

> + return;
> + }
> +
> + if (submodule_uses_worktrees(path))
> + die(_("relocate_gitdir for submodule '%s' with "
> +   "more than one worktree not supported"), path);

No current support for worktrees (yet!).

> +
>   if (!prefix)
>   prefix = get_super_prefix();
>  
> @@ -1437,22 +1448,14 @@ void absorb_git_dir_into_superproject(const char 
> *prefix,
> const char *path,
> unsigned flags)
>  {
> - const char *sub_git_dir, *v;
> - char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
>   struct strbuf gitdir = STRBUF_INIT;
> -
>   strbuf_addf(, "%s/.git", path);
> - sub_git_dir = resolve_gitdir(gitdir.buf);
>  
>   /* Not populated? */
> - if (!sub_git_dir)
> + if (!file_exists(gitdir.buf))
>   goto out;

There should be a is_submodule_populated() function now, maybe
we should start using it when performing population checks?

>  
> - /* Is it already absorbed into the superprojects git dir? */
> - real_sub_git_dir = real_pathdup(sub_git_dir);
> - real_common_git_dir = real_pathdup(get_git_common_dir());
> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))
> - relocate_single_git_dir_into_superproject(prefix, path);
> + relocate_single_git_dir_into_superproject(prefix, path);

So the check was just pushed into the relocation function.

>  
>   if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
>   struct child_process cp = CHILD_PROCESS_INIT;
> @@ -1481,6 +1484,4 @@ void absorb_git_dir_into_superproject(const char 
> *prefix,
>  
>  out:
>   strbuf_release();
> - free(real_sub_git_dir);
> - free(real_common_git_dir);
>  }
> diff --git a/t/t7412-submodule-absorbgitdirs.sh 
> b/t/t7412-submodule-absorbgitdirs.sh
> index 1c47780e2b..e2bbb449b6 100755
> --- a/t/t7412-submodule-absorbgitdirs.sh
> +++ b/t/t7412-submodule-absorbgitdirs.sh
> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested 
> submodule' '
>   test_cmp expect.2 actual.2
>  '
>  
> +test_expect_success 're-setup nested submodule' '
> + # un-absorb the direct submodule, to test if the nested submodule
> + # is still correct (needs a rewrite of the gitfile only)
> + rm -rf sub1/.git &&
> + mv .git/modules/sub1 sub1/.git &&
> + GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
> + # fixup the nested submodule
> + echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
> + GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
> + core.worktree "../../../nested" &&
> + # make sure this re-setup is correct
> + git status --ignore-submodules=none
> +'
> +
> +test_expect_success 'absorb the git dir in a nested submodule' '
> + git status >expect.1 &&
> + git -C sub1/nested rev-parse HEAD >expect.2 &&
> + git submodule absorbgitdirs &&
> + test -f sub1/.git &&
> + test -f sub1/nested/.git &&
> + test -d .git/modules/sub1/modules/nested &&
> + git status >actual.1 &&
> + git -C sub1/nested rev-parse HEAD >actual.2 &&
> + test_cmp expect.1 actual.1 &&
> + test_cmp expect.2 actual.2
> +'
> +
>  test_expect_success 'setup a gitlink with missing .gitmodules entry' '
>   git init sub2 &&
>   test_commit -C sub2 first &&
> -- 
> 2.11.0.486.g67830dbe1c


Aside from my one question the rest of this looks good to me.

-- 
Brandon Williams

Re: [RFC 1/2] grep: only add delimiter if there isn't one already

[Jakub Narębski]

W dniu 23.01.2017 o 14:15, Stefan Hajnoczi pisze:
> On Fri, Jan 20, 2017 at 10:16:31AM -0800, Junio C Hamano wrote:

>> My only piece of advice to folks who feel that way is to learn Git
>> more and get comfortable.  You can do neat things like
>>
>>$ git grep -e pattern rev -- t ':!t/helper/'
>>
>> that you cannot do with "rev:t", for example ;-)
> 
> Neat, thanks for showing the path exclusion syntax.  I wasn't aware of
> it.

That reminds me of mu TODO item: moving extended pathspec information
from gitglossary(7) manpage (sic!) to to-be-created gitpathspec(7).

-- 
Jakub Narębski

Re:

[Stefan Beller]

On Tue, Jan 24, 2017 at 4:43 PM, Cornelius Weig
 wrote:

> -(5) Sign your work
> +(5) Certify your work by signing off.

That sounds better than what I proposed.

Thanks,
Stefan

Re:

[Linus Torvalds]

On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller  wrote:
>
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway.

I think even that could be further simplified - by just removing all
comments about pgp email

Because it's not that the PGP keys would be hard to get, it's that
PGP-signed email is an abject failure, and nobody sane does it.

Google for "phil zimmerman doesn't use pgp email".

It's dead. So I'm not sure it's worth mentioning at all.

You might as well talk about how you shouldn't use EBCDIC encoding for
your patches, or about why git assumes that an email address has an
'@' sign in it, instead of being an UUCP bang path address.

  Linus

Re:

[Eric Wong]

Linus Torvalds  wrote:
> On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller  wrote:
> >
> > +Do not PGP sign your patch. Most likely, your maintainer or other
> > +people on the list would not have your PGP key and would not bother
> > +obtaining it anyway.
> 
> I think even that could be further simplified - by just removing all
> comments about pgp email
> 
> Because it's not that the PGP keys would be hard to get, it's that
> PGP-signed email is an abject failure, and nobody sane does it.
> 
> Google for "phil zimmerman doesn't use pgp email".
> 
> It's dead. So I'm not sure it's worth mentioning at all.

I disagree, we still see it, and Debian still advocates it.
In fact, we may also want to mention S/MIME in the same breath:

  https://public-inbox.org/git/20170110004031.57985-2-hans...@google.com/

Richard's more recent mails seem to indicate he's reformed :)

[PATCH v1 3/3] blame: add --color option

[Edmundo Carmona Antoranz]

Revision information will be highlighted so that it's easier
to tell from content of the file being "blamed".

Signed-off-by: Edmundo Carmona Antoranz 
---
 Documentation/blame-options.txt |  5 +
 Documentation/git-blame.txt |  2 +-
 builtin/blame.c | 13 +
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 64858a631..4abb4eb7e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -57,6 +57,11 @@ include::line-range-format.txt[]
Show revision hints on aggregated information about the revision.
Implies --aggregate.
 
+--[no-]color::
+   Revision information will be highlighted on normal output
+   when running git from a terminal. This option allows
+   for color to be forcibly enabled or disabled.
+
 --encoding=::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ed8b119fa..d25cbc5ef 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--progress] [--abbrev=] [--aggregate] [--hint]
+   [--progress] [--abbrev=] [--aggregate] [--hint] [--color]
[ | --contents  | --reverse ..] [--] 
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 7473b50e9..c661dd538 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1886,6 +1886,7 @@ static const char *format_time(unsigned long time, const 
char *tz_str,
 #define OUTPUT_LINE_PORCELAIN 01000
 #define OUTPUT_AGGREGATE  02000
 #define OUTPUT_HINT   04000
+#define OUTPUT_COLOR 01
 
 static void emit_porcelain_details(struct origin *suspect, int repeat)
 {
@@ -1943,6 +1944,8 @@ static void emit_porcelain(struct scoreboard *sb, struct 
blame_entry *ent,
 static void print_revision_info(char* revision_hex, int revision_length, 
struct blame_entry* ent,
struct commit_info ci, int opt, int show_raw_time, int 
line_number)
 {
+   if (opt & OUTPUT_COLOR)
+   printf("%s", GIT_COLOR_BOLD);
if (!line_number)
printf("\t");
int length = revision_length;
@@ -2008,6 +2011,8 @@ static void print_revision_info(char* revision_hex, int 
revision_length, struct
printf(" %s", ci.summary.buf);
printf("\n");
}
+   if (opt & OUTPUT_COLOR)
+   printf("%s", GIT_COLOR_RESET);
 }
 
 static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2608,6 +2613,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
char *final_commit_name = NULL;
enum object_type type;
struct commit *final_commit = NULL;
+   int show_color;
 
struct string_list range_list = STRING_LIST_INIT_NODUP;
int output_option = 0, opt = 0;
@@ -2647,6 +2653,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
OPT_STRING_LIST('L', NULL, _list, N_("n,m"), N_("Process 
only line range n,m, counting from 1")),
OPT_BIT(0, "aggregate", _option, N_("Aggregate output"), 
OUTPUT_AGGREGATE),
OPT_BIT(0, "hint", _option, N_("Show revision hints"), 
OUTPUT_HINT),
+   OPT_BOOL(0, "color", _color, N_("Show colors on revision 
information")),
OPT__ABBREV(),
OPT_END()
};
@@ -2666,6 +2673,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
save_commit_buffer = 0;
dashdash_pos = 0;
show_progress = -1;
+   show_color = -1;
 
parse_options_start(, argc, argv, prefix, options,
PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);
@@ -2698,6 +2706,11 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
} else if (show_progress < 0)
show_progress = isatty(2);
 
+   if (show_color < 0)
+   show_color = isatty(1);
+   if (show_color)
+   output_option = output_option | OUTPUT_COLOR;
+
if (0 < abbrev && abbrev < GIT_SHA1_HEXSZ)
/* one more abbrev length is needed for the boundary commit */
abbrev++;
-- 
2.11.0.rc1

[PATCH v1 2/3] blame: add --hint option

[Edmundo Carmona Antoranz]

When printing aggregate information about revisions, this option
allows to add the 1-line summary of the revision to provide the
reader with some additional context about the revision itself.

Signed-off-by: Edmundo Carmona Antoranz 
---
 Documentation/blame-options.txt |  4 
 Documentation/git-blame.txt |  2 +-
 builtin/blame.c | 13 +++--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index be2a327ff..64858a631 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -53,6 +53,10 @@ include::line-range-format.txt[]
Aggregate information about revisions. This option makes no
difference on incremental or porcelain format.
 
+--hint::
+   Show revision hints on aggregated information about the revision.
+   Implies --aggregate.
+
 --encoding=::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 385eaf7be..ed8b119fa 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--progress] [--abbrev=] [--aggregate] 
+   [--progress] [--abbrev=] [--aggregate] [--hint]
[ | --contents  | --reverse ..] [--] 
 
 DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 09b3b0c8a..7473b50e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1885,6 +1885,7 @@ static const char *format_time(unsigned long time, const 
char *tz_str,
 #define OUTPUT_SHOW_EMAIL  0400
 #define OUTPUT_LINE_PORCELAIN 01000
 #define OUTPUT_AGGREGATE  02000
+#define OUTPUT_HINT   04000
 
 static void emit_porcelain_details(struct origin *suspect, int repeat)
 {
@@ -2001,8 +2002,12 @@ static void print_revision_info(char* revision_hex, int 
revision_length, struct
if (line_number && (opt & OUTPUT_AGGREGATE))
printf(opt & OUTPUT_ANNOTATE_COMPAT ? "%*d)" : " %*d) ",
   max_digits, line_number);
-   if (!line_number)
-   printf(")\n");
+   if (!line_number) {
+   printf(")");
+   if (opt & OUTPUT_HINT)
+   printf(" %s", ci.summary.buf);
+   printf("\n");
+   }
 }
 
 static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2018,6 +2023,9 @@ static void emit_other(struct scoreboard *sb, struct 
blame_entry *ent, int opt)
get_commit_info(suspect->commit, , 1);
sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
 
+   if (~(opt & OUTPUT_AGGREGATE) & (opt & OUTPUT_HINT))
+   opt=opt|OUTPUT_AGGREGATE;
+
if (opt & OUTPUT_AGGREGATE)
print_revision_info(hex, revision_length, ent, ci, opt, 
show_raw_time, 0);
 
@@ -2638,6 +2646,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
{ OPTION_CALLBACK, 'M', NULL, , N_("score"), N_("Find line 
movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
OPT_STRING_LIST('L', NULL, _list, N_("n,m"), N_("Process 
only line range n,m, counting from 1")),
OPT_BIT(0, "aggregate", _option, N_("Aggregate output"), 
OUTPUT_AGGREGATE),
+   OPT_BIT(0, "hint", _option, N_("Show revision hints"), 
OUTPUT_HINT),
OPT__ABBREV(),
OPT_END()
};
-- 
2.11.0.rc1

[PATCH v1 1/3] blame: add --aggregate option

[Edmundo Carmona Antoranz]

To avoid taking up so much space on normal output printing duplicate
information on consecutive lines that "belong" to the same revision,
this option allows to print a single line with the information about
the revision before the lines of content themselves.

Signed-off-by: Edmundo Carmona Antoranz 
---
 Documentation/blame-options.txt |  4 ++
 Documentation/git-blame.txt |  4 +-
 builtin/blame.c | 85 +++--
 3 files changed, 63 insertions(+), 30 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 2669b87c9..be2a327ff 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -49,6 +49,10 @@ include::line-range-format.txt[]
Show the result incrementally in a format designed for
machine consumption.
 
+--aggregate::
+   Aggregate information about revisions. This option makes no
+   difference on incremental or porcelain format.
+
 --encoding=::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index fdc3aea30..385eaf7be 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,8 +10,8 @@ SYNOPSIS
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] 
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
-   [--progress] [--abbrev=] [ | --contents  | --reverse 
..]
-   [--] 
+   [--progress] [--abbrev=] [--aggregate] 
+   [ | --contents  | --reverse ..] [--] 
 
 DESCRIPTION
 ---
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..09b3b0c8a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const 
char *tz_str,
 #define OUTPUT_NO_AUTHOR   0200
 #define OUTPUT_SHOW_EMAIL  0400
 #define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_AGGREGATE  02000
 
 static void emit_porcelain_details(struct origin *suspect, int repeat)
 {
@@ -1931,43 +1932,41 @@ static void emit_porcelain(struct scoreboard *sb, 
struct blame_entry *ent,
putchar('\n');
 }
 
-static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
-{
-   int cnt;
-   const char *cp;
-   struct origin *suspect = ent->suspect;
-   struct commit_info ci;
-   char hex[GIT_SHA1_HEXSZ + 1];
-   int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
-
-   get_commit_info(suspect->commit, , 1);
-   sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
-
-   cp = nth_line(sb, ent->lno);
-   for (cnt = 0; cnt < ent->num_lines; cnt++) {
-   char ch;
-   int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : 
abbrev;
-
-   if (suspect->commit->object.flags & UNINTERESTING) {
+/**
+ * Print information about the revision.
+ * This information can be used in either aggregated output
+ * or prepending each line of the content of the file being blamed
+ * 
+ * if line_number == 0, it's an aggregate line
+ */
+static void print_revision_info(char* revision_hex, int revision_length, 
struct blame_entry* ent,
+   struct commit_info ci, int opt, int show_raw_time, int 
line_number)
+{
+   if (!line_number)
+   printf("\t");
+   int length = revision_length;
+   if (!line_number || !(opt & OUTPUT_AGGREGATE)) {
+   if (ent->suspect->commit->object.flags & UNINTERESTING) {
if (blank_boundary)
-   memset(hex, ' ', length);
+   memset(revision_hex, ' ', length);
else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
length--;
putchar('^');
}
}
 
-   printf("%.*s", length, hex);
+   printf("%.*s", length, revision_hex);
if (opt & OUTPUT_ANNOTATE_COMPAT) {
const char *name;
if (opt & OUTPUT_SHOW_EMAIL)
name = ci.author_mail.buf;
else
name = ci.author.buf;
-   printf("\t(%10s\t%10s\t%d)", name,
+   printf("\t(%10s\t%10s", name,
   format_time(ci.author_time, ci.author_tz.buf,
-  show_raw_time),
-  ent->lno + 1 + cnt);
+  show_raw_time));
+   if (line_number)
+   printf("\t%d)", line_number);
} else {
if (opt & OUTPUT_SHOW_SCORE)
printf(" %*d %02d",
@@ 

Re: [PATCH] tag: add tag.createReflog option

[Pranit Bauva]

Hey Cornelius,

On Wed, Jan 25, 2017 at 5:49 AM,   wrote:
> From: Cornelius Weig 
>
> Git does not create a history for tags, in contrast to common
> expectation to simply version everything. This can be changed by using
> the `--create-reflog` flag when creating the tag. However, a config
> option to enable this behavior by default is missing.
>
> This commit adds the configuration variable `tag.createReflog` which
> enables reflogs for new tags by default.
>
> Signed-off-by: Cornelius Weig 

You have also added the option --no-create-reflog so would it be worth
to mention it in the commit message?
> ---
>  Documentation/config.txt  |  5 +
>  Documentation/git-tag.txt |  8 +---
>  builtin/tag.c |  6 +-
>  t/t7004-tag.sh| 14 ++
>  4 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index af2ae4c..9e5f6f6 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
> as computed via `submodule.alternateLocation`. Possible values are
> `ignore`, `info`, `die`. Default is `die`.
>
> +tag.createReflog::
> +   A boolean to specify whether newly created tags should have a reflog.
> +   If `--[no-]create-reflog` is specified on the command line, it takes
> +   precedence. Defaults to `false`.

This follows the convention, good! :)

>  tag.forceSignAnnotated::
> A boolean to specify whether annotated tags created should be GPG 
> signed.
> If `--annotate` is specified on the command line, it takes
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 5055a96..f2ed370 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
>  [ | ]
>  'git tag' -d ...
>  'git tag' [-n[]] -l [--contains ] [--points-at ]
> -   [--column[=] | --no-column] [--create-reflog] [--sort=]
> +   [--column[=] | --no-column] [--[no-]create-reflog] 
> [--sort=]
> [--format=] [--[no-]merged []] [...]
>  'git tag' -v ...
>
> @@ -149,8 +149,10 @@ This option is only applicable when listing tags without 
> annotation lines.
> all, 'whitespace' removes just leading/trailing whitespace lines and
> 'strip' removes both whitespace and commentary.
>
> ---create-reflog::
> -   Create a reflog for the tag.
> +--[no-]create-reflog::
> +   Force to create a reflog for the tag, or no reflog if 
> `--no-create-reflog`
> +   is used. Unless the `tag.createReflog` config variable is set to 
> true, no
> +   reflog is created by default. See linkgit:git-config[1].
>
>  ::
> The name of the tag to create, delete, or describe.
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 73df728..1f13e4d 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {
>
>  static unsigned int colopts;
>  static int force_sign_annotate;
> +static int create_reflog;
>
>  static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, 
> const char *format)
>  {
> @@ -165,6 +166,10 @@ static int git_tag_config(const char *var, const char 
> *value, void *cb)
> force_sign_annotate = git_config_bool(var, value);
> return 0;
> }
> +   if (!strcmp(var, "tag.createreflog")) {
> +   create_reflog = git_config_bool(var, value);
> +   return 0;
> +   }
>
> if (starts_with(var, "column."))
> return git_column_config(var, value, "tag", );
> @@ -325,7 +330,6 @@ int cmd_tag(int argc, const char **argv, const char 
> *prefix)
> const char *object_ref, *tag;
> struct create_tag_options opt;
> char *cleanup_arg = NULL;
> -   int create_reflog = 0;
> int annotate = 0, force = 0;
> int cmdmode = 0, create_tag_object = 0;
> const char *msgfile = NULL, *keyid = NULL;
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 1cfa8a2..67b39ec 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -90,6 +90,20 @@ test_expect_success '--create-reflog does not create 
> reflog on failure' '
> test_must_fail git reflog exists refs/tags/mytag
>  '
>
> +test_expect_success 'option tag.createreflog creates reflog by default' '
> +   test_when_finished "git tag -d tag_with_reflog" &&
> +   git config tag.createReflog true &&
> +   git tag tag_with_reflog &&
> +   git reflog exists refs/tags/tag_with_reflog
> +'
> +
> +test_expect_success 'option tag.createreflog overridden by command line' '
> +   test_when_finished "git tag -d tag_without_reflog" &&
> +   git config tag.createReflog true &&
> +   git tag --no-create-reflog tag_without_reflog &&
> +   test_must_fail git reflog exists refs/tags/tag_without_reflog
> 

Re: SubmittingPatches: drop temporal reference for PGP signing

[Philip Oakley]

From: "Stefan Beller" 



Do not PGP sign your patch, at least *for now*. (...)




And maybe these 2 small words are the bug in the documentation?
Shall we drop the "at least for now" part, like so:

---8<---
From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
From: Stefan Beller 
Date: Tue, 24 Jan 2017 16:19:13 -0800
Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP 
signing


Signed-off-by: Stefan Beller 
---
Documentation/SubmittingPatches | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/SubmittingPatches 
b/Documentation/SubmittingPatches

index 08352deaae..28da4ad2d4 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -216,12 +216,12 @@ that it will be postponed.
Exception:  If your mailer is mangling patches then someone may ask
you to re-send them using MIME, that is OK.

-Do not PGP sign your patch, at least for now.  Most likely, your
-maintainer or other people on the list would not have your PGP
-key and would not bother obtaining it anyway.  Your patch is not
-judged by who you are; a good patch from an unknown origin has a
-far better chance of being accepted than a patch from a known,
-respected origin that is done poorly or does incorrect things.
+Do not PGP sign your patch. Most likely, your maintainer or other
+people on the list would not have your PGP key and would not bother
+obtaining it anyway. Your patch is not judged by who you are; a good
+patch from an unknown origin has a far better chance of being accepted
+than a patch from a known, respected origin that is done poorly or
+does incorrect things.


Wouldn't this also benefit from a forward reference to the section 5 on the 
DOC signining? This would avoid Cornelius's case where he felt that section 
5 no longer applied.



If you really really really really want to do a PGP signed
patch, format it as "multipart/signed", not a text/plain message
--
2.11.0.495.g04f60290a0.dirty

--
Philip 

Re: submodule network operations [WAS: Re: [RFC/PATCH 0/4] working tree operations: support superprefix]

[Brian J. Davis]

On 1/24/2017 12:49 PM, Stefan Beller wrote:

On Sat, Jan 21, 2017 at 7:53 AM, Brian J. Davis  wrote:

On 1/19/2017 7:22 PM, Stefan Beller wrote:

Between the init and the update step you can modify the URLs.
These commands are just a repetition from the first email, but the
git commands can be viewed as moving from one state to another
for submodules; submodules itself can be seen as a state machine
according to that proposed documentation. Maybe such a state machine
makes it easier to understand for some people.


"Between the init and the update step you can modify the URLs."  Yes I
can
and have to... wish it was not this way.

So how would yo u rather want to do it?
look at the .gitmodules file beforehand and then run a "submodule update"
?
Or a thing like

  git -c url.https://internal.insteadOf git://github.com/ \
  -c submodule.record-rewritten-urls submodule update

(no need for init there as theoretically there is not
need for such an intermediate step)


"Yes please and thank you" ... both.

My thought was to simply allow addition to .gitmodules.  If I understand
correctly you are proposing, to override these at the command line and
possibly rewrite them on submodule update, but maybe not save or add to
.gitmodules. I would then propose both.
1) allow user to add to .gitmodules for those who do not care that
"outsiders" know the internal dev server
and
2) allow to rewrite while not saving to .gitmodules on fresh clone and
submodule update for thoes that do not want ousiders to known internal dev
server.
and possibly:
3) allow at command line to add remote to .gitmodules on submodule commands
(note add optoin in -c  =  pair)

.gitmodules before:

[submodule "subprojects/wrangler"]
 path = subprojects/wrangler
 url = git://github.com/

Then your adapted command:

git -c url.https://internal.insteadOf git://github.com/ \
 -c submodule.record-rewritten-urls=add,internal --add submodule
update

would produce

[submodule "subprojects/projname"]
 path = subprojects/projname
 remote.origin.url = git://github.com/
 remote.internal.url =https://internal.insteadOf

Or similar support.

I think this was avoided until now as it would rewrite/add history.
So what if you want ot "just mirror" a large project with a lot
of submodules? You would want to do that without touching
the history, hence we'd need to do such a configuration in a separate
place. IIRC there was a proposal to have a ref e.g.
"refs/submodule/config", that can overwrite/extend the .gitmodules
file with your own configuration. It is a ref, such that mirroring would
work, but not part of the main history, such that yoiu can still change it.
How would this handle the hashes for the submodules tracked in the super 
project.  This gets to my earlier statement that git submodules is not 
really that well thought out.  When/if /refs/submodule/config 
overwrites/extends .gitmoduels what does git checkout for a hash code 
(version) for the subproject?  Maybe there are some simple ways of doing 
this, but it really seems to me that git submodules needs an overhaul to 
support multiple remotes.  Would the refs/submodule/config also include 
the versions hashes for the submodule for both remotes?  When switching 
between remotes would the git submodule update pull the correct hash 
version for the subproject and pull the correct version of the subproject?

I think to get it right we need to enable a workflow that allows easy
"multi-step" mirroring, e.g. A (source of truth) can be mirrored to B,
who can overlay the .gitmodules to point to server-B, which then can
be mirrored by C, who can have its own serverC.

When C forgot to configure all the submodules, it should fall back to
serverB as that was closest, and if that is unconfigured it should
fallback to A IMO.




[remote "origin"]
 url = https://github.com/..
[remote "inhouse"]
 url = https://inhouse.corp/..

But where do we clone it from?
(Or do we just do a "git init" on that submodule and fetch
from both remotes? in which order?)

origin by default and inhouse if specified. There is already a implied
default (origin). The idea was not to do both but rather what is
specified.
Origin and inhouse are just names for remotes. If one wanted a
"--all-remotes" could pull from everywhere in the Ether if feature was to
be
implemented.

How is origin implied to be the default?
Should there be an order (e.g. if you cannot find it at inhouse get it
from github,
if they are down, get it from kernel.org)

As it is in the Highlander series... "there can be only one" (remote).   So
that is what I mean by origin.  The only remote allowed is the "origin"
unless changed by the user... but there can still only be one *currently*.
Though I see your point as it is not labeled "origin".  It is not labeled at
all.  Apologies for confusion there.

"origin" is just a common name for a remote, like "master" is a common name
for 

Re: [PATCH] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves

[Stefan Beller]

On Tue, Jan 24, 2017 at 1:58 PM, Brandon Williams  wrote:
> On 01/24, Stefan Beller wrote:
>> + if (read_gitfile_gently(old_git_dir, _code) ||
>> + err_code == READ_GITFILE_ERR_NOT_A_REPO) {
>> + /*
>> +  * If it is an actual gitfile, it doesn't need migration,
>> +  * however in case of a recursively nested submodule, the
>> +  * gitfile content may be stale, as its superproject
>> +  * (which may be a submodule of another superproject)
>> +  * may have been moved. So expect a bogus pointer to be read,
>> +  * which materializes as error READ_GITFILE_ERR_NOT_A_REPO.
>> +  */
>> + connect_work_tree_and_git_dir(path, real_new_git_dir);
>
> So connect_work_tree_and_git_dir() will update the .gitfile if it is
> stale.
>
>> + return;
>> + }
>> +
>> + if (submodule_uses_worktrees(path))
>> + die(_("relocate_gitdir for submodule '%s' with "
>> +   "more than one worktree not supported"), path);
>
> No current support for worktrees (yet!).
>
>> +
>>   if (!prefix)
>>   prefix = get_super_prefix();
>>
>> @@ -1437,22 +1448,14 @@ void absorb_git_dir_into_superproject(const char 
>> *prefix,
>> const char *path,
>> unsigned flags)
>>  {
>> - const char *sub_git_dir, *v;
>> - char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
>>   struct strbuf gitdir = STRBUF_INIT;
>> -
>>   strbuf_addf(, "%s/.git", path);
>> - sub_git_dir = resolve_gitdir(gitdir.buf);
>>
>>   /* Not populated? */
>> - if (!sub_git_dir)
>> + if (!file_exists(gitdir.buf))
>>   goto out;
>
> There should be a is_submodule_populated() function now, maybe
> we should start using it when performing population checks?

Yes I am aware of that, but the problem is we cannot use it here.
is_submodule_populated[1], just like the code here, uses
resolve_gitdir, which is

const char *resolve_gitdir(const char *suspect)
{
if (is_git_directory(suspect))
   return suspect;
return read_gitfile(suspect);
}

And there you see the problem: read_gitfile will die on error.
we'd have to have use read_gitfile_gently(old_git_dir, _code),
and then allow READ_GITFILE_ERR_NOT_A_REPO to go through,
just as above.

And that is also the reason why we had to move submodule_uses_worktrees
down, as it also uses no gentle function to look for a git directory
(read: it would die as well). When you have bogus content in your
.git file, there is really nothing you can do to determine if the submodule
is part of a worktree setup, so it is fine to postpone the check until after we
fixed up the link.

So here is the bug you spotted: If it is a worktree already, then
read_gitfile_gently would work fine, no need to "fix" it.

I'll resend with logic as follows:

char *retvalue = read_gitfile_gently(old_git_dir, _code);
if (retvalue)
// return early; a worktree is fine here, no need to check
// because we do nothing

if (err_code == READ_GITFILE_ERR_NOT_A_REPO)
// connect; then check for worktree and return early;

// do the actual relocation.


[1] as found e.g. at
https://public-inbox.org/git/1481915002-162130-2-git-send-email-bmw...@google.com/

>
>>
>> - /* Is it already absorbed into the superprojects git dir? */
>> - real_sub_git_dir = real_pathdup(sub_git_dir);
>> - real_common_git_dir = real_pathdup(get_git_common_dir());
>> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))
>> - relocate_single_git_dir_into_superproject(prefix, path);
>> + relocate_single_git_dir_into_superproject(prefix, path);
>
> So the check was just pushed into the relocation function.

The check was pushed down, so we can use the
connect_work_tree_and_git_dir instead.

>
>>
>>   if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
>>   struct child_process cp = CHILD_PROCESS_INIT;
>> @@ -1481,6 +1484,4 @@ void absorb_git_dir_into_superproject(const char 
>> *prefix,
>>
>>  out:
>>   strbuf_release();
>> - free(real_sub_git_dir);
>> - free(real_common_git_dir);
>>  }
>> diff --git a/t/t7412-submodule-absorbgitdirs.sh 
>> b/t/t7412-submodule-absorbgitdirs.sh
>> index 1c47780e2b..e2bbb449b6 100755
>> --- a/t/t7412-submodule-absorbgitdirs.sh
>> +++ b/t/t7412-submodule-absorbgitdirs.sh
>> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested 
>> submodule' '
>>   test_cmp expect.2 actual.2
>>  '
>>
>> +test_expect_success 're-setup nested submodule' '
>> + # un-absorb the direct submodule, to test if the nested submodule
>> + # is still correct (needs a rewrite of the gitfile only)
>> + rm -rf sub1/.git &&
>> + mv .git/modules/sub1 sub1/.git &&
>> + GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
>> + 

Re: [PATCH] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves

[Brandon Williams]

On 01/24, Stefan Beller wrote:
> On Tue, Jan 24, 2017 at 1:58 PM, Brandon Williams  wrote:
> > On 01/24, Stefan Beller wrote:
> >> + if (read_gitfile_gently(old_git_dir, _code) ||
> >> + err_code == READ_GITFILE_ERR_NOT_A_REPO) {
> >> + /*
> >> +  * If it is an actual gitfile, it doesn't need migration,
> >> +  * however in case of a recursively nested submodule, the
> >> +  * gitfile content may be stale, as its superproject
> >> +  * (which may be a submodule of another superproject)
> >> +  * may have been moved. So expect a bogus pointer to be read,
> >> +  * which materializes as error READ_GITFILE_ERR_NOT_A_REPO.
> >> +  */
> >> + connect_work_tree_and_git_dir(path, real_new_git_dir);
> >
> > So connect_work_tree_and_git_dir() will update the .gitfile if it is
> > stale.
> >
> >> + return;
> >> + }
> >> +
> >> + if (submodule_uses_worktrees(path))
> >> + die(_("relocate_gitdir for submodule '%s' with "
> >> +   "more than one worktree not supported"), path);
> >
> > No current support for worktrees (yet!).
> >
> >> +
> >>   if (!prefix)
> >>   prefix = get_super_prefix();
> >>
> >> @@ -1437,22 +1448,14 @@ void absorb_git_dir_into_superproject(const char 
> >> *prefix,
> >> const char *path,
> >> unsigned flags)
> >>  {
> >> - const char *sub_git_dir, *v;
> >> - char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
> >>   struct strbuf gitdir = STRBUF_INIT;
> >> -
> >>   strbuf_addf(, "%s/.git", path);
> >> - sub_git_dir = resolve_gitdir(gitdir.buf);
> >>
> >>   /* Not populated? */
> >> - if (!sub_git_dir)
> >> + if (!file_exists(gitdir.buf))
> >>   goto out;
> >
> > There should be a is_submodule_populated() function now, maybe
> > we should start using it when performing population checks?
> 
> Yes I am aware of that, but the problem is we cannot use it here.
> is_submodule_populated[1], just like the code here, uses
> resolve_gitdir, which is
> 
> const char *resolve_gitdir(const char *suspect)
> {
> if (is_git_directory(suspect))
>return suspect;
> return read_gitfile(suspect);
> }
> 
> And there you see the problem: read_gitfile will die on error.
> we'd have to have use read_gitfile_gently(old_git_dir, _code),
> and then allow READ_GITFILE_ERR_NOT_A_REPO to go through,
> just as above.

Hmm, then maybe is_submodule_populated should be rewritten to not die on
an error then?

> 
> And that is also the reason why we had to move submodule_uses_worktrees
> down, as it also uses no gentle function to look for a git directory
> (read: it would die as well). When you have bogus content in your
> .git file, there is really nothing you can do to determine if the submodule
> is part of a worktree setup, so it is fine to postpone the check until after 
> we
> fixed up the link.
> 
> So here is the bug you spotted: If it is a worktree already, then
> read_gitfile_gently would work fine, no need to "fix" it.
> 
> I'll resend with logic as follows:
> 
> char *retvalue = read_gitfile_gently(old_git_dir, _code);
> if (retvalue)
> // return early; a worktree is fine here, no need to check
> // because we do nothing
> 
> if (err_code == READ_GITFILE_ERR_NOT_A_REPO)
> // connect; then check for worktree and return early;
> 
> // do the actual relocation.
> 
> 
> [1] as found e.g. at
> https://public-inbox.org/git/1481915002-162130-2-git-send-email-bmw...@google.com/
> 
> >
> >>
> >> - /* Is it already absorbed into the superprojects git dir? */
> >> - real_sub_git_dir = real_pathdup(sub_git_dir);
> >> - real_common_git_dir = real_pathdup(get_git_common_dir());
> >> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))
> >> - relocate_single_git_dir_into_superproject(prefix, path);
> >> + relocate_single_git_dir_into_superproject(prefix, path);
> >
> > So the check was just pushed into the relocation function.
> 
> The check was pushed down, so we can use the
> connect_work_tree_and_git_dir instead.
> 
> >
> >>
> >>   if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
> >>   struct child_process cp = CHILD_PROCESS_INIT;
> >> @@ -1481,6 +1484,4 @@ void absorb_git_dir_into_superproject(const char 
> >> *prefix,
> >>
> >>  out:
> >>   strbuf_release();
> >> - free(real_sub_git_dir);
> >> - free(real_common_git_dir);
> >>  }
> >> diff --git a/t/t7412-submodule-absorbgitdirs.sh 
> >> b/t/t7412-submodule-absorbgitdirs.sh
> >> index 1c47780e2b..e2bbb449b6 100755
> >> --- a/t/t7412-submodule-absorbgitdirs.sh
> >> +++ b/t/t7412-submodule-absorbgitdirs.sh
> >> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested 
> >> submodule' '
> >>   

Re: [PATCH] difftool.c: mark a file-local symbol with static

[Jeff King]

On Tue, Jan 24, 2017 at 01:52:13PM -0800, Junio C Hamano wrote:

> > I dunno. As ugly as the "%s" thing is in the source, at least it doesn't
> > change the output. Not that an extra space is the end of the world, but
> > it seems like it's letting the problem escape from the source code.
> >
> > Do people still care about resolving this? -Wno-format-zero-length is in
> > the DEVELOPER options. It wasn't clear to me if that was sufficient, or
> > if we're going to get a bunch of reports from people that need to be
> > directed to the right compiler options.
> 
> I view both as ugly, but probably "%s", "" is lessor of the two
> evils.
> 
> Perhaps
> 
>   #define JUST_SHOW_EMPTY_LINE "%s", ""
> 
>   ...
>   warning(JUST_SHOW_EMPTY_LINE);
> ...
> 
> or something silly like that?

Gross, but at least it's self documenting. :)

I guess a less horrible version of that is:

  static inline warning_blank_line(void)
  {
warning("%s", "");
  }

We'd potentially need a matching one for error(), but at last it avoids
macro trickery.

-Peff

Re: [PATCH 7/7] completion: recognize more long-options

[Junio C Hamano]

Cornelius Weig  writes:

>> Please study item (5) "Sign your work" in
>> Documentation/SubmittingPatches and sign off your work.
>
> I followed the recommendations to submitting work, and in the first
> round signing is discouraged.

Just this point.  You found a bug in our documentation if that is
the case; it should not be giving that impression to you.  

Re: [PATCH 7/7] completion: recognize more long-options

[Cornelius Weig]

On 01/25/2017 12:24 AM, Junio C Hamano wrote:
> Cornelius Weig  writes:
> 
>>> Please study item (5) "Sign your work" in
>>> Documentation/SubmittingPatches and sign off your work.
>>
>> I followed the recommendations to submitting work, and in the first
>> round signing is discouraged.
> 
> Just this point.  You found a bug in our documentation if that is
> the case; it should not be giving that impression to you.  
> 

Well, I am referring to par. (4) of Documentation/SubmittingPatches
(emphasis mine):

<<
*Do not PGP sign your patch, at least for now*.  Most likely, your
maintainer or other people on the list would not have your PGP
key and would not bother obtaining it anyway.  Your patch is not
judged by who you are; a good patch from an unknown origin has a
far better chance of being accepted than a patch from a known,
respected origin that is done poorly or does incorrect things.
>>

If first submissions should be signed as well, then I find this quite
misleading.

[PATCH] gpg-interface: Add some output from gpg when it errors out.

[Mike Hommey]

When e.g. signing a tag fails, the user is left with the following
output on their console:
  error: gpg failed to sign the data
  error: unable to sign the tag

There is no indication of what specifically failed, and no indication
how they might solve the problem.

It turns out, gpg still does output some messages without a [GNUPG:]
prefix. The same messages it outputs when run standalone, in fact.

Those messages can be helpful to find what made the gpg command fail.

For instance, after changing my laptop for a new one, I copied my
configs, but had some environment differences that broke gpg.
With this change applied, the output becomes, on this new machine:
  gpg: keyblock resource '/usr/share/keyrings/debian-keyring.gpg': No
such file or directory
  error: gpg failed to sign the data
  error: unable to sign the tag

which makes it clearer what's wrong.

Signed-off-by: Mike Hommey 
---
 gpg-interface.c | 28 
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index e44cc27da..2768bb307 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -149,6 +149,26 @@ const char *get_signing_key(void)
return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
 }
 
+static int pipe_gpg_command(struct child_process *cmd,
+   const char *in, size_t in_len,
+   struct strbuf *out, size_t out_hint,
+   struct strbuf *err, size_t err_hint)
+{
+   int ret = pipe_command(cmd, in, in_len, out, out_hint, err, err_hint);
+   /* Print out any line that doesn't start with [GNUPG:] if the gpg
+* command failed. */
+   if (ret) {
+   struct strbuf **err_lines = strbuf_split(err, '\n');
+   for (struct strbuf **line = err_lines; *line; line++) {
+   if (memcmp((*line)->buf, "[GNUPG:]", 8)) {
+   strbuf_rtrim(*line);
+   fprintf(stderr, "%s\n", (*line)->buf);
+   }
+   }
+   strbuf_list_free(err_lines);
+   }
+   return ret;
+}
 /*
  * Create a detached signature for the contents of "buffer" and append
  * it after "signature"; "buffer" and "signature" can be the same
@@ -175,8 +195,8 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
*signature, const char *sig
 * because gpg exits without reading and then write gets SIGPIPE.
 */
sigchain_push(SIGPIPE, SIG_IGN);
-   ret = pipe_command(, buffer->buf, buffer->len,
-  signature, 1024, _status, 0);
+   ret = pipe_gpg_command(, buffer->buf, buffer->len,
+  signature, 1024, _status, 0);
sigchain_pop(SIGPIPE);
 
ret |= !strstr(gpg_status.buf, "\n[GNUPG:] SIG_CREATED ");
@@ -232,8 +252,8 @@ int verify_signed_buffer(const char *payload, size_t 
payload_size,
gpg_status = 
 
sigchain_push(SIGPIPE, SIG_IGN);
-   ret = pipe_command(, payload, payload_size,
-  gpg_status, 0, gpg_output, 0);
+   ret = pipe_gpg_command(, payload, payload_size,
+  gpg_status, 0, gpg_output, 0);
sigchain_pop(SIGPIPE);
 
delete_tempfile();
-- 
2.11.0.dirty

Re: [PATCH 7/7] completion: recognize more long-options

[Stefan Beller]

On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig
 wrote:
> On 01/25/2017 12:24 AM, Junio C Hamano wrote:
>> Cornelius Weig  writes:
>>
 Please study item (5) "Sign your work" in
 Documentation/SubmittingPatches and sign off your work.
>>>
>>> I followed the recommendations to submitting work, and in the first
>>> round signing is discouraged.
>>
>> Just this point.  You found a bug in our documentation if that is
>> the case; it should not be giving that impression to you.
>>
>
> Well, I am referring to par. (4) of Documentation/SubmittingPatches
> (emphasis mine):
>
> <<
> *Do not PGP sign your patch, at least for now*.  Most likely, your
> maintainer or other people on the list would not have your PGP
> key and would not bother obtaining it anyway.  Your patch is not
> judged by who you are; a good patch from an unknown origin has a
> far better chance of being accepted than a patch from a known,
> respected origin that is done poorly or does incorrect things.
>>>
>
> If first submissions should be signed as well, then I find this quite
> misleading.
>

Please read on; While this part addresses PGP signing,
which is discouraged at any round,
later on we talk about another type of signing.
(not cryptographic strong signing, but signing the intent;)
the DCO in the commit message.

So no PGP signing (in any round of the patch).

But DCO signed (in anything that you deem useful for the
project and that you are allowed to contribute)

[PATCH] tag: add tag.createReflog option

[cornelius . weig]

From: Cornelius Weig 

Git does not create a history for tags, in contrast to common
expectation to simply version everything. This can be changed by using
the `--create-reflog` flag when creating the tag. However, a config
option to enable this behavior by default is missing.

This commit adds the configuration variable `tag.createReflog` which
enables reflogs for new tags by default.

Signed-off-by: Cornelius Weig 
---
 Documentation/config.txt  |  5 +
 Documentation/git-tag.txt |  8 +---
 builtin/tag.c |  6 +-
 t/t7004-tag.sh| 14 ++
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4c..9e5f6f6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2945,6 +2945,11 @@ submodule.alternateErrorStrategy
as computed via `submodule.alternateLocation`. Possible values are
`ignore`, `info`, `die`. Default is `die`.
 
+tag.createReflog::
+   A boolean to specify whether newly created tags should have a reflog.
+   If `--[no-]create-reflog` is specified on the command line, it takes
+   precedence. Defaults to `false`.
+
 tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG 
signed.
If `--annotate` is specified on the command line, it takes
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..f2ed370 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 [ | ]
 'git tag' -d ...
 'git tag' [-n[]] -l [--contains ] [--points-at ]
-   [--column[=] | --no-column] [--create-reflog] [--sort=]
+   [--column[=] | --no-column] [--[no-]create-reflog] 
[--sort=]
[--format=] [--[no-]merged []] [...]
 'git tag' -v ...
 
@@ -149,8 +149,10 @@ This option is only applicable when listing tags without 
annotation lines.
all, 'whitespace' removes just leading/trailing whitespace lines and
'strip' removes both whitespace and commentary.
 
---create-reflog::
-   Create a reflog for the tag.
+--[no-]create-reflog::
+   Force to create a reflog for the tag, or no reflog if 
`--no-create-reflog`
+   is used. Unless the `tag.createReflog` config variable is set to true, 
no
+   reflog is created by default. See linkgit:git-config[1].
 
 ::
The name of the tag to create, delete, or describe.
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df728..1f13e4d 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 static int force_sign_annotate;
+static int create_reflog;
 
 static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, 
const char *format)
 {
@@ -165,6 +166,10 @@ static int git_tag_config(const char *var, const char 
*value, void *cb)
force_sign_annotate = git_config_bool(var, value);
return 0;
}
+   if (!strcmp(var, "tag.createreflog")) {
+   create_reflog = git_config_bool(var, value);
+   return 0;
+   }
 
if (starts_with(var, "column."))
return git_column_config(var, value, "tag", );
@@ -325,7 +330,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *object_ref, *tag;
struct create_tag_options opt;
char *cleanup_arg = NULL;
-   int create_reflog = 0;
int annotate = 0, force = 0;
int cmdmode = 0, create_tag_object = 0;
const char *msgfile = NULL, *keyid = NULL;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1cfa8a2..67b39ec 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -90,6 +90,20 @@ test_expect_success '--create-reflog does not create reflog 
on failure' '
test_must_fail git reflog exists refs/tags/mytag
 '
 
+test_expect_success 'option tag.createreflog creates reflog by default' '
+   test_when_finished "git tag -d tag_with_reflog" &&
+   git config tag.createReflog true &&
+   git tag tag_with_reflog &&
+   git reflog exists refs/tags/tag_with_reflog
+'
+
+test_expect_success 'option tag.createreflog overridden by command line' '
+   test_when_finished "git tag -d tag_without_reflog" &&
+   git config tag.createReflog true &&
+   git tag --no-create-reflog tag_without_reflog &&
+   test_must_fail git reflog exists refs/tags/tag_without_reflog
+'
+
 test_expect_success 'listing all tags if one exists should succeed' '
git tag -l &&
git tag
-- 
2.10.2

Re:

[Cornelius Weig]

On 01/25/2017 01:21 AM, Stefan Beller wrote:
>>
>>> Do not PGP sign your patch, at least *for now*. (...)
>>
> 
> And maybe these 2 small words are the bug in the documentation?
> Shall we drop the "at least for now" part, like so:
> 
> ---8<---
> From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
> From: Stefan Beller 
> Date: Tue, 24 Jan 2017 16:19:13 -0800
> Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing
> 
> Signed-off-by: Stefan Beller 
> ---
>  Documentation/SubmittingPatches | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 08352deaae..28da4ad2d4 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -216,12 +216,12 @@ that it will be postponed.
>  Exception:  If your mailer is mangling patches then someone may ask
>  you to re-send them using MIME, that is OK.
>  
> -Do not PGP sign your patch, at least for now.  Most likely, your
> -maintainer or other people on the list would not have your PGP
> -key and would not bother obtaining it anyway.  Your patch is not
> -judged by who you are; a good patch from an unknown origin has a
> -far better chance of being accepted than a patch from a known,
> -respected origin that is done poorly or does incorrect things.
> +Do not PGP sign your patch. Most likely, your maintainer or other
> +people on the list would not have your PGP key and would not bother
> +obtaining it anyway. Your patch is not judged by who you are; a good
> +patch from an unknown origin has a far better chance of being accepted
> +than a patch from a known, respected origin that is done poorly or
> +does incorrect things.
>  
>  If you really really really really want to do a PGP signed
>  patch, format it as "multipart/signed", not a text/plain message
> 

It definitely is an improvement. Though it would still leave me puzzled
when finding a section about signing just below.

Is changing heading (5) too big a change? Like so:

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352de..71898dc 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -246,7 +246,7 @@ patch.
  *2* The mailing list: git@vger.kernel.org
 
 
-(5) Sign your work
+(5) Certify your work by signing off.
 
 To improve tracking of who did what, we've borrowed the
 "sign-off" procedure from the Linux kernel project on patches

[PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves

[Stefan Beller]

Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
When nested is already absorbed into sub, but sub is not absorbed into
its superproject, then we need to fixup the gitfile and core.worktree
setting for 'nested' when absorbing 'sub', but we do not need to move
its git dir around.

Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".

An alternative I considered to do this work lazily, i.e. when resolving
"../.git/modules/nested", we would notice the ".git" being a gitfile
linking to another path.  That seemed to be robuster by design, but harder
to get the implementation right.  Maybe we have to do that anyway once we
try to have submodules and worktrees working nicely together, but for now
just produce 'correct' (i.e. direct) pointers.

Signed-off-by: Stefan Beller 
---
 submodule.c| 43 ++
 t/t7412-submodule-absorbgitdirs.sh | 27 
 2 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/submodule.c b/submodule.c
index 4c4f033e8a..95437105bf 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1437,22 +1437,47 @@ void absorb_git_dir_into_superproject(const char 
*prefix,
  const char *path,
  unsigned flags)
 {
+   int err_code;
const char *sub_git_dir, *v;
char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
struct strbuf gitdir = STRBUF_INIT;
-
strbuf_addf(, "%s/.git", path);
-   sub_git_dir = resolve_gitdir(gitdir.buf);
+   sub_git_dir = resolve_gitdir_gently(gitdir.buf, _code);
 
/* Not populated? */
-   if (!sub_git_dir)
-   goto out;
+   if (!sub_git_dir) {
+   char *real_new_git_dir;
+   const char *new_git_dir;
+   const struct submodule *sub;
+
+   if (err_code == READ_GITFILE_ERR_STAT_FAILED)
+   goto out; /* unpopulated as expected */
+   if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
+   /* We don't know what broke here. */
+   read_gitfile_error_die(err_code, path, NULL);
 
-   /* Is it already absorbed into the superprojects git dir? */
-   real_sub_git_dir = real_pathdup(sub_git_dir);
-   real_common_git_dir = real_pathdup(get_git_common_dir());
-   if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))
-   relocate_single_git_dir_into_superproject(prefix, path);
+   /*
+   * Maybe populated, but no git directory was found?
+   * This can happen if the superproject is a submodule
+   * itself and was just absorbed. The absorption of the
+   * superproject did not rewrite the git file links yet,
+   * fix it now.
+   */
+   sub = submodule_from_path(null_sha1, path);
+   if (!sub)
+   die(_("could not lookup name for submodule '%s'"), 
path);
+   new_git_dir = git_path("modules/%s", sub->name);
+   if (safe_create_leading_directories_const(new_git_dir) < 0)
+   die(_("could not create directory '%s'"), new_git_dir);
+   real_new_git_dir = real_pathdup(new_git_dir);
+   connect_work_tree_and_git_dir(path, real_new_git_dir);
+   } else {
+   /* Is it already absorbed into the superprojects git dir? */
+   real_sub_git_dir = real_pathdup(sub_git_dir);
+   real_common_git_dir = real_pathdup(get_git_common_dir());
+   if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))
+   relocate_single_git_dir_into_superproject(prefix, path);
+   }
 
if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
struct child_process cp = CHILD_PROCESS_INIT;
diff --git a/t/t7412-submodule-absorbgitdirs.sh 
b/t/t7412-submodule-absorbgitdirs.sh
index 1c47780e2b..e2bbb449b6 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested 
submodule' '
test_cmp expect.2 actual.2
 '
 
+test_expect_success 're-setup nested submodule' '
+   # un-absorb the direct submodule, to test if the nested submodule
+   # is still correct (needs a rewrite of the gitfile only)
+   rm -rf sub1/.git &&
+   mv .git/modules/sub1 sub1/.git &&
+   GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
+   # fixup the nested submodule
+   echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
+   GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
+   core.worktree "../../../nested" &&
+   # make sure this re-setup is correct
+   git status 

[PATCHv2 2/3] cache.h: expose the dying procedure for reading gitlinks

[Stefan Beller]

In a later patch we want to react to only a subset of errors, defaulting
the rest to die as usual. Separate the block that takes care of dying
into its own function so we have easy access to it.

Signed-off-by: Stefan Beller 
---
 cache.h |  1 +
 setup.c | 48 ++--
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/cache.h b/cache.h
index cafa3d10ae..d55f5dccb1 100644
--- a/cache.h
+++ b/cache.h
@@ -507,6 +507,7 @@ extern int is_nonbare_repository_dir(struct strbuf *path);
 #define READ_GITFILE_ERR_NO_PATH 6
 #define READ_GITFILE_ERR_NOT_A_REPO 7
 #define READ_GITFILE_ERR_TOO_LARGE 8
+extern void read_gitfile_error_die(int error_code, const char *path, const 
char *dir);
 extern const char *read_gitfile_gently(const char *path, int 
*return_error_code);
 #define read_gitfile(path) read_gitfile_gently((path), NULL)
 extern const char *resolve_gitdir_gently(const char *suspect, int 
*return_error_code);
diff --git a/setup.c b/setup.c
index 4605fd3c3c..967f289f1e 100644
--- a/setup.c
+++ b/setup.c
@@ -486,6 +486,30 @@ int verify_repository_format(const struct 
repository_format *format,
return 0;
 }
 
+void read_gitfile_error_die(int error_code, const char *path, const char *dir)
+{
+   switch (error_code) {
+   case READ_GITFILE_ERR_STAT_FAILED:
+   case READ_GITFILE_ERR_NOT_A_FILE:
+   /* non-fatal; follow return path */
+   break;
+   case READ_GITFILE_ERR_OPEN_FAILED:
+   die_errno("Error opening '%s'", path);
+   case READ_GITFILE_ERR_TOO_LARGE:
+   die("Too large to be a .git file: '%s'", path);
+   case READ_GITFILE_ERR_READ_FAILED:
+   die("Error reading %s", path);
+   case READ_GITFILE_ERR_INVALID_FORMAT:
+   die("Invalid gitfile format: %s", path);
+   case READ_GITFILE_ERR_NO_PATH:
+   die("No path in gitfile: %s", path);
+   case READ_GITFILE_ERR_NOT_A_REPO:
+   die("Not a git repository: %s", dir);
+   default:
+   die("BUG: unknown error code");
+   }
+}
+
 /*
  * Try to read the location of the git directory from the .git file,
  * return path to git directory if found.
@@ -559,28 +583,8 @@ const char *read_gitfile_gently(const char *path, int 
*return_error_code)
 cleanup_return:
if (return_error_code)
*return_error_code = error_code;
-   else if (error_code) {
-   switch (error_code) {
-   case READ_GITFILE_ERR_STAT_FAILED:
-   case READ_GITFILE_ERR_NOT_A_FILE:
-   /* non-fatal; follow return path */
-   break;
-   case READ_GITFILE_ERR_OPEN_FAILED:
-   die_errno("Error opening '%s'", path);
-   case READ_GITFILE_ERR_TOO_LARGE:
-   die("Too large to be a .git file: '%s'", path);
-   case READ_GITFILE_ERR_READ_FAILED:
-   die("Error reading %s", path);
-   case READ_GITFILE_ERR_INVALID_FORMAT:
-   die("Invalid gitfile format: %s", path);
-   case READ_GITFILE_ERR_NO_PATH:
-   die("No path in gitfile: %s", path);
-   case READ_GITFILE_ERR_NOT_A_REPO:
-   die("Not a git repository: %s", dir);
-   default:
-   assert(0);
-   }
-   }
+   else if (error_code)
+   read_gitfile_error_die(error_code, path, dir);
 
free(buf);
return error_code ? NULL : path;
-- 
2.11.0.495.g04f60290a0.dirty

[PATCHv2 1/3] Add gentle version of resolve_git_dir

[Stefan Beller]

This follows a93bedada (setup: add gentle version of read_gitfile,
2015-06-09), and assumes the same reasoning. resolve_git_dir is unsuited
for speculative calls, so we want to use the gentle version to find out
about potential errors.

Signed-off-by: Stefan Beller 
---
 cache.h | 4 +++-
 setup.c | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 00a029af36..cafa3d10ae 100644
--- a/cache.h
+++ b/cache.h
@@ -509,7 +509,9 @@ extern int is_nonbare_repository_dir(struct strbuf *path);
 #define READ_GITFILE_ERR_TOO_LARGE 8
 extern const char *read_gitfile_gently(const char *path, int 
*return_error_code);
 #define read_gitfile(path) read_gitfile_gently((path), NULL)
-extern const char *resolve_gitdir(const char *suspect);
+extern const char *resolve_gitdir_gently(const char *suspect, int 
*return_error_code);
+#define resolve_gitdir(path) resolve_gitdir_gently((path), NULL)
+
 extern void set_git_work_tree(const char *tree);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
diff --git a/setup.c b/setup.c
index 1b534a7508..4605fd3c3c 100644
--- a/setup.c
+++ b/setup.c
@@ -1017,11 +1017,11 @@ const char *setup_git_directory(void)
return setup_git_directory_gently(NULL);
 }
 
-const char *resolve_gitdir(const char *suspect)
+const char *resolve_gitdir_gently(const char *suspect, int *return_error_code)
 {
if (is_git_directory(suspect))
return suspect;
-   return read_gitfile(suspect);
+   return read_gitfile_gently(suspect, return_error_code);
 }
 
 /* if any standard file descriptor is missing open it to /dev/null */
-- 
2.11.0.495.g04f60290a0.dirty

[PATCHv2 0/3] fix recursive submodule absorbing

[Stefan Beller]

In this second iteration, only absorb_git_dir_into_superproject is touched,
which does the check if connect_work_tree_and_git_dir is needed.

Internally I also had a patch that converts is_submodule_populated
to be gentle, but it is not needed here, so I dropped it before sending out.

Thanks,
Stefan


Stefan Beller (3):
  Add gentle version of resolve_git_dir
  cache.h: expose the dying procedure for reading gitlinks
  submodule absorbing: fix worktree/gitdir pointers recursively for
non-moves

 cache.h|  5 +++-
 setup.c| 52 --
 submodule.c| 43 ---
 t/t7412-submodule-absorbgitdirs.sh | 27 
 4 files changed, 93 insertions(+), 34 deletions(-)

-- 
2.11.0.495.g04f60290a0.dirty

Re: [PATCH 7/7] completion: recognize more long-options

[Cornelius Weig]

On 01/25/2017 12:43 AM, Stefan Beller wrote:
> On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig
>  wrote:
>> On 01/25/2017 12:24 AM, Junio C Hamano wrote:
>>> Cornelius Weig  writes:
>>>
> Please study item (5) "Sign your work" in
> Documentation/SubmittingPatches and sign off your work.

 I followed the recommendations to submitting work, and in the first
 round signing is discouraged.
>>>
>>> Just this point.  You found a bug in our documentation if that is
>>> the case; it should not be giving that impression to you.
>>>
>>
>> Well, I am referring to par. (4) of Documentation/SubmittingPatches
>> (emphasis mine):
>>
>> <<
>> *Do not PGP sign your patch, at least for now*.  Most likely, your
>> maintainer or other people on the list would not have your PGP
>> key and would not bother obtaining it anyway.  Your patch is not
>> judged by who you are; a good patch from an unknown origin has a
>> far better chance of being accepted than a patch from a known,
>> respected origin that is done poorly or does incorrect things.
>> <<
>>
>> If first submissions should be signed as well, then I find this quite
>> misleading.
>>
> 
> Please read on; While this part addresses PGP signing,
> which is discouraged at any round,
> later on we talk about another type of signing.
> (not cryptographic strong signing, but signing the intent;)
> the DCO in the commit message.
> 
> So no PGP signing (in any round of the patch).
> 
> But DCO signed (in anything that you deem useful for the
> project and that you are allowed to contribute)
> 

Right, it's crystal clear now. What confused me was the combination of

> Do not PGP sign your patch, at least *for now*. (...)

and then the section with heading

> (5) *Sign* your work

So I didn't even bother to read (5) because I deemed it irrelevant. I
think if it had said `(5) *Certify* your work` this would not have happened.

[no subject]

[Stefan Beller]

>
>> Do not PGP sign your patch, at least *for now*. (...)
>

And maybe these 2 small words are the bug in the documentation?
Shall we drop the "at least for now" part, like so:

---8<---
>From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001
From: Stefan Beller 
Date: Tue, 24 Jan 2017 16:19:13 -0800
Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing

Signed-off-by: Stefan Beller 
---
 Documentation/SubmittingPatches | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08352deaae..28da4ad2d4 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -216,12 +216,12 @@ that it will be postponed.
 Exception:  If your mailer is mangling patches then someone may ask
 you to re-send them using MIME, that is OK.
 
-Do not PGP sign your patch, at least for now.  Most likely, your
-maintainer or other people on the list would not have your PGP
-key and would not bother obtaining it anyway.  Your patch is not
-judged by who you are; a good patch from an unknown origin has a
-far better chance of being accepted than a patch from a known,
-respected origin that is done poorly or does incorrect things.
+Do not PGP sign your patch. Most likely, your maintainer or other
+people on the list would not have your PGP key and would not bother
+obtaining it anyway. Your patch is not judged by who you are; a good
+patch from an unknown origin has a far better chance of being accepted
+than a patch from a known, respected origin that is done poorly or
+does incorrect things.
 
 If you really really really really want to do a PGP signed
 patch, format it as "multipart/signed", not a text/plain message
-- 
2.11.0.495.g04f60290a0.dirty

Re: [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves

[Brandon Williams]

On 01/24, Stefan Beller wrote:
> Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
> When nested is already absorbed into sub, but sub is not absorbed into
> its superproject, then we need to fixup the gitfile and core.worktree
> setting for 'nested' when absorbing 'sub', but we do not need to move
> its git dir around.
> 
> Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
> it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
> 
> An alternative I considered to do this work lazily, i.e. when resolving
> "../.git/modules/nested", we would notice the ".git" being a gitfile
> linking to another path.  That seemed to be robuster by design, but harder
> to get the implementation right.  Maybe we have to do that anyway once we
> try to have submodules and worktrees working nicely together, but for now
> just produce 'correct' (i.e. direct) pointers.
> 
> Signed-off-by: Stefan Beller 
> ---
>  submodule.c| 43 
> ++
>  t/t7412-submodule-absorbgitdirs.sh | 27 
>  2 files changed, 61 insertions(+), 9 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 4c4f033e8a..95437105bf 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1437,22 +1437,47 @@ void absorb_git_dir_into_superproject(const char 
> *prefix,
> const char *path,
> unsigned flags)
>  {
> + int err_code;
>   const char *sub_git_dir, *v;
>   char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
>   struct strbuf gitdir = STRBUF_INIT;
> -
>   strbuf_addf(, "%s/.git", path);
> - sub_git_dir = resolve_gitdir(gitdir.buf);
> + sub_git_dir = resolve_gitdir_gently(gitdir.buf, _code);
>  
>   /* Not populated? */
> - if (!sub_git_dir)
> - goto out;
> + if (!sub_git_dir) {
> + char *real_new_git_dir;
> + const char *new_git_dir;
> + const struct submodule *sub;
> +
> + if (err_code == READ_GITFILE_ERR_STAT_FAILED)
> + goto out; /* unpopulated as expected */
> + if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
> + /* We don't know what broke here. */
> + read_gitfile_error_die(err_code, path, NULL);
>  
> - /* Is it already absorbed into the superprojects git dir? */
> - real_sub_git_dir = real_pathdup(sub_git_dir);
> - real_common_git_dir = real_pathdup(get_git_common_dir());
> - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))
> - relocate_single_git_dir_into_superproject(prefix, path);
> + /*
> + * Maybe populated, but no git directory was found?
> + * This can happen if the superproject is a submodule
> + * itself and was just absorbed. The absorption of the
> + * superproject did not rewrite the git file links yet,
> + * fix it now.
> + */
> + sub = submodule_from_path(null_sha1, path);
> + if (!sub)
> + die(_("could not lookup name for submodule '%s'"), 
> path);
> + new_git_dir = git_path("modules/%s", sub->name);
> + if (safe_create_leading_directories_const(new_git_dir) < 0)
> + die(_("could not create directory '%s'"), new_git_dir);
> + real_new_git_dir = real_pathdup(new_git_dir);
> + connect_work_tree_and_git_dir(path, real_new_git_dir);

Memory leak with 'real_new_git_dir'

> + } else {
> + /* Is it already absorbed into the superprojects git dir? */
> + real_sub_git_dir = real_pathdup(sub_git_dir);
> + real_common_git_dir = real_pathdup(get_git_common_dir());

The scope of 'real_sub_git_dir' and 'real_common_git_dir' variable can
be narrowed.  Move their declaration here and free it prior to exiting
the else block.

> + if (!skip_prefix(real_sub_git_dir, real_common_git_dir, ))

'v' isn't ever used, just use starts_with() and get rid of 'v'

> + relocate_single_git_dir_into_superproject(prefix, path);
> + }
>  

There's a label 'out' at the end of the function which can be removed as
there is no 'goto' statement using it.

>   if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
>   struct child_process cp = CHILD_PROCESS_INIT;
> diff --git a/t/t7412-submodule-absorbgitdirs.sh 
> b/t/t7412-submodule-absorbgitdirs.sh
> index 1c47780e2b..e2bbb449b6 100755
> --- a/t/t7412-submodule-absorbgitdirs.sh
> +++ b/t/t7412-submodule-absorbgitdirs.sh
> @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested 
> submodule' '
>   test_cmp expect.2 actual.2
>  '
>  
> +test_expect_success 're-setup nested submodule' '
> + # un-absorb the direct submodule, to test if the nested submodule
> + # is 

Re: [PATCH v2 7/7] Makefile: add a knob to enable the use of Asciidoctor

[Øyvind A . Holm]

On 2017-01-23 04:09:17, brian m. carlson wrote:
> On Mon, Jan 23, 2017 at 03:57:13AM +0100, Øyvind A. Holm wrote:
> > On 2017-01-22 02:41:56, brian m. carlson wrote:
> > > While Git has traditionally built its documentation using 
> > > AsciiDoc, some people wish to use Asciidoctor for speed or other 
> > > reasons.  Add a Makefile knob, USE_ASCIIDOCTOR, that sets various 
> > > options in order to produce acceptable output.  For HTML output, 
> > > XHTML5 was chosen, since the AsciiDoc options also produce XHTML, 
> > > albeit XHTML 1.1.
> >
> > I applied and tested the patches on the current master, commit 
> > 787f75f0567a ("Sixth batch for 2.12"), and "make doc" with 
> > USE_ASCIIDOCTOR fails:
> >
> > [...]
> >
> >   $ asciidoctor --version
> >   Asciidoctor 0.1.4 [http://asciidoctor.org]
>
> I think you need a newer version of Asciidoctor.  I fixed one or two 
> issues upstream in 1.5.2, I think, that made it work properly.

I've tried on Linux Mint 18 with Asciidoctor 1.5.4 now, and it works 
there, so the version is probably too old, yes.

> You could try to do the build with the "html5" target instead of 
> "xhtml5" and see if that works.  If so, we could switch to that 
> instead if we want to support older Asciidoctor versions.

It went a little better, but after a while it died with

  $ make doc USE_ASCIIDOCTOR=1
  [Cut 249 lines]
  GEN technical/api-index.txt
  ASCIIDOC technical/api-index.html
  ASCIIDOC git-init-db.xml
  sed "s|@@MAN_BASE_URL@@|file:///home/sunny/share/doc/git-doc/|" 
manpage-base-url.xsl.in > manpage-base-url.xsl
  XMLTO git-init-db.1
  xmlto: 
/home/sunny/src/git/src-other/devel/git/git/Documentation/git-init-db.xml does 
not validate (status 3)
  xmlto: Fix document syntax or use --skip-validation option
  /home/sunny/src/git/src-other/devel/git/git/Documentation/git-init-db.xml:5: 
element article: validity error : root and DTD name do not match 'article' and 
'manpage'
  Document 
/home/sunny/src/git/src-other/devel/git/git/Documentation/git-init-db.xml does 
not validate
  Makefile:343: recipe for target 'git-init-db.1' failed
  make[1]: *** [git-init-db.1] Error 13
  make[1]: Leaving directory 
'/home/sunny/src/git/src-other/devel/git/git/Documentation'
  Makefile:2091: recipe for target 'doc' failed
  make: *** [doc] Error 2
  $

and that's fair enough, since the generated html isn't well-formed. 
Adding --skip-validation to XMLTO_EXTRA gave a slightly different 
result:

  GEN technical/api-index.txt
  ASCIIDOC technical/api-index.html
  ASCIIDOC git-init-db.xml
  sed "s|@@MAN_BASE_URL@@|file:///home/sunny/share/doc/git-doc/|" 
manpage-base-url.xsl.in > manpage-base-url.xsl
  XMLTO git-init-db.1
  Note: namesp. cut : stripped namespace before processing   
git-init-db(1)
  Note: namesp. cut : processing stripped document   
git-init-db(1)
  Erro:  no refentry: No refentry elements found in "git-init-db(1)  
git-init-db(1)
  Makefile:343: recipe for target 'git-init-db.1' failed
  make[1]: *** [git-init-db.1] Error 1
  make[1]: Leaving directory 
'/home/sunny/src/git/src-other/devel/git/git/Documentation'
  Makefile:2091: recipe for target 'doc' failed
  make: *** [doc] Error 2
  $

But frankly, this probably isn't a showstopper. Even though this is the 
newest stable version of Debian, Asciidoctor 0.1.4 was released 
2013-09-05, 3y5m ago. USE_ASCIIDOCTOR isn't the default, so people can 
build the docs with asciidoc, and that works in Debian 8.7.

Regards,
Øyvind

+-| Øyvind A. Holm  - N 60.37604° E 5.9° |-+
| OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |
| Fingerprint: A006 05D6 E676 B319 55E2  E77E FB0C BEE8 94A5 06E5 |
+| 1698e7f6-e257-11e6-bfa0-db5caa6d21d3 |-+


signature.asc
Description: PGP signature

Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)

[Lars Schneider]

> On 24 Jan 2017, at 14:27, Jeff King  wrote:
> 
> On Tue, Jan 24, 2017 at 11:04:21AM +0100, Lars Schneider wrote:
> 
>> "fsck: prepare dummy objects for --connectivity-check" seems to
>> make t1450-fsck.sh fail on macOS with TravisCI:
>> 
>> Initialized empty Git repository in /Users/travis/build/git/git/t/trash 
>> directory.t1450-fsck/connectivity-only/.git/
>> [master (root-commit) 86520b7] empty
>> Author: A U Thor 
>> 2 files changed, 1 insertion(+)
>> create mode 100644 empty
>> create mode 100644 empty.t
>> override r--r--r--  travis/staff for 
>> .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391? (y/n [n]) not 
>> overwritten
>> dangling blob c21c9352f7526e9576892a6631e0e8cf1fccd34d
> 
> Looks like "mv" prompts and then fails to move the file (so we get the
> dangling blob for the source blob, and fsck doesn't report failure because
> we didn't actually corrupt the destination blob).
> 
> If I'm understanding the behavior correctly, this violates POSIX, which
> says:
> 
>  1. If the destination path exists, the −f option is not specified, and
> either of the following conditions is true:
> 
> a. The permissions of the destination path do not permit writing
> and the standard input is a terminal.
> 
> b. The −i option is specified.
> 
> the mv utility shall write a prompt to standard error and read a
> line from standard input. If the response is not affirmative,
> mv shall do nothing more with the current source_file and go on
> to any remaining source_files.
> 
> Our stdin isn't a terminal, and we do not specify "-i", so I think it
> shouldn't prompt.  It also exits with code 0 after failing to move,
> which is another surprise.
> 
> Here's a patch (tested by me that it works on Linux, but I don't know
> for sure that it fixes the Travis problem).
> 
> -- >8 --
> Subject: [PATCH] t1450: use "mv -f" within loose object directory
> 
> The loose objects are created with mode 0444. That doesn't
> prevent them being overwritten by rename(), but some
> versions of "mv" will be extra careful and prompt the user,
> even without "-i".
> 
> Reportedly macOS does this, at least in the Travis builds.
> The prompt reads from /dev/null, defaulting to "no", and the
> object isn't moved. Then to make matters even more
> interesting, it still returns "0" and the rest of the test
> proceeds, but with a broken setup.
> 
> We can work around it by using "mv -f" to override the
> prompt. This should work as it's already used in t5504 for
> the same purpose.
> 
> Signed-off-by: Jeff King 
> ---
> t/t1450-fsck.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index efaf41b68..33a51c9a6 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -536,7 +536,7 @@ test_expect_success 'fsck --connectivity-only' '
>   # free to examine the type if it chooses.
>   empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 &&
>   blob=$(echo unrelated | git hash-object -w --stdin) &&
> - mv $(sha1_file $blob) $empty &&
> + mv -f $(sha1_file $blob) $empty &&
> 
>   test_must_fail git fsck --strict &&
>   git fsck --strict --connectivity-only &&
> -- 
> 2.11.0.840.gd37c5973a

Ack. This patch fixes the issue:
https://travis-ci.org/larsxschneider/git/builds/194819605

Thanks,
Lars

Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)

[Jeff King]

On Tue, Jan 24, 2017 at 11:04:21AM +0100, Lars Schneider wrote:

> "fsck: prepare dummy objects for --connectivity-check" seems to
> make t1450-fsck.sh fail on macOS with TravisCI:
> 
> Initialized empty Git repository in /Users/travis/build/git/git/t/trash 
> directory.t1450-fsck/connectivity-only/.git/
> [master (root-commit) 86520b7] empty
>  Author: A U Thor 
>  2 files changed, 1 insertion(+)
>  create mode 100644 empty
>  create mode 100644 empty.t
> override r--r--r--  travis/staff for 
> .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391? (y/n [n]) not 
> overwritten
> dangling blob c21c9352f7526e9576892a6631e0e8cf1fccd34d

Looks like "mv" prompts and then fails to move the file (so we get the
dangling blob for the source blob, and fsck doesn't report failure because
we didn't actually corrupt the destination blob).

If I'm understanding the behavior correctly, this violates POSIX, which
says:

  1. If the destination path exists, the −f option is not specified, and
 either of the following conditions is true:

 a. The permissions of the destination path do not permit writing
 and the standard input is a terminal.

 b. The −i option is specified.

 the mv utility shall write a prompt to standard error and read a
 line from standard input. If the response is not affirmative,
 mv shall do nothing more with the current source_file and go on
 to any remaining source_files.

Our stdin isn't a terminal, and we do not specify "-i", so I think it
shouldn't prompt.  It also exits with code 0 after failing to move,
which is another surprise.

Here's a patch (tested by me that it works on Linux, but I don't know
for sure that it fixes the Travis problem).

-- >8 --
Subject: [PATCH] t1450: use "mv -f" within loose object directory

The loose objects are created with mode 0444. That doesn't
prevent them being overwritten by rename(), but some
versions of "mv" will be extra careful and prompt the user,
even without "-i".

Reportedly macOS does this, at least in the Travis builds.
The prompt reads from /dev/null, defaulting to "no", and the
object isn't moved. Then to make matters even more
interesting, it still returns "0" and the rest of the test
proceeds, but with a broken setup.

We can work around it by using "mv -f" to override the
prompt. This should work as it's already used in t5504 for
the same purpose.

Signed-off-by: Jeff King 
---
 t/t1450-fsck.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index efaf41b68..33a51c9a6 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -536,7 +536,7 @@ test_expect_success 'fsck --connectivity-only' '
# free to examine the type if it chooses.
empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 &&
blob=$(echo unrelated | git hash-object -w --stdin) &&
-   mv $(sha1_file $blob) $empty &&
+   mv -f $(sha1_file $blob) $empty &&
 
test_must_fail git fsck --strict &&
git fsck --strict --connectivity-only &&
-- 
2.11.0.840.gd37c5973a

Re: GSoC 2017: application open, deadline = February 9, 2017

[Christian Couder]

On Tue, Jan 24, 2017 at 12:28 PM, Johannes Schindelin
 wrote:
> Hi Matthieu,
>
> On Mon, 23 Jan 2017, Matthieu Moy wrote:
>
>> * Who's willing to mentor?
>
> As in the years before, I am willing to mentor.

I am also willing to mentor.

Thanks!

Re: [PATCH v2 0/5] string-list: make string_list_sort() reentrant

[Jeff King]

On Tue, Jan 24, 2017 at 12:44:10PM +0100, Johannes Schindelin wrote:

> Hi Peff,
> 
> On Mon, 23 Jan 2017, Jeff King wrote:
> 
> > Is there any interest in people adding the ISO qsort_s() to their libc
> > implementations? It seems like it's been a fair number of years by now.
> 
> Visual C supports it *at least* since Visual Studio 2005:
> 
> https://msdn.microsoft.com/en-us/library/4xc60xas(v=vs.80).aspx
> 
> With René's patch, we have an adapter for GNU libc, and if anybody comes
> up with the (equally trivial) adapter for BSD libc's qsort_r(), we have a
> lot of bases covered.

Sadly, no. Microsoft's qsort_s() is not compatible with the ISO C one.
And BSD's qsort_r() has a similar problem acting as a wrapper, because
the order of arguments in the callback functions is different (so you'd
have to actually wrap the callback, too, and rearrange the arguments for
each call, or do some macro trickery).

Gory details are in:

  
https://stackoverflow.com/questions/39560773/different-declarations-of-qsort-r-on-mac-and-linux/39561369

and the original thread:

  http://public-inbox.org/git/3083fbf7-d67e-77e4-e05f-94a7e7e15...@web.de/

-Peff

Re: [PATCH] difftool.c: mark a file-local symbol with static

[Jeff King]

mn Sat, Jan 21, 2017 at 09:26:08PM -0800, David Aguilar wrote:

> > > An obvious second
> > > best option would be to drop -Wall from the "everybody" CFLAGS and
> > > move it to DEVELOPER_CFLAGS instead.
> > 
> > Yeah, though that doesn't help the example above.
> > 
> > As ugly as warning("%s", "") is, I think it may be the thing that annoys
> > the smallest number of people.
> > 
> > -Peff
> 
> How about using warning(" ") instead?
> 
> For difftool.c specifically, the following is a fine solution,
> and doesn't require that we change our warning flags just for
> this one file.

I dunno. As ugly as the "%s" thing is in the source, at least it doesn't
change the output. Not that an extra space is the end of the world, but
it seems like it's letting the problem escape from the source code.

Do people still care about resolving this? -Wno-format-zero-length is in
the DEVELOPER options. It wasn't clear to me if that was sufficient, or
if we're going to get a bunch of reports from people that need to be
directed to the right compiler options.

The problematic code just hit 'next', so I suppose we'll see soon (OTOH,
the real test is when it get shipped as part of a release).

-Peff